Skip to content

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

Merged
danoswaltCL merged 5 commits intorelease/v5.1from
hotfix/exclusions-bug-mark-call-opt-issue1301
Mar 15, 2024
Merged

🐛 Hotfix: Fix for Exclusions logic(Simple & Within Subject Exps) and code optimization for mark call#1324
danoswaltCL merged 5 commits intorelease/v5.1from
hotfix/exclusions-bug-mark-call-opt-issue1301

Conversation

@ppratikcr7
Copy link
Copy Markdown
Contributor

@ppratikcr7 ppratikcr7 commented Feb 27, 2024

This PR resolves issue #1301

Below are the changes made in this PR.

For Simple experiment:

  1. So, basically the experiment level exclusions were not getting stored in DB as filteredExperiment returned from experimentLevelExclusionInclusion() was empty.
  2. Group Level exclusions were not getting stored in DB, as code block to fetch groupexperiments was checking for empty/invalid groups and returning empty experiments list. This code block was not required as we are already checking for experimentsWithInvalidGroupAndWorkingGroup() inside updateEnrollmentExclusion().
  3. Added conditions to check if group object has any keys or not for group level exclusions.
  4. Also, moved a try catch code snippet in the mark apis' exclusions code logic to be executed only in case of enrolling or enrollmentComplete experiment state, else its not required to be executed for all other states of the experiment.
  5. There was duplicate code for storing monitoredDecisionPoint and monitoredDecisionPointLog documents in both individual/group level exclusions and experiment level exclusions logic. The individual/group level exclusions' monitoredDecisionPointLog document didn't had the check for within-subjects. And the experiment level exclusions' monitoredDecisionPoint existing document was not fetched and always a new document was getting stored. So, basically I have used a single code snippet common for both after the if-else branch executes.
  6. Added a few comments in the exclusion code enum.
  7. Updated wrong variable names using 'Code Spell Checker' VSCode extension (ID: streetsidesoftware.code-spell-checker). Much helpful for oversight typos in variable naming.

For Within subjects experiment:

  1. REACHED_PRIOR exclusion was not getting set as consistencyRule is null for Within subjects experiments.
  2. Individual Enrollment document was not getting overwritten, so added a condition to check if the individualEnrollment is not present.

Please find attached document to verify the exclusions and their expected behaviour. Might take some time to verify all, in case of doubts, do let me know.

Note:

  1. Will be adding detailed test cases to check alll exclusion codes in a separate PR using a separate ticket. It's better if these changes go in release branch asap.

Comment thread backend/packages/Upgrade/src/api/services/ExperimentAssignmentService.ts Outdated
)

* 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
@danoswaltCL danoswaltCL merged commit 4d5d361 into release/v5.1 Mar 15, 2024
@danoswaltCL danoswaltCL deleted the hotfix/exclusions-bug-mark-call-opt-issue1301 branch March 15, 2024 15:33
danoswaltCL added a commit that referenced this pull request Mar 15, 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>

---------

Co-authored-by: Yagnik Hingrajiya <50392803+Yagnik56@users.noreply.github.com>
Co-authored-by: Pratik Prajapati <33730817+ppratikcr7@users.noreply.github.com>
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exclusions are not handled as expected in mark call and code needs optimization

3 participants