feat: send post-processed data in reports #15953
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
betodealmeida
commented
Jul 29, 2021
| @@ -637,9 +632,7 @@ def get_data(self, pk: int) -> Response: | |||
| except (TypeError, json.decoder.JSONDecodeError): | |||
| form_data = {} | |||
|
|
|||
| return self.get_data_response( | |||
| command, viz_type=chart.viz_type, form_data=form_data | |||
There was a problem hiding this comment.
viz_type is also a field in form_data, so we just need to pass form_data around.
Codecov Report
@@ Coverage Diff @@
## master #15953 +/- ##
==========================================
+ Coverage 76.98% 77.05% +0.07%
==========================================
Files 988 988
Lines 52378 52378
Branches 6622 6622
==========================================
+ Hits 40322 40360 +38
+ Misses 11833 11795 -38
Partials 223 223
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
hughhhh
reviewed
Jul 29, 2021
| @@ -55,7 +55,7 @@ | |||
|
|
|||
| class Slice( | |||
| Model, AuditMixinNullable, ImportExportMixin | |||
| ): # pylint: disable=too-many-public-methods | |||
| ): # pylint: disable=too-many-public-methods, too-many-instance-attributes | |||
There was a problem hiding this comment.
we could just update the max attributes
https://github.com/apache/superset/blob/master/.pylintrc#L385
There was a problem hiding this comment.
Hmmmm, I'm not sure I want to change the settings for the whole codebase. I like being conservative.
hughhhh
reviewed
Jul 29, 2021
betodealmeida
force-pushed
the
send_pivoted_text_report
branch
from
30279d2
to
e0ea3aa
Compare
eschutho
approved these changes
Jul 30, 2021
opus-42
pushed a commit
to opus-42/incubator-superset
that referenced
this pull request
Nov 14, 2021
* feat: send post-processed data in reports * Fix tests and lint * Use enums * Limit Slack message to 4k chars
cccs-RyanS
pushed a commit
to CybercentreCanada/superset
that referenced
this pull request
Dec 17, 2021
* feat: send post-processed data in reports * Fix tests and lint * Use enums * Limit Slack message to 4k chars
QAlexBall
pushed a commit
to QAlexBall/superset
that referenced
this pull request
Dec 29, 2021
* feat: send post-processed data in reports * Fix tests and lint * Use enums * Limit Slack message to 4k chars
cccs-rc
pushed a commit
to CybercentreCanada/superset
that referenced
this pull request
Mar 6, 2024
* feat: send post-processed data in reports * Fix tests and lint * Use enums * Limit Slack message to 4k chars
mistercrunch
added
🏷️ bot
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.
SUMMARY
For text-based chart reports (pivot tables and regular tables) we want to send the data transformed, so that the user sees the same data they would see on the chart. This PR changes reports to use the new
ChartDataResultType.POST_PROCESSEDresult type for that.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Here's a simple table:
Here's how the new "text" report data format looks like in Slack:
Here's how it looks like in the email:
Here's a simple pivot table:
Here's how it looks like in Slack as "text":
And email:
The CSV is also pivoted, here's an example in the email notification:
Pivot v2 looks just like Pivot v1.
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION