Data upload now considers group in upload when possible #1022
Conversation
WalkthroughThe changes involve an update to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Utils
User->>App: Initiate upload
App->>Utils: Call reformatInstrumentData()
Utils->>Utils: Retrieve currentGroup
Utils->>Utils: Set groupId = currentGroup.id
Utils-->>App: Return reformatted data
App-->>User: Upload complete
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
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
🧹 Outside diff range and nitpick comments (2)
apps/web/src/features/upload/utils.ts (2)
120-120: Document the root-level data behavior.Consider adding a comment to document what happens when
currentGroupis null (i.e., data is uploaded at root level without group association).+ // When currentGroup is null, data will be uploaded without group association (root-level data) groupId: currentGroup?.id,
Line range hint
108-125: Consider enhancing type safety for group handling.The function accepts
currentGroupas nullable but doesn't enforce group presence. Consider splitting this into two separate functions or using a discriminated union type to handle cases where group association is required vs. optional.Example approach:
type WithRequiredGroup = { currentGroup: Group; requireGroup: true; data: FormTypes.Data[]; instrument: UnilingualInstrumentInfo; }; type WithOptionalGroup = { currentGroup: Group | null; requireGroup: false; data: FormTypes.Data[]; instrument: UnilingualInstrumentInfo; }; export function reformatInstrumentData(params: WithRequiredGroup | WithOptionalGroup): UploadInstrumentRecordsData { // If requireGroup is true, TypeScript will ensure currentGroup is not null const { currentGroup, data, instrument } = params; // ... rest of the implementation }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/src/features/upload/utils.ts(1 hunks)
🔇 Additional comments (1)
apps/web/src/features/upload/utils.ts (1)
120-120: LGTM! Verify root-level data handling.
The change correctly includes the group ID during data upload, which will fix the data visibility issues in the user's data hub. The use of optional chaining safely handles the null case.
Let's verify the handling of root-level data (when currentGroup is null):
✅ Verification successful
Let's search more specifically for the handling of group data in the upload context and instrument records.
Based on the search results, I can now generate the final response since we have clear evidence of how group IDs are handled in the instrument records system.
Null groupId is properly handled throughout the system
The backend service (instrument-records.service.ts) is designed to handle both cases where groupId is present or null:
- Records can be created and queried without a group ID
- Group connection in the database is only established when
groupIdis provided - Export and query operations work correctly for both grouped and ungrouped data
The optional chaining in the upload utils is therefore correct and safe to use as is.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any special handling of root-level data or documentation about groupId being null
rg -l "root.*data|group.*null|null.*group" --type ts
Length of output: 917
Script:
#!/bin/bash
# Check how instrument records are handled with respect to groups
rg -A 5 "groupId|group.*id" apps/web/src/features/upload/ apps/api/src/instrument-records/
Length of output: 12182
2 participants
Fixes the issue of upload not including the users group onto the data when they upload it. Thus making it not visible to the users data hub.
resolves issue #1021
Summary by CodeRabbit
New Features
Bug Fixes
groupIdwas previously set toundefined, enhancing data integrity.Chores