access-control: added new playbackPolicy & webhook trigger #1665
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov Report
@@ Coverage Diff @@
## master #1665 +/- ##
===================================================
- Coverage 53.49990% 53.00491% -0.49499%
===================================================
Files 74 74
Lines 4843 4892 +49
Branches 939 952 +13
===================================================
+ Hits 2591 2593 +2
- Misses 1923 1967 +44
- Partials 329 332 +3
Continue to review full report in Codecov by Sentry.
|
| ); | ||
| } | ||
| const webhook = await db.webhook.get(content.playbackPolicy.webhookId); | ||
| if (!webhook) { |
There was a problem hiding this comment.
Also! We must check if the webhook can listen to the event playback.accessControl (or whatever). I know you haven't created it yet, but commenting here as a reminder.
There was a problem hiding this comment.
This is still needed 👀
And thinking about it, maybe we should allow fetching an account webhook of the playback.accessControl type, WDYT? So it defaults to idk the oldest active webhook for that event type, if you don't specify it in the playbackPolicy 🤔
| ); | ||
| } | ||
| const webhook = await db.webhook.get(content.playbackPolicy.webhookId); | ||
| if (!webhook) { |
There was a problem hiding this comment.
This is still needed 👀
And thinking about it, maybe we should allow fetching an account webhook of the playback.accessControl type, WDYT? So it defaults to idk the oldest active webhook for that event type, if you don't specify it in the playbackPolicy 🤔
| (playbackPolicy && asset.playbackPolicy?.type === "webhook") || | ||
| playbackPolicy?.type === "webhook" |
There was a problem hiding this comment.
Perhaps we actually need a helper like isPrivatePlaybackPolicy which does the !== public check, and use it here and in the other place where we choose the OS ID.
Here, the logic we should check is
| (playbackPolicy && asset.playbackPolicy?.type === "webhook") || | |
| playbackPolicy?.type === "webhook" | |
| isPrivatePlaybackPolicy(playbackPolicy) !== isPrivatePlaybackPolicy(asset.playbackPolicy) |
WDYT?
There was a problem hiding this comment.
Makes sense, I arrived late on this and now I don't want to touch too much, can open a separate PR
There was a problem hiding this comment.
Yeah do it. This is not only a nit, the functionality is incomplete (its ignoring jwt policies for example)
victorges
force-pushed
the
gio/feat/access-control
branch
from
b1d2663
to
607f2c1
Compare
What does this pull request do? Explain your changes. (required)
Specific updates (required)
How did you test each of these updates (required)
Does this pull request close any open issues?
Screenshots (optional)
Checklist