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

Restrict startsWith comparisons in CheckboxTree more #1126

Merged
merged 2 commits into from
Sep 30, 2022

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Sep 27, 2022

While working on taxonomy dropdowns, I found a small 🐛 where checking user.biometric would also check user.biometric_health even though they are siblings to each other. This was because of some scrappy startsWith comparisons we were doing. This PR firms up that comparison so it is a little more sophisticated.

Code Changes

  • Compare against the value with a period instead of just the value
  • Update tests with a new case that shows this

Steps to Confirm

  • Visit the data category selector in a dataset and click "User > Biometric Data". It should no longer also check "User > Biometric Health Data"

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

Before:

Screen.Recording.2022-09-27.at.4.45.51.PM.mov

After:

Screen.Recording.2022-09-27.at.4.44.45.PM.mov

@allisonking allisonking marked this pull request as ready for review September 28, 2022 00:22
@allisonking allisonking requested a review from a team September 28, 2022 00:22
To fix a bug where checking user.biometric would also check user.biometric_health
@allisonking allisonking force-pushed the aking-fix-checkbox-tree-bug branch from cfb6f7f to 3c938fa Compare September 28, 2022 19:39
Copy link
Contributor

@ssangervasi ssangervasi left a comment

Choose a reason for hiding this comment

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

👩‍👧‍👧🥷

@ThomasLaPiana ThomasLaPiana merged commit 0cd4d77 into main Sep 30, 2022
@ThomasLaPiana ThomasLaPiana deleted the aking-fix-checkbox-tree-bug branch September 30, 2022 16:26
ssangervasi added a commit that referenced this pull request Oct 1, 2022
* main:
  Restrict startsWith comparisons in CheckboxTree more (#1126)
  [1076] ui/plus: Approve dataset classification button (#1129)
  Do not rely on order for checking intercept results (#1131)
  Prepare 1.9.1 release (#1137)
  Bump fideslang to 1.3.1 (#1136)
  Prepare changelog for 1.9.0 release (#1134)
  Update CHANGELOG.md (#1132)
  Fix bug causing missing datamap rows (#1124)
  cls migration (#1060)
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.

3 participants