rdmorganiser / rdmo

A tool to support the planning, implementation, and organization of research data management.
https://rdmorganiser.github.io
Apache License 2.0
100 stars 48 forks source link

Importing and re-exporting a full XML removes `<additional_input>` #998

Closed Zack-83 closed 2 months ago

Zack-83 commented 4 months ago

Description / Beschreibung

Edit some options and export them in/from the subpage ./management/optionsets/. Open the XML file with a text editor. The fields <additional_input> are populated.

Edit the same options within a catalogue in the subpage /management/catalogs/, and export the full XML. Open the XML file with a text editor. The fields <additional_input> are empty!

Edit a XML optionset file on your computer and import it into the subpage ./management/optionsets/. The fields <additional_input> are populated.

Edit a full XML file on your computer and then import it into the subpage /management/catalogs/. The fields <additional_input> are reset to "---"!

Expected behaviour / Erwartetes Verhalten

The content of <additional_input> should be maintained on either way.

I have attached an exemplary full XML before and after the export. The information loss is striking.

epm.zip

jochenklar commented 4 months ago

Hi @Zack-83 above you write "options" but you are on the optionset page or the catalogs page, can you clarify a bit what you did edit on which page?

Zack-83 commented 4 months ago

I follow the links from the catalogue https://rdmocat.aip.de/management/catalogs/19/nested/ to the optionset https://rdmocat.aip.de/management/optionsets/134/ to the option https://rdmocat.aip.de/management/options/995/ , tick the radio buttons on the last line, save, go back to https://rdmocat.aip.de/management/catalogs/19/nested/ , download.

jochenklar commented 4 months ago

Hmm, I cannot really reproduce the problem here, are you sure the option was saved? Maybe there is another bug preventing this. I currently have no account on https://rdmocat.aip.de or my pw is not working anymore. Maybe you or @triole can create/update mine.

Zack-83 commented 4 months ago

I created an account for you.
Yes, the options were saved. I can tell more about the mistake:

MyPyDavid commented 3 months ago

thanks for reporting @Zack-83 , I think there is a bug in the current xml parser function convert_additional_input, it ignores the xml value in some cases and sets only '' for the additional_input https://github.com/rdmorganiser/rdmo/blob/065d195043b7ac34a7b6b3180dac69829a3974bc/rdmo/core/xml.py#L225

I can address this in my import feature PR https://github.com/rdmorganiser/rdmo/pull/810

jochenklar commented 3 months ago

Ah, so it will only apear when importing the old format, right? This I did not check.

MyPyDavid commented 3 months ago

I've added the first if case, that was the only thing that was missing I think.

additional_input = element.get('additional_input')
if additional_input in ['', 'text', 'textarea']:  # from Option.ADDITIONAL_INPUT_CHOICES
    pass
elif additional_input == 'True':
    element['additional_input'] = 'text'
else:
    element['additional_input'] = ''

PS made a mistake with element['additional_input'] = additional_input ;) a pass should suffice

jochenklar commented 3 months ago

Yes, this should work. I think the files that cause these problems have the wrong version. If the XML files are edited to use the new additional_input syntax, the version attributes of the rdmo tag at the beginning of the file needs to be updated as well.

MyPyDavid commented 3 months ago

yes, but who knows what the right version is when writing the XML file? Or how can an editor find this out?

jochenklar commented 3 months ago

hmm, the idea was never to edit the xml files directly, but people do it anyway. I guess they just need to be careful, but any convert_... functions should somehow be graceful with those fields, like your change. We could also raise errors when things cannot be catched like this.

Zack-83 commented 3 months ago

With which version was the additional_input syntax changed? I am one who often edits XMLs by hand and I often downgrade the version from the present 2.1.3. to 2.0.0. to increase "inclusivity".

jochenklar commented 3 months ago

In 2.1.0 so this would explain why it did not work. But we add @MyPyDavid 's fix nevertheless. We have only 2 "compatibility breaks" to 2.0.0 and to 2.1.0.

jochenklar commented 2 months ago

@MyPyDavid this is also implemented in dev-2.2.0 right?

MyPyDavid commented 2 months ago

yes, this will be part of RDMO 2.2.0