Delete all instances of historical values when deleteProperties is called
Currently if you've altered the visibility of the property you want to delete in the past, that historical value is not deleted.
Issue also exists for soft deleted properties
@joeferner @mwizeman Thoughts about these historical values enhancements:
- [ ] Hard deletes by property name should delete all instances of that property (including historical values of altering property visibility of that name)
- [ ] Hard deletes by property name + key should delete all instances of that property with that key name combo (including historical values of altering property visibility of that name + key)
- [ ] Soft delete by property name should soft delete all instances of that property (including historical values of altering property visibility of that name)
- [ ] Soft delete by property name + key should delete all instances of that property with that key name combo (including historical values of altering property visibility of that name + key)
- [ ] Re-adding a soft deleted property should add a new entry into historical values rather than modifying the delete property entry
@sfeng88 I think having hard deletes wipe out all instances including history sounds ok. I'm not sure about the case of having soft deletes also soft delete history. I usually think of soft deletes being used to maintain a historical record (say for auditing) of the data. Would marking the historical values deleted misrepresent the way things actually happened? I just worry about trying to piece together a picture of what data looked like at a particular point in time and not being able to tell if it was soft deleted at that time or some point later. Maybe I just don't understand the extent of the change you're proposing for soft deletes.
@mwizeman there's a isDeleted flag right now on historical values, but it doesn't always mark all instances of that property name or property key + name combo with a isDeleted=true. For soft deletes, I'm proposing that fix.
For soft deletes, are you imagining just appending a new history value when we do soft deletes? So for example lets say we have
Added Property test1 with key k1 in our history, if we soft delete property (k1, test1), would the history look like
Deleted property test1 with key1
Added Property test1 with key1
For hard deletes, I'm proposing that we always delete the historical value rather than marking the isDeleted flag as true.
I'll type up the unit tests for each of those 5 scenarios and put it on a branch.
@sfeng88 when altering visibility of a property in general what does the history look like? Should altering the visibility optionally cause a hard delete on the old property? The reason I mention this is that if the purpose of altering the visibility is to increase the security of that property would a user not be able to see the value by looking at the history?
This is the behavior I'm seeing:
History before in newest to oldest order (key, name, visibility, timestamp, value, isDeleted):
k1, name1, A, TS2, value1, false
k1, name1, "", TS1, value1, false
History after calling graph.deleteProperties('name1', AUTH_A) in newest to oldest order:
k1, name1, A, TS2, value1, true
k1, name1, "", TS1, value1, false
What I was expecting was no history values after making that call.
For deletes we can go the route of hard deleting the old property which should solve this issue. The other option is to have the history look like this for soft deletes (hard deletes in my mind should still delete the history):
k1, name1, A, TS2, value1, true
k1, name1, "", TS1, value1, true
I can go either way.