fix(docs): Fix & make verifyOwnership consistent

[drikusroor]

Currently, Chapter 7 (Accessing currentUser in the API Side) introduces a helper function to check whether the currentUser "owns" the post it wants to modify. However, the helper function is referred to as verifyOwnership and later as validateOwnership.

This PR aims to:

• De-duplicate this naming conflict.
• De-duplicate the two functions' parameters (destructured id)
• Add missing arrow in arrow function

Also, before the function is introduced, an inline check is done (see here) which returns true instead of the db.post.update(...) result. We might want to convert that throw an early error, although it doesn't look very pretty.

// highlight-next-line
import { ForbiddenError } from '@redwoodjs/graphql-server'

// highlight-start
export const updatePost = async ({ id, input }) => {
  if (!(await adminPost({ id }))) {
    throw new ForbiddenError("You don't have access to this post")
  }
  // highlight-end

  return db.post.update({
    data: input,
    where: { id },
  })
}

[jtoar]

Hey @drikusroor, the name change seems sensible and was probably just an oversight on our part, thanks! @cannikin confirming the name and signature are good?

And @drikusroor for your second point, is it just that it's a matter of three lines vs one line for the conditional? I feel like that was also a stylistic choice we made for readability, but @cannikin can you confirm that for me too?

[cannikin]

Yeah those different names must have been an oversight on my part, whoops!

I'm not sure what I was thinking in that updatePost function, since it would never update, just return true! Yeah that negated check is a sea of parenthesis...I guess the alternative would be run the update inside of the if and throw in the else:

export const updatePost = async ({ id, input }) => {
  if (await adminPost({ id })) {
    return db.post.update({
      data: input,
      where: { id },
    })
  } else {
    throw new ForbiddenError("You don't have access to this post")
  }
}

Which might flow a little nicer? But do all the other errors actually short circuit? We should probably stick to that convention if that's the case.

[drikusroor]

Hi @cannikin, thank you for your response.

With regards to:

Which might flow a little nicer? But do all the other errors actually short circuit?

I agree, your alternative "flows" nicer. I'm not sure if all the other errors will short circuit here. For now, this is just an example that is better than the one in the current documentation.

We should probably stick to that convention if that's the case.

What do you mean by this? Should the later verifyOwnership function in the documentation have the same form, or? I'm not sure what you are expecting from me here. 😅 I could already commit your proposed updatePost example if you want me to?

[cannikin]

What do you mean by this? Should the later verifyOwnership function in the documentation have the same form, or? I'm not sure what you are expecting from me here. 😅 I could already commit your proposed updatePost example if you want me to?

Sorry, I just meant that if we have other errors in the tutorial that short circuit then this would should too. But if not, I would stick with the longer one that's a little easier to read (do the update in the if and throw the error in the else).

[drikusroor]

Hey, sorry for the days of silence. I just wanted to let you know I've updated the updatePost method in the docs according to your proposal. ;)

[cannikin]

Awesome, thank you!