Refactor mass import page to use base list view instead of base edit view by solth · Pull Request #6983 · kitodo/kitodo-production

solth · 2026-04-13T16:40:45Z

Fixes #6805 by refactoring the mass import page so that it is not based upon the baseEditView.xhtml template anymore and thus can continue to use the required enctype="multipart/form-data"

BartChris · 2026-04-13T16:41:49Z

also to check: #6883

codacy-production · 2026-04-13T16:42:28Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results

Complexity 0

Duplication 0

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

solth · 2026-04-14T06:08:13Z

also to check: #6883

@BartChris would you mind reviewing this pull request, with that particular issue in mind, as you worked on the mass import quite a lot yourself?

BartChris · 2026-04-15T07:33:15Z

When the uploaded csv file gets bigger i run into problems:

I uploaded a csv file with 453 rows:

When removing one of the red marked pseudo columns or when trying to switch the catalog the table is emptied:

BartChris · 2026-04-15T07:58:10Z

When uploading a bigger csv file the "Add metadata" dialog is just an empty dialog:

vs

Edit: OK the fix for this button is the same:

   <p:commandButton id="addMetadataColumn"
                                         title="#{msgs['dataEditor.addMetadata.newMetadata']}"
                                         disabled="#{MassImportForm.records.size() eq 0}"
                                         process="@this"
                                         partialSubmit="true"
                                         style="margin: 3px"
                                         icon="fa fa-plus"
                                         styleClass="secondary"
                                         action="#{MassImportForm.addMetadataDialog.prepareMetadataTypes()}"
                                         update="addMetadataDialog"
                                         oncomplete="PF('addMetadataDialog').show();"/>

BartChris · 2026-04-15T08:04:42Z

Maybe @henning-gerhardt can also check if his issue with the role settings is fixed, because the error always comes down to the number of elements edited.

BartChris · 2026-04-15T08:10:28Z

I just noticed that the problems i encountered also appear in 3.9.0.

henning-gerhardt · 2026-04-15T08:11:21Z

As we at SLUB did not use the mass import feature and all changes are only focused on mass import I don't think that this pull request is fixing #6805 in first place. The comments in #6805 are switching fast from the original issue to an other maybe related issue and so I don't think that the main issue is fixed with this changes here.

henning-gerhardt · 2026-04-15T08:19:28Z

I tried it again and #6805 is already fixed by some changes between December 2025 and April 2026. So this pull request is not really related to the original issue of #6805 but maybe still to the comments in #6805.

solth · 2026-04-15T08:28:16Z

I tried it again and #6805 is already fixed by some changes between December 2025 and April 2026. So this pull request is not really related to the original issue of #6805 but maybe still to the comments in #6805.

Yes, exactly. The linked issue was already fixed by #6810, I think, and the necessity to refactor the mass import form resulted from the changes in that pull request.

solth · 2026-04-15T12:16:20Z

@BartChris thanks a lot for your review and suggestions, which fixed the problems you reported. (I commited them manually instead of applying the suggestions via GitHub because some indentations seemed to be lost in the suggestions).

I also increased the sizeLimit to a hopefully more reasonble value (and rebased against the current main branch).

Would you mind checking the pull request again?

BartChris · 2026-04-16T14:07:33Z

I noticed that the MassImport button to trigger the import is never activated. (Maybe because of the changes i suggested)

I suppose we have to update it explicitely whenever we execute an action. I tested it with the catalogueSelect box and added massImportButtonForm to the list of to be updated components. Now the button is enabled after switching the catalog.

  <p:selectOneMenu id="catalogueSelect"
                                     disabled="#{not MassImportForm.firstColumnContainsRecordsIdentifier()}"
                                     required="#{not empty param['massImportButtonForm:importButton']}"
                                     converter="#{importConfigurationConverter}"
                                     value="#{MassImportForm.importConfigurationId}">
                        <f:selectItem itemValue="#{null}"
                                      itemLabel="-- #{msgs.selectCatalog} --"
                                      noSelectionOption="true"/>
                        <f:selectItems value="#{CreateProcessForm.catalogImportDialog.importConfigurations}"
                                       var="configuration"
                                       itemLabel="#{configuration.title}"
                                       itemValue="#{configuration}"/>
                        <p:ajax process="@this"
                                partialSubmit="true"
                                update="recordsForm massImportButtonForm"/>
                    </p:selectOneMenu>

