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

✨ Enhancement: Update data CSV export format #1378

Merged

Conversation

ppratikcr7
Copy link
Collaborator

@ppratikcr7 ppratikcr7 commented Mar 19, 2024

This PR updates the data CSV export format as per this [format] (https://docs.google.com/spreadsheets/d/1qAy7SNklPBWTtJed0Bqc8IppZn5DsQ4nBRsjtP-wtm0/edit?usp=sharing) as per the ticket #1182 .

For the above format, we have updated the query to be build using MonitoredDecisionPoint table to capture all the mark calls. This will include both enrolled and excluded users data.

For the excluded user's groupId data, we had to add a column groupId in the IndividualExclusion table similar to what we have in the IndividualEnrollment table.

@ppratikcr7 ppratikcr7 self-assigned this Mar 19, 2024
@ppratikcr7 ppratikcr7 linked an issue Mar 19, 2024 that may be closed by this pull request
@ppratikcr7
Copy link
Collaborator Author

@amurphy-cl In the above export format shared by you, do we need to store na in stratifications column for the user's whose stratification data is not available, or should we keep it blank in the export csv?

Also, you have kept na data in Payloads column, but now that we store default payload names in the DB, we wont have any such na values. Correct me if I am getting this wrong?

Screenshot 2024-03-19 at 2 57 41 PM

MonitoredDecisionPoint,
'monitored',
'monitored.userId = individualEnrollment.userId AND monitored.site = decisionPoint.site AND monitored.target = decisionPoint.target'
.innerJoin(Experiment, 'experiment', 'experiment.id::text = "monitoredDecisionPoint"."experimentId"')
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are adding too many joins. We need to check the performance of this query in a production-like environment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, the next step is this. I and Saumya would be looking into this query for optimization.

@danoswaltCL danoswaltCL merged commit f054019 into release/v5.1 Apr 1, 2024
8 checks passed
@danoswaltCL danoswaltCL deleted the enhancement/update-export-format-new-issue1182 branch April 1, 2024 20:33
danoswaltCL added a commit that referenced this pull request May 13, 2024
* storing default condition payload values in db with conditions (#1309)

* storing default condition payload values in db with conditions

* version bump to v5.1.1

* code clean up review cmt

* removing old condition payload when new are added

* version bump to 5.1.1

---------

Co-authored-by: danoswaltCL <97542869+danoswaltCL@users.noreply.github.com>

* Bugfix/1365 assignedcondition null mark issue (#1366)

* proper null coalescing in mark call for assignedcondition

* make assignedCondition an optional value in MarkData object, as we can safely as assume null is the intended condition

* version bump

* 🐛 Hotfix: Fix for Exclusions logic(Simple & Within Subject Exps) and code optimization for mark call (#1324)

* bugfixes for exclusions logic and mark call opt

* removing extra call to fetch experiment

* Storing proper group exclusion and individual exclusion documents (#1343)

* correct group exclusion and individual exclusion docs with code optimization for assign call

* code optimizations for mark and assign call for exclusions

* resolved peer review comments for group exclusions bug

* version bump release branch

---------

Co-authored-by: danoswaltCL <97542869+danoswaltCL@users.noreply.github.com>

* Bugfix/1382 experiment type in assign response (#1384)

* add experimentType to IExperimentAssignmentv5 response

* changes to main.java

* remove extra ts lib experimentType logic

* spec update

* spec update dataservice

* version bump

* ✨ Enhancement: Update data CSV export format (#1378)

* bugfixes for exclusions logic and mark call opt

* correct group exclusion and individual exclusion docs with code optimization for assign call

* code optimizations for mark and assign call for exclusions

* updating data csv export format to capture each mark call

* resolve peer review comments to confirm workingGroup is defined

* fix for integration test cases with old DP keys

* use new relic env var for prod and staging (#1438)

* Bugfix/use new relic var (#1442)

* use new relic env var for prod and staging

* version bump 5.1.5

* switch default user role from cretor to reader (#1448)

* experiment list context chip issue is resolved

* send whole url string in email link (#1464)

* send whole url string in email link

* change version to 5.1.7 for pipeline

* snackbar for import and delete experiment (#1468)

* snackbar issue resolved for import and delete operation of experiment

* version bump

---------

Co-authored-by: danoswaltCL <97542869+danoswaltCL@users.noreply.github.com>

* Bugfix for consistent metrics statistics view (#1467)

* bugfix for metrics statistics view

* metrics consistent dictionary usage across stepper and details page

* bump to 5.1.9

---------

Co-authored-by: danoswaltCL <97542869+danoswaltCL@users.noreply.github.com>

* Merge down release hotfixes 5.0 into 5.1 (#1470)

* cherry-pick ea63855 (#1159)

Co-authored-by: pratik <ppratik.cr7@gmail.com>

* Hotfix/enrollment complete fix (#1161)

* cherry-pick ea63855

* bump version after enrollment complete  hotfix

---------

Co-authored-by: pratik <ppratik.cr7@gmail.com>

* Fix/version root only (#1163)

* revert, change version back to major-minor on root only

* add backend

* Fix/version root only (#1164)

* revert, change version back to major-minor on root only

* add backend

* commit the backend package.json

* no assignedCondition null in java lib (#1441)

* fix missing imports

* version bump 5.1.10

---------

Co-authored-by: pratik <ppratik.cr7@gmail.com>

* peer review comments to improve enrollment code testcases

* Resolved review comment on PR

* Disabled dp and condition table and Allow payload edit (#1473)

* disabled dp and condition table edit and allowed change in condition payload while enrolling

* same changes for factorial experiment

* removed unnecessary the code change

* Grey out the decision points and conditions/factors tables

* Revert "Grey out the decision points and conditions/factors tables"

This reverts commit 91c7494.

* Grey out the decision points and conditions/factors tables (recommit)

---------

Co-authored-by: Zack Lee <zlee@carnegielearning.com>

* add prefix to keys for cache lookup (#1477)

* add prefix to keys for cache lookup

* version bump

* ✨ Toggle for within-subjects experiment type support (#1471)

* toggle for within-subjects experiment type support

* version bump

---------

Co-authored-by: danoswaltCL <97542869+danoswaltCL@users.noreply.github.com>

* 🐛 Bugfix to overwrite monitored document for unused decision points (#1482)

* bugfix to overwrite monitored document for unused decision points

* unit test cases fixed for multiple monitored document getting stored for unused dp

* version bump

---------

Co-authored-by: danoswaltCL <97542869+danoswaltCL@users.noreply.github.com>

* ✨ Detailed Integration Testcases: Exclusion codes (#1433)

* detailed integration test cases for exclusion codes

* peer review comments to improve exclusion code testcases

* added missing mock experiments while resolving conflicts

* review comments fixed for enrollment code and other integration test cases

---------

Co-authored-by: Yagnik Hingrajiya <50392803+Yagnik56@users.noreply.github.com>
Co-authored-by: danoswaltCL <97542869+danoswaltCL@users.noreply.github.com>
Co-authored-by: Yagnik <yagnikhingrajiya56@gmail.com>
Co-authored-by: Ben Blanchard <bblanchard@carnegielearning.com>
Co-authored-by: Zack Lee <zlee@carnegielearning.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update export format
3 participants