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

Feature/DOI-keywords-and-integration-tests #715

Merged
merged 8 commits into from
Apr 7, 2022
Merged

Conversation

hannyle
Copy link
Contributor

@hannyle hannyle commented Mar 21, 2022

Description

  • Keywords is a mandatory field in DOI form and users can type and save as many keywords as possible. Keywords are displayed in the form of tags so after typing some text, users should press , or Enter to mark the keyword as a tag.
  • Fix integration tests for Study object, now Study form has a mandatory Study Abstract field and we can only submit one Study object.

Related issues

Solves #628 and #695

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Changes Made

  • In WizardJSONSchemaParser.tsx, MUI Chip component was used to render Keywords field. A hidden input is used to register value to the form and MUI Textfield is used to receive input text from user.
  • Updated Cypress tests to include:
    • Study Abstraction when filling a Study form and only one Study form is submitted per time.
    • Dataset object is mandatorily submitted before publishing a submission
    • DOI form is also saved before publishing a submission. A Cypress command saveDoiForm is added to be reused.

Testing

  • Integration Tests

@hannyle hannyle self-assigned this Mar 21, 2022
@hannyle hannyle added the enhancement New feature or request label Mar 21, 2022
@hannyle hannyle added this to the 0.13.0 milestone Mar 21, 2022
@hannyle hannyle changed the title Feature/keywords Feature/DOI-keywords-and-integration-tests Mar 21, 2022
@blankdots
Copy link
Contributor

it would be nice to update changelog

@hannyle
Copy link
Contributor Author

hannyle commented Mar 22, 2022

@blankdots I understand that we can only submit one Study object, but is it also forbidden to edit or replace a Study object? It seems so to me with the current backend changes.

@blankdots
Copy link
Contributor

blankdots commented Mar 22, 2022

@blankdots I understand that we can only submit one Study object, but is it also forbidden to edit or replace a Study object? It seems so to me with the current backend changes.

are you referring to XML or form submissions?

from what I see in integration tests in backend we are able to do PATCH on Study https://github.com/CSCfi/metadata-submitter/blob/feature/metax-integration/tests/integration/run_tests.py#L1770

@hannyle
Copy link
Contributor Author

hannyle commented Mar 22, 2022

@blankdots I understand that we can only submit one Study object, but is it also forbidden to edit or replace a Study object? It seems so to me with the current backend changes.

are you referring to XML or form submissions?

from what I see in integration tests in backend we are able to do PATCH on Study https://github.com/CSCfi/metadata-submitter/blob/feature/metax-integration/tests/integration/run_tests.py#L1770

For me it happens in both form and XML. Other objects work fine but with Study form, for example, editing and updating a submitted form caused error: "Some items (e.g: "accessionId", "publishDate", "dateCreated", "metaxIdentifier") cannot be changed."

@blankdots
Copy link
Contributor

For me it happens in both form and XML. Other objects work fine but with Study form, for example, editing and updating a submitted form caused error: "Some items (e.g: "accessionId", "publishDate", "dateCreated", "metaxIdentifier") cannot be changed."

Yes, that is correct i think "accessionId", "publishDate", "dateCreated" was never possible to update any of them previously as well, it is good not to include them in the PATCH JSON. if the PATCH JSON does not include them, then we have a bug in backend.
Could you provide the JSON and XML you are sending for PATCH update ?

Similar fields cannot be updated for XML as well.
I think that is similar for all objects including datasets.

@lilachic
Copy link
Contributor

I'm not sure how keywords should be added, one at a field or can I but all the keywords in a field?
Trying the DOI form manually I got a bit weird error for 2nd and 3rd keywords, although there are some characters it says "must NOT be shorter than 1 character"
Screenshot 2022-03-22 at 15 18 09

@hannyle
Copy link
Contributor Author

hannyle commented Mar 22, 2022

