Skip to content

refactor(maintainer): use unwrapJoin to safely parse github_installations relations#406

Merged
jakharmonika364 merged 2 commits into
Coder-s-OG-s:mainfrom
diksha78dev:fix/issue-390
Jun 24, 2026
Merged

refactor(maintainer): use unwrapJoin to safely parse github_installations relations#406
jakharmonika364 merged 2 commits into
Coder-s-OG-s:mainfrom
diksha78dev:fix/issue-390

Conversation

@diksha78dev

Copy link
Copy Markdown
Contributor

Summary

Replaces manual object casts of github_installations joins in detect.ts with the unwrapJoin helper. This ensures the relationship extraction is type-safe and resilient, preventing runtime errors if Supabase returns the inner join as an array instead of an object.

Type of Change

  • Bug fix
  • New feature
  • UI / UX improvement
  • Refactor
  • Documentation
  • Other

Related Issue

Closes #390

What was changed?

  • Imported unwrapJoin utility from @/lib/supabase/inner-join.
  • Refactored isUserMaintainer to extract the relation payload securely using unwrapJoin<{ uninstalled_at: string | null }> rather than enforcing strict object coercion.
  • Refactored listMaintainerInstalls by typing the raw github_installations property as unknown and implementing an initial map iteration to safely unwrap the data before executing the subsequent .filter() logic.

Screenshots

N/A

Checklist

  • My code follows the project structure and conventions
  • I tested this locally (npm run dev)
  • No hardcoded secrets or credentials
  • I have updated documentation if needed

@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

@diksha78dev is attempting to deploy a commit to the codersogs-3057's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Ayush-Patel-56 Ayush-Patel-56 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks right, matches process-pr-event.ts's pattern. But this still doesn't have a test for the array-shape case - every existing test in detect.test.ts mocks github_installations as an object, so the fix is unverified against the actual bug it's for. Can you add one mocking it as [{ uninstalled_at: null }]? Also, isUserMaintainer uses as any while listMaintainerInstalls uses a typed unknown cast - worth matching styles.

@Ayush-Patel-56 Ayush-Patel-56 added the Needs author reply Author need to reply label Jun 23, 2026
@diksha78dev

Copy link
Copy Markdown
Contributor Author

@Ayush-Patel-56 I have added the requested changes and updated the detect.test.ts .Kindly review my pr and let me know anything needs to fix i'd be happy to resolve them.

@jakharmonika364 jakharmonika364 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array-shape tests added for both functions, cast styles unified. LGTM

@jakharmonika364 jakharmonika364 added level:critical Critical level difficulty quality:exceptional Exceptional quality contribution type:refactor Code refactor gssoc:approved Approved by GSSOC admin mentor:Ayush-Patel-56 Replace Ayush-Patel-56 with mentor's GitHub handle to credit them nsoc26 SSoC26 level3 Hard and removed Needs author reply Author need to reply labels Jun 24, 2026
@jakharmonika364 jakharmonika364 merged commit c1f40c2 into Coder-s-OG-s:main Jun 24, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc:approved Approved by GSSOC admin Hard level:critical Critical level difficulty level3 mentor:Ayush-Patel-56 Replace Ayush-Patel-56 with mentor's GitHub handle to credit them nsoc26 quality:exceptional Exceptional quality contribution SSoC26 type:refactor Code refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

detect.ts uses manual join casts instead of the existing unwrapJoin helper

3 participants