-
Notifications
You must be signed in to change notification settings - Fork 0
feat: enhance Club schema with additional contact fields #30
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
Conversation
- Added `additionalContacts`, `email`, and `resultServiceEmail` fields to the Club type in the GraphQL schema. - Updated input types `SaveClubInput` and `ModifyClubInput` to include `additionalContactIds`, `email`, and `resultServiceEmail`.
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
WalkthroughAdds optional fields (additionalContacts/additionalContactIds, email, resultServiceEmail) to Club and related inputs in the GraphQL schema; updates generated docs and doc asset payloads; introduces exported REGEX_EMAIL_FORMAT and uses it to validate emails in validation logic; adds unit tests for club email validation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Validation as ValidationModule
participant Schema
Client->>Validation: submit Club payload (email?, resultServiceEmail?, ...)
Validation->>Validation: validate fields using REGEX_EMAIL_FORMAT
alt valid
Validation-->>Schema: forward validated payload
Schema-->>Client: success / saved
else invalid
Validation-->>Client: "Ungültiges E-Mail-Format" error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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 (1)
schema.graphql (1)
76-78: Add validation for new Club email and contact fields in validation.tsThree new optional fields added to Club type and inputs. The generated TypeScript types are correct, but the validation layer is incomplete:
Current
checkClub()function (lines 129-173) validatesname,shortName,website, andaddress, but does not validate the new fields:
resultServiceEmail(AWSEmail scalar) should be validated for proper email formatadditionalContactIdsarray should validate that IDs are non-null stringsAdd email format validation for both email fields and array validation for
additionalContactIds:// Email validation if (club.email) { if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(club.email)) { result.addError('email', 'Ungültige E-Mail-Adresse'); } } if (club.resultServiceEmail) { if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(club.resultServiceEmail)) { result.addError('resultServiceEmail', 'Ungültige E-Mail-Adresse'); } } // Additional contacts validation if (club.additionalContactIds && club.additionalContactIds.length === 0) { result.addWarning('additionalContactIds', 'Leeres Array für zusätzliche Kontakte'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/generated/graphql.model.generated.tsis excluded by!**/generated/**
📒 Files selected for processing (5)
docs/assets/search.js(1 hunks)docs/types/Club.html(1 hunks)docs/types/ModifyClubInput.html(1 hunks)docs/types/SaveClubInput.html(1 hunks)schema.graphql(3 hunks)
🔇 Additional comments (6)
docs/assets/search.js (1)
1-1: Auto-generated search index updated.This file is auto-generated documentation artifact (search index) that has been updated to reflect the new schema fields. No review needed.
docs/types/Club.html (1)
1-13: Auto-generated documentation updated correctly.This TypeDoc-generated HTML documentation correctly reflects the three new Club fields (additionalContacts, email, resultServiceEmail) added in schema.graphql. The documentation structure and navigation have been properly updated.
schema.graphql (2)
266-268: SaveClubInput correctly uses IDs for input.The input type correctly uses
additionalContactIds: [ID](array of IDs) rather than full Person objects. This follows GraphQL best practices where:
- Input types accept IDs
- Output types return full objects
The email fields use
AWSEmailscalar which provides validation.
279-281: ModifyClubInput correctly mirrors SaveClubInput.The ModifyClubInput includes the same three new optional fields as SaveClubInput, maintaining consistency for create and update operations. All fields are optional, allowing partial updates.
docs/types/SaveClubInput.html (1)
1-10: Auto-generated documentation for SaveClubInput updated correctly.TypeDoc-generated documentation properly reflects the new optional input fields (additionalContactIds, email, resultServiceEmail) with correct types and navigation entries.
docs/types/ModifyClubInput.html (1)
1-10: Auto-generated documentation for ModifyClubInput updated correctly.TypeDoc-generated documentation properly reflects the new optional input fields, consistent with SaveClubInput and the schema changes.
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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)
test/validation.test.ts (2)
301-309: Consider adding positive test cases for email validation.The test correctly verifies that invalid email formats are rejected. Consider adding test cases for valid email formats and edge cases (e.g., valid emails, undefined/null values) to ensure the validation behaves correctly across all scenarios.
Example:
describe('email validation', () => { + it('should accept valid email format', () => { + const club = { + ...clubTemplateLong, + email: 'contact@example.com', + }; + expect(() => validateClub(club)).not.toThrow(); + }); + + it('should accept undefined email', () => { + const club = { + ...clubTemplateLong, + email: undefined, + }; + expect(() => validateClub(club)).not.toThrow(); + }); + it('should throw error for invalid email format', () => { const club = { ...clubTemplateLong, email: 'joasdohi', }; expect(() => validateClub(club)).toThrow('Ungültiges E-Mail-Format'); }); });
310-318: Consider adding positive test cases for result service email validation.Similar to the email validation tests, consider adding test cases for valid email formats and edge cases to ensure comprehensive coverage of the resultServiceEmail field validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/assets/navigation.js(1 hunks)docs/assets/search.js(1 hunks)docs/variables/REGEX_EMAIL_FORMAT.html(1 hunks)src/validation.ts(4 hunks)test/validation.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/variables/REGEX_EMAIL_FORMAT.html
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/assets/search.js
🧰 Additional context used
🧬 Code graph analysis (1)
test/validation.test.ts (1)
src/validation.ts (1)
validateClub(184-187)
🔇 Additional comments (5)
docs/assets/navigation.js (1)
1-1: Auto-generated documentation asset.This file contains base64-encoded navigation data that is auto-generated by the documentation tool. No review needed.
src/validation.ts (4)
3-4: LGTM! Good refactor to centralize email validation.Exporting
REGEX_EMAIL_FORMATpromotes consistency and maintainability across different validation functions. The regex pattern is appropriately permissive for general email validation without being overly complex.
160-167: LGTM! Email validation correctly implemented for new Club fields.The validation logic correctly:
- Treats email fields as optional (only validates if truthy)
- Uses the centralized
REGEX_EMAIL_FORMATfor consistency- Provides field-specific error messages
- Follows the established validation pattern
324-324: LGTM! Good refactor to use centralized email regex.Replacing the inline email regex with
REGEX_EMAIL_FORMATimproves consistency and maintainability.
383-383: LGTM! Consistent use of centralized email validation.Good refactor to use
REGEX_EMAIL_FORMATfor association email validation, maintaining consistency across the codebase.
additionalContacts,email, andresultServiceEmailfields to the Club type in the GraphQL schema.SaveClubInputandModifyClubInputto includeadditionalContactIds,email, andresultServiceEmail.Summary by CodeRabbit
New Features
Documentation
Tests