Yes, that is correct i think "accessionId", "publishDate", "dateCreated" was never possible to update any of them previously as well, it is good not to include them in the PATCH JSON. if the PATCH JSON does not include them, then we have a bug in backend. Could you provide the JSON and XML you are sending for PATCH update ?

Similar fields cannot be updated for XML as well. I think that is similar for all objects including datasets.

Yes, an example of updating a submitted Study form where I only changed the studyTitle:

POST request:
Screenshot 2022-03-22 at 15 39 16

PATCH request:
Screenshot 2022-03-22 at 15 34 33

As of now I have checked again, the error only happens with updating Study form, replacing the XML file works fine.

@blankdots
Copy link
Contributor

blankdots commented Mar 22, 2022

@hannyle yes, so you should not send metaxIdentifier and doi in PATCH request that is why it fails - well metaxIdentifieris now a forbidden key and probably we will makedoi` a forbidden key as well

@hannyle
Copy link
Contributor Author

hannyle commented Mar 22, 2022

I'm not sure how keywords should be added, one at a field or can I but all the keywords in a field? Trying the DOI form manually I got a bit weird error for 2nd and 3rd keywords, although there are some characters it says "must NOT be shorter than 1 character" Screenshot 2022-03-22 at 15 18 09

Sorry for not making it clear in the description, Keywords are display in the form of tag so you need to press , or Enter after typing the keyword. I updated the description here:

Screenshot 2022-03-22 at 15 47 22

@blankdots
Copy link
Contributor

@hannyle yes, so you should not send metaxIdentifier and doi in PATCH request that is why it fails - well metaxIdentifieris now a forbidden key and probably we will makedoi` a forbidden key as well

the same fields should be omitted for dataset object as well

@hannyle
Copy link
Contributor Author

hannyle commented Mar 22, 2022

@hannyle yes, so you should not send metaxIdentifier and doi in PATCH request that is why it fails - well metaxIdentifieris now a forbidden key and probably we will makedoi` a forbidden key as well

@hannyle yes, so you should not send metaxIdentifier and doi in PATCH request that is why it fails - well metaxIdentifieris now a forbidden key and probably we will makedoi` a forbidden key as well

the same fields should be omitted for dataset object as well

You're right, metaxIdentifier and doi shouldn't be in the PATCH. I will modify the request in frontend.

@saulipurhonen
Copy link
Contributor

Not sure if this should be tackled in another PR but input field doesn't seem to adjust with the keywords
image

@hannyle
Copy link
Contributor Author

hannyle commented Mar 28, 2022

Not sure if this should be tackled in another PR but input field doesn't seem to adjust with the keywords image

Good catch! I will fix it in this PR.

@saulipurhonen
Copy link
Contributor

Should we ditch index number in input field label for keywords
image

I think index number doesn't make sense if user is restricted to one keywords field and "Add new item" button for keywords is removed as discussed in todays weekly meeting.

@hannyle hannyle force-pushed the feature/keywords branch from 9854ece to bf8f8a3 Compare April 4, 2022 14:20
@hannyle hannyle force-pushed the feature/keywords branch from bf8f8a3 to c9c13cd Compare April 5, 2022 06:12
@hannyle hannyle requested review from saulipurhonen, lilachic, rquazi and blankdots and removed request for blankdots, rquazi and lilachic April 5, 2022 06:27
Copy link
Contributor

@blankdots blankdots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UI is work in progress and it shows, but that is understandable as we transition. The data looks good and seems to be registered correctly in the backend

@saulipurhonen
Copy link
Contributor

Looks good to me. Keywords behave as supposed and tests looks comprehensive.

@hannyle hannyle merged commit 5749d44 into develop Apr 7, 2022
@hannyle hannyle deleted the feature/keywords branch April 7, 2022 07:34
@blankdots blankdots mentioned this pull request Apr 7, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Study requires Abstract and Dataset requires Description add list of keywords to DOI form
4 participants