-
Notifications
You must be signed in to change notification settings - Fork 7
tweak: RN-1773: trim value and label attributes of option relation
#6530
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
base: master
Are you sure you want to change the base?
Conversation
| const uniqueOptionsCreated = new Map( | ||
| optionsCreated.map(option => [hashOption(option), option]), | ||
| ).values(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JavaScript Set uses reference equality here, and we can’t provide a custom hash function; so we achieve the same effect with a Map
| const sortOrder = | ||
| existingOption?.sort_order ?? | ||
| ((await models.option.getLargestSortOrder(optionSetId)) ?? 0) + 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing wrong with just using getLargestSortOrder every time; but this has the useless side effect of moving options from the middle of the sort order to the very end (and leaving a “gap”)
| await db.runSql(` | ||
| BEGIN; | ||
|
|
||
| -- Delete duplicates to ensure (option_set_id, TRIM(value)) is unique |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this step, TRIM()-ing option.value will cause 16 conflicts (as of 20 October 2025)
packages/database/src/migrations/20251020010249-TrimAndDedupeOptions-modifies-data.js
Show resolved
Hide resolved
| ROW_NUMBER() OVER ( | ||
| PARTITION BY "option_set_id", TRIM("value") | ||
| ORDER BY "id" | ||
| ) > 1 AS "is_duplicate" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeps the option with the “lowest” id (presumably the oldest entry); but this is arbitrary
|
bugbot run |
37247c3 to
584a575
Compare
75472d7 to
57dddae
Compare
57dddae to
284006c
Compare
RN-1773
Causes headaches with duplicate options in option sets.
Stack 🥞
This is PR 2 of 2 in a stack: