-
Notifications
You must be signed in to change notification settings - Fork 64
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
Deactivate export buttons when process does not contain images #4224
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First issue: Each time the user interacts with the user interface, for each process that is displayed in the list, it is checked eight times on the file system to determine whether the process contains images.
In the Restore View phase:
- to determine the style for the commandLink exportMets
- to determine the disabled status for the commandLink exportMets
- to determine the style for the commandLink exportDms
- to determine the disabled status for the commandLink exportDms
Four more times in the Render Response phase. Caching should be implemented here.
Second issue: The way checking for images is error-prone, as it doesn’t handle all special cases. I would suggest the following.
public boolean hasImages(int processId) throws DAOException {
Process process = ServiceManager.getProcessService().getById(processId);
Folder generatorSource = process.getProject().getGeneratorSource();
if(Objects.isNull(generatorSource)) {
return false;
}
Subfolder sourceFolder = new Subfolder(process, generatorSource);
return !sourceFolder.listContents().isEmpty();
}
BTW, I am unsure if this method should be in FileService
or ProcessService
.
*/ | ||
public boolean hasImages(int processId) { | ||
try { | ||
return FileService.hasImages(processId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, we do access File Service using the Service Loader. However, I prefer static access where possible. See #3327.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I take it you agree with hasImages
remaining static
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kathrin-Huber should decide
Kitodo/src/main/java/org/kitodo/production/services/file/FileService.java
Outdated
Show resolved
Hide resolved
Kitodo/src/main/webapp/WEB-INF/templates/includes/processes/processActionsColumn.xhtml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to cover all requirements.
Fixes #3292