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

Hotfix: fix export data #2141

Merged
merged 2 commits into from
Dec 5, 2024
Merged

Hotfix: fix export data #2141

merged 2 commits into from
Dec 5, 2024

Conversation

bcb37
Copy link
Collaborator

@bcb37 bcb37 commented Nov 26, 2024

This pr:

  • removes the queries to 'monitored_decision_point' and 'monitored_decision_point_log' and replaces multiple rows with different MarkExperimentPointTime values with a single row with MarkExperimentPointTime coming from the enrollment table
  • avoids including metrics columns that do not correspond to queries defined in the experiment
  • fixes the display of metric keys

@bcb37 bcb37 linked an issue Nov 26, 2024 that may be closed by this pull request
Copy link
Collaborator

@ppratikcr7 ppratikcr7 left a comment

Choose a reason for hiding this comment

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

@bcb37 So wanted to confirm one thing, you are now showing the attribute metric (timeSeconds) instead of the class (masteryWorkspace) for a grouped metric in the metrics column, right? Should we show the complete grouped metric (Class -> Key -> Attribute)? I have attached screenshot for other to see the difference. The old export gives 2 rows for each of the 2 students and the new one as per this PR gives 1 row for each of the 2 students. This is working perfectly as expected.

Screenshot 2024-11-27 at 6 38 28 PM

Also, didn't get the 2nd point of this PR description. Which metric column will not be shown? We already show the metrics that are used in the experiment. Can you please elaborate this scenario a bit. Thanks

@bcb37
Copy link
Collaborator Author

bcb37 commented Nov 27, 2024

Also, didn't get the 2nd point of this PR description. Which metric column will not be shown? We already show the metrics that are used in the experiment. Can you please elaborate this scenario a bit. Thanks

@ppratikcr7 it was including metrics columns from other experiments because the metrics list wasn't restricted to just the metrics in the current experiment. So I fixed that.
The screenshot is from a prod export of an experiment that does not have queries "Percent Graduated", "Swap", etc.

Screenshot 2024-11-27 at 2 42 34 PM

@ppratikcr7 ppratikcr7 self-requested a review November 29, 2024 04:21
ppratikcr7
ppratikcr7 previously approved these changes Nov 29, 2024
@bcb37 bcb37 merged commit 8f55f4d into release/6.0 Dec 5, 2024
14 checks passed
@bcb37 bcb37 deleted the hotfix/fix-export-data branch December 5, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix export data and formatting for 6.0
4 participants