Skip to content

Conversation

@dancastellon
Copy link
Contributor

Impact: minor
Type: bugfix

Issue

The createdAt field of the Accounts schema is currently required. This makes it impossible to validate against the schema when performing updates.

Solution

This PR simply adds optional: true to the createdAt field.

Breaking changes

None

Testing

  1. Add the following piece of test code in /imports/plugins/core/accounts/server/index.js:
async function testAccountUpdate() {
  import { Accounts } from "/imports/collections/schemas";

  const testDoc = {
    userId: "mZv3WqQTC8MshuAHc",
    shopId: "J8Bhq3uTtdgwZx3rz",
    name: "Dan"
  }
  
  Accounts.validate(testDoc);

  console.log("Validation passed.");
}

testAccountUpdate();
  1. Confirm you see "Validation passed." in console
  2. Remove line 209 (optional: true) in /imports/collections/schemas/accounts.js
  3. When the console restarts, confirm you see ERROR Reaction: Created at is required in console.

@rosshadden rosshadden merged commit 43d5003 into develop Jan 7, 2019
@rosshadden rosshadden deleted the fix-dancastellon-accounts-createdat-optional branch January 7, 2019 19:25
@brent-hoover
Copy link
Collaborator

@dancastellon Can someone explain to me why making this field optional is a good idea? This should be an autogenerated field and really should remain required. I am pretty sure there are things that rely on it.

@dancastellon
Copy link
Contributor Author

@zenweasel It shouldn't be required when validating against the schema for updates. It has an autoValue function that sets the date on insert. When I was trying to validate against the schema I kept getting "Created at is required".

@brent-hoover
Copy link
Collaborator

@dancastellon I think you have another problem there that make this critical field not required is probably not the correct solution.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants