api: task: add projectId to tasks #2268
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
gioelecerati
force-pushed
the
gio/task/projectId
branch
from
8d98d37 to
ba44679
Compare
gioelecerati
force-pushed
the
gio/task/projectId
branch
from
45f2b28 to
30ea8a2
Compare
There was a problem hiding this comment.
LGTM but I think this is overcomplicating things grabbing the userId from several different places and re-fetching it from the DB all the time. I think it could be greatly simplified by not making arguments optional and just passing the user object around. The tasks logic should never execute without a user.
packages/api/src/task/scheduler.ts
Outdated
| inputAsset?: Asset, | ||
| outputAsset?: Asset, | ||
| userId?: string, | ||
| requesterId?: string, | ||
| ) { | ||
| const projectId = inputAsset?.projectId || user?.defaultProjectId; |
There was a problem hiding this comment.
Shouldn't this come from the request API key instead? i.e. req.project?.id || user.defaultProjectId (user is never null, should not ?)
There was a problem hiding this comment.
Makes sense, but in the clip case the owner is different than the requester - I could pass req.project.id everywhere but there, but I instead removed userId and I am passing the user object, passing the owner during the clip gen, how does it look?
There was a problem hiding this comment.
Seeing a bunch of test failure after addressing the comments tho, looking into it
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