BartChris · 2026-04-16T14:14:13Z

This is not directly related to the PR and can maybe be fixed later, but i am wondering if we should spam the screen with tons of messages for every imported row:

codacy-production · 2026-04-16T16:03:55Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 2 duplication

Metric Results

Complexity 0

Duplication 2

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

solth · 2026-04-16T18:41:08Z

This is not directly related to the PR and can maybe be fixed later, but i am wondering if we should spam the screen with tons of messages for every imported row:

Yes, that's suboptimal, and yes, we should address that in a separate pull request.

solth · 2026-04-17T07:38:11Z

@BartChris I get a FacesException when trying to edit cell contents from slightly larger CSV files (about 100 rows), which I cannot explain, yet. Maybe it has something to do with lazy loading additional entries, but I don't now for sure.

…view

…age layout

BartChris · 2026-04-17T13:52:33Z

@BartChris I get a FacesException when trying to edit cell contents from slightly larger CSV files (about 100 rows), which I cannot explain, yet. Maybe it has something to do with lazy loading additional entries, but I don't now for sure.

I do not get an error here (maybe you fixed that) but i also cannot edit and save the values (it keeps the old value). I would have to check if this ever worked on large tables.

solth · 2026-04-17T14:00:38Z

I do not get an error here (maybe you fixed that) but i also cannot edit and save the values (it keeps the old value). I would have to check if this ever worked on large tables.

Yes, I was able to fix the problem with larger files by using the enctype="multipart/form-data" in the recordsForm, but editing CSV cells is currently not possible.

I tried updating the current row in

https://github.com/solth/kitodo-production/blob/c078685857381483bfff1233f0d19009c8d177da/Kitodo/src/main/webapp/WEB-INF/templates/includes/massImport/massImportTab.xhtml#L148

but that did not work, either.

BartChris · 2026-04-17T14:08:00Z

I do not get an error here (maybe you fixed that) but i also cannot edit and save the values (it keeps the old value). I would have to check if this ever worked on large tables.

Yes, I was able to fix the problem with larger files by using the enctype="multipart/form-data" in the recordsForm, but editing CSV cells is currently not possible.

Did this work in 3.8? I would say this is a Mass Import and values should be provided by the CSV in general, so i do not see it as a critical problem, if values cannot be edited directly in the UI.

solth · 2026-04-17T14:20:38Z

Did this work in 3.8? I would say this is a Mass Import and values should be provided by the CSV in general, so i do not see it as a critical problem, if values cannot be edited directly in the UI.

Yes, it did work in 3.8. And in 3.9 it works as well. In main, the mass import is pretty broken right now, though, to the point where it is not usable at all. The pull request does improve things and restores usabilty in general, but not being able to edit uploaded data (in contrast editing manually entered rows without a CSV upload seems to work fine) still is quite a regression when compared to previous releases, in my opinion.

solth · 2026-04-17T14:36:05Z

@BartChris as a temporary step I deactivated the option to edit datatable cells after uploading a CSV file, so that this feature is not offered in a broken state.

I would suggest to go ahead and merge this pull request if you don't find any other problems with it, to at least restore the basic functionality of the mass import again. Once we get cell editing working again we can reactivate it afterwards in a separate pull request. What do you think?

BartChris

Yes, i agree. The rest of the functionality works for me

BartChris · 2026-04-17T14:51:02Z

Hmm, right now i got a problem when starting the import i have not seen before although i suppose this is not connected to the PR:

solth · 2026-04-17T14:56:39Z

Hmm, right now i got a problem when starting the import i have not seen before although i suppose this is not connected to the PR:

We should probably make sure it isn't. Perhaps you could test with the same case in the main branch?

I just noticed that the "Mass import" button still isn't activated immediately after uploading a CSV files that already contains all data (thus metadata of type documentType in the first column) and doesn't require catalog queries.

BartChris · 2026-04-17T14:57:44Z

I will check. Right now i suspect that this is again about external schema files i cannot resolve...

codacy-production · 2026-04-17T15:01:14Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 2 duplication

Metric Results

Complexity 0

Duplication 2

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

BartChris · 2026-04-17T15:05:21Z

Ok, after commenting out

validationService.validateMappingFiles(importConfiguration.getMappingFiles());

and deactivating validation here

private String importProcessAndReturnParentID(String recordId, LinkedList<TempProcess> allProcesses,
                                                  ImportConfiguration importConfiguration, int projectID,
                                                  int templateID, boolean isParentInRecord, String parentIdMetadata,
                                                  boolean validateAgainstXmlSchema)

it works. So this is not directly connected and about some validation issue.

solth · 2026-04-17T15:08:47Z

It should be possible to deactivate XML validation of external data for an import configuration in general, so you don't have to tamper with the code to achieve that:

BartChris · 2026-04-17T15:09:21Z

Same error on main, so definitely not connected!

BartChris · 2026-04-17T15:14:31Z

It should be possible to deactivate XML validation of external data for an import configuration in general, so you don't have to tamper with the code to achieve that:

In importProcessForMassImport the validation flag seems hardcoded and not influenced by the setting.

Edit:It seems to be additionally checked in convertDataRecordToInternal, sorry. But validateExternal is always true.

// validate external record against corresponding XML metadata schema(ta)
if (validateExternal && importConfiguration.getValidateExternalData()) {
           validationService.validateExternalRecord(xmlContent, importConfiguration, identifier);
}

But it seems like

// validate mapping files against xslt schema
validationService.validateMappingFiles(importConfiguration.getMappingFiles());

is always called. In my case i suppose the mapping file validation let the process fail.

solth · 2026-04-17T15:17:11Z

In importProcessForMassImport the validation flag seems hardcoded and not influenced by the setting.

Good that we found this out so it can be fixed as well!

…upload

solth · 2026-04-17T15:44:35Z

In importProcessForMassImport the validation flag seems hardcoded and not influenced by the setting.

Good that we found this out so it can be fixed as well!

Interestingly, the setting in the import configuration actually seemed to work for me.

When schema validation was activated in the configuration, I got the following result in mass import:

After deactivating the schema validation in the configuration, though, the import of the same IDs from the same catalog interface was succesful:

It seems the static parameter true in https://github.com/solth/kitodo-production/blob/e1fdd90482a14ce073dac77044db74c8a9e1884c/Kitodo/src/main/java/org/kitodo/production/services/data/ImportService.java#L1577 does not actually control whether schema validation is applied during mass import or not 🤔

Maybe your import didn't really fail because of schema invalid metadata, but because the imported XML file or the import mapping XSLT did not contain valid XML to begin with?

BartChris · 2026-04-17T15:54:06Z

Maybe your import didn't really fail because of schema invalid metadata, but because the imported XML file or the import mapping XSLT did not contain valid XML to begin with?

I have to check deeper, but my investigations right now seem to indicate that is not the validation of the catalog entries which fails for me, but the validation of the mapping files. And the latter might come down to the well known problem of external schema files, which are maybe blocked for me.

https://github.com/solth/kitodo-production/blob/e1fdd90482a14ce073dac77044db74c8a9e1884c/Kitodo/src/main/java/org/kitodo/production/services/data/ImportService.java#L962

kitodo-production/Kitodo/src/main/resources/schemata/xslt20.xsd

Lines 57 to 64 in e1fdd90

    
           <xs:import namespace="http://www.w3.org/XML/1998/namespace" schemaLocation="http://www.w3.org/2001/xml.xsd"/> 
        
           <!--  
        
               An XSLT stylesheet may contain an in-line schema within an xsl:import-schema element, 
        
               so the Schema for schemas needs to be imported 
        
           --> 
        
           <xs:import namespace="http://www.w3.org/2001/XMLSchema" schemaLocation="http://www.w3.org/2001/XMLSchema.xsd"/>

Edit: OK, i get the same error in "normal" catalog import... will make an issue next week.

solth requested a review from thomaslow April 13, 2026 16:40

solth added the import Import mappings and configurations label Apr 14, 2026

solth requested review from BartChris and removed request for thomaslow April 14, 2026 11:52

BartChris reviewed Apr 15, 2026

View reviewed changes

Comment thread Kitodo/src/main/webapp/WEB-INF/templates/includes/massImport/massImportTab.xhtml Outdated

BartChris reviewed Apr 15, 2026

View reviewed changes

Comment thread Kitodo/src/main/webapp/WEB-INF/templates/includes/massImport/massImportTab.xhtml

BartChris reviewed Apr 15, 2026

View reviewed changes

Comment thread Kitodo/src/main/webapp/WEB-INF/templates/includes/massImport/massImportTab.xhtml Outdated

BartChris reviewed Apr 15, 2026

View reviewed changes

Comment thread Kitodo/src/main/webapp/WEB-INF/templates/includes/massImport/massImportTab.xhtml

solth force-pushed the refactor-mass-import-page branch from 3415193 to 1022eef Compare April 15, 2026 12:17

solth requested a review from BartChris April 15, 2026 13:19

solth force-pushed the refactor-mass-import-page branch from 1022eef to 0c08d89 Compare April 16, 2026 09:13

BartChris reviewed Apr 16, 2026

View reviewed changes

Comment thread Kitodo/src/main/webapp/WEB-INF/templates/includes/massImport/massImportTab.xhtml Outdated

solth marked this pull request as draft April 16, 2026 14:11

Refactor mass import page to use base list view instead of base edit …

8d351ad

…view

solth added 5 commits April 17, 2026 10:49

Implement change requests

24278cf

Fix mass import button never being activated and refine mass import p…

f928adf

…age layout

Add Selenium tests for adding and removing CSV rows during mass import

1cfe4bb

Add Selenium test for edting cell contents during mass import

a3345b5

Add missing 'form' element and some more 'partialSubmit's

8ddb6f9

solth force-pushed the refactor-mass-import-page branch from d2ae1ff to 8ddb6f9 Compare April 17, 2026 08:50

Add 'enctype' of records form to 'multipart/form-data'

c078685

Deactivate cell editing after CSV upload

730328b

BartChris approved these changes Apr 17, 2026

View reviewed changes

solth marked this pull request as ready for review April 17, 2026 14:46

Activate import button after CSV file upload in mass import form

3477b1c

Temporarily disable test to edit mass import cell contents after CSV …

877f383

…upload

solth merged commit cc93607 into kitodo:main Apr 17, 2026
6 checks passed

solth deleted the refactor-mass-import-page branch April 17, 2026 19:10

Conversation

solth commented Apr 13, 2026

Uh oh!

BartChris commented Apr 13, 2026

Uh oh!

codacy-production bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

solth commented Apr 14, 2026

Uh oh!

Uh oh!

BartChris commented Apr 15, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BartChris commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BartChris commented Apr 15, 2026

Uh oh!

BartChris commented Apr 15, 2026

Uh oh!

henning-gerhardt commented Apr 15, 2026

Uh oh!

henning-gerhardt commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solth commented Apr 15, 2026

Uh oh!

solth commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

BartChris commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BartChris commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codacy-production bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

solth commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solth commented Apr 17, 2026

Uh oh!

BartChris commented Apr 17, 2026

Uh oh!

solth commented Apr 17, 2026

Uh oh!

BartChris commented Apr 17, 2026

Uh oh!

solth commented Apr 17, 2026

Uh oh!

solth commented Apr 17, 2026

Uh oh!

BartChris left a comment

Choose a reason for hiding this comment

Uh oh!

BartChris commented Apr 17, 2026

Uh oh!

solth commented Apr 17, 2026

Uh oh!

BartChris commented Apr 17, 2026

Uh oh!

codacy-production bot commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

BartChris commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solth commented Apr 17, 2026

Uh oh!

BartChris commented Apr 17, 2026

Uh oh!

codacy-production bot commented Apr 13, 2026 •

edited

Loading

BartChris commented Apr 15, 2026 •

edited

Loading

henning-gerhardt commented Apr 15, 2026 •

edited

Loading

solth commented Apr 15, 2026 •

edited

Loading

BartChris commented Apr 16, 2026 •

edited

Loading

BartChris commented Apr 16, 2026 •

edited

Loading

codacy-production bot commented Apr 16, 2026 •

edited

Loading

solth commented Apr 16, 2026 •

edited

Loading

codacy-production bot commented Apr 17, 2026 •

edited

Loading

BartChris commented Apr 17, 2026 •

edited

Loading

BartChris commented Apr 17, 2026 •

edited

Loading

BartChris commented Apr 17, 2026 •

edited

Loading