-
-
Notifications
You must be signed in to change notification settings - Fork 868
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
Develop : fixed birthdate update at user portal for all users #3497
Develop : fixed birthdate update at user portal for all users #3497
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 18 minutes and 45 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)
WalkthroughThe pull request introduces modifications to two React components: Changes
Possibly related PRs
Suggested reviewers
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
|
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)
src/screens/UserPortal/Settings/Settings.tsx (1)
261-266
: Fix template literal formatting.The template literal formatting doesn't match Prettier standards.
Apply this diff to fix the formatting:
- className={`d-flex flex-row ${styles.containerHeight} ${hideDrawer === null - ? '' - : hideDrawer - ? styles.expand - : styles.contract - }`} + className={`d-flex flex-row ${styles.containerHeight} ${ + hideDrawer === null + ? '' + : hideDrawer + ? styles.expand + : styles.contract + }`}🧰 Tools
🪛 ESLint
[error] 261-261: Insert
⏎··········
(prettier/prettier)
[error] 266-266: Delete
··
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
src/screens/MemberDetail/MemberDetail.tsx (1)
244-247
: Fix mutation input formatting.The mutation input structure is correct, but there's a missing comma after the object.
Apply this diff to fix the formatting:
data: { id: currentUrl, ...formState, - } + },🧰 Tools
🪛 ESLint
[error] 247-247: Insert
,
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/MemberDetail/MemberDetail.tsx
(2 hunks)src/screens/UserPortal/Settings/Settings.tsx
(4 hunks)
🧰 Additional context used
🪛 ESLint
src/screens/MemberDetail/MemberDetail.tsx
[error] 247-247: Insert ,
(prettier/prettier)
src/screens/UserPortal/Settings/Settings.tsx
[error] 261-261: Insert ⏎··········
(prettier/prettier)
[error] 266-266: Delete ··
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
src/screens/MemberDetail/MemberDetail.tsx
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/screens/UserPortal/Settings/Settings.tsx
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
🔇 Additional comments (4)
src/screens/UserPortal/Settings/Settings.tsx (3)
102-104
: LGTM! Consistent mutation input structure.The change standardizes the mutation input structure by wrapping
updatedUserDetails
within adata
object, aligning with the structure used in other components.🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
117-117
: LGTM! Enhanced error visibility.Adding error logging before error handling improves debugging capabilities.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
460-462
: LGTM! Proper birthdate validation.The implementation correctly handles birthdate changes and includes validation to prevent future dates.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
src/screens/MemberDetail/MemberDetail.tsx (1)
76-76
: LGTM! Proper initialization.Initializing birthdate as an empty string instead of a hardcoded value is the correct approach.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
working on the failing steps. |
@palisadoes PTAL |
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 (1)
src/screens/UserPortal/Settings/Settings.tsx (1)
461-463
: Optimize birthdate validation.The date validation in the onChange handler is redundant since
handleFieldChange
already includes date validation. Themax
attribute effectively prevents future date selection.- onChange={(e) => { - handleFieldChange('birthDate', e.target.value); - }} + onChange={(e) => handleFieldChange('birthDate', e.target.value)}🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/screens/UserPortal/Settings/Settings.tsx
(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: PR Workflow
src/screens/UserPortal/Settings/Settings.tsx
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
🔇 Additional comments (1)
src/screens/UserPortal/Settings/Settings.tsx (1)
102-104
: LGTM! Standardized mutation input structure.The change to wrap user details in a
data
object aligns with the structure used in other components, ensuring consistent mutation calls.Let's verify the mutation structure consistency across the codebase:
✅ Verification successful
✓ Mutation structure change is correct and consistent
The change to wrap user details in a
data
object aligns with:
- The mutation's internal structure in
mutations.ts
- Similar usage patterns in other components like
MemberDetail.tsx
- GraphQL schema expectations for the
updateUserProfile
mutation🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar UPDATE_USER_MUTATION usage patterns ast-grep --pattern 'updateUserDetails({ variables: { $$$ } })'Length of output: 444
Script:
#!/bin/bash # Search for UPDATE_USER_MUTATION definition ast-grep --pattern 'const UPDATE_USER_MUTATION = gql` $$$ `' # Find all imports and usages of UPDATE_USER_MUTATION rg "UPDATE_USER_MUTATION" -A 2Length of output: 4942
Script:
#!/bin/bash # View the mutation definition cat src/GraphQl/Mutations/mutations.tsLength of output: 15821
Script:
#!/bin/bash # View the mutation usage in MemberDetail.tsx rg -A 5 "updateUser\(" src/screens/MemberDetail/MemberDetail.tsxLength of output: 226
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3497 +/- ##
============================================
+ Coverage 3.70% 89.51% +85.81%
============================================
Files 315 337 +22
Lines 8270 8714 +444
Branches 1859 1937 +78
============================================
+ Hits 306 7800 +7494
+ Misses 7926 640 -7286
- Partials 38 274 +236
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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
🔭 Outside diff range comments (1)
src/screens/UserPortal/Settings/Settings.spec.tsx (1)
Line range hint
31-63
: Update the initial mock to use the new structure.The initial
MOCKS
array also needs to be updated to use the newdata
wrapper structure for consistency.Apply this diff to fix the mock data structure:
{ request: { query: UPDATE_USER_MUTATION, variables: { - firstName: 'Noble', - lastName: 'Mittal', - createdAt: '2021-03-01', - gender: 'MALE', - phoneNumber: '+174567890', - birthDate: '2024-03-01', - grade: 'GRADE_1', - empStatus: 'UNEMPLOYED', - maritalStatus: 'SINGLE', - address: 'random', - state: 'random', - country: 'IN', + data: { + firstName: 'Noble', + lastName: 'Mittal', + createdAt: '2021-03-01', + gender: 'MALE', + phoneNumber: '+174567890', + birthDate: '2024-03-01', + grade: 'GRADE_1', + empStatus: 'UNEMPLOYED', + maritalStatus: 'SINGLE', + address: 'random', + state: 'random', + country: 'IN', + }, }, result: { data: { updateUserProfile: { _id: '453', }, }, }, }, },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/UserPortal/Settings/Settings.spec.tsx
(1 hunks)src/screens/UserPortal/Settings/Settings.tsx
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/screens/UserPortal/Settings/Settings.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (1)
src/screens/UserPortal/Settings/Settings.spec.tsx (1)
132-148
: LGTM! The mutation variables structure has been updated correctly.The variables are now properly wrapped in a
data
object, which aligns with the API changes for user profile updates. The structure includes all necessary fields includingbirthDate
, which is relevant to the bug fix.
@palisadoes the check is failing as the base branch is not |
No. I meant, will this change affect the develop branch of Talawa Admin so that its code will have to be modified? It has implications for us running our cloud demo instance |
@palisadoes Sorry I did not get you. If you are asking for |
Thanks, that's what I wanted to know |
3104525
into
PalisadoesFoundation:develop
What kind of change does this PR introduce?
a bugfix
Issue Number:
#3405
Snapshots/Videos:
user profile updated successfully
https://www.loom.com/share/8b5ef692ca614679be9a46e3ed127550
admin and superadmin profile updated successfully
https://www.loom.com/share/6d258c571d304c9db097fd447b91249d?sid=bd718d23-595a-404a-9476-e25dd2934776
If relevant, did you update the documentation?
No
Summary
Fixed the mutation call.
Does this PR introduce a breaking change?
Not sure.
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores