Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

add deactivate method to Participant #586

Closed
chadwhitacre opened this issue Jan 31, 2013 · 4 comments
Closed

add deactivate method to Participant #586

chadwhitacre opened this issue Jan 31, 2013 · 4 comments
Labels
★★★

Comments

@chadwhitacre
Copy link
Contributor

When the user clicks the deactivate button, we want to convert them back to a stub participant, which is what gets created whenever we first hit /on/twitter/fooname/. We do keep an elsewhere record in that case, and all elsewhere records have a stub participant. For example, we should reset statement and goal, claimed_time should be null, and id should be a random value.

Look at how the merge flow works. We archive accounts as part of that.

I wish we didn't have to overwrite claimed_time. I don't like losing data. :-/
@chadwhitacre chadwhitacre mentioned this issue Jan 31, 2013
Closed
@chadwhitacre
Copy link
Contributor Author

Hmmm ... this might not work, actually, because we need to keep the old participant_id around to display in other people's history pages. Let's leave this open pending discussion on #54.

@chadwhitacre
Copy link
Contributor Author

Okay, I think what we do is archive accounts like we do with the absorptions table, where we record the following info:

  • timestamp
  • absorbed_was
  • absorbed_by
  • archived_as

Let's add a deletions table with these columns:

  • timestamp
  • deleted_was
  • archived_as

The archived_as column in both cases is a random string to which the participant_id was changed. The *_was key is the old participant_id (it's not a foreign key but ideally would be immutable).

The niggle with history pages is iinm already present with absorbed accounts. The answer is to update the history page to use the *_was keys where relevant (#663).

@chadwhitacre chadwhitacre mentioned this issue Feb 18, 2013
Closed
@chadwhitacre
Copy link
Contributor Author

Let's use this ticket for adding this functionality to the Participant object, and #664 for UI.

chadwhitacre added a commit that referenced this issue Feb 20, 2013
@chadwhitacre
Modify the schema to record deletions; #586
4969f78
chadwhitacre added a commit that referenced this issue Feb 20, 2013
@chadwhitacre
Add a delete method with broken tests; #586
5853c27 
Deletion is like absorption, and absorption is not ported to SQLAlchemy
yet. I factored out the common part (archiving) and implemented deletion
using the old wiring. This includes a few tests, but they're borked atm.
chadwhitacre added a commit that referenced this issue Feb 22, 2013
@chadwhitacre
Fix regressions in test suite; #586
925759c 
I needed an import to properly register the Deletion model with our
SQLAlchemy wiring, so that the deletions table was emptied on teardown.
chadwhitacre added a commit that referenced this issue Feb 22, 2013
@chadwhitacre
Add more tests for delete, uncovering bug; #586
440274a 
This adds more tests for the delete method. One of the areas I'm
concerned about is the interaction between Participant.delete and the
auth system. How does user.ANON behave with a deleted account? In
exploring this I've uncovered a bug in User.from_session_token where the
SQL "session_token = NULL" was translated into the SQLAlchemy expression
"filter_by(session_token=None)", which generates the non-equivalent SQL
expression "session_token is NULL". This is a potential security issue
that needs to be addressed on master immediately. Therefore, I'm
committing on this branch and switching over there.
@chadwhitacre
Copy link
Contributor Author

This was a step towards #54. #54 is done now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
★★★
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@chadwhitacre