Bugfix/1365 assignedcondition null mark issue#1366
Merged
danoswaltCL merged 3 commits intorelease/v5.1from Mar 14, 2024
Merged
Conversation
…n safely as assume null is the intended condition
bcb37
approved these changes
Mar 14, 2024
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>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
#1365
This was found in testing v5 Java Lib in Mathia. When no condition is returned by upgrade (aka 200 response but no not enrolled in any experiments), one would expect to be able to use
new MarkExperimentRequestData(site, target)but this will send an object withdata.assignedConditionas null, which is not currently allowed, we get a Bad RequestHowever, there's no good reason I can see to force this
data.assignedConditionto be required, if it's null but there's a site and target supplied, we can safely assume a condition ofnull.With a small adjustment in the null-coalescing code in the controller we can easily allow this to be an optional value and assume
nullfor the conditionCode.