fix: Minimize data while creating connection request #1486
+30
−40
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Ankita Patidar <[email protected]>
WalkthroughRepository method getInvitationDidByOrgId now returns a single agent_invitations record via findFirst with ordering, instead of an array via findMany. Service createConnectionInvitation aligns to consume a single object, simplifying variable naming and logging to debug. Other calls are reformatted without behavioral changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Service as ConnectionService
participant Repo as ConnectionRepository
participant DB as Database
Client->>Service: createConnectionInvitation(orgId, ...)
Service->>Repo: getInvitationDidByOrgId(orgId)
Repo->>DB: findFirst(agent_invitations where orgId) + orderBy
DB-->>Repo: agent_invitations (single record)
Repo-->>Service: agent_invitations
Service->>Service: Resolve connectionInvitationDid (invitationDid || legacyInvitationDid)
Service-->>Client: Return invitation details
note over Service: Logging reduced to debug
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
ankita-p17
changed the title
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/connection/src/connection.repository.ts (1)
376-387: Unify getInvitationDidByOrgId to return only invitationDid with null-safety
- In apps/connection/src/connection.repository.ts, apps/verification/src/repositories/verification.repository.ts and apps/issuance/src/issuance.repository.ts: change the method to
async getInvitationDidByOrgId(orgId: string): Promise<{ invitationDid: string } | null> { return this.prisma.agent_invitations.findFirst({ where: { orgId }, select: { invitationDid: true }, orderBy: { createDateTime: 'desc' } // verify: newest vs oldest }); }- Update all callers to handle a null return before dereferencing.
- Confirm with the business whether ordering should be
'desc'(newest) or'asc'(oldest).
🧹 Nitpick comments (1)
apps/connection/src/connection.service.ts (1)
538-541: Use template strings for debug logs.Second arg is treated as Logger context, not value. Interpolate values instead.
- this.logger.debug('connectionInvitationDid:', connectionInvitationDid); + this.logger.debug(`connectionInvitationDid: ${connectionInvitationDid}`); - - this.logger.debug(`logoUrl:::, ${organisation.logoUrl}`); + this.logger.debug(`logoUrl: ${organisation.logoUrl}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/connection/src/connection.repository.ts(2 hunks)apps/connection/src/connection.service.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/connection/src/connection.repository.ts (2)
apps/connection/src/connection.service.ts (2)
getConnectionRecords(103-112)deleteConnectionRecords(662-703)apps/connection/src/connection.controller.ts (1)
deleteConnectionRecords(100-103)
🔇 Additional comments (1)
apps/connection/src/connection.repository.ts (1)
348-360: Good optimization: selecting only required fields.Limiting fields in findMany aligns with the PR goal and reduces payload/IO. LGTM.
Comment on lines
+531
to
537
| let legacyInvitationDid; | ||
| if (IsReuseConnection) { | ||
| const data: agent_invitations[] = await this.connectionRepository.getInvitationDidByOrgId(orgId); | ||
| if (data && 0 < data.length) { | ||
| const [firstElement] = data; | ||
| legacyinvitationDid = firstElement?.invitationDid ?? undefined; | ||
|
|
||
| this.logger.log('legacyinvitationDid:', legacyinvitationDid); | ||
| } | ||
| const data: agent_invitations = await this.connectionRepository.getInvitationDidByOrgId(orgId); | ||
| legacyInvitationDid = data.invitationDid ?? undefined; | ||
| } | ||
| const connectionInvitationDid = invitationDid ? invitationDid : legacyinvitationDid; | ||
| const connectionInvitationDid = invitationDid ? invitationDid : legacyInvitationDid; | ||
|
|
There was a problem hiding this comment.
Fix potential runtime crash when no invitation exists.
getInvitationDidByOrgId can return null; dereferencing data.invitationDid will throw. Use null-safe access and avoid forcing the type.
Apply:
- let legacyInvitationDid;
- if (IsReuseConnection) {
- const data: agent_invitations = await this.connectionRepository.getInvitationDidByOrgId(orgId);
- legacyInvitationDid = data.invitationDid ?? undefined;
- }
- const connectionInvitationDid = invitationDid ? invitationDid : legacyInvitationDid;
+ let legacyInvitationDid: string | undefined;
+ if (IsReuseConnection) {
+ const data = await this.connectionRepository.getInvitationDidByOrgId(orgId);
+ legacyInvitationDid = data?.invitationDid ?? undefined;
+ }
+ const connectionInvitationDid = invitationDid ?? legacyInvitationDid;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let legacyInvitationDid; | |
| if (IsReuseConnection) { | |
| const data: agent_invitations[] = await this.connectionRepository.getInvitationDidByOrgId(orgId); | |
| if (data && 0 < data.length) { | |
| const [firstElement] = data; | |
| legacyinvitationDid = firstElement?.invitationDid ?? undefined; | |
| this.logger.log('legacyinvitationDid:', legacyinvitationDid); | |
| } | |
| const data: agent_invitations = await this.connectionRepository.getInvitationDidByOrgId(orgId); | |
| legacyInvitationDid = data.invitationDid ?? undefined; | |
| } | |
| const connectionInvitationDid = invitationDid ? invitationDid : legacyinvitationDid; | |
| const connectionInvitationDid = invitationDid ? invitationDid : legacyInvitationDid; | |
| let legacyInvitationDid: string | undefined; | |
| if (IsReuseConnection) { | |
| const data = await this.connectionRepository.getInvitationDidByOrgId(orgId); | |
| legacyInvitationDid = data?.invitationDid ?? undefined; | |
| } | |
| const connectionInvitationDid = invitationDid ?? legacyInvitationDid; |
🤖 Prompt for AI Agents
In apps/connection/src/connection.service.ts around lines 531 to 537, the call
to this.connectionRepository.getInvitationDidByOrgId(orgId) can return null so
dereferencing data.invitationDid may throw; update the code to check that the
returned value is not null (or use optional chaining) before accessing
invitationDid and avoid forcing the type—assign legacyInvitationDid =
data?.invitationDid ?? undefined (or explicitly handle the null return by
setting legacyInvitationDid = undefined when data is null) so the subsequent
connectionInvitationDid computation is safe.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is linked to the reported bug #1485
Fetched only relevant data from table to optimize the performance
Summary by CodeRabbit