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

Add optional metadata form in ERC creation #203

Merged
merged 2 commits into from
Mar 11, 2021

Conversation

Fmazin
Copy link
Contributor

@Fmazin Fmazin commented Feb 28, 2021

This PR should resolve #146.

With this pull request a fourth form page for the ERC creation is added. On this form page you can edit the DOI, the languages of the paper(ISO 639-1 codes) and the keywords.
Full optional metadata page:
Bildschirmfoto von 2021-02-26 21-12-21

They are also shown on the metadata overview on the ERC site:
Bildschirmfoto von 2021-02-26 21-13-08

Checks

I have included a few checks for the metadata.

  1. The DOI is checked for validity:
    Bildschirmfoto von 2021-02-26 21-11-40

  2. The keywords should always be at least two characters long.
    Bildschirmfoto von 2021-02-26 21-10-36

    1. The codes must be from the ISO Standard.
      Bildschirmfoto von 2021-02-26 21-08-05
    1. The languages must be unique.
      Bildschirmfoto von 2021-02-26 21-09-27

If any of the checks are to much or a more general problem arises please let me know.

@njakuschona njakuschona marked this pull request as ready for review March 8, 2021 18:12
Copy link
Contributor

@njakuschona njakuschona left a comment

Choose a reason for hiding this comment

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

The code looks good so far. I would suggest to test if the reset and tab change works as expected after the merge. I can take care of it after the merge.

Copy link
Member

@nuest nuest left a comment

Choose a reason for hiding this comment

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

Looks good to me (judging from the screenshots), I trust @NJaku01 to confirm it works :-)

@nuest
Copy link
Member

nuest commented Mar 11, 2021

@NJaku01 please go ahead with the merge, ideally you can follow up with the mentioned tests right afterwards.

@njakuschona njakuschona merged commit b21230f into o2r-project:dev Mar 11, 2021
njakuschona added a commit that referenced this pull request Mar 11, 2021
@njakuschona
Copy link
Contributor

@nuest I tested the reset. I fixed some minor errors; the fixes are in #204. After @Fmazin reviewed these changes, I would merge them and merge the tests afterwards.

njakuschona added a commit that referenced this pull request Mar 11, 2021
njakuschona added a commit to njakuschona/o2r-UI that referenced this pull request Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants