Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Writing a robust updatedAt autoValue #419

Closed
Floriferous opened this issue Sep 7, 2020 · 4 comments
Closed

Writing a robust updatedAt autoValue #419

Floriferous opened this issue Sep 7, 2020 · 4 comments

Comments

@Floriferous
Copy link

In the docs, there is a very helpful example of how to define a updatedAt field that stores a date on each update.

We've found for our use case, that storing the updatedBy is very helpful to trace things down to the user who performs the update.

However, it has proven to be quite difficult to write that in a nested way like this, without any way to bypass the schema:

{
  updatedAt: { date: Date, userId: String }
}

The following collection operations each should behave:

Collection.create({ title: 'hello' }); // Should not set the field
Collection.update(id, { $set: { title: 'hello' } }); // Should set the field
Collection.update(id, { $unset: { updatedAt: true } }); // Should not unset the field
Collection.update(id, { $unset: { "updatedAt.date": true } }); // Should not unset the nested date
Collection.update(id, { $set: { "updatedAt.date": someDate } }); // Should not set the date to a custom `someDate`
Collection.update(id, { $set: { updatedAt: { date: someDate, userId: 'someUserId' } } }); // Should not set the entire object

So here's what that schema looks like:

new SimpleSchema({
  updatedAt: {
    type: Object,
    autoValue() {
      if (!this.isInsert && this.operator !== '$set') {
        this.unset();
        return;
      }

      if (this.isUpdate || this.isInsert || this.isUpsert) {
        return { date: new Date(), userId: this.userId };
      }
      this.unset();
    },
    optional: true,
    denyInsert: true,
  },
  'updatedAt.date': {
    type: Date,
    optional: true,
    autoValue() {
      if (!this.parentField().isSet) {
        this.unset();
      }
    },
    denyInsert: true,
  },
  'updatedAt.userId': {
    type: String,
    optional: true,
    autoValue() {
      if (!this.parentField().isSet) {
        this.unset();
      }
    },
    denyInsert: true,
  },
})

I find it a bit intense that you need to add all these extra bits to prevent any wrongdoing, where the specs are pretty simple to write in words:

  • On each update, let the server set the date and userId
  • Don't let anything else modify this field

Does someone know of a simpler way to do this? I think it'd be useful to add to the docs, as this is a simple piece of information that can be helpful in all meteor codebases :)

@harryadel
Copy link
Member

harryadel commented Sep 14, 2020

Indeed, it feels quite verbose versus a much intuitive schema you provided. I'd happily accept a PR that updates the docs.

We could definitely provide certain helpers like https://github.com/aldeed/meteor-schema-deny denyInsert and denyUpdate that'd help with your use case. But I'm unsure whether we should add those changes to collection2 or have them in a separate package like meteor-schema-deny and aldeed/meteor-schema-index. 🤔

@StorytellerCZ @coagmano Your input is greatly appreciated guys.

@StorytellerCZ
Copy link
Member

I'm fine with adding additional directives like denyInsert, but that would have to go into aldeed:schema-deny which already has denyUpdate.

I like the thinking behind updatedBy, though personally I would make it more of an audit. An array that would add a new unalterable entry every time the object gets updated, though depending on the application that might be better to have with more data in a separate collection, but I'm getting off-topic.

We either should bring schema-deny under community maintenance as well or if my memory serves me right there was a talk about re-integrating it and schema-index back into this package.

@copleykj
Copy link
Member

copleykj commented Sep 25, 2020

It seems to me that this could be easily solved by just having two top level fields, updatedAt and updatedBy. Keeping anything else from modifying the field is as simple as calling unset if the operation isn't an update or upsert.

new SimpleSchema({
  updatedAt: {
    type: Date,
    autoValue() {
      if (this.isUpdate || this.isUpsert) {
        return new Date();
      }
      this.unset();
    },
    optional: true,
  },
  updatedBy: {
    type: SimpleSchema.RegEx.Id,
    autoValue() {
      if (this.isUpdate || this.isUpsert) {
        return this.userId;
      }
      this.unset();
    },
    optional: true,
  },
});

@Floriferous
Copy link
Author

Yeah, nested objects always add a lot of extra headaches with simple schema, even though the compartmentalisation helps avoid clutter when looking through objects.

Anyways, I think @copleykj is fairly straightforward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants