Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to deal with deletions? #7

Open
Danwhy opened this issue Oct 8, 2018 · 8 comments
Open

How to deal with deletions? #7

Danwhy opened this issue Oct 8, 2018 · 8 comments
Assignees
Labels
discuss help wanted Extra attention is needed question Further information is requested

Comments

@Danwhy
Copy link
Member

Danwhy commented Oct 8, 2018

Although we are never deleting any records from our append only database, we still need a way to mark data as 'deleted', so it isn't shown to to users.

One solution could be to have an extra column in each table for 'deleted', and to mark that as true every time a user 'deletes' something. This would have the advantage of being easily reversible, ie. we could have a page that lists all 'deleted' items, and a button that 'undeletes' them, simply by marking 'deleted' as false.

The main issue we are facing is how to deal with deleting associations. We've come up with a way to keep associations among updated records, which should also work for deleted records (see #6). But this doesn't account for simply removing an association between two records.

The proposed solution above could work, but it might mean re-implementing a lot of what we already have. Ecto uses 'many to many' fields in schemas to automatically update composite tables. These tables currently consist of just two ids. It would probably be worth looking into how Ecto goes about updating these tables, and how easily we could instead do it ourselves.

Any suggestions or thoughts on the above two issues are welcome

@Danwhy Danwhy added question Further information is requested discuss labels Oct 8, 2018
@nelsonic
Copy link
Member

nelsonic commented Oct 8, 2018

@Danwhy this is one of the difficult things that is actually quite easy.
You've "hit the nail on the head" with the deleted=true approach.
This needs to be built-in to the macro we will be using for the queries.
And into the permissions system for viewing the "deleted" data.
When we deploy this and there is user data that must be deleted by law,
we need to create a "batch process" that actually deletes records for real from the DB & backups.

To make this work we need to have an anonymisation step in user registration.
But for now please don't "worry" about that, it's a separate issue that we will be working on soon.

Go ahead with your intuition on how to set a record to "deleted" and we will fill-in the gaps shortly!

@nelsonic nelsonic removed their assignment Oct 8, 2018
@Danwhy
Copy link
Member Author

Danwhy commented Oct 8, 2018

In terms of the deleting associations, there are two ways we could go about it.

The first is to simply do what we are already doing, which is updating the associations when we update the records, but to add a step where we filter out those associations that no longer exist.
This involves very little extra work, but means we could be adding an extra row to the database, even when nothing in it has changed, just the id.

The second is to add a 'deleted' column to the composite table, and every time an update happens, check if any data has been updated, or just the association, and then performing the relevant update. This would be a little more work, but would mean only updating the three columns in the composite table if no other data was changed.

@nelsonic
Copy link
Member

nelsonic commented Oct 8, 2018

@Danwhy technically if the "primary" content type is "deleted" then the "WHERE" clause will exclude it and the association does not matter.
e.g: if a product is "deleted", we aren't interested in the reviews and PSQL select query will ignore them.

@Danwhy
Copy link
Member Author

Danwhy commented Oct 9, 2018

@nelsonic True, it was more the deletion of the actual association I was thinking of. For example, in CS if a venue wanted to remove a drink, but both the venue and drink still existed.

@nelsonic
Copy link
Member

nelsonic commented Oct 9, 2018

For CS, YAGNI. (come back to it when the CS assoc table has 100k+ rows)
If the drink is "deleted" the WHERE clause will exclude it.
If the Venue is "deleted" the WHERE clause will exclude it.
therefore the union query of the two record types using the "lookup" table,
will (should) also exclude records where either one is "deleted". 😉

@nelsonic
Copy link
Member

@Danwhy can we automatically add a :deleted column to any table where alog is used?

@Danwhy
Copy link
Member Author

Danwhy commented Oct 22, 2018

I was thinking about this, but hadn't had a chance to look into it yet. The same with :entry_id.
I'll take a look into it and report back. The less somebody is required to do to use this module, the better

@nelsonic
Copy link
Member

@Danwhy please see: dwyl/alog#5 (thanks!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants