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.

Sign up for GitHub

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

Jump to bottom

Mass import #4901

Closed
wants to merge 4 commits into from
Closed

Mass import #4901

wants to merge 4 commits into from

Conversation

IkramMaalej
Copy link
Collaborator

Redesign massImport

  • Combining the two tabs 'file MassImport' and 'Text MassImport' in one tab.
  • Add metadataTable in order to offer the possibility to add more Metadata to the imported metadata.

Part of #4322

markusweigelt
markusweigelt reviewed Jan 10, 2022
View reviewed changes
Kitodo/src/main/resources/messages/messages_en.properties Outdated
@@ -592,6 +592,7 @@ messages=Messages
metadata=Metadata
metadataCorrupt=The metadata file is faulty, please inform the administrator.
metadataDelete=Delete metadata
metadataDeleteNotAllowed=Metadata is set as'Required' oder "alwaysShowing" in the ruleset
metadataDoesNotEndWith=not end with the value
Copy link
Collaborator

Choose a reason for hiding this comment

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

From the user's point of view, a little more context may be required, for example "The deletion of the metadata is not possible because metadata is set as 'Required' oder "alwaysShowing" in the ruleset.

markusweigelt
markusweigelt reviewed Jan 10, 2022
View reviewed changes
Kitodo/src/main/resources/messages/messages_de.properties Outdated
@@ -580,6 +580,7 @@ messages=Nachrichten
metadata=Metadaten
metadataCorrupt=Die Metadatendatei ist fehlerhaft, bitte informieren Sie ihren Administrator.
metadataDelete=Metadaten l\u00F6schen
metadataDeleteNotAllowed=Metadaten ist in Regelsatz als 'Required' oder "alwaysShowing" konfiguriert
metadataDoesNotEndWith=endet nicht mit dem Wert
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest change with more context see en properties

markusweigelt
markusweigelt suggested changes Jan 10, 2022
View reviewed changes
Copy link
Collaborator

@markusweigelt markusweigelt left a comment

Choose a reason for hiding this comment

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

From my point of view, code is save except for the small changes and Codacy. However, I still have to test it in the application.

@IkramMaalej IkramMaalej force-pushed the mass-import branch 2 times, most recently from 3f19cc9 to 93b1f19 Compare January 11, 2022 08:53
solth
solth requested changes Jan 14, 2022
View reviewed changes
Kitodo/src/main/webapp/pages/massImport.xhtml Outdated
rendered="#{MassImportForm.importMode.equals('file') and MassImportForm.selectedCatalog ne null}">
<div>
<p:fileUpload value="#{MassImportForm.file}"
fileUploadListener="#{MassImportForm.handleFileUpload}"
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs to be updated to the Primefaces 8 syntax (see #4909)

@solth
Copy link
Member

solth commented Jan 14, 2022

metadata-button-design

This vertical offset between input fields and command buttons could probably be fixed with something combination of CSS rules display: inline-block and vertical-align: text-bottom for the buttons.

@IkramMaalej IkramMaalej force-pushed the mass-import branch 2 times, most recently from 999adfa to 5d2d693 Compare January 19, 2022 18:03
@IkramMaalej IkramMaalej force-pushed the mass-import branch 2 times, most recently from e898201 to 3f38e5e Compare February 8, 2022 13:36
@BartChris
Copy link
Collaborator

I created two other issues regarding mass import today. I do not know if you want to adress them in this pull request as well, but maybe you want to tackle those problems as well while you are at it.
I leave theme here for reference

#4988
#4989

@solth
Copy link
Member

solth commented Feb 16, 2022

I created two other issues regarding mass import today. I do not know if you want to adress them in this pull request as well, but maybe you want to tackle those problems as well while you are at it.
I leave theme here for reference

I would opt to solve those issues in separate pull requests because this one is already huge enough as it is and shouldn't be delayed further.

The release management also prefers many small pull requests over few large ones, AFAIK.

@IkramMaalej IkramMaalej force-pushed the mass-import branch 3 times, most recently from 573dca8 to a0da846 Compare March 4, 2022 08:16
@IkramMaalej IkramMaalej marked this pull request as draft March 7, 2022 11:14
@IkramMaalej IkramMaalej marked this pull request as ready for review March 18, 2022 10:15
@IkramMaalej IkramMaalej marked this pull request as draft March 24, 2022 09:40
@IkramMaalej IkramMaalej deleted the mass-import branch April 5, 2022 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markusweigelt markusweigelt markusweigelt requested changes

@solth solth solth requested changes

@Kathrin-Huber Kathrin-Huber Awaiting requested review from Kathrin-Huber

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@IkramMaalej @solth @BartChris @markusweigelt