Skip to content

Conversation

@CanRau
Copy link

@CanRau CanRau commented Oct 23, 2019

Add option to exclude certain fields from history.
As discussed in #20 I, for example, have the need for a history table but want to exclude a field containing an image blob.

Fixes a typo in readme as well.

I'm still not too experienced regarding tests, I added one but many of the existing plus mine throw

SequelizeUniqueConstraintError: Validation error

Please let me know what you think.

Uh I used array.includes() let me know if that's fine regarding Node support.

Resolves #20

@ddolcimascolo
Copy link
Collaborator

Hi @CanRau

Sorry for the late reply. The failing tests are because _.filter probably not does what you think it should. It actually returns an array of values your function returns true for, not an object with only the keys your function returns true for. See the difference ?
That being said, the whole _.mapValues thing can probably be rewritten to use something more simpler, like _.reduce which would allow iterating only once on the object which would improve the performance as an added benefit :)

Can you rework your code?

Cheers,
David

@CanRau
Copy link
Author

CanRau commented Dec 25, 2019

No worries, I'm late as well^^
Like so? Tests are working now, too.
Maybe not the cleanest but it seems to work.

I'm not using Sequelize anymore so I hacked this only using the tests to not abandon my PR 😉

@CanRau CanRau mentioned this pull request Dec 25, 2019
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.

Option to exclude certain fields

2 participants