Using pre validate hook instead of pre save

[lasselukkari]

Currently the incremented field can not be set as required because the plugin uses pre save hook instead of pre validate.

I can create a pull request to change this if you don't see any problems with the idea.

[hasezoey]

I had decided to have it in a pre-save hook as the code actually interacts with the database (read / update / create) and changes values on the document. And my perception of the "validate" hook is to, well, validate, not transform.

[lasselukkari]

The pre validate doesn't mean the hook is directly involved in validation. It simply indicates that the hook runs before the validation step. If your hook modifies the model data and you want those changes to be validated, those modifications must occur before validation. In most cases, you’ll want to perform the data transformations before validation unless you have a compelling reason to do otherwise.

[hasezoey]

The pre validate doesn't mean the hook is directly involved in validation. It simply indicates that the hook runs before the validation step

I knew this, but was still thinking what i wrote above.

---

Then let me ask you this, for AutoIncrementID it would be easy to transition to pre-validate as (on a high level) the requirements are this.isNew and the field in the document being unset. Though what about AutoIncrementSimple whose literal purpose is about "incrementing a value on each save" (as long as it is not this.isNew)?
Or is this issue only targeted at AutoIncrementID?

[lasselukkari]

I'm only using the AutoIncrementID. To me the AutoIncrementSimple is so easy to implement with a hook that would not add a dependency to my project just to get that functionality.