feat(event-handlers): introduce event handler system for GitProxy lifecycle events#1519
feat(event-handlers): introduce event handler system for GitProxy lifecycle events#1519Andreybest wants to merge 4 commits into
Conversation
✅ Deploy Preview for endearing-brigadeiros-63f9d0 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1519 +/- ##
==========================================
+ Coverage 90.10% 90.33% +0.22%
==========================================
Files 69 75 +6
Lines 5657 5957 +300
Branches 990 1053 +63
==========================================
+ Hits 5097 5381 +284
- Misses 541 556 +15
- Partials 19 20 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@Andreybest Could I vote for an additional phase? We have several custom actions that run when a push is first received and the proxy blocks it for review. This state is not really We also have custom actions that would work really well as plugin events. For example, we:
It would be great if plugins could also add data that can be rendered in the UI. This would be a hugely helpful change, as it would allow us to separate our custom code from the base git-proxy. Keeping the two in sync is painful at the moment. |
|
Hey @andypols ! Thanks for the suggestion, this seems like a good use cases for events. Will try to add mechanisms that would be able to support your use cases :) |
re-vlad
left a comment
re-vlad
left a comment
There was a problem hiding this comment.
Just a wish: while the code is well-documented, there's no external docs (README, docs, etc.) explaining how to use the event system. This should be added.
| "contactEmail": "", | ||
| "csrfProtection": true, | ||
| "plugins": [], | ||
| "eventHandlers": [], |
There was a problem hiding this comment.
The config schema shows the eventHandlers array, but there are no examples of how to configure event handlers in proxy.config.json. Consider to document example
There was a problem hiding this comment.
Added documentation for that in architecture.md. Thanks!
| if (!action.user && !action.userEmail) { | ||
| return undefined; | ||
| } | ||
| return { username: action.user, email: action.userEmail }; |
There was a problem hiding this comment.
EventDetails.user and repository must be immutable copies (using String()) of Action fields — not references. Otherwise, async handlers may see stale or modified data if Action is mutated later in the chain. This rather critical I believe, but easy to fix:
return {
username = action.user ? String(action.user) : undefined;
email = action.userEmail ? String(action.userEmail) : undefined;
}```.
There was a problem hiding this comment.
Both values are strings, and strings are copied by value, not by reference, thus why this is unnecessary.
| } from './types'; | ||
|
|
||
| const buildRepositoryContext = (action: Action): RepositoryContext => ({ | ||
| url: action.url, |
There was a problem hiding this comment.
The same problem like line 38, don't return references, return values instead:
name: action.repoName? String(action.repoName) : undefined,
url: String(action.url),
There was a problem hiding this comment.
Responded on comment above
Hey @andypols! Added a new phase - On "add data that can be rendered in the UI", not sure if this should be the part of this PR, since the current architecture of event hooks is to inform a plugin that something happened in a manner of "fire-and-forget". And as I understand this will also require changes to database schema itself. Please let me know if I understood what you have meant. :) |
3 participants
Implements #1449.
Introduces a lightweight, async, observer-only event system so external integrations (notifications, audit feeds) can react to push/pull lifecycle events without coupling to chain internals.
Event hooks can be added the same way as the
pluginsinconfig.proxy.jsonSupports 2 operations:
pullandpushWith 5 phases:
startedcompletederrorpermissionDeniedpendingReviewThis is not well documented, and not sure where to add info on that, would appreciate suggestions on that matter :)