fix(controllers): allow decline join request to remove outdated records #1630
Conversation
WalkthroughThe deployment workflows in GitHub Actions were updated to replace Heroku with Koyeb for service deployment and redeployment. Additionally, the congregation administration controller's user removal logic was changed to use congregation-specific removal instead of global deletion, and a join request deletion function no longer checks for user existence before declining the request. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Congregation Admin
participant Controller as congregation_admin_controller.ts
participant User as User Instance
participant Congregation as Congregation
Admin->>Controller: congregationDeleteUser(userId)
Controller->>User: removeCongregation()
User-->>Controller: (User removed from congregation)
Controller-->>Admin: Success/Failure response
sequenceDiagram
participant Admin as Congregation Admin
participant Controller as congregation_admin_controller.ts
participant Congregation as Congregation
Admin->>Controller: deleteJoinRequest(userId)
Controller->>Congregation: declineJoinRequest(userId)
Congregation-->>Controller: (Join request declined)
Controller-->>Admin: Success/Failure response
sequenceDiagram
participant GitHubActions
participant KoyebCLI
participant KoyebService
GitHubActions->>KoyebCLI: Install and authenticate with API token
GitHubActions->>KoyebCLI: Redeploy service (service name from secrets)
KoyebCLI->>KoyebService: Trigger redeployment
KoyebService-->>KoyebCLI: Deployment status
KoyebCLI-->>GitHubActions: Deployment result
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/deploy.yml (1)
42-48: Fix typo in step name and consider environment variable naming consistency.There's a typo in the step name "Deploy API to Koeyb" - it should be "Deploy API to Koyeb". Additionally, the environment variable names use "KOEYB" instead of "KOYEB", which is inconsistent with the actual service name.
Apply this diff to fix the typo:
- - name: Deploy API to Koeyb + - name: Deploy API to KoyebConsider whether the environment variable names should be updated to use "KOYEB" for consistency, or if "KOEYB" is intentionally used to match existing secret names.
.github/workflows/redeploy.yml (1)
22-28: Fix typo in step name and maintain consistency with deploy.yml.Same issue as in the deploy workflow - there's a typo in "Deploy API to Koeyb" which should be "Deploy API to Koyeb". The environment variable naming should also be consistent across both workflows.
Apply this diff to fix the typo:
- - name: Deploy API to Koeyb + - name: Deploy API to KoyebEnsure the environment variable naming is consistent with the deploy.yml workflow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/deploy.yml(2 hunks).github/workflows/redeploy.yml(1 hunks)src/v3/controllers/congregation_admin_controller.ts(1 hunks)
🔇 Additional comments (2)
src/v3/controllers/congregation_admin_controller.ts (2)
729-729: Excellent scope refinement for user removal.This change properly scopes the user removal to congregation-specific removal rather than global deletion. This is more appropriate for congregation admin functionality and prevents admins from accidentally deleting user accounts entirely.
913-915:✅ Verification successful
Verify error handling in downstream join request decline logic.
The removal of the user existence check aligns with the PR objective to "allow decline join request to remove outdated records." This enables cleanup of orphaned join requests where the user account may have been deleted but join requests remain.
Please verify that the
cong.declineJoinRequest(userId)method handles cases where the user doesn't exist gracefully, and that no unhandled errors will be thrown downstream.
🏁 Script executed:
#!/bin/bash # Description: Check the implementation of declineJoinRequest method to verify error handling # Expected: Method should handle non-existent users gracefully # Search for the declineJoinRequest method implementation ast-grep --pattern $'declineJoinRequest($userId) { $$$ }' # Also search for any error handling patterns in join request methods rg -A 10 "declineJoinRequest|decline.*join.*request" --type tsLength of output: 2028
DeclineJoinRequest handles missing users gracefully
I reviewed the implementation in src/v3/classes/Congregation.ts:
- The
declineJoinRequest(user)method filtersthis.join_requeststo remove any record matching the given user ID, regardless of whether that user still exists.- It also drops any stale requests whose
record.useris no longer found inUsersList.list.- The updated list is then saved via
setCongJoinRequestsand assigned back tothis.join_requests.Because it merely filters and persists the array, no errors are thrown if the passed-in
userIddoesn’t exist. No additional error handling is required here.
|
🎉 This PR is included in version 3.25.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.