blitzjs.com
blitzjs.com copied to clipboard
docs(multitenancy): fix example update mutation
In the current example, the code snippet is invalid. update()
is limited to unique model fields, so the only way to execute this call is to add a unique constraint on id
and organizationId
. There's also updateMany()
, but I wouldn't use that as an example as it will scan all table rows and can hurt performance. To my knowledge Prisma doesn't yet support limiting updateMany()
to the first updated record.
Do let me know if explanation can be improved and if we should add more examples/info here.
This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/blitzjs/blitzjs-com/52BgfyKnrtwrPftEL93BN4ARf47Y
✅ Preview: https://blitzjs-com-git-fork-krszwsk-patch-1-blitzjs.vercel.app
Looks nice. One question: in this case, there could be multiple Projects with the same id
?
@JuanM04 No, what the query is aiming for is to verify that user can only update a project from current organisation.
~~We could also findFirst
and then evaluate just the returned data, but I'd generally prefer not returning anything from the DB query unless it's actually relevant. Might be overcomplicating though, open to simplify.~~
EDIT: The above would be valid for fetching data, not for updating. Ignore it. I don't think there's any other option to perform the update with one query and not sift through the entire table of records, so my first explanation is still valid.
Wouldn't it be better to do two queries: a findUnique
and then an update
? in that way, you can check the organization id manually and throw an error like "This user cannot update this project because they doesn't belong to this organization". I understand the efficiency of only one update
query, but the other way is more verbose (and we are doing documentation for getting started) and doesn't need an extra index
Example:
import { resolver } from "blitz"
import db from "db"
import * as z from "zod"
const UpdateProject = z
.object({
id: z.number(),
name: z.string(),
})
.nonstrict()
export default resolver.pipe(
resolver.zod(UpdateProject),
resolver.authorize(),
async ({ id, ...data }, ctx) => {
if (!ctx.session.orgId) throw new Error("Missing session.orgId")
const project = await db.project.findUnique({
where: { id },
rejectOnNotFound: true,
})
if (project.organizationId !== ctx.session.orgId) {
throw new Error("This user cannot update this project because they doesn't belong to this organization")
}
return db.project.update({
where: { id },
data,
})
}
)
I think we should document both ways, the unique constraint that @krszwsk has and @JuanM04's approach.
@krszwsk what happens in your case if the id doesn't exist, the org id doesn't exist, or the combo doesn't exist? Does it just return null? If so, we should check that and throw NotFoundError() as is our pattern for getItem().
And for @JuanM04's snippet, we should use that same pattern, so don't use rejectOnNotFound
but instead manually throw NotFoundError() (as a side note, we should add support so that use of rejectOnNotFound
will display as NotFoundError on the frontend).
Lastly on Juan's snippet, I think it would be slightly better to use findMany
and then you can filter on both id and orgId.
@JuanM04 update
will return if it has changed a record, so we can detect in case we haven't found anything and return a verbose error.
@flybayer findMany
will cycle through the table, won't stop on the first found record AFAIK, findFirst
would be best there I guess.
As for other suggestions, agreed, let's use both, I'll update the PR soon
@flybayer also, @JuanM04 's snippet above already checks if user is allowed to modify the org's project manually, so no need to query that in the query I think 🤔
Ah yes, I meant findFirst
🤦♂️