Fix: Added organization membership sample data for API_ADMINISTRATOR_USER #3183
Conversation
…USER_EMAIL_ADDRESS user
WalkthroughThis pull request introduces five new administrator membership entries to the organization_memberships.json file and modifies the seedInitialData.ts file. The JSON file receives new entries with distinct organization IDs but a consistent fixed memberId and timestamps. In seedInitialData.ts, the unique UUID generation using uuidv7() is replaced with a fixed UUID for the administrator user. These changes ensure that the administrator user is consistently identified across the sample data seeding process. Changes
Sequence Diagram(s)sequenceDiagram
participant Seeder as DataSeeder
participant DB as Database
participant Membership as OrgMembership
Seeder->>DB: Check if administrator user exists
DB-->>Seeder: User not found
Seeder->>Seeder: Assign fixed admin userId ("0194e562-0a52-70f3-9e7b-2e789a5aebdc")
Seeder->>DB: Create administrator user with fixed userId
DB-->>Seeder: User created
Seeder->>DB: Append membership entries for admin across multiple organizations
DB-->>Seeder: Membership entries added
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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
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
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3183 +/- ##
====================================================
- Coverage 40.11% 40.11% -0.01%
====================================================
Files 455 455
Lines 33543 33542 -1
Branches 433 433
====================================================
- Hits 13457 13456 -1
Misses 20086 20086 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/plugins/seedInitialData.ts (1)
89-106: Consider adding validation for environment variables.While the implementation is correct, consider adding validation for the environment variables before using them to ensure they are properly set.
+ // Add at the beginning of the plugin function + const requiredEnvVars = [ + 'API_ADMINISTRATOR_USER_EMAIL_ADDRESS', + 'API_ADMINISTRATOR_USER_NAME', + 'API_ADMINISTRATOR_USER_PASSWORD' + ]; + + for (const envVar of requiredEnvVars) { + if (!fastify.envConfig[envVar]) { + throw new Error(`Missing required environment variable: ${envVar}`); + } + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
sample_data/organization_memberships.json(1 hunks)src/plugins/seedInitialData.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (2)
src/plugins/seedInitialData.ts (1)
87-88: LGTM! Good practice using a fixed UUID for consistent sample data.The use of a fixed UUID for the administrator user ensures consistent identification across the sample data, making it easier to track and manage administrator memberships.
sample_data/organization_memberships.json (1)
100-127: Verify organization IDs exist in the organizations table.The new entries reference organization IDs. We should verify these organizations exist in the sample data.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the referenced organization IDs exist in the organizations sample data echo "Checking for organization IDs in sample data..." ORGS_TO_CHECK=( "ab1c2d3e-4f5b-6a7c-8d9e-0f1a2b3c4d5e" "ab1c2d3e-4f5b-6a7c-8d9e-0f1a2b3c4d5f" "cd3e4f5b-6a7c-8d9e-0f1a-2b3c4d5e6f7a" "bc2d3e4f-5a6b-7c8d-9e0f-1a2b3c4d5e6f" ) for ORG_ID in "${ORGS_TO_CHECK[@]}"; do echo "Checking organization ID: $ORG_ID" rg -q "\"$ORG_ID\"" sample_data/organizations.json || echo "Warning: Organization ID $ORG_ID not found" doneLength of output: 1290
Organization IDs Verified in Organizations Table
The script output shows that all referenced organization IDs exist in sample_data/organizations.json, with no missing entries.
| { | ||
| "organizationId": "ab1c2d3e-4f5b-6a7c-8d9e-0f1a2b3c4d5e", | ||
| "memberId": "0194e562-0a52-70f3-9e7b-2e789a5aebdc", | ||
| "role": "administrator", | ||
| "createdAt": "2023-04-13T04:53:17.742Z", | ||
| "creatorId": "0194e562-0a52-70f3-9e7b-2e789a5aebdc" | ||
| }, | ||
| { | ||
| "organizationId": "ab1c2d3e-4f5b-6a7c-8d9e-0f1a2b3c4d5f", | ||
| "memberId": "0194e562-0a52-70f3-9e7b-2e789a5aebdc", | ||
| "role": "administrator", | ||
| "createdAt": "2023-04-13T04:53:17.742Z", | ||
| "creatorId": "0194e562-0a52-70f3-9e7b-2e789a5aebdc" | ||
| }, | ||
| { | ||
| "organizationId": "cd3e4f5b-6a7c-8d9e-0f1a-2b3c4d5e6f7a", | ||
| "memberId": "0194e562-0a52-70f3-9e7b-2e789a5aebdc", | ||
| "role": "administrator", | ||
| "createdAt": "2023-04-13T04:53:17.742Z", | ||
| "creatorId": "0194e562-0a52-70f3-9e7b-2e789a5aebdc" | ||
| }, | ||
| { | ||
| "organizationId": "bc2d3e4f-5a6b-7c8d-9e0f-1a2b3c4d5e6f", | ||
| "memberId": "0194e562-0a52-70f3-9e7b-2e789a5aebdc", | ||
| "role": "administrator", | ||
| "createdAt": "2023-04-13T04:53:17.742Z", | ||
| "creatorId": "0194e562-0a52-70f3-9e7b-2e789a5aebdc" | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding a schema validation for the JSON data.
To ensure data integrity, consider adding a JSON schema validation for the organization memberships data structure.
+ // Add a new file: sample_data/schemas/organization_memberships.schema.json
+ {
+ "$schema": "http://json-schema.org/draft-07/schema#",
+ "type": "array",
+ "items": {
+ "type": "object",
+ "required": ["organizationId", "memberId", "role", "createdAt", "creatorId"],
+ "properties": {
+ "organizationId": {
+ "type": "string",
+ "pattern": "^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$"
+ },
+ "memberId": {
+ "type": "string",
+ "pattern": "^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$"
+ },
+ "role": {
+ "type": "string",
+ "enum": ["administrator", "regular"]
+ },
+ "createdAt": {
+ "type": "string",
+ "format": "date-time"
+ },
+ "creatorId": {
+ "type": "string",
+ "pattern": "^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$"
+ }
+ }
+ }
+ }
There was a problem hiding this comment.
A better approach would be to update the src/utilities/loadSampleData.ts script to update the sample database so that the API_ADMINISTRATOR_USER is automatically made administrator for all organizations through an API call.
Hard coding the UUID in the data opens up security vulnerabilities.
Please change the logic.
| @@ -85,7 +84,8 @@ const plugin: FastifyPluginAsync = async (fastify) => { | |||
| "Administrator user does not exist in the database. Creating the administrator.", | |||
| ); | |||
|
|
|||
| const userId = uuidv7(); | |||
| // Administrator is assigned fixed UUIDv7 id to load the sample data with this id. | |||
| const userId = "0194e562-0a52-70f3-9e7b-2e789a5aebdc"; | |||
There was a problem hiding this comment.
This method hard codes a UUID. This is a security vulnerability.
There was a problem hiding this comment.
Yes sir, I understood
I will change the logic as you guided.
|
@palisadoes I am closing this PR, I will raise another PR with the approach you suggested to me. |
PratapRathi
deleted the
Fix/Add-org-membership-for-API_ADMINISTRATOR_USER-in-sample-DB-import
branch
What kind of change does this PR introduce?
This PR will add the
organization_membershipsrecords forAPI_ADMINISTRATOR_USER_EMAIL_ADDRESSuser.Issue Number:
Fixes #3178
Snapshots/Videos:
If relevant, did you update the documentation?
N/A
Summary
A fixed user_Id is assigned to
API_ADMINISTRATOR_USER_EMAIL_ADDRESSuser inseedInitialData.ts, using this id we added some more records inorganization_membershipstable. This fix and and show the organizations on login withAPI_ADMINISTRATOR_USER_EMAIL_ADDRESSafter loading sample data.Does this PR introduce a breaking change?
No
Checklist
CodeRabbit AI Review
Test Coverage
Other information
N/A
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Chores
These updates streamline administrative functions and promote greater system consistency and reliability for end-users.