Discussion: Strategies for handling null deltas when updating an existing record

[Rory-Gaddin]

I was doing some testing on an API recently with Paper Trail enabled, and found that submitting the same PATCH request multiple times increments the revision number and updatedAt fields on the record on the entity table, but does not create additional entries in the Revisions table.

This feels somewhat inconsistent to me, and I was thinking perhaps it would be good to have an option that allows us to decide whether to discard Revisions entirely (essentially abort the update operation) if the delta is null, or to write a new Revision record even in the case of a null delta. The current behaviour is somewhere in between the two, and this may cause some confusion for anyone looking for the "missing" revisions in the Revisions table.

This seems to be due to the fact that beforeHook in index.js unconditionally increments the revision number of the instance being updated...

instance.set(options.revisionAttribute, (currentRevisionId || 0) + 1);
    if (delta && delta.length > 0) {
      if (!instance.context) {
        instance.context = {};
      }
      instance.context.delta = delta;
    }
    if (debug) {
      log('end of beforeHook');
    }

...but afterHook does not build and save the Revision change record if the delta is null...

if (instance.context && instance.context.delta && instance.context.delta.length > 0) {
      var Revision = sequelize.model(options.revisionModel);
      if (options.enableRevisionChangeModel) {
        var RevisionChange = sequelize.model(options.revisionChangeModel);
      }
      var delta = instance.context.delta;

      ...

      // Build revision
      var revision = Revision.build({
        model: this.name,
        documentId: instance.id,
        document: JSON.stringify(currentVersion),
        userId: ns.get(options.continuationKey) || opt[options.continuationKey]
      });
}

If we have agreement in principle on this, I'm happy to have a crack at it and submit a pull request when done. Let me know your thoughts.

Kind regards,
Rory

[nielsgl]

Interesting point, had not thought about this as much as you (I have been using PATCH but had not realized the effect). It definitely makes sense and is a good idea to correct this behavior to be consistent across the hooks and I also agree that this makes it easier to understand and it simplifies the behavior and expectations.

PRs are definitely welcome and I'd be happy to have a look at a PR and test the new behavior!