frab / frab

conference management system
https://frab.github.io/frab/
Other
715 stars 151 forks source link

Add user merge function #215

Closed zonque closed 8 years ago

zonque commented 8 years ago

Installations with an enabled CfP may suffer from the problem that users create more than one account and then forget about them. It should hence be possible for an administrator to merge two or more account into one.

erdgeist commented 8 years ago

In order to merge two users, we first need to consider the user and the corresponding person objects. Given a user without a person matching its user_id, we only need to consider the ConferenceUser Table and replace and unify all occurrences of the source user_id with the dest user_id.

If for each user we do have a person record, we just delete the second instance from person table and keep its id. We need to fix up EventRating, EventPerson and Availabilities table and let the rest of the attribute be fixed manually.

Would that be enough?

manno commented 8 years ago

erd.pdf

I'd suggest merging the User and Person models, the one with the most recent updated_at should take precedence. A merge form isn't really needed, as a first step it would be sufficient to enter two user ids, keep the latest profile and account, attach the related models from the deleted ones.

erdgeist commented 8 years ago

Please take a look at my feature branch at erdgeist/frab@5a8203ed0f219dd4cccc65c44da580820b8cab55 , I'd like some feedback on how the attributes are merged.

manno commented 8 years ago

I don't think there is a beautiful way to merge attributes. I'm happy there are tests, because I couldn't bring myself to trust p.user_id = nil; p.destroy at first. Was afraid that would delete the user.

erdgeist commented 8 years ago

So was I, same went with the corresponding code in the User model. But it worked. I wonder if it is better to (and if so how to implement) prevent User.merge_with being called from any other place than Person.merge_with. If someone calls User.merge_with, the Persons would not be merged but destroyed.