Permitted require option for edit, destroy, findOne

[allouis]

no-issue

With the new framework it is hard to handle 404 errors outside of the
serialization layer, this is because we cannot force deestroy or edit
to error if the model is missing. This lets us do that.

[allouis]

Yo @kirrg001 I assigned you to this because i'm not 100% this isn't gonna have bad repercussions for the v0.1 api?

[kirrg001]

@allouis Could you share why it's hard? And what the use case is? :)

[allouis]

// in a controller object for new api
query(data, options) {
  return SomeModel.destroy(Object.assign({id: data.id}, options}))
}

If you DELETE with a non existent id and this runs, you will encounter an error here (Cannot read property destroy of undefined) : https://github.com/TryGhost/Ghost/blob/master/core/server/models/base/index.js#L828

So we look at how posts does it, posts sets the required property to true https://github.com/TryGhost/Ghost/blob/master/core/server/api/v0.1/posts.js#L247

So we change query to:

// in a controller object for new api
query(data, options) {
  return SomeModel.destroy(Object.assign({id: data.id, require: true}, options}))
}

But still doesn't work because it's not a permitted option. We could just add it in the ApiKey model - but it doesn't not seem correct behaviour, it seems to me that destroy should never give an error like cannot read property of undefined

[allouis]

Other options instead of this:

• Add check to destroy method, if (object) { object.destroy()
• Add permitted option just to ApiKey model
• others...

[kirrg001]

Can't we just force options.require=true in the model layer?
There are a couple of examples in the model layer already. (Search for require=true)

[allouis]

Yeah I can do that 👍

[allouis]

@kirrg001 reopened this as discussed on #9954 🎉

[allouis]

I've added support for findOne as well 👍

[kirrg001]

this looks klitzeklein

[allouis]

🔸