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

Don't delete past speakers upon user.delete #1917

Closed
luisa-beerboom opened this issue Sep 26, 2023 · 2 comments · Fixed by #1921
Closed

Don't delete past speakers upon user.delete #1917

luisa-beerboom opened this issue Sep 26, 2023 · 2 comments · Fixed by #1921
Assignees
Labels
Milestone

Comments

@luisa-beerboom
Copy link
Member

For background see OpenSlides/openslides-client#2639

Upon deletion of a user, any speaker instance related to him is currently also deleted.
This is fine for speakers who are still in the waiting list, but those who are either currently speaking (i.e. start_time is defined) or finished (i.e. end_time is defined) should not be deleted for the sake of keeping a coherent protocol.

So:
If a user is deleted:

  • All speakers of this user with neither a defined start_time or end_time should be deleted.
  • All speakers of this user with either a defined start_time or end_time should have the user removed, but should not be deleted.

Also:
While testing we've noticed that removing a user from a meeting via removal from the relevant groups doesn't remove him from the meetings speaker lists either.
This should show a similar behaviour to the deletion:

If a user is removed from a meeting:

  • All speakers of this user in this meeting with neither a defined start_time or end_time should be deleted.
  • All speakers of this user in this meeting with either a defined start_time or end_time should not be deleted. In this case the user should NOT be removed from the speaker though.
@r-peschke
Copy link
Member

For background see OpenSlides/openslides-client#2639

Upon deletion of a user, any speaker instance related to him is currently also deleted. This is fine for speakers who are still in the waiting list, but those who are either currently speaking (i.e. start_time is defined) or finished (i.e. end_time is defined) should not be deleted for the sake of keeping a coherent protocol.

So: If a user is deleted:

* All speakers of this user with neither a defined `start_time` or `end_time` should be deleted.

* All speakers of this user with either a defined `start_time` or `end_time` should have the user removed, but should not be deleted.

The deletion rules are set in the models.yml, in the head of that file they are explained.
This way it is not possible to implement the rules like you want. Surely, we could code this individual in the relation handling, but that would be a real pain and hard to detect on every change.
Proposal: In the models.yml meeting_user/speaker_ids we could set the on_delete to SET_NULL instead of CASCADE. I hope it doesn't happen regularly that a user on a speaker list will be deleted or removed from the meeting before he can talk.

Also: While testing we've noticed that removing a user from a meeting via removal from the relevant groups doesn't remove him from the meetings speaker lists either. This should show a similar behaviour to the deletion:

If a user is removed from a meeting:

* All speakers of this user in this meeting with neither a defined `start_time` or `end_time` should be deleted.

* All speakers of this user in this meeting with either a defined `start_time` or `end_time` should not be deleted. _In this case the user should NOT be removed from the speaker though._

see above, I don't think it is used regularly. But this is easier to implement, although there will be a performance penalty on user.update, where the removal from groups is made,

@r-peschke
Copy link
Member

After discussion (@rrenkert, @luisa-beerboom @m-schieder @r-peschke ) we decided to follow the proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants