feat(paranoid): Revert paranoid to work like in v3

[holm]

Pull Request check-list

Please make sure to review and check all of these items:

• Does npm run test or npm run test-DIALECT pass with this change (including linting)?
• Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
• Have you added new tests to prevent regressions?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
• Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

We are currently upgrading to Sequelize v4. As many others we have run into issue #8496. I do not believe having this functionality is good in any way, and I think it was a mistake for make this the new default in v4. The problems around this is well explained in the linked issue.

This PR tries to move forward by reverting paranoid to work like in v3, with an option to have it work like v4. This will likely be a semver-major change to land in v5.

Closes #8496

[codecov]

Codecov Report

Merging #9007 into master will increase coverage by <.01%.
The diff coverage is 100%.

[eseliger]

This looks okay to me, if this is a big problem for many people, I accept this as a workaround. But I'm not really sure about the term 'simple', and the multi data type for the paranoid option, as it might be confusing for people and if someone checks for if(paranoid), 'simple' will also evaluate to true. This might be handy for some cases but I think there could also be others. Maybe we could add another option, which toggles 'legacy' mode?

[holm]

Thanks for the review. I agree that the term simple is not ideal, and happy to take suggestions. Not sure legacy is great either, as it implies it shouldn't be used.

I initially added this as a separate option, but found that I liked the shared option better. It also has a nice resemblance with the timestamps properties, where you can use a boolean or a string. Maybe the best option is to have a boolean value be the backwards compatible value, and encourage new users to use a string only, either time-based or simple (or whatever name we come up with). true would be equivalent to time-based.

[holm]

Any chance for getting a review of this?

[janmeier]

Honestly, I'd be fine with going for better query performance over semantic correctness.

I'd be open to going back to the old behaviour and releasing v5 now.

[holm]

I guess we could also invert this and have the v4 logic be ‘legacy’. It would fix the issue with the simple terminology and have most users be on the default config.

[sushantdhiman]

Looks good, can you deprecate time-based approach. Possibly give short reason for deprecation, then link to target issue

in v5 we can have simple way by default

[holm]

I changed this PR now to restore the old functionality, with an option to use legacy to have it work like in v4.

I have updated the docs, but it would be great if the maintainers could take care of any other communication required, as I think that would be much more efficient.

[holm]

What can I do to help move this PR forward?

[blacksun1]

Any word on a version 5 release or when this code will be available to start using.

[janmeier]

@sushantdhiman Is working on v5 here, should be ready soon

[sushantdhiman]

This fix is now included in v5 branch 014f2f5