Fix/120 case insensitive repo removal#121
Conversation
|
Hi. @anderdc |
anderdc
left a comment
There was a problem hiding this comment.
Keep the remove-path change — LOWER(repo_full_name) = LOWER(:name) is correct and matches the convention used by installation.deleted and the admin controller.
Drop the add-path change (repoFullName: repo.full_name.toLowerCase() in installation.handler.ts). The repos table is the canonical-case source of truth (see the comment at pulls.service.ts:50-51), and three sites match it exact-case: webhook.service.ts:85 (findOneBy({ repoFullName }), the registration gate), miners.service.ts:183/232 (LEFT JOIN repos r ON r.repo_full_name = p.repo_full_name), and pulls.service.ts:50. Lowercasing only the install-insert path, while pull_requests/issues keep GitHub's canonical casing, means any mixed-case repo would fail the webhook.service.ts:85 lookup and get silently skipped as 'not registered' — the same silent-ingestion-failure this PR is trying to prevent.
The case-insensitive removal alone captures the win. Normalizing stored casing would have to be done consistently across every write path and lookup, which is a much larger change and not what #120 needs.
|
Duplicate of #127 which is better scoped/implemented. Closing. |
Summary
Fixes a bug where removing a GitHub repository via webhook could silently fail if the repo name was stored with different casing than what the webhook payload sent. Two changes:
repoFullNameis now normalized to lowercase before insert, so all new rows are stored consistently.UPDATEquery now usesLOWER(repo_full_name) = LOWER(:name)viacreateQueryBuilder, making the lookup case-insensitive and matching rows regardless of how they were originally stored.Related Issues
Fixes #120
Type of Change
Testing
Org/Repo) is correctly matched and cleared when the webhook sendsorg/repo(all lowercase) or any other casing variant.Checklist