Don't send results works at the card level #37546
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a user enables **Don't send if there aren't results** this was the previous behavior: - If no results appeared on any card, no emails was sent. This was expected. - If any card contained results, all cards were sent in the email, including those that had no results. This PR modifieds that behavior to only send those cards that contain results for a dashboard pulse. For example, if 5 cards are present on a dashboard and 3 have no results, only results for the 2 cards containing data are included in the pulse. Primary changes: - `metabase.pulse/execute-dashboard` now removes any card with a row count of 0 in its computed result if `skip_if_empty` is true for the pulse. - The addition of tests covering the matrix of tests where `skip_if_empty` is true or false, all cards contain data, some cards contain data, and no cards contain data. - The `with-skip-if-empty-pulse-result` makes the above set of tests nice and concise. Minor changes: - Fixing a minor issue in `with-metadata-data-cards` in which the macroexpansion logic would go bad if the user used a different symbol name for `base-card-id`. - Spelling and formatting issues in `metabase.pulse` Fixes #34777
markbastian
added
the
backport
|
@markbastian Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone? |
markbastian
added a commit
that referenced
this pull request
Jan 11, 2024
* Don't send results works at the card level When a user enables **Don't send if there aren't results** this was the previous behavior: - If no results appeared on any card, no emails was sent. This was expected. - If any card contained results, all cards were sent in the email, including those that had no results. This PR modifieds that behavior to only send those cards that contain results for a dashboard pulse. For example, if 5 cards are present on a dashboard and 3 have no results, only results for the 2 cards containing data are included in the pulse. Primary changes: - `metabase.pulse/execute-dashboard` now removes any card with a row count of 0 in its computed result if `skip_if_empty` is true for the pulse. - The addition of tests covering the matrix of tests where `skip_if_empty` is true or false, all cards contain data, some cards contain data, and no cards contain data. - The `with-skip-if-empty-pulse-result` makes the above set of tests nice and concise. Minor changes: - Fixing a minor issue in `with-metadata-data-cards` in which the macroexpansion logic would go bad if the user used a different symbol name for `base-card-id`. - Spelling and formatting issues in `metabase.pulse` Fixes #34777 * lint fix for #_{:clj-kondo/ignore [:unresolved-symbol]} (cherry picked from commit 6f0161a)
markbastian
added a commit
that referenced
this pull request
Jan 11, 2024
* Don't send results works at the card level When a user enables **Don't send if there aren't results** this was the previous behavior: - If no results appeared on any card, no emails was sent. This was expected. - If any card contained results, all cards were sent in the email, including those that had no results. This PR modifieds that behavior to only send those cards that contain results for a dashboard pulse. For example, if 5 cards are present on a dashboard and 3 have no results, only results for the 2 cards containing data are included in the pulse. Primary changes: - `metabase.pulse/execute-dashboard` now removes any card with a row count of 0 in its computed result if `skip_if_empty` is true for the pulse. - The addition of tests covering the matrix of tests where `skip_if_empty` is true or false, all cards contain data, some cards contain data, and no cards contain data. - The `with-skip-if-empty-pulse-result` makes the above set of tests nice and concise. Minor changes: - Fixing a minor issue in `with-metadata-data-cards` in which the macroexpansion logic would go bad if the user used a different symbol name for `base-card-id`. - Spelling and formatting issues in `metabase.pulse` Fixes #34777 * lint fix for #_{:clj-kondo/ignore [:unresolved-symbol]} (cherry picked from commit 6f0161a)
markbastian
added a commit
that referenced
this pull request
Jan 11, 2024
As a follow on to #37546, this fixes an issue in which the macroexpansion of `with-skip-if-empty-pulse-result` doesn't capture the scope of the arguments in `skip-if-empty-test`. The solution is just to _correctly_ wrap all of the `mt/id` functions in `mt/dataset`.
markbastian
added a commit
that referenced
this pull request
Jan 11, 2024
* Don't send results works at the card level (#37546) * Don't send results works at the card level When a user enables **Don't send if there aren't results** this was the previous behavior: - If no results appeared on any card, no emails was sent. This was expected. - If any card contained results, all cards were sent in the email, including those that had no results. This PR modifieds that behavior to only send those cards that contain results for a dashboard pulse. For example, if 5 cards are present on a dashboard and 3 have no results, only results for the 2 cards containing data are included in the pulse. Primary changes: - `metabase.pulse/execute-dashboard` now removes any card with a row count of 0 in its computed result if `skip_if_empty` is true for the pulse. - The addition of tests covering the matrix of tests where `skip_if_empty` is true or false, all cards contain data, some cards contain data, and no cards contain data. - The `with-skip-if-empty-pulse-result` makes the above set of tests nice and concise. Minor changes: - Fixing a minor issue in `with-metadata-data-cards` in which the macroexpansion logic would go bad if the user used a different symbol name for `base-card-id`. - Spelling and formatting issues in `metabase.pulse` Fixes #34777 * lint fix for #_{:clj-kondo/ignore [:unresolved-symbol]} (cherry picked from commit 6f0161a) * Fixing test dataset reference
markbastian
added a commit
that referenced
this pull request
Jan 11, 2024
As a follow on to #37546, this fixes an issue in which the macroexpansion of `with-skip-if-empty-pulse-result` doesn't capture the scope of the arguments in `skip-if-empty-test`. The solution is just to _correctly_ wrap all of the `mt/id` functions in `mt/dataset`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a user enables Don't send if there aren't results this was the previous behavior:
This PR modifieds that behavior to only send those cards that contain results for a dashboard pulse. For example, if 5 cards are present on a dashboard and 3 have no results, only results for the 2 cards containing data are included in the pulse.
Primary changes:
metabase.pulse/execute-dashboardnow removes any card with a row count of 0 in its computed result ifskip_if_emptyis true for the pulse.skip_if_emptyis true or false, all cards contain data, some cards contain data, and no cards contain data.with-skip-if-empty-pulse-resultmakes the above set of tests nice and concise.Minor changes:
with-metadata-data-cardsin which the macroexpansion logic would go bad if the user used a different symbol name forbase-card-id.metabase.pulseFixes #34777