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

[imports] Relocate csv-imports to the backend: committee #1709

Closed
r-peschke opened this issue Apr 25, 2023 · 5 comments · Fixed by #1878
Closed

[imports] Relocate csv-imports to the backend: committee #1709

r-peschke opened this issue Apr 25, 2023 · 5 comments · Fixed by #1878
Assignees
Labels
Milestone

Comments

@r-peschke
Copy link
Member

Details see in actions https://github.com/OpenSlides/OpenSlides/wiki/without_templ.committee.json_upload and https://github.com/OpenSlides/OpenSlides/wiki/without_templ.committee.import.

@emanuelschuetze @MSoeb @rrenkert @jsangmeister @luisa-beerboom see specification of actions(links above) and let's discuss or give your okay for the factual requirements.

@r-peschke r-peschke added this to the 4.1 milestone Apr 25, 2023
@reiterl reiterl linked a pull request May 11, 2023 that will close this issue
@reiterl reiterl self-assigned this May 11, 2023
@reiterl
Copy link
Member

reiterl commented May 17, 2023

I am not sure, if I have the current action json_upload description.

What should happen, if

data= [{"name": "n1"}, {"name": "n1"}]

in json_upload? The info of the second data set is interesting. Is there a difference, if a committee with name n1 already exists?

Same with organization_tags, etc.
E.G.

data=[{"name": "n1", "organization_tags": ["ot1", "ot2", "ot1"]}, {"name": "n2", "organization_tags": ["ot1"]}]

There are two interesting points here.

Can we handle every instance separated?

Committee update. Should we handle empty fields special?
e.g. The committee has already managers configured. and now:

data =[{"name": "n1", "orga_tags": ["ot1"], "committee_managers": []}}
vs.
data=[{"name": "n1", "orga_tags": ["ot1"]}]

How does the handling of add and remove committee_managers, meeting_admins, etc work?
It should be possible to add and remove items. But is there a special mark in the json_upload
result to show that? As far as I know, there is no special mark.

@r-peschke
Copy link
Member Author

r-peschke commented May 17, 2023

I am not sure, if I have the current action json_upload description.

I looked at it and it seems to the current version

What should happen, if

data= [{"name": "n1"}, {"name": "n1"}]

That isn't a very useful format. If it is easy for you throw a warning, that it is duplicate and let the user decide, whether he wants to import or not. If the committee doesn't exist, the second will throw an error on import, because we will generate 2 create events. IMO this doesn't matter, because it is no real use case to import the same committee twice in 1 import.

in json_upload? The info of the second data set is interesting. Is there a difference, if a committee with name n1 already exists?

Same with organization_tags, etc. E.G.

With organization_tags we should prevent it. If 2 committees want to use the same, we should generate only 1 create event, because it is a real use case to use an organization on more than 1 committee.

data=[{"name": "n1", "organization_tags": ["ot1", "ot2", "ot1"]}, {"name": "n2", "organization_tags": ["ot1"]}]

There are two interesting points here.

Can we handle every instance separated?

Committee update. Should we handle empty fields special? e.g. The committee has already managers configured. and now:

data =[{"name": "n1", "orga_tags": ["ot1"], "committee_managers": []}}
vs.
data=[{"name": "n1", "orga_tags": ["ot1"]}]

With this data it is easy:
First case removes all committee_managers from committee, second one don't touch the committee managers.
Problem still not solved, how to transfer these different cases from csv-file to backend-interpreted format!

How does the handling of add and remove committee_managers, meeting_admins, etc work? It should be possible to add and remove items. But is there a special mark in the json_upload result to show that? As far as I know, there is no special mark.

Neither committee_managers, nor meeting_admins will be created, if they not exist. The committee_managers can be added or removed (don't mention it in he payload) from beeing committee_manager.
The meeting_admins are different, because we don't update meetings. Each time the meeting_name is filled, we create a new meeting for the committee with it's own meeting_admins.

@reiterl
Copy link
Member

reiterl commented May 22, 2023

Some questions about the import action:

  • Why does the payload has command instead of import like the others?
  • What should be done, if a row is no longer create or no longer update?
  • What do you mean by column objects? S.th. like organization_tags? The orga tags don't need to have an id in create case. How can I use the id to create an instance?
  • What about the done case?
  • What errors could occure and should be catched?
  • The code doesn't support the exception and return rows stuff. How should that work? We need to throw an exception in error case, so nothing is changed.
  • What is the forward to committees feature? If a name is not found, what should be done? It seems, that orga tags and forward to committees are similar.

@reiterl
Copy link
Member

reiterl commented May 22, 2023

At import:
The two user fields committee_managers and meeting_admins are similar. I won't create new users and will collect the ids of done entries and will use them.

@r-peschke
Copy link
Member Author

r-peschke commented May 25, 2023

Some questions about the import action:

* Why does the payload has _command_ instead of _import_ like the others?

changed in the import action description

* What should be done, if a row is no longer create or no longer update?

IMO show an error in the line. The consequence is that the whole import fails.

* What do you mean by _column objects_? S.th. like `organization_tags`? The orga tags don't need to have an _id_ in create case. How can I use the _id_ to _create_ an instance?

In the action description object is now a link with description. But it is: To show/store extra information in a column, these column can't be a simple value type like string or integer. It has to be the defined object type with some more properties like info, which contain a predefined action state, an optional id and the fields value and type.

* What about the done case?

The done case is no case. It is an ImportState, that has a specific meaning in it's specific context. The ImportState is just a set of predefined possibilities to inform the client (or import-action) about a specific state in context of row, column... If you miss an action state to express a specific state, invent a new one. But try to reuse the existing ones, they are just for development, not for the user.

* What errors could occure and should be catched?

Besides common errors like action worker with data not found, errors are differences between the preview state and the state during import, i.e. an object expected to exist don't exist any more and vice versa. The user must be informed, that something he saw in the preview, has changed!

* The code doesn't support the exception and return rows stuff. How should that work? We need to throw an exception in error case, so nothing is changed.

If there is a real exception, it is okay to be thrown.
Our error checking for changed data between preview and import must be reported in a new preview.
If there is an error and we don't want to write the data to the database, overwrite the build_write_request method:

def build_write_request(self) -> Optional[WriteRequest]:
        """
        Prevent data from beeing written in error case
        """
       if self.ERROR:
             return NONE
       else:
            return super().build_write_request()
* What is the `forward to committees` feature? If a name is not found, what should be done? It seems, that orga tags and forward to committees are similar.

Look for existing committees in database and in your action_data for creating committees. Because of possible circular relations you should handle the forwarding after the creation of all committees.

@reiterl reiterl assigned r-peschke and unassigned reiterl Jun 14, 2023
@reiterl reiterl linked a pull request Aug 18, 2023 that will close this issue
@jsangmeister jsangmeister modified the milestones: 4.1, 4.2 Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants