-
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 include the creation, editing, validation of case study elements #4482
Conversation
…rately created answer collections
📝 WalkthroughWalkthroughThis pull request introduces modifications across multiple files related to answer collections, internationalization, and Cypress testing. The changes primarily focus on updating user notifications, test cases for answer collections, and internationalization messages. The modifications include updating the warning message for answer options, refactoring Cypress test commands for deleting answer collections, and expanding test coverage for selection and case study questions. Changes
Possibly related PRs
✨ Finishing Touches
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: 0
🧹 Nitpick comments (2)
cypress/cypress/e2e/D-questions-workflow.cy.ts (1)
1875-1900
: TODO comments provide clear roadmap for case study testing.The TODO comments effectively outline the test cases needed for case study elements, covering creation, validation, editing, and cleanup aspects.
Would you like me to help implement these test cases? I can generate the test code following the same patterns used in other question type tests.
cypress/cypress/fixtures/D-questions.json (1)
89-92
: LGTM! Test data updated with more diverse examples.The test data has been appropriately updated from vegetables to cities, with a clear description and a good mix of solutions.
Consider adding cities from different continents to make the test data more globally representative. For example:
- "solutionsNotChosen": ["Tokyo", "Berlin", "Rome", "Madrid"], + "solutionsNotChosen": ["Tokyo", "Berlin", "Cairo", "Sydney"],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/frontend-manage/src/components/resources/answerCollections/AnswerCollectionEditModal.tsx
(1 hunks)cypress/cypress/e2e/D-questions-workflow.cy.ts
(8 hunks)cypress/cypress/e2e/K-resources-workflow.cy.ts
(6 hunks)cypress/cypress/fixtures/D-questions.json
(1 hunks)cypress/cypress/support/commands.ts
(1 hunks)packages/i18n/messages/de.ts
(1 hunks)packages/i18n/messages/en.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/i18n/messages/en.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: cypress-run
- GitHub Check: check
- GitHub Check: build
- GitHub Check: Analyze (javascript)
- GitHub Check: SonarCloud
🔇 Additional comments (7)
apps/frontend-manage/src/components/resources/answerCollections/AnswerCollectionEditModal.tsx (1)
52-52
: LGTM! Message key updated to be more generic.The change from
answerOptionUsedAsSolution
toanswerOptionUsed
appropriately broadens the warning message context to cover both solution and case study usages.cypress/cypress/support/commands.ts (1)
198-200
: LGTM! Enhanced deletion verification.The added assertion improves test robustness by verifying that the answer collection is actually removed from the DOM after deletion.
cypress/cypress/e2e/K-resources-workflow.cy.ts (1)
293-293
: LGTM! Consistent use of enhanced deletion command.The changes improve test maintainability by using the centralized
cy.deleteAnswerCollection
command with built-in verification.Also applies to: 393-393, 952-952, 978-978
cypress/cypress/e2e/D-questions-workflow.cy.ts (2)
1531-1550
: LGTM! Well-structured test setup for selection questions.The test properly creates answer collections with different access levels and descriptions, setting up the required test data.
2292-2309
: LGTM! Thorough cleanup with database verification.The cleanup section properly removes test data and verifies the database state, ensuring no test artifacts remain.
packages/i18n/messages/de.ts (1)
2005-2006
: LGTM! Message key and content updated to be more inclusive.The message has been appropriately updated to include both solutions and case study elements, and the key has been renamed to reflect this broader scope.
Let's verify that the key is consistently updated across the codebase:
✅ Verification successful
Key update verified: All occurrences consistently use
answerOptionUsed
The message key has been properly updated across all relevant files including translations, frontend components, and tests. No instances of the old key remain in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usages of the old key. # Search for any remaining instances of the old key echo "Searching for old key 'answerOptionUsedAsSolution'..." rg "answerOptionUsedAsSolution" # Search for usages of the new key to ensure it's being used echo "Searching for new key 'answerOptionUsed'..." rg "answerOptionUsed"Length of output: 665
cypress/cypress/fixtures/D-questions.json (1)
97-100
: LGTM! Test data consistently updated with meal-related examples.The edited collection data has been appropriately updated with a diverse set of meal types, maintaining consistency with the previous changes.
klicker-uzh
|
Project |
klicker-uzh
|
Branch Review |
case-study-test-suite
|
Run status |
|
Run duration | 20m 26s |
Commit |
|
Committer | Julius Schlapbach |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
324
|
View all changes introduced in this branch ↗︎ |
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
🧹 Nitpick comments (2)
apps/frontend-manage/src/components/questions/manipulation/options/CaseStudyCriteriaFields.tsx (2)
116-123
: Add aria-label for better accessibility.While the data attribute is well-structured for testing, the button should have an aria-label to improve accessibility for screen readers.
<Button onClick={() => remove(index)} className={{ root: 'mt-6 h-8 w-8 items-center justify-center border-red-600 hover:border-red-600 hover:text-red-600', }} data={{ cy: `remove-criterion-${index}` }} + aria-label={t('manage.questionForms.removeCriterion')} > <FontAwesomeIcon icon={faTrashCan} /> </Button>
Line range hint
128-143
: Consider initializing with default values.While the data attribute is well-structured, initializing new criteria with undefined values might lead to validation issues. Consider providing default values that align with your validation rules.
onClick={() => push({ id: nanoid(), - name: undefined, - min: undefined, - max: undefined, - step: undefined, - unit: undefined, + name: '', + min: 0, + max: 100, + step: 1, + unit: '', }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/frontend-manage/src/components/questions/manipulation/options/CaseStudyCasesFields.tsx
(5 hunks)apps/frontend-manage/src/components/questions/manipulation/options/CaseStudyCriteriaFields.tsx
(6 hunks)cypress/cypress/e2e/D-questions-workflow.cy.ts
(8 hunks)cypress/cypress/fixtures/D-questions.json
(1 hunks)packages/graphql/src/services/questions.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: cypress-run
- GitHub Check: check
- GitHub Check: SonarCloud
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (14)
apps/frontend-manage/src/components/questions/manipulation/options/CaseStudyCriteriaFields.tsx (1)
49-49
: LGTM! Well-structured test attributes.The data attributes follow a consistent and clear naming pattern (
criterion-${index}-${fieldName}
), making the form fields easily targetable in Cypress tests.Also applies to: 66-66, 83-83, 98-98, 113-113
apps/frontend-manage/src/components/questions/manipulation/options/CaseStudyCasesFields.tsx (4)
78-78
: LGTM! Test attribute follows naming convention.The data-cy attribute follows the established pattern of using descriptive, hyphen-separated names with dynamic indices.
116-116
: LGTM! Test attribute follows naming convention.The data-cy attribute for case description follows the same pattern as the title attribute.
175-177
: LGTM! Test attributes for solution bounds follow naming convention.The data-cy attributes for lower and upper bounds follow a consistent pattern that clearly identifies the case, item, and criterion indices.
Also applies to: 193-195
218-218
: LGTM! Test attribute for add button follows naming convention.The data-cy attribute for the add new case button follows the established pattern.
packages/graphql/src/services/questions.ts (1)
217-219
: LGTM! Improved filter logic for case study items.The filter logic correctly identifies items that are no longer included in the case study's collection items.
cypress/cypress/fixtures/D-questions.json (3)
89-92
: LGTM! Updated selection question test data.The test data for selection questions has been updated with more realistic city-based examples.
97-100
: LGTM! Updated edited selection question test data.The test data for edited selection questions uses a consistent theme of meals.
102-314
: LGTM! Comprehensive test data for case study questions.The test data includes all necessary aspects of case study questions:
- Basic question metadata (title, content, explanation)
- Collection configuration
- Items and criteria
- Cases with descriptions
- Solutions with lower/upper bounds
- Edited versions of all data
cypress/cypress/e2e/D-questions-workflow.cy.ts (5)
1531-1550
: LGTM! Test setup for selection questions.The test properly creates answer collections with appropriate access levels and descriptions before running selection question tests.
1877-1896
: LGTM! Test setup for case study questions.The test properly creates answer collections for case study questions with appropriate access levels and descriptions.
1898-2062
: LGTM! Comprehensive case study creation test.The test thoroughly validates:
- Basic question creation
- Collection selection
- Item selection
- Criteria configuration
- Case creation and deletion
2226-2429
: LGTM! Thorough validation testing.The test covers all critical validation scenarios:
- Required fields
- Numeric constraints
- Solution bounds
- Criteria relationships
3131-3149
: LGTM! Proper test cleanup.The test properly cleans up all created answer collections and verifies the cleanup was successful.
|
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores