Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: Add script to post report results in a topic regularly #328

Merged
merged 2 commits into from
Oct 10, 2024

Conversation

martin-brennan
Copy link
Contributor

This script is similar to the existing one that schedules
report results to to be sent to a PM on a regular basis,
but instead takes a topic ID and posts to that. This way
people can have report results sent to a public topic regularly
too and not have to deal with PM recipients and so on.

@martin-brennan martin-brennan force-pushed the feature/scheduled-report-post-automation branch 3 times, most recently from ee727c7 to be2abcc Compare October 8, 2024 05:16
This script is similar to the existing one that schedules
report results to to be sent to a PM on a regular basis,
but instead takes a topic ID and posts to that. This way
people can have report results sent to a public topic regularly
too and not have to deal with PM recipients and so on.
@martin-brennan martin-brennan force-pushed the feature/scheduled-report-post-automation branch from be2abcc to 3603393 Compare October 8, 2024 06:28
@martin-brennan martin-brennan marked this pull request as ready for review October 8, 2024 06:28
topic.reload.posts.count
}.by(1)

expect(topic.posts.last.raw).to include("Scheduled Report for #{query.name}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we are generating stuff here ... might as well test that the query is rendered, even something basic like query is "select 'abctestabc'" and look for the text abctestabc?

additionally we need url support for our specific use case:

eg: select abc,https://test.com as some_url

data explorer has special handling for _url.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep makes sense, will do that and handle URLs 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URL special handling is on the client so we don't need to test for that here. For our internal example, the DE table generator returns something like this:

| team | in_progress_url | up_next_url | maybe_next_url | maybe_later_url | unprioritized_url |
| :----- | :----- | :----- | :----- | :----- | :----- |
| blah | 4,https://test.com/xyz | 13,https://test.com/xyz | 18,https://test.com/xyz | 15,https://test.com/xyz | 11,https://test.com/xyz |

So the client itself must wrap the count with the URL

@martin-brennan
Copy link
Contributor Author

@SamSaffron made the suggested changes and left a comment about the URL stuff :)

@SamSaffron
Copy link
Member

thanks martin, let's give it a shot and see what Dave says.

@martin-brennan martin-brennan merged commit da1c99e into main Oct 10, 2024
5 checks passed
@martin-brennan martin-brennan deleted the feature/scheduled-report-post-automation branch October 10, 2024 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants