Removing users from meeting, using user.delete or remove from groups, as one new action

[luisa-beerboom]

Reproduce:

1. Create a meeting with multiple users. Make a subset but not all of those users also participants of another meeting. Ensure that the groups of the users are the same.
2. Go into said meeting and select all users
3. Delete them

What will happen:

The client will send multiple actions in the same request: one user.delete for the users who had only one meeting and one user.update to remove the users that had multiple meetings from our meeting.
This will cause the datastore to send an error response along the lines of: "The following locks were broken: groups/"

Clearly the two requests both try to access the same field at the same time, causing conflicts.
We should either separate them, or look for a solution in the backend.

The problem is solved by using 2 separate transaction by changing the route to backend to handle_separately. The disadvantage of this solution: it require an extended error handling, because it may be, that 1 transaction fails. Therefore I let the issue open with level low. Then we will realize solution3, do it in an own action, see below. That would imply changes for the client, using the new action, too. We should change the interface, because the client suggestion of separating the cases may be outdated.

[luisa-beerboom]

What do you think @jsangmeister

[jsangmeister]

This is a backend problem; since they are sent in the same request, the backend should be able to handle this.

[r-peschke]

2 ways for the client to handle this case:

1. route handle_separately: advises the backend to use separate database-transactions(=datastore.writer calls). This works, because the 2 transactions in this case are executed sequentially in the given sequence. The lock fields for the 2nd transaction are read after finishing the first transaction. Disadvantage: 2 datastore.write calls and 2 transactions, whose possible results the client has to handle
2. route handle_request: advise the backend to use only 1 database transaction. The current implementation gathers the results of 2 independent actions( user.update and user.delete). Each action determines its own locked fields, but potentially on the same database state. This will not work, if there is 1 common locked field, because it's position will be updated in the datastore. Checking the lock fields of the 2nd action will fail, because the new position will be read in the same database transaction.

For the moment we have to use the route with handle separately.
Ideas, using route handle_request-route:

1. The easiest would be to gather all locked_fields in the first write_request, only in case of the handle_request/atomic route.

   advantage: easy to implement
   requirement: To do this in a generic way, all combinations of independent actions should be written in a manner, to use always the datastore.changed_models of the other(s) independent action. For example, the user.update removes user from a specific group, the user.delete must remove itself from the same group from the changed_model of this group. Between the different actions this is realized in action_handler.py, method perform_action by resetting the datastore with hard=False. But in relation_handler.py, method get_template_field_db_value I found a read from db with use_changed_models=False. This may be necessary to have a kind of WriteRequest-isolation level, but would be wrong to work on a after-previous-write-request level. There are various more places in code using this flag.

2. We could join the 2 or more write request and their locked fields to send only 1 write request to the datastore: Would simplify the interface and the datastore implementation. But the problem remains the same.

3. We could invent an own action for this specific combination of actions with individual programming. IMO best done with a changed interface with meeting_id and list of users to remove from meeting. The backend would decide, whether a user will be removed by deleting or by removing from group(s).

@rrenkert, @jsangmeister What do you think?

[rrenkert]

After talking to @r-peschke i would prefer idea 3.

[jsangmeister]

I'm fine with using the handle_seperately route for now, after all that's why we implemented it. 2 Datastore calls is still O(1) and does not seem that bad to me.

[r-peschke]

to simplify the client error-handling IMO we should resolve it using solution 3, but here is no hurry;-)

[emanuelschuetze]

OpenSlides/openslides-client#1524 is merged. Can we close this issue?

[luisa-beerboom]

OpenSlides/openslides-client#1524 is merged. Can we close this issue?

As far as I know that PR is more of a quick client-side fix for the immediate problem, while this is supposed to be the permanent solution

[jsangmeister]

currently not needed.