-
Notifications
You must be signed in to change notification settings - Fork 14
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
chore(cypress): extend test suite to cover creation and editing of selection questions #4438
Conversation
Warning Rate limit exceeded@sjschlapbach has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 50 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request introduces a new selection question type to the application, enhancing the question creation and management workflow. The changes span multiple files across the frontend, testing, internationalization, and data layers. Key modifications include adding user notifications for collection requirements, updating Cypress test coverage for selection questions, introducing German and English localization messages, and expanding the test data for answer collections. 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 (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/frontend-manage/src/components/questions/manipulation/options/SelectionOptions.tsx (1)
149-166
: Consider adding ARIA attributes for better accessibility.While the implementation is functionally correct, consider enhancing accessibility for screen readers.
<div data-cy="choose-correct-answer-options"> <Select isClearable isMulti value={selectedAnswers} options={collectionAnswers} classNames={{ container: () => 'w-full', }} + aria-label={t('manage.questionForms.selectAnswerOptions')} onChange={(newValue) => helpers.setValue(newValue.map((tag) => tag.value)) } placeholder={t('manage.questionForms.selectAnswerOptions')} noOptionsMessage={() => t('manage.questionForms.noMatchingOptionFound') } /> </div>
cypress/cypress/e2e/K-resources-workflow.cy.ts (1)
219-220
: Track implementation of missing test cases.Several TODO comments indicate missing test coverage for:
- Verifying answer collections availability for selection questions
- Checking restricted answer catalogue usage
- Verifying no answer catalogue message
Would you like me to create GitHub issues to track these test implementations?
Also applies to: 378-379, 388-389
packages/i18n/messages/de.ts (1)
997-998
: Fix punctuation and quotes for consistency.The German translation should match the English version's punctuation and quote style.
Apply this diff to fix the inconsistencies:
- SEAnswerCollectionRequired: - "Zur Erstellung von Auswahl-Fragen benötigen Sie Zugriff auf mindestens eine Antwort-Sammlung!. Sie können diese entweder unter dem Reiter 'Ressourcen' selbst erstellen oder dort eine bestehende Sammlung anderer Nutzer importieren.", + SEAnswerCollectionRequired: + 'Zur Erstellung von Auswahl-Fragen benötigen Sie Zugriff auf mindestens eine Antwort-Sammlung! Sie können diese entweder unter dem Reiter "Ressourcen" selbst erstellen oder dort eine bestehende Sammlung anderer Nutzer importieren.',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/frontend-manage/src/components/questions/manipulation/options/SelectionOptions.tsx
(5 hunks)cypress/cypress/e2e/D-questions-workflow.cy.ts
(2 hunks)cypress/cypress/e2e/K-resources-workflow.cy.ts
(3 hunks)packages/i18n/messages/de.ts
(1 hunks)packages/i18n/messages/en.ts
(1 hunks)packages/prisma/src/data/data/TEST.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
cypress/cypress/e2e/D-questions-workflow.cy.ts
[error] 1196-1196: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.
(lint/suspicious/noFocusedTests)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: cypress-run
- GitHub Check: check
- GitHub Check: build
- GitHub Check: Analyze (javascript)
- GitHub Check: SonarCloud
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (5)
apps/frontend-manage/src/components/questions/manipulation/options/SelectionOptions.tsx (2)
Line range hint
1-18
: LGTM! Clean imports and interface definition.The imports are well-organized and the component interface is clearly defined.
86-94
: LGTM! Good user feedback implementation.The warning notification provides clear feedback when no collections are available, improving the user experience.
packages/prisma/src/data/data/TEST.ts (1)
542-547
: LGTM! Test data entries are consistent.The new entries maintain the same structure and alphabetical order as existing entries.
cypress/cypress/e2e/D-questions-workflow.cy.ts (1)
1070-1117
: LGTM! Comprehensive test coverage for selection questions.The test cases thoroughly verify the creation and configuration of selection questions.
packages/i18n/messages/en.ts (1)
998-999
: LGTM! Clear and informative message.The message clearly explains the requirement for answer collections and provides actionable guidance for users on how to proceed.
…of collection availability
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
cypress/cypress/e2e/D-questions-workflow.cy.ts (1)
1196-1196
:⚠️ Potential issueRemove .only from the test case.
The focused test will prevent other tests from running. This is typically used during development and should be removed before merging.
- it.only('Verify that the edited state of the selection question persists', () => { + it('Verify that the edited state of the selection question persists', () => {
🧹 Nitpick comments (1)
cypress/cypress/e2e/D-questions-workflow.cy.ts (1)
1115-1116
: TODO: Implement preview verification test.The TODO comment indicates missing test coverage for the preview functionality of selection questions.
Would you like me to help implement the preview verification test or create a GitHub issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cypress/cypress/e2e/D-questions-workflow.cy.ts
(2 hunks)cypress/cypress/e2e/K-resources-workflow.cy.ts
(4 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 (7)
- GitHub Check: build
- GitHub Check: test
- GitHub Check: cypress-run
- GitHub Check: build
- GitHub Check: SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: check
🔇 Additional comments (3)
cypress/cypress/e2e/D-questions-workflow.cy.ts (3)
68-79
: LGTM! Well-structured constants for selection questions.The constants follow consistent naming patterns and provide clear, descriptive values for both initial and edited states.
1070-1216
: LGTM! Comprehensive test coverage for selection questions.The test cases thoroughly cover:
- Question creation with validation
- Content verification
- Sample solution management
- Editing functionality
- State persistence
The structure follows the established patterns in the test suite, making it maintainable and consistent.
1082-1106
: LGTM! Robust implementation with proper validations.The implementation follows Cypress best practices:
- Uses data-cy attributes for reliable element selection
- Includes appropriate wait times after actions
- Validates both UI state and data persistence
- Covers error cases with proper assertions
Also applies to: 1141-1151
…o ensure full functionality when running workflow multiple times
|
klicker-uzh
|
Project |
klicker-uzh
|
Branch Review |
selection-question-testing
|
Run status |
|
Run duration | 14m 47s |
Commit |
|
Committer | Julius Schlapbach |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
199
|
View all changes introduced in this branch ↗︎ |
klicker-uzh
|
Project |
klicker-uzh
|
Branch Review |
v3-new-elements
|
Run status |
|
Run duration | 15m 00s |
Commit |
|
Committer | Julius Schlapbach |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
189
|
View all changes introduced in this branch ↗︎ |
Summary by CodeRabbit
Release Notes
New Features
Improvements
Localization
Testing