update gitlab webhook flow #738
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #738 +/- ##
================================================
+ Coverage 96.14000 96.16000 +0.02000
================================================
Files 814 814
Lines 18441 18438 -3
================================================
Hits 17731 17731
+ Misses 710 707 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
nora-codecov
force-pushed
the
nora/366
branch
2 times, most recently
from
e068893 to
5132710
Compare
webhook_handlers/views/gitlab.py
Outdated
| ) | ||
| message = "User created" | ||
| elif event_name in ("project_rename", "project_transfer"): | ||
| self._try_initiate_sync_for_owner() |
There was a problem hiding this comment.
will this sync for both owners? do we need it to?
There was a problem hiding this comment.
so my current hypothesis is this - and i think we were thinking about it wrong before:
A user can only transfer a repo if they have access to the previous location and the new location (both OwnerOrgs).
So doing a sync on the OwnerUser making the transfer request should give us access to both projects (OwnerOrgs), and the sync process must be able to self-correct project/group structure and organization because that's the sync function we use when the user isn't seeing their repos correctly...
So I think this sync (on the OwnerUser) is able to initiate a sync for both OwnerOrgs :)
nora-codecov
force-pushed
the
nora/366
branch
2 times, most recently
from
fa46ff9 to
51d8b8c
Compare
There was a problem hiding this comment.
looks good!
i think the Owner model also has a bot column and that might give us another way to make sure we successfully sync: if owner_email, path_with_namespace, and old_path_with_namespace all point to Owners without oauth_token set, we can maybe check if they have bot set and use that bot
up to you if you want to look into that in this PR or merge the webhook validation stuff and follow up with that idea separately
Purpose/Motivation
Refine the flow, use webhook_secret if it is set on repo.
Links to relevant tickets
https://github.com/codecov/internal-issues/issues/359