Fix issue with Unique Records not being removed when the unique fields are value fields

[AlexStansfield]

See Issue #509 for all the details.

I've since taken the tests created for that issue and improved them slightly.

Changes to Model.js:

• Prepare the properties earlier so that we can tell the unique fields have actually changed
• Made a change to the how it determines hasUniqueProperties by also checking params.remove
• Create new copies of properties and params to pass to prepareProperties to avoid issues due to the method mutating the objects causing conflicts if run multiple times

[AlexStansfield]

I'm not really happy with the way I had to do this, but because prepareProperties mutates the properties and params in certain situations this caused issues (like adding remove twice on fields).

Personally I think the prepareProperties shouldn't mutate these objects and should return new objects with the changes, but that seems like a huge task so this seemed like the way around it.

If you know a better way I'd be happy to hear them and update accordingly

[mobsense]

Alex, thanks for the work on this.

I agree with the comment about mutating properties.

How is this working for you in practice?

I'll wait for 2 other PRs to have some test, then I'll merge this one.

[AlexStansfield]

Actually I was a little worried about the code just because that prepareProperties is so complicated that I really couldn't be sure that even though the tests passed this wasn't still messing something up so for now we've taken uniqueness off the schema and doing it only with the existing validation code that checked for duplicates. Not ideal but better than broken data.

Was waiting to hear back from you guys what you thought.

[mobsense]

We use uniqueness rarely. It does add a lot of complexity, so we use it for things that really require it like email addresses and account names.

[AlexStansfield]

We use uniqueness rarely. It does add a lot of complexity, so we use it for things that really require it like email addresses and account names.

Our use case is ensuring unique email on user records. The issue for us is we don't want to hard delete users, we want to soft delete them. So a simple unique on email field doesn't suffice, no user would be able to sign back up with the same email if their account is deleted.

Solution we found was to use a value function to set a uniqueEmail field based on the value of email and deletedAt and have uniqueEmail be the unique field. But then we ran into this problem.

If you think my PR is a good fix then I'm happy for you to merge it so we can turn the uniqueness back on.

[mobsense]

Alex, sorry for the delay in looking into this. Been swamped with other stuff.

I'm concerned about the risk of change in the PR -- so that is why I've not just merged it already. I need to dig into it closely.

I'll try to get to this soon.

Michael

[AlexStansfield]

Alex, sorry for the delay in looking into this. Been swamped with other stuff.

I'm concerned about the risk of change in the PR -- so that is why I've not just merged it already. I need to dig into it closely.

No problem, take your time

[mobsense]

Just an update, digging into this now and should have feedback very soon.

[mobsense]

Sorry for the delay.