fix(apps/frontend-manage): do not allow duplicate option values in answer collections #4449
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new validation rule for answer collections to ensure that all entries are unique. The changes span across multiple files, including the frontend component, Cypress tests, and internationalization (i18n) message files. The primary goal is to prevent duplicate entries when creating answer collections by adding a uniqueness check to the validation schema and providing appropriate error messages in German and English. Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…wed to be the same as any existing one
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
v3-new-elements
|
| Run status |
|
| Run duration | 16m 42s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
240
|
| View all changes introduced in this branch ↗︎ | |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/frontend-manage/src/components/resources/answerCollections/AddAnswerCollectionEntry.tsx (2)
54-54: Comprehensive form validation implementationThe validation setup effectively prevents duplicate values through:
- Immediate validation on mount
- Initial touched state for immediate feedback
- Clear error message for unique value requirement
However, consider adding input sanitization before the uniqueness check.
Consider adding value trimming to prevent spaces from creating false uniqueness:
newValue: Yup.string() .required(t('manage.resources.valueRequired')) + .transform((value) => value?.trim()) .notOneOf(entryValues, t('manage.resources.uniqueValuesRequired')),Also applies to: 58-58, 60-62
31-34: Consider case-insensitive comparison for better UXCurrently, the uniqueness check is case-sensitive, meaning "Option" and "option" would be considered different values. Consider implementing case-insensitive comparison for better user experience.
const entryValues = useMemo( - () => entries.map((entry) => entry.value), + () => entries.map((entry) => entry.value.toLowerCase()), [entries] ) newValue: Yup.string() .required(t('manage.resources.valueRequired')) + .transform((value) => value?.toLowerCase()) .notOneOf(entryValues, t('manage.resources.uniqueValuesRequired')),Also applies to: 60-62
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/frontend-manage/src/components/resources/answerCollections/AddAnswerCollectionEntry.tsx(3 hunks)apps/frontend-manage/src/components/resources/answerCollections/AnswerCollectionEditModal.tsx(1 hunks)cypress/cypress/e2e/K-resources-workflow.cy.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cypress/cypress/e2e/K-resources-workflow.cy.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run
🔇 Additional comments (3)
apps/frontend-manage/src/components/resources/answerCollections/AnswerCollectionEditModal.tsx (1)
70-70: LGTM: Proper prop passing with fallbackThe addition of the
entriesprop with a fallback to an empty array is a good defensive programming practice that enables duplicate value validation.apps/frontend-manage/src/components/resources/answerCollections/AddAnswerCollectionEntry.tsx (2)
7-7: LGTM: Clean type imports and prop interfaceThe addition of necessary imports and proper TypeScript typing for the entries prop ensures type safety.
Also applies to: 13-13, 18-18, 22-22
31-34: Good use of useMemo for performance optimizationThe memoization of entry values prevents unnecessary recalculations when the component re-renders, optimizing performance when the entries array is large.
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
deny-duplicate-answer-options
|
| Run status |
|
| Run duration | 16m 15s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
240
|
| View all changes introduced in this branch ↗︎ | |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/frontend-manage/src/components/resources/answerCollections/AnswerCollectionOption.tsx (1)
193-204: Consider enhancing button state feedback and accessibility.While the implementation is functional, consider these improvements:
- Add a loading spinner or indicator when
isSubmittingis true- Use ARIA attributes to communicate the button state to screen readers
- Ensure sufficient color contrast in the disabled state
<Button type="submit" className={{ root: twMerge( 'border-primary-80 hover:border-primary-80 bg-primary-100 h-8 w-8 items-center justify-center border text-white', - (isSubmitting || !isValid) && - 'bg-primary-60 cursor-not-allowed' + (isSubmitting || !isValid) && + 'bg-primary-60 cursor-not-allowed opacity-75' ), }} disabled={isSubmitting || !isValid} + aria-busy={isSubmitting} data={{ cy: 'save-edit-answer-option' }} > - <FontAwesomeIcon icon={faSave} /> + {isSubmitting ? ( + <FontAwesomeIcon icon={faSpinner} spin /> + ) : ( + <FontAwesomeIcon icon={faSave} /> + )} </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/frontend-manage/src/components/resources/answerCollections/AnswerCollectionEditModal.tsx(2 hunks)apps/frontend-manage/src/components/resources/answerCollections/AnswerCollectionOption.tsx(3 hunks)cypress/cypress/e2e/K-resources-workflow.cy.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cypress/cypress/e2e/K-resources-workflow.cy.ts
- apps/frontend-manage/src/components/resources/answerCollections/AnswerCollectionEditModal.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: check
- GitHub Check: test
- GitHub Check: cypress-run
- GitHub Check: SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (3)
apps/frontend-manage/src/components/resources/answerCollections/AnswerCollectionOption.tsx (3)
13-13: LGTM! Props and imports are well structured.The changes improve type safety with
otherEntriesarray and add proper i18n support.Also applies to: 16-16, 20-20, 28-28
126-136: Well-implemented validation logic!The Yup schema effectively prevents duplicate entries with proper i18n error messages. The form configuration ensures immediate validation on mount.
131-134: Verify otherEntries population in parent components.Let's ensure that
otherEntriesis properly populated by parent components to maintain the uniqueness constraint.✅ Verification successful
✓ Uniqueness validation is properly implemented
The
otherEntriesprop is correctly populated by filtering out the current entry's value from the collection and passing all other values for validation, ensuring uniqueness across all options.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components that render AnswerCollectionOption rg -A 5 "AnswerCollectionOption" apps/frontend-manage/src/ # Search for components that prepare otherEntries rg -A 5 "otherEntries" apps/frontend-manage/src/Length of output: 9276
|
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests