fix : add support to queries and mutations to update user profiles #3535
Conversation
|
Warning Rate limit exceeded@bandhan-majumder has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 34 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 (2)
WalkthroughThis pull request updates several localization JSON files to add, modify, and remove translation keys, especially in the "settings" and related sections. It also restructures GraphQL schema enums and introduces a new mutation ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as Settings Component
participant G as GraphQL Server
U->>S: Update profile details (e.g., address, phone, description)
S->>G: Call updateCurrentUser(input)
G-->>S: Return updated user details
S->>U: Display update confirmation
sequenceDiagram
participant U as User
participant D as ChangeLanguageDropdown Component
participant G as GraphQL Server
U->>D: Select new language
D->>G: Call updateCurrentUser(input: { naturalLanguageCode: selectedLanguage })
G-->>D: Return updated user details
D->>U: Notify language update success
Assessment against linked issues
Possibly related PRs
Poem
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
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
|
writing tests and fixing failing steps. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (11)
src/GraphQl/Queries/Queries.ts (2)
9-33: Minor code style issues detected by ESLint/Prettier.
Several lines exhibit spacing discrepancies (e.g., “Delete··” or “Replace········with······”). These formatting problems can be addressed by running Prettier with the--writeflag to keep the codebase consistent and maintainable.🧰 Tools
🪛 ESLint
[error] 9-9: Delete
··(prettier/prettier)
[error] 10-10: Delete
··(prettier/prettier)
[error] 11-11: Delete
··(prettier/prettier)
[error] 12-12: Replace
········with······(prettier/prettier)
[error] 13-13: Delete
··(prettier/prettier)
[error] 14-14: Delete
··(prettier/prettier)
[error] 15-15: Replace
········with······(prettier/prettier)
[error] 16-16: Delete
··(prettier/prettier)
[error] 17-17: Replace
········with······(prettier/prettier)
[error] 18-18: Delete
··(prettier/prettier)
[error] 19-19: Delete
··(prettier/prettier)
[error] 20-20: Delete
··(prettier/prettier)
[error] 21-21: Delete
··(prettier/prettier)
[error] 22-22: Delete
··(prettier/prettier)
[error] 23-23: Delete
··(prettier/prettier)
[error] 24-24: Delete
··(prettier/prettier)
[error] 25-25: Replace
········with······(prettier/prettier)
[error] 26-26: Delete
··(prettier/prettier)
[error] 27-27: Delete
··(prettier/prettier)
[error] 28-28: Replace
········with······(prettier/prettier)
[error] 29-29: Delete
··(prettier/prettier)
[error] 30-30: Replace
········with······(prettier/prettier)
[error] 31-31: Delete
··(prettier/prettier)
[error] 32-32: Delete
··(prettier/prettier)
[error] 33-33: Delete
··(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
35-35: Ensure bracket placement and formatting align with convention.
The final}is tracked as changed. Possibly a minor bracket realignment. Confirm that structural changes here are intentional and run automated formatting to resolve.🧰 Tools
🪛 ESLint
[error] 35-35: Insert
··(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
src/screens/UserPortal/Settings/Settings.tsx (5)
24-24: Remove unused type import.
InterfaceEventis imported but never used. Eliminating this from imports will reduce warnings and keep the code clean.- import type { InterfaceEvent } from 'components/EventManagement/EventAttendance/InterfaceEvents';🧰 Tools
🪛 ESLint
[error] 24-24: 'InterfaceEvent' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
88-98:removeEmptyFieldsfunction logic.
Filtering out null/blank fields before the GraphQL mutation is a great approach to avoid overwriting data with empty values. However, note that using([_, value]) => ...can raise warnings for an unused variable. You can rename_to_keyor simply ignore it to satisfy ESLint rules.- .filter(([_, value]) => + .filter(([key, value]) =>🧰 Tools
🪛 ESLint
[error] 89-90: Delete
⏎(prettier/prettier)
[error] 91-91: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 93-93: Insert
⏎··········(prettier/prettier)
[error] 93-93: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 94-94: Replace
··········value·!==·null·&&with············value·!==·null·&&⏎···········(prettier/prettier)
[error] 95-95: Replace
(typeof·value·!==·"string"·||·value.trim()·!==·"")with··(typeof·value·!==·'string'·||·value.trim()·!==·''),(prettier/prettier)
[error] 96-96: Insert
,(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
147-148: Preventing future dates is a good call.
It's helpful to log an error; you may also want user-facing feedback (like a tooltip or toast) explaining why the date remains unchanged.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
410-447: Home and work phone fields.
Adding granular phone fields increases clarity. Consider using a validation library to confirm phone number formats (e.g., with E.164 support).🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
24-24: Address code style & lint warnings.
You have several ESLint/Prettier warnings (unused vars, missing return type, quotes, etc.). Address them to maintain consistency and keep the CI pipeline green.Also applies to: 91-91, 366-366
🧰 Tools
🪛 ESLint
[error] 24-24: 'InterfaceEvent' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
src/components/ChangeLanguageDropdown/ChangeLanguageDropDown.tsx (1)
45-45: Prettier style fixes suggested.
Insert missing semicolons or commas per Prettier’s recommendations to keep code style uniform.Also applies to: 51-51
🧰 Tools
🪛 ESLint
[error] 45-45: Insert
;(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
src/components/UserPortal/UserProfile/UserAddressFields.tsx (1)
38-49: Consider adding aria-label for better accessibility.While the implementation is correct, consider adding aria-label attributes to improve screen reader support.
<Form.Control type="text" placeholder="Eg: lane 123, Main Street" id="address1" value={userDetails.addressLine1} onChange={(e) => handleFieldChange('addressLine1', e.target.value)} className={styles.cardControl} data-testid="inputAddress1" + aria-label={`${t('addressLine1')} input field`} />public/locales/zh/translation.json (1)
1123-1125: New Work and Home Phone Number Fields in Settings
The insertion of"workPhoneNumber": "工作电话号码。"and"homePhoneNumber": "家庭电话号码。"improves the settings section by including both work and home phone numbers. Confirm that these keys are used uniformly across all localization files and in forms where phone numbers are required.public/locales/hi/translation.json (1)
939-941: Address Fields in Member Detail: Check Consistency Across Sections.
The newly added keys"postalCode": "पोस्टल कोड","addressLine1": "पता पंक्ति 1", and"addressLine2": "पता पंक्ति 2"enhance the address details under thememberDetailsection. Note that in theorgUpdatesection, the postal code is translated as"डाक कोड". Consider aligning these translations to maintain consistency across sections, unless a deliberate distinction is intended.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
public/locales/en/translation.json(2 hunks)public/locales/fr/translation.json(3 hunks)public/locales/hi/translation.json(3 hunks)public/locales/sp/translation.json(3 hunks)public/locales/zh/translation.json(3 hunks)schema.graphql(4 hunks)src/GraphQl/Mutations/mutations.ts(1 hunks)src/GraphQl/Queries/Queries.ts(1 hunks)src/components/ChangeLanguageDropdown/ChangeLanguageDropDown.tsx(2 hunks)src/components/UserPortal/UserProfile/UserAddressFields.tsx(4 hunks)src/screens/UserPortal/Settings/Settings.tsx(15 hunks)src/utils/formEnumFields.ts(1 hunks)
🧰 Additional context used
🪛 ESLint
src/components/ChangeLanguageDropdown/ChangeLanguageDropDown.tsx
[error] 45-45: Insert ;
(prettier/prettier)
[error] 46-46: Delete ····
(prettier/prettier)
[error] 51-51: Insert ,
(prettier/prettier)
src/screens/UserPortal/Settings/Settings.tsx
[error] 24-24: 'InterfaceEvent' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 89-90: Delete ⏎
(prettier/prettier)
[error] 91-91: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 93-93: Insert ⏎··········
(prettier/prettier)
[error] 93-93: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 94-94: Replace ··········value·!==·null·&& with ············value·!==·null·&&⏎···········
(prettier/prettier)
[error] 95-95: Replace (typeof·value·!==·"string"·||·value.trim()·!==·"") with ··(typeof·value·!==·'string'·||·value.trim()·!==·''),
(prettier/prettier)
[error] 96-96: Insert ,
(prettier/prettier)
[error] 121-121: Insert ,
(prettier/prettier)
[error] 127-127: Insert ,
(prettier/prettier)
[error] 131-131: Replace tCommon('updatedSuccessfully',·{·item:·'Profile'·})·as·string with ⏎··········tCommon('updatedSuccessfully',·{·item:·'Profile'·})·as·string,⏎········
(prettier/prettier)
[error] 169-169: Insert ,
(prettier/prettier)
[error] 206-206: Insert ⏎··········
(prettier/prettier)
[error] 207-207: Replace ·········· with ············
(prettier/prettier)
[error] 208-208: Insert ··
(prettier/prettier)
[error] 209-209: Insert ··
(prettier/prettier)
[error] 210-210: Insert ··
(prettier/prettier)
[error] 211-211: Delete ··
(prettier/prettier)
[error] 286-286: Replace 'image/jpeg',·'image/png',·'image/gif' with ⏎································'image/jpeg',⏎································'image/png',⏎································'image/gif',⏎······························
(prettier/prettier)
[error] 292-292: Replace 'Invalid·file·type.·Please·upload·a·JPEG,·PNG,·or·GIF.' with ⏎··································'Invalid·file·type.·Please·upload·a·JPEG,·PNG,·or·GIF.',⏎································
(prettier/prettier)
[error] 297-297: Replace 'File·is·too·large.·Maximum·size·is·5MB.' with ⏎··································'File·is·too·large.·Maximum·size·is·5MB.',⏎································
(prettier/prettier)
[error] 366-366: Replace 'enter·new·password' with "enter·new·password"
(prettier/prettier)
src/GraphQl/Queries/Queries.ts
[error] 9-9: Delete ··
(prettier/prettier)
[error] 10-10: Delete ··
(prettier/prettier)
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Replace ········ with ······
(prettier/prettier)
[error] 13-13: Delete ··
(prettier/prettier)
[error] 14-14: Delete ··
(prettier/prettier)
[error] 15-15: Replace ········ with ······
(prettier/prettier)
[error] 16-16: Delete ··
(prettier/prettier)
[error] 17-17: Replace ········ with ······
(prettier/prettier)
[error] 18-18: Delete ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 20-20: Delete ··
(prettier/prettier)
[error] 21-21: Delete ··
(prettier/prettier)
[error] 22-22: Delete ··
(prettier/prettier)
[error] 23-23: Delete ··
(prettier/prettier)
[error] 24-24: Delete ··
(prettier/prettier)
[error] 25-25: Replace ········ with ······
(prettier/prettier)
[error] 26-26: Delete ··
(prettier/prettier)
[error] 27-27: Delete ··
(prettier/prettier)
[error] 28-28: Replace ········ with ······
(prettier/prettier)
[error] 29-29: Delete ··
(prettier/prettier)
[error] 30-30: Replace ········ with ······
(prettier/prettier)
[error] 31-31: Delete ··
(prettier/prettier)
[error] 32-32: Delete ··
(prettier/prettier)
[error] 33-33: Delete ··
(prettier/prettier)
[error] 35-35: Insert ··
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
src/components/ChangeLanguageDropdown/ChangeLanguageDropDown.tsx
[warning] Code style issues found. Run Prettier with --write to fix.
schema.graphql
[error] File is unauthorized to change/delete.
src/screens/UserPortal/Settings/Settings.tsx
[warning] Code style issues found. Run Prettier with --write to fix.
src/GraphQl/Queries/Queries.ts
[warning] Code style issues found. Run Prettier with --write to fix.
🪛 GitHub Check: CodeQL
src/screens/UserPortal/Settings/Settings.tsx
[warning] 242-242: DOM text reinterpreted as HTML
DOM text is reinterpreted as HTML without escaping meta-characters.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (26)
src/GraphQl/Queries/Queries.ts (1)
7-7: Recommend naming your queries consistently for clarity.
Renaming the query toCurrentUseris a good practice, making it easier to reference and debug.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
src/screens/UserPortal/Settings/Settings.tsx (6)
6-6: Migration toUPDATE_CURRENT_USER_MUTATIONlooks good.
Switching from the old mutation to the new one aligns with the updated backend schema for user settings. No immediate issues found.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
25-25: ImportingEventsAttendedByUser.
This import is expanded usage in the settings screen. If no references are made or if it’s unused, consider removing to avoid bundling unnecessary code.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
33-34: Component-level i18n initialization is correct.
Your usage ofuseTranslation('translation', { keyPrefix: 'settings' })helps keep translation keys well-scoped. Good job!🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
36-36: Track changes vs. initial state.
UsingisUpdatedto conditionally display the “Save” and “Reset” actions is a solid approach to avoid unintentional saves.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
48-69: Thoroughly expandeduserDetailsstate.
The new fields (e.g., home/work phone, additional address lines) align with your updated user profile specification. Ensure all relevant validations and constraints are documented and tested in your forms.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
357-373: Password field addition.
Ensure that password policies (minimum length, complexity, etc.) are validated. Otherwise, invalid passwords might be accepted.🧰 Tools
🪛 ESLint
[error] 366-366: Replace
'enter·new·password'with"enter·new·password"(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
src/components/ChangeLanguageDropdown/ChangeLanguageDropDown.tsx (2)
6-6: Switched toUPDATE_CURRENT_USER_MUTATION.
This correctly aligns with the new schema for changing language preferences.Also applies to: 34-34
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
42-46: Encapsulate language code in aninputobject.
PassingnaturalLanguageCodeas part ofinputis consistent with the new mutation signature. Looks good.🧰 Tools
🪛 ESLint
[error] 45-45: Insert
;(prettier/prettier)
[error] 46-46: Delete
····(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
src/components/UserPortal/UserProfile/UserAddressFields.tsx (2)
10-17: LGTM! Interface changes improve address field granularity.The updated interface provides better structure for address data by:
- Splitting address into addressLine1 and addressLine2
- Adding city and postalCode fields
- Using standardized countryCode instead of country
113-114: Ensure consistent case handling for country codes.The country code is being converted to lowercase before being stored. Verify that this aligns with the backend expectations.
src/utils/formEnumFields.ts (2)
202-267: LGTM! Standardized enum values improve consistency.The change to lowercase with underscores aligns with common enum naming conventions and matches the schema definitions.
296-309: Improved gender inclusivity with 'intersex' option.The update to gender options is more inclusive and aligns with modern best practices.
src/GraphQl/Mutations/mutations.ts (1)
83-114: LGTM! Comprehensive user update mutation.The UPDATE_CURRENT_USER_MUTATION provides a complete set of user fields and follows GraphQL best practices by:
- Using input types for mutation arguments
- Returning all relevant user fields
- Following consistent naming conventions
public/locales/zh/translation.json (2)
939-941: New Address Fields in Member Details Added
The addition of"postalCode","addressLine1", and"addressLine2"with their Chinese translations enhances the user address information in the memberDetail section. Ensure that these keys are consistently propagated and mapped in the application for displaying user addresses.
1143-1144: New Description Keys in Settings Section
The new keys"enterDescription": "输入描述"and"descripción": "描述"have been added. While"enterDescription"clearly invites user input, the key"descripción"appears to be using a Spanish term for description. Verify if this naming is intentional for a specific use case in the Chinese locale. If not, consider renaming it to a more generic key (e.g."description") for consistency across translations.public/locales/en/translation.json (3)
1123-1132: Enhancements in Settings: Phone Numbers, State, City, and Cancel Key
The updated settings object now includes"workPhoneNumber": "Work Phone No.","homePhoneNumber": "Home Phone No.", and changes the"state"value to"State"along with adding"city": "City"and"cancel": "Cancel". These changes help clarify the labels for users. Please make sure that these modifications correctly override any previous values and that the terms align with the overall UX vocabulary across the app.
1138-1139: New Address Line Keys in Settings
The addition of"addressLine1": "Address Line 1"and"addressLine2": "Address Line 2"provides more granularity in address inputs. Confirm that any frontend components referencing these keys are updated accordingly so that the display text is consistent.
1147-1148: New Description Keys in Settings
The new keys"description": "Description"and"enterDescription": "Enter Description"have been added in the settings section. This mirrors the changes in other locales. Verify that these keys are used in input forms and validation messages as intended.public/locales/hi/translation.json (2)
1126-1127: Addition of Work and Home Phone Number in Settings.
The updates introducing"workPhoneNumber": "कार्यस्थल का फ़ोन नंबर"and"homePhoneNumber": "घर का फ़ोन नंबर"are correctly implemented and improve the contact information display. Please ensure these entries are also updated in other locale files for uniformity.
1146-1147: Update to Description Fields in Settings.
The modified keys"enterDescription": "विवरण दर्ज करें"and"description": "विवरण"provide clearer instructions for users. Verify that the wording matches similar fields in other language files so that the overall user experience remains consistent.public/locales/fr/translation.json (3)
935-937: Moved Address Fields to Member Detail Section
The new keys"postalCode","addressLine1", and"addressLine2"have been added to thememberDetailsection. This change improves the logical grouping of address-related information by moving these keys from theorgList(where they were previously defined) to the user’s personal details.
1126-1127: New Contact Phone Number Keys in Settings
The addition of"workPhoneNumber"and"homePhoneNumber"in thesettingssection enhances the capture of user contact details. Please double-check that the translations are consistent across other locale files.
1148-1149: New Description Fields in Settings
The new keys"enterDescription"and"description"have been introduced in thesettingssection. Ensure their usage in the UI aligns with similar keys in other locales and that the language style remains consistent throughout the application.public/locales/sp/translation.json (2)
942-944: Addition of Address Fields in Member Detail
New keys"postalCode","addressLine1", and"addressLine2"have been added to thememberDetailsection. This mirrors the changes in the French locale and supports a more coherent user profile by centralizing address information. Confirm that these keys now replace the previous usage fromorgList.
1127-1130: Enhanced Contact and Description Fields in Settings
Thesettingssection now includes"workPhoneNumber","homePhoneNumber","enterDescription", and"description". These additions will allow for more detailed and accurate user information. Please verify that the Spanish translations and formatting match the design guidelines and are consistent with other language files.
| <h5>{tCommon('address')} :-</h5> | ||
| <UserAddressFields | ||
| tCommon={tCommon} | ||
| t={t} | ||
| handleFieldChange={handleFieldChange} | ||
| userDetails={userDetails} | ||
| /> | ||
| <Row className="mb-1"> | ||
| <Col lg={12}> | ||
| <Form.Label | ||
| htmlFor="description" | ||
| className={`${styles.cardLabel}`} | ||
| > | ||
| {t('description')} | ||
| </Form.Label> | ||
| <Form.Control | ||
| as="textarea" | ||
| id="description" | ||
| placeholder={t('enterDescription')} | ||
| value={userDetails.description} | ||
| onChange={(e) => | ||
| handleFieldChange('description', e.target.value) | ||
| } | ||
| className={`${styles.cardControl}`} | ||
| rows={3} | ||
| data-testid="inputDescription" | ||
| /> | ||
| </Col> | ||
| </Row> | ||
| {isUpdated && ( |
There was a problem hiding this comment.
description field addition.
A multiline text area is a useful addition. Consider adding length checks or sanitization if the backend environment requires it (e.g., to guard against malicious input).
Be mindful of potential XSS if user descriptions are displayed unescaped.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/screens/UserPortal/Settings/Settings.tsx (3)
24-24: Remove unused import.The
InterfaceEventtype is imported but never used in the code.-import type { InterfaceEvent } from 'components/EventManagement/EventAttendance/InterfaceEvents';🧰 Tools
🪛 ESLint
[error] 24-24: 'InterfaceEvent' is defined but never used.
(@typescript-eslint/no-unused-vars)
91-98: Add return type to removeEmptyFields function.The function is missing a TypeScript return type annotation.
-function removeEmptyFields(obj: typeof userDetails) { +function removeEmptyFields(obj: typeof userDetails): Partial<typeof userDetails> {🧰 Tools
🪛 ESLint
[error] 91-91: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 93-93: Insert
⏎··········(prettier/prettier)
[error] 93-93: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 94-94: Replace
··········value·!==·null·&&with············value·!==·null·&&⏎···········(prettier/prettier)
[error] 95-95: Replace
(typeof·value·!==·"string"·||·value.trim()·!==·"")with··(typeof·value·!==·'string'·||·value.trim()·!==·''),(prettier/prettier)
[error] 96-96: Insert
,(prettier/prettier)
291-299: Replace alerts with toast notifications.Using browser alerts for validation feedback is not consistent with the rest of the application which uses toast notifications.
if (!validTypes.includes(file.type)) { - alert('Invalid file type. Please upload a JPEG, PNG, or GIF.'); + toast.error('Invalid file type. Please upload a JPEG, PNG, or GIF.'); return; } if (file.size > maxSize) { - alert('File is too large. Maximum size is 5MB.'); + toast.error('File is too large. Maximum size is 5MB.'); return; }🧰 Tools
🪛 ESLint
[error] 292-292: Replace
'Invalid·file·type.·Please·upload·a·JPEG,·PNG,·or·GIF.'with⏎··································'Invalid·file·type.·Please·upload·a·JPEG,·PNG,·or·GIF.',⏎································(prettier/prettier)
[error] 297-297: Replace
'File·is·too·large.·Maximum·size·is·5MB.'with⏎··································'File·is·too·large.·Maximum·size·is·5MB.',⏎································(prettier/prettier)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/screens/UserPortal/Settings/Settings.tsx(15 hunks)
🧰 Additional context used
🪛 ESLint
src/screens/UserPortal/Settings/Settings.tsx
[error] 24-24: 'InterfaceEvent' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 89-90: Delete ⏎
(prettier/prettier)
[error] 91-91: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 93-93: Insert ⏎··········
(prettier/prettier)
[error] 93-93: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 94-94: Replace ··········value·!==·null·&& with ············value·!==·null·&&⏎···········
(prettier/prettier)
[error] 95-95: Replace (typeof·value·!==·"string"·||·value.trim()·!==·"") with ··(typeof·value·!==·'string'·||·value.trim()·!==·''),
(prettier/prettier)
[error] 96-96: Insert ,
(prettier/prettier)
[error] 121-121: Insert ,
(prettier/prettier)
[error] 127-127: Insert ,
(prettier/prettier)
[error] 131-131: Replace tCommon('updatedSuccessfully',·{·item:·'Profile'·})·as·string with ⏎··········tCommon('updatedSuccessfully',·{·item:·'Profile'·})·as·string,⏎········
(prettier/prettier)
[error] 169-169: Insert ,
(prettier/prettier)
[error] 206-206: Insert ⏎··········
(prettier/prettier)
[error] 207-207: Replace ·········· with ············
(prettier/prettier)
[error] 208-208: Insert ··
(prettier/prettier)
[error] 209-209: Insert ··
(prettier/prettier)
[error] 210-210: Insert ··
(prettier/prettier)
[error] 211-211: Delete ··
(prettier/prettier)
[error] 286-286: Replace 'image/jpeg',·'image/png',·'image/gif' with ⏎································'image/jpeg',⏎································'image/png',⏎································'image/gif',⏎······························
(prettier/prettier)
[error] 292-292: Replace 'Invalid·file·type.·Please·upload·a·JPEG,·PNG,·or·GIF.' with ⏎··································'Invalid·file·type.·Please·upload·a·JPEG,·PNG,·or·GIF.',⏎································
(prettier/prettier)
[error] 297-297: Replace 'File·is·too·large.·Maximum·size·is·5MB.' with ⏎··································'File·is·too·large.·Maximum·size·is·5MB.',⏎································
(prettier/prettier)
[error] 366-366: Replace 'enter·new·password' with "enter·new·password"
(prettier/prettier)
🔇 Additional comments (1)
src/screens/UserPortal/Settings/Settings.tsx (1)
567-588: Add validation and sanitization for description field.The description field should have length limits and input sanitization to prevent XSS attacks.
#!/bin/bash # Check if DOMPurify or similar sanitization library is used in the project rg -l "DOMPurify|xss|sanitize" package.json
|
Please change the title so that it's more searchable |
palisadoes
added
the
ignore-sensitive-files-pr
bandhan-majumder
changed the title
|
hi @palisadoes . I have done the refactoring for updating section. There are also things included in that page which gets rendered (ex. |
Create a suitable solution. Discuss it in the slack channels |
|
@palisadoes Sure. I will add/update queries if needed to match the requirement. |
for reference: current schema of event:
|
|
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (2)
src/screens/MemberDetail/MemberDetail.tsx (1)
295-299: Capture the file object instead of input value.
Using e.target.value for file uploads will not provide the File object. Retrieve the actual file from e.target.files[0] instead, ensuring the avatar upload flows correctly.- onChange={(e) => handleFieldChange('avatar', e.target.value)} + onChange={(e) => { + if (e.target.files && e.target.files.length > 0) { + handleFieldChange('avatar', e.target.files[0]); + } + }}schema.graphql (1)
2177-2188: UpdateUserInput Fields Using Legacy Enums
Within the definition of UpdateUserInput, the fields “educationGrade”, “employmentStatus”, and “maritalStatus” are still typed using the old enums (EducationGrade, EmploymentStatus, MaritalStatus). Since the new enums have been introduced and are used in other parts of the schema (e.g. in UserPg), these input fields should be updated accordingly. This change is required for API consistency and to avoid runtime type errors.
♻️ Duplicate comments (3)
src/screens/UserPortal/Settings/Settings.tsx (3)
259-271:⚠️ Potential issueSanitize user avatar URL to prevent XSS.
The avatar URL is rendered directly in the
srcattribute without sanitization.Apply this diff to prevent XSS:
src={ userDetails.avatar instanceof File ? URL.createObjectURL(userDetails.avatar) - : userDetails.avatar + : encodeURI(userDetails.avatar) }
364-373:⚠️ Potential issueEnhance password field security.
The password field lacks validation and strength requirements.
Apply this diff to improve security:
<Form.Control type="password" id="inputPassword" data-testid="inputPassword" placeholder="enter new password" + pattern="^(?=.*[A-Za-z])(?=.*\d)(?=.*[@$!%*#?&])[A-Za-z\d@$!%*#?&]{8,}$" + title="Password must be at least 8 characters long and include letters, numbers, and special characters" onChange={(e) => handleFieldChange('password', e.target.value) } className={`${styles.cardControl}`} /> +<Form.Text className="text-muted"> + Password must be at least 8 characters long and include letters, numbers, and special characters. +</Form.Text>
577-589:⚠️ Potential issueSanitize user description to prevent XSS.
The description field needs sanitization before display to prevent XSS attacks.
Add sanitization when displaying the description:
<Form.Control as="textarea" id="description" placeholder={t('enterDescription')} - value={userDetails.description} + value={userDetails.description ? DOMPurify.sanitize(userDetails.description) : ''} onChange={(e) => handleFieldChange('description', e.target.value) } className={`${styles.cardControl}`} rows={3} data-testid="inputDescription" />
🧹 Nitpick comments (9)
src/screens/UserPortal/Settings/SettingsMocks.ts (1)
12-12: Address the TODO comment about avatar handling.The TODO comment indicates that avatar handling is not implemented yet.
Would you like me to help implement the avatar handling functionality or create an issue to track this task?
src/GraphQl/Queries/Queries.ts (1)
10-34: Verify field order for readability.The fields are not alphabetically ordered, which can make it harder to scan and maintain. Consider reordering them alphabetically for better readability.
Apply this diff to reorder the fields:
query CurrentUser { currentUser { addressLine1 addressLine2 avatarMimeType avatarURL birthDate city countryCode createdAt description educationGrade emailAddress employmentStatus homePhoneNumber id isEmailAddressVerified maritalStatus mobilePhoneNumber name natalSex naturalLanguageCode postalCode role state updatedAt workPhoneNumber } }src/screens/UserPortal/Settings/Settings.tsx (1)
45-66: Consider using TypeScript interfaces for state.Define a TypeScript interface for the user details state to improve type safety and maintainability.
Add this interface definition:
interface UserDetails { addressLine1: string; addressLine2: string; emailAddress: string; birthDate: string | null; city: string; avatar: File | null; countryCode: string; description: string; educationGrade: string; employmentStatus: string; homePhoneNumber: string; maritalStatus: string; mobilePhoneNumber: string; name: string; natalSex: string; naturalLanguageCode: string; password: string; postalCode: string; state: string; workPhoneNumber: string; }public/locales/en/translation.json (1)
1128-1141: Check Consistency of “state” and “enterState” Labels
In the “settings” section, the keys for “state” and “enterState” have been updated to “State” and “Enter State”, respectively. However, note that in the “orgUpdate” section (lines around 905–913) the key “state” is still labeled “State / Province”. For a unified user experience, consider standardizing these labels across the application.public/locales/hi/translation.json (3)
939-944: New translation keys added in memberDetail for extended contact information
The new keys “mobilePhoneNumber”, “workPhoneNumber”, “homePhoneNumber”, “postalCode”, “addressLine1”, and “addressLine2” have been added to the memberDetail object to support more granular contact details. They appear accurate; however, note that the translation for “workPhoneNumber” here (“कार्य फ़ोन नंबर”) is slightly different from the one in other sections (e.g. in settings, it is “कार्यस्थल का फ़ोन नंबर”). Please review for consistency across the application if a unified terminology is preferred.
1129-1130: New phone number keys added in settings
The settings section now includes “workPhoneNumber” and “homePhoneNumber”. In settings, “workPhoneNumber” is translated as “कार्यस्थल का फ़ोन नंबर”, which differs from the memberDetail translation. Consider aligning these translations if they refer to the same concept for a consistent user experience.
1136-1136: Review translation for “cancel” in settings
The “cancel” key at line 1136 has been updated to “रद्द करें”. This translation is consistent with many other parts of the file; please confirm it meets the overall language style used in the application.public/locales/fr/translation.json (1)
1128-1147: New Settings Fields for Contact Information
In the "settings" section, the addition of keys for "workPhoneNumber", "homePhoneNumber", "enterDescription", and "description" helps streamline user settings. Please double-check that the punctuation (e.g. period at end of workPhoneNumber) is intentional for consistency in UI copy.public/locales/sp/translation.json (1)
1132-1137: Settings Update for Contact Information
In the "settings" section, the addition of "workPhoneNumber" and "homePhoneNumber" (along with corresponding descriptions "enterDescription" and "description") ensures that contact details are uniformly captured across languages. Double-check for any inconsistencies in phrasing compared to other locales.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
public/locales/en/translation.json(3 hunks)public/locales/fr/translation.json(3 hunks)public/locales/hi/translation.json(3 hunks)public/locales/sp/translation.json(3 hunks)public/locales/zh/translation.json(3 hunks)schema.graphql(6 hunks)src/GraphQl/Mutations/mutations.ts(1 hunks)src/GraphQl/Queries/Queries.ts(1 hunks)src/components/ChangeLanguageDropdown/ChangeLanguageDropDown.tsx(2 hunks)src/components/UserPortal/UserProfile/EventsAttendedByUser.tsx(1 hunks)src/screens/MemberDetail/MemberDetail.spec.tsx(6 hunks)src/screens/MemberDetail/MemberDetail.tsx(16 hunks)src/screens/MemberDetail/MemberDetailMocks.ts(3 hunks)src/screens/UserPortal/Settings/Settings.spec.tsx(7 hunks)src/screens/UserPortal/Settings/Settings.tsx(15 hunks)src/screens/UserPortal/Settings/SettingsMocks.ts(1 hunks)src/utils/languages.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/GraphQl/Mutations/mutations.ts
- src/components/ChangeLanguageDropdown/ChangeLanguageDropDown.tsx
🧰 Additional context used
📓 Learnings (2)
src/screens/UserPortal/Settings/Settings.spec.tsx (2)
Learnt from: NishantSinghhhhh
PR: PalisadoesFoundation/talawa-admin#3400
File: src/screens/UserPortal/UserScreen/UserScreen.spec.tsx:94-97
Timestamp: 2025-01-25T15:29:46.898Z
Learning: In test files, when using the useLocalStorage utility's setItem function, always provide both the prefix ('Talawa-admin' by default) and the key: setItem(prefix, key, value). Example: setItem('Talawa-admin', 'name', 'John Doe')
Learnt from: IITI-tushar
PR: PalisadoesFoundation/talawa-admin#2718
File: src/screens/UserPortal/Settings/Settings.spec.tsx:0-0
Timestamp: 2024-12-22T17:58:17.818Z
Learning: The matchMedia mock implementation in `Settings.spec.tsx` no longer includes the deprecated addListener and removeListener methods, opting solely for addEventListener and removeEventListener.
src/screens/MemberDetail/MemberDetail.spec.tsx (1)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2425
File: src/screens/MemberDetail/MemberDetail.test.tsx:100-100
Timestamp: 2024-11-12T10:40:58.654Z
Learning: In `src/screens/MemberDetail/MemberDetail.test.tsx`, using `await wait();` is acceptable to wait for the render to complete.
🪛 ESLint
src/screens/MemberDetail/MemberDetailMocks.ts
[error] 1-1: 'UPDATE_CURRENT_USER_MUTATION' is defined but never used.
(@typescript-eslint/no-unused-vars)
src/screens/UserPortal/Settings/Settings.spec.tsx
[error] 185-185: Delete ········
(prettier/prettier)
src/screens/MemberDetail/MemberDetail.tsx
[error] 52-52: 'setSelectedAvatar' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 84-84: 'refetchUserDetails' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 85-85: 'fetchMoreAssignedTags' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 142-142: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 145-145: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
src/screens/UserPortal/Settings/Settings.tsx
[error] 87-87: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 90-90: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 161-161: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 286-286: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Actions: PR Workflow
src/screens/UserPortal/Settings/Settings.spec.tsx
[warning] 1-1: Code style issues found in the above file. Run Prettier with --write to fix.
🔇 Additional comments (14)
src/screens/MemberDetail/MemberDetailMocks.ts (1)
15-42: LGTM! Comprehensive mock data structure.The mock data provides good test coverage with a complete set of user attributes, including address, contact information, and personal details. The structure aligns well with the GraphQL schema changes.
Also applies to: 74-101, 116-143
src/screens/UserPortal/Settings/SettingsMocks.ts (1)
4-43: LGTM! Well-structured mock data for different test scenarios.The mock data effectively covers various test scenarios:
- Successful user profile update
- Empty user profile
- Error handling for invalid input
- Reset functionality
Also applies to: 45-83, 85-123, 125-167, 169-183
src/screens/MemberDetail/MemberDetail.spec.tsx (1)
102-321: LGTM! Comprehensive test coverage.The test cases effectively cover:
- Rendering of UI elements
- Form input handling
- Reset functionality
- Error scenarios
- User profile updates
src/screens/UserPortal/Settings/Settings.spec.tsx (1)
331-434: LGTM! Excellent test coverage for edge cases.The test cases effectively cover important edge cases:
- Birth date validation preventing future dates
- Successful profile updates with toast notifications
- Error handling for invalid phone numbers
src/GraphQl/Queries/Queries.ts (1)
8-8: Name the query for better debugging and caching.Good practice to name the query as
CurrentUser. Named queries help with debugging and caching in Apollo Client.public/locales/zh/translation.json (1)
939-944: LGTM! Chinese translations added for new user profile fields.The translations are accurate and consistent with the English terms.
Also applies to: 1129-1131, 1148-1149
schema.graphql (4)
246-263: Schema Enum “UserEducationGrade” Introduced
A new enum “UserEducationGrade” with lowercase values has been added. Please ensure these schema changes have been properly authorized—note that previous reviews flagged unauthorized modifications to schema.graphql. Also, verify that clients using the previous uppercase values are updated accordingly.
267-271: New Enum “UserEmploymentStatus” Added
The new enum “UserEmploymentStatus” is introduced with values (full_time, part_time, unemployed). The naming appears consistent; please confirm that any usage in mutations or type definitions aligns with this new enum and that legacy references have been removed.
896-898: New Mutation “updateCurrentUser” Added
A new mutation “updateCurrentUser(input: MutationUpdateCurrentUserInput!): UserPg” has been introduced to handle user profile updates. Please double-check that its input type and the returned type align with the new user profile structure and that any necessary client-side adjustments are made.
674-681: 🛠️ Refactor suggestionTypo in “UserMaritalStatus” Enum
The new enum “UserMaritalStatus” is defined with values including “seperated”. Please note the likely typo: “seperated” should be “separated” (with an “a”) to maintain correct spelling and consistency with standard English usage. Consider the following diff:-enum UserMaritalStatus { - divorced - engaged - married - seperated - single - widowed -} +enum UserMaritalStatus { + divorced + engaged + married + separated + single + widowed +}Likely invalid or redundant comment.
public/locales/en/translation.json (1)
989-996: New Translation Keys for User Profile Fields Added
New keys for “mobilePhoneNumber”, “workPhoneNumber”, “homePhoneNumber”, “addressLine1”, “addressLine2”, and “postalCode” have been introduced in the “memberDetail” section. These keys align with the backend changes for extended user profiles. Verify that the labels (e.g. “Phone No” vs. “Mobile Phone No”) are consistent with other parts of the UI.public/locales/hi/translation.json (1)
1149-1150: Addition of description text keys in settings
The entries for “enterDescription” (“विवरण दर्ज करें”) and “description” (“विवरण”) have been added to the settings section. These keys are clear and straightforward. Ensure that their usage in the UI aligns with other similar descriptions across the project.public/locales/fr/translation.json (1)
935-940: User Profile Fields Added in memberDetail
New translation keys for "postalCode", "addressLine1", "addressLine2", "mobilePhoneNumber", "workPhoneNumber", and "homePhoneNumber" have been added under the "memberDetail" section. These updates correctly reflect the expanded user profile support and align with the backend changes.public/locales/sp/translation.json (1)
942-947: User Profile Fields Added in memberDetail
New keys "postalCode", "mobilePhoneNumber", "workPhoneNumber", "homePhoneNumber", "addressLine1", and "addressLine2" have been added to the "memberDetail" section. This update mirrors the enhancements in other locales and supports the expanded user profile features.
@palisadoes . Sure. I have refactored the code which are currently already implemented and removed which are not but was still there in the same component. (tags, events..). Currently working on the coderabbit's suggestions |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/screens/MemberDetail/MemberDetail.tsx (2)
83-84: Remove unused variables.The variables
refetchUserDetailsandfetchMoreAssignedTagsare defined but never used.- refetch: refetchUserDetails, - fetchMore: fetchMoreAssignedTags,🧰 Tools
🪛 ESLint
[error] 83-83: 'refetchUserDetails' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 84-84: 'fetchMoreAssignedTags' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
141-150: Add return type and rename unused parameter.The
removeEmptyFieldsfunction lacks a return type and uses an unused parameter name.-function removeEmptyFields(obj: typeof formState) { +function removeEmptyFields(obj: typeof formState): Record<string, unknown> { return Object.fromEntries( - Object.entries(obj).filter(([_, value]) => + Object.entries(obj).filter(([key, value]) => value !== null && value !== undefined && (typeof value !== 'string' || value.trim() !== '') ) ); }🧰 Tools
🪛 ESLint
[error] 144-144: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
schema.graphql(6 hunks)src/screens/MemberDetail/MemberDetail.tsx(16 hunks)src/screens/MemberDetail/MemberDetailMocks.ts(1 hunks)src/screens/UserPortal/Settings/Settings.spec.tsx(7 hunks)src/screens/UserPortal/Settings/Settings.tsx(15 hunks)
🧰 Additional context used
📓 Learnings (2)
src/screens/UserPortal/Settings/Settings.spec.tsx (2)
Learnt from: NishantSinghhhhh
PR: PalisadoesFoundation/talawa-admin#3400
File: src/screens/UserPortal/UserScreen/UserScreen.spec.tsx:94-97
Timestamp: 2025-01-25T15:29:46.898Z
Learning: In test files, when using the useLocalStorage utility's setItem function, always provide both the prefix ('Talawa-admin' by default) and the key: setItem(prefix, key, value). Example: setItem('Talawa-admin', 'name', 'John Doe')
Learnt from: IITI-tushar
PR: PalisadoesFoundation/talawa-admin#2718
File: src/screens/UserPortal/Settings/Settings.spec.tsx:0-0
Timestamp: 2024-12-22T17:58:17.818Z
Learning: The matchMedia mock implementation in `Settings.spec.tsx` no longer includes the deprecated addListener and removeListener methods, opting solely for addEventListener and removeEventListener.
src/screens/MemberDetail/MemberDetail.tsx (1)
Learnt from: bandhan-majumder
PR: PalisadoesFoundation/talawa-admin#3535
File: src/screens/MemberDetail/MemberDetail.tsx:399-414
Timestamp: 2025-02-10T03:48:14.082Z
Learning: In the talawa-admin application, password security (hashing/encryption) is handled on the backend side, while the frontend components only collect and pass the plain text password to the backend.
🪛 ESLint
src/screens/MemberDetail/MemberDetail.tsx
[error] 83-83: 'refetchUserDetails' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 84-84: 'fetchMoreAssignedTags' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 144-144: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
src/screens/UserPortal/Settings/Settings.tsx
[error] 92-92: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 288-288: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
🔇 Additional comments (15)
src/screens/UserPortal/Settings/Settings.spec.tsx (5)
92-94: LGTM: URL mock implementation.The URL mock is correctly implemented to handle avatar file uploads in tests.
132-162: Verify all form field assertions.The test case thoroughly validates all input fields and their values. However, consider adding assertions for validation error messages when invalid data is entered.
expect(screen.getByTestId('inputPassword')).toHaveValue('12qw!@QW'); + +// Add validation assertions +userEvent.type(screen.getByTestId('inputPhoneNumber'), 'invalid'); +expect(screen.getByText('Invalid phone number format')).toBeInTheDocument();
330-359: LGTM: Future date validation test.Good test coverage for preventing future dates in birth date field.
361-399: LGTM: Profile update success test.The test case properly verifies the success toast message after profile update.
401-433: LGTM: Error handling test.Good test coverage for error handling with invalid phone numbers.
src/screens/UserPortal/Settings/Settings.tsx (2)
269-273:⚠️ Potential issuePotential XSS vulnerability in avatar URL.
The avatar URL is directly rendered in the src attribute without sanitization.
-src={ - userDetails.avatar instanceof File - ? URL.createObjectURL(userDetails.avatar) - : userDetails.avatar -} +src={ + userDetails.avatar instanceof File + ? URL.createObjectURL(userDetails.avatar) + : encodeURI(userDetails.avatar) +}Likely invalid or redundant comment.
360-376: 🛠️ Refactor suggestionEnhance password field security.
The password field lacks validation and strength requirements.
<Form.Control type="password" id="inputPassword" data-testid="inputPassword" placeholder="enter new password" + pattern="^(?=.*[A-Za-z])(?=.*\d)(?=.*[@$!%*#?&])[A-Za-z\d@$!%*#?&]{8,}$" + title="Password must be at least 8 characters long and include letters, numbers, and special characters" onChange={(e) => handleFieldChange('password', e.target.value) } className={`${styles.cardControl}`} /> +<Form.Text className="text-muted"> + Password must be at least 8 characters long and include letters, numbers, and special characters. +</Form.Text>Likely invalid or redundant comment.
schema.graphql (2)
246-263: LGTM: Enum value standardization.The new enums (
UserEducationGrade,UserEmploymentStatus,UserMaritalStatus) follow a consistent lowercase naming convention.Also applies to: 267-271, 674-681
2177-2201: LGTM: Comprehensive user update input type.The
MutationUpdateUserInputtype includes all necessary fields for user profile updates with proper types.src/screens/MemberDetail/MemberDetailMocks.ts (6)
1-1: Imports are properly used
The import statements for UPDATE_CURRENT_USER_MUTATION and CURRENT_USER are now utilized in the mocks. This resolves the previously flagged issue about an unused import.Also applies to: 3-3
64-100: Repeated currentUser structure
This block mirrors the structure validated above (lines 15–41). The same schema checks and test coverage apply here.
[duplicate_comment, verify]
106-142: Third currentUser mock
Similar to previous mocks, ensure these fields match the updated schema and are tested.
[duplicate_comment, verify]
149-186: Review partial update fields in updateMock
The updateCurrentUser mutation sets many fields to null or empty strings. Verify the backend supports such partial updates without errors.Potential check:
✅ Verification successful
🏁 Script executed:
#!/bin/bash rg 'updateCurrentUser' -A 10Length of output: 3544
Partial Update Fields in Mocks Verified
The backend mutation for updateCurrentUser is defined to return all the same fields shown in the mocks. The search results from the GraphQL mutation file and schema indicate that the input type (MutationUpdateCurrentUserInput) accepts the partial updates (with null or empty strings) provided in the mocks. Therefore, the partial update fields in updateMock are consistent with the backend expectations and supported without errors.
• The mutation implementation in src/GraphQl/Mutations/mutations.ts returns the same fields as those defined in the mocks.
• The GraphQL schema (schema.graphql) uses MutationUpdateCurrentUserInput, supporting these kinds of partial values.
48-59: Ensure test coverage for unassigning user tags
This mock for UNASSIGN_USER_TAG appears correct, but confirm that tests cover the unassignUserTag response shape.You can run:
✅ Verification successful
🏁 Script executed:
#!/bin/bash rg 'unassignUserTag' -A 10Length of output: 25785
UnassignUserTag Response Shape and Test Coverage Verified
Based on our investigation:
• The mocks in both MemberDetailMocks (lines 48–59) and ManageTagMocks correctly define the response shape for UNASSIGN_USER_TAG by returning an object with the _id property.
• The GraphQL mutation in TagMutations.ts is consistent with the schema and returns only _id, matching what the mocks provide.
• The UI components (ManageTag.tsx and UnassignUserTagModal.tsx) rely on these mocks, indicating that tests using these mocks already simulate the expected response shape for unassigning user tags.No additional changes are required unless you want explicit assertions on the _id field in unit tests.
15-41: Verify that each user field exists in the GraphQL schema
Ensure that the fields (e.g., addressLine1, countryCode, birthDate, etc.) align with the actual schema for currentUser.You might run a script like:
✅ Verification successful
🏁 Script executed:
#!/bin/bash rg "addressLine1|addressLine2|avatarMimeType|avatarURL|birthDate|city|countryCode|description|educationGrade|emailAddress|employmentStatus|homePhoneNumber" -A 5Length of output: 65698
GraphQL Schema Fields Verified
- Confirmed that each field (addressLine1, addressLine2, avatarMimeType, avatarURL, birthDate, city, countryCode, description, educationGrade, emailAddress, employmentStatus, homePhoneNumber) is present in the GraphQL schema.
- The schema definitions (e.g., addressLine1: String, countryCode: Iso3166Alpha2CountryCode, birthDate: Date, educationGrade as a custom type, employmentStatus as a custom type, homePhoneNumber: PhoneNumber) align with the values shown in the MemberDetail mocks.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/screens/UserPortal/Settings/Settings.tsx (1)
468-517: Add phone number validation.The phone number fields lack proper format validation. Consider adding:
- International phone number format validation
- Country code validation
- Area code validation
Apply this diff to add phone number validation:
+const validatePhoneNumber = (number: string): boolean => { + // Matches international phone number format: +1234567890 + const phoneRegex = /^\+[1-9]\d{1,14}$/; + return phoneRegex.test(number); +}; const handleFieldChange = (fieldName: string, value: string): void => { + if (fieldName.includes('PhoneNumber')) { + if (value && !validatePhoneNumber(value)) { + toast.error('Please enter a valid phone number in international format (e.g., +1234567890)'); + return; + } + } setIsUpdated(true); setUserDetails((prevState) => ({ ...prevState, [fieldName]: value, })); };
🧹 Nitpick comments (4)
src/screens/UserPortal/Settings/Settings.tsx (3)
359-376: Enhance password field security.While password security is handled on the backend, consider adding client-side validation for better user experience:
- Password strength indicators
- Minimum length requirements
- Special character requirements
Apply this diff to enhance the password field:
<Form.Control type="password" id="inputPassword" data-testid="inputPassword" placeholder="enter new password" + pattern="^(?=.*[A-Za-z])(?=.*\d)(?=.*[@$!%*#?&])[A-Za-z\d@$!%*#?&]{8,}$" + title="Password must be at least 8 characters long and include letters, numbers, and special characters" onChange={(e) => handleFieldChange('password', e.target.value) } className={`${styles.cardControl}`} /> +<Form.Text className="text-muted"> + Password must be at least 8 characters long and include letters, numbers, and special characters. +</Form.Text>
163-188: Enhance avatar file validation.While the current implementation validates file type and size, consider adding:
- Image dimension validation
- Aspect ratio validation
- File name sanitization
Apply this diff to enhance the avatar validation:
const fileUploadHandler = (e: React.ChangeEvent<HTMLInputElement>): void => { const file = e.target?.files?.[0]; if (file) { const validTypes = ['image/jpeg', 'image/png', 'image/gif']; const maxSize = 5 * 1024 * 1024; // 5MB + // Validate file name + const sanitizedFileName = file.name.replace(/[^a-zA-Z0-9.-]/g, '_'); + if (sanitizedFileName !== file.name) { + toast.error('File name contains invalid characters.'); + return; + } if (!validTypes.includes(file.type)) { toast.error('Invalid file type. Please upload a JPEG, PNG, or GIF.'); return; } if (file.size > maxSize) { toast.error('File is too large. Maximum size is 5MB.'); return; } + // Validate image dimensions + const img = new Image(); + img.onload = () => { + const maxDimension = 1024; + if (img.width > maxDimension || img.height > maxDimension) { + toast.error(`Image dimensions must not exceed ${maxDimension}x${maxDimension} pixels.`); + return; + } + + // Update state only if validation passes + setUserDetails((prevState) => ({ + ...prevState, + avatar: file, + })); + setSelectedAvatar(file); + setIsUpdated(true); + }; + img.src = URL.createObjectURL(file); + return; } - // Update all states properly - setUserDetails((prevState) => ({ - ...prevState, - avatar: file, - })); - setSelectedAvatar(file); - setIsUpdated(true); } };
87-98: Add return type and rename unused parameter.The function implementation is correct but could be improved:
Apply this diff to enhance the function:
-function removeEmptyFields(obj: typeof userDetails) { +function removeEmptyFields(obj: typeof userDetails): Partial<typeof userDetails> { return Object.fromEntries( Object.entries(obj).filter( - ([_, value]) => + ([key, value]) => value !== null && value !== undefined && (typeof value !== 'string' || value.trim() !== ''), ), ); }🧰 Tools
🪛 ESLint
[error] 92-92: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
src/screens/MemberDetail/MemberDetail.tsx (1)
687-695: Optimize getLanguageName function.The function uses map incorrectly for finding a value. Use find instead for better performance.
Apply this diff to optimize the function:
export const getLanguageName = (code: string): string => { - let language = 'Unavailable'; - languages.map((data) => { - if (data.code == code) { - language = data.name; - } - }); - return language; + const language = languages.find((data) => data.code === code); + return language?.name ?? 'Unavailable'; };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/screens/MemberDetail/MemberDetail.tsx(16 hunks)src/screens/UserPortal/Settings/Settings.spec.tsx(7 hunks)src/screens/UserPortal/Settings/Settings.tsx(15 hunks)
🧰 Additional context used
📓 Learnings (2)
src/screens/UserPortal/Settings/Settings.spec.tsx (2)
Learnt from: NishantSinghhhhh
PR: PalisadoesFoundation/talawa-admin#3400
File: src/screens/UserPortal/UserScreen/UserScreen.spec.tsx:94-97
Timestamp: 2025-01-25T15:29:46.898Z
Learning: In test files, when using the useLocalStorage utility's setItem function, always provide both the prefix ('Talawa-admin' by default) and the key: setItem(prefix, key, value). Example: setItem('Talawa-admin', 'name', 'John Doe')
Learnt from: IITI-tushar
PR: PalisadoesFoundation/talawa-admin#2718
File: src/screens/UserPortal/Settings/Settings.spec.tsx:0-0
Timestamp: 2024-12-22T17:58:17.818Z
Learning: The matchMedia mock implementation in `Settings.spec.tsx` no longer includes the deprecated addListener and removeListener methods, opting solely for addEventListener and removeEventListener.
src/screens/MemberDetail/MemberDetail.tsx (1)
Learnt from: bandhan-majumder
PR: PalisadoesFoundation/talawa-admin#3535
File: src/screens/MemberDetail/MemberDetail.tsx:399-414
Timestamp: 2025-02-10T03:48:14.082Z
Learning: In the talawa-admin application, password security (hashing/encryption) is handled on the backend side, while the frontend components only collect and pass the plain text password to the backend.
🪛 Biome (1.9.4)
src/screens/MemberDetail/MemberDetail.tsx
[error] 120-120: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 ESLint
src/screens/UserPortal/Settings/Settings.tsx
[error] 92-92: '_' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 288-288: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (4)
src/screens/UserPortal/Settings/Settings.spec.tsx (3)
330-359: LGTM! Good test coverage for birth date validation.The test case effectively validates that future dates are not allowed for birth dates, ensuring data integrity.
361-399: LGTM! Good test coverage for profile update.The test case effectively validates the success path of profile updates, including proper toast notifications.
401-433: LGTM! Good test coverage for error handling.The test case effectively validates error handling for invalid phone numbers, ensuring proper error feedback.
src/screens/UserPortal/Settings/Settings.tsx (1)
571-592: LGTM! Description field is properly sanitized.The implementation uses DOMPurify.sanitize to prevent XSS attacks in the description field.
What kind of change does this PR introduce?
Refactoring of queries and mutations to support user profile updates
Issue Number:
Fixes #3522
Snapshots/Videos:
video:
https://www.loom.com/share/a50c0505678c4847a3a37e9baec31118
photo:
Regular user
Admin user
If relevant, did you update the documentation?
Not yet
Summary
Refactored user profile update queries and mutations along with translations support .
Does this PR introduce a breaking change?
Not sure
Checklist
CodeRabbit AI Review
Test Coverage
Other information
N/A
Have you read the contributing guide?
Summary by CodeRabbit
New Features
Bug Fixes
Chores