mint-o-badges / badgr-server

Open Badge issuing and management with Django
GNU Affero General Public License v3.0
2 stars 1 forks source link

Feat/change email of institution on ownership change #182

Closed timber-they closed 1 month ago

timber-they commented 2 months ago

fix: infinite recursion

Remove the storage of the original information in __init__, since this caused infinite recursion on deletion of users associated with an institution. Instead in saving I receive the old version from the database to compare for changes.

feat: transfer ownership on user deletion

If a user is deleted the ownership of all associated issuers is transferred to the next staff with the highest privileges. Also the contact email of the issuer is set to the primary email of the new owner. "Owner" in this sense also implicates creator, to ensure that no issuer exists without a creator (even though this is kind of a lie).

timber-they commented 2 months ago

@timber-they If I understood correctly this PR resolve the issue of updating issuer contact email when the owner/creator of the issuer is removed from members list. I created a new issuer and deleted the owner after adding a new member with owner role but the issuer contact E-mail still the same as previous owner. If there's anything I'm missing here, can you please elaborate it?

The situation described in the ticket was a bit different. To reproduce:

With the fixes in this PR this will lead to the member added in step 2 to become the "creator" and have it's primary E-Mail set as contact address. What you're describing (that the mail of a member that's added to an issuer without any member becomes the contact address) is a bit different. Since it's not the scenario Julia described, I'm also not entirely sure it's a requirement.

YouQam commented 2 months ago

@timber-they If I understood correctly this PR resolve the issue of updating issuer contact email when the owner/creator of the issuer is removed from members list. I created a new issuer and deleted the owner after adding a new member with owner role but the issuer contact E-mail still the same as previous owner. If there's anything I'm missing here, can you please elaborate it?

The situation described in the ticket was a bit different. To reproduce:

* Create an issuer

* Add a new member (doesn't matter which role)

* Delete the original owner

With the fixes in this PR this will lead to the member added in step 2 to become the "creator" and have it's primary E-Mail set as contact address. What you're describing (that the mail of a member that's added to an issuer without any member becomes the contact address) is a bit different. Since it's not the scenario Julia described, I'm also not entirely sure it's a requirement.

I have a question regarding doesn't matter which role, if the new staff member is not owner it's impossible to remove other owner (real creator). Unless you mean deleting the user using admin panel (which I don't think Julia did when she found this issue).

timber-they commented 2 months ago

I have a question regarding doesn't matter which role, if the new staff member is not owner it's impossible to remove other owner (real creator). Unless you mean deleting the user using admin panel (which I don't think Julia did when she found this issue).

Hang on that's an interestingly different understanding. In the issue Julia said:

I created the institution "Deutsche Telekom Stiftung". after that I added another person as owner and got deleted.

Note the and got deleted part. To me that clearly means that the user was deleted, which is currently only possible via the admin panel. To be honest until now I wasn't even aware that it was possible to remove the creator of an institution via the frontend, thus this fix also doesn't solve that case. I do see though that it might make sense to also consider this case, so I'll add that functionality.

timber-they commented 2 months ago

@YouQam I adjusted it to now also cover the case you described.

YouQam commented 2 months ago

@YouQam I adjusted it to now also cover the case you described.

Removing owner from issuer staff list works properly, but deleting user (from admin panel) has three issue:

  1. the deleted owner still exist at staff members list (although it was deleted)
  2. the role of added member remain the same (ex: editor or staff ) and never change to owner
  3. contact email belongs to deleted user
timber-they commented 1 month ago

@YouQam I adjusted it to now also cover the case you described.

Removing owner from issuer staff list works properly, but deleting user (from admin panel) has three issue:

1. the deleted owner still exist at staff members list (although it was deleted)

2. the role of added member remain the same (ex: editor or staff ) and never change to owner

3. contact email belongs to deleted user

I couldn't reproduce the same error because for me the creation of an issuer failed with the newest version. I fixed that error and now I can't reproduce your error anymore. So I think I fixed it ^^