ojsde / dnb

OJS plugin that exports full texts and metadata to the Deutsche Nationalbibliothek (DNB)
GNU General Public License v2.0
3 stars 3 forks source link

Plugin should give a more precise error description if articles cannot be exported #24

Open BartChris opened 4 months ago

BartChris commented 4 months ago

The export of articles might fail for different reasons, e.g. if the genre of the submission/article has the wrong category

https://github.com/ojsde/dnb/blob/aa5b0e7fcc83e150b832908fb4b4909c2af180b1/DNBExportPlugin.php#L1193

The error message if something cannot be exported is always the same and indicates missing pdf or epub files. This is confusing if there is in fact a PDF but the export fails for other reasons.

https://github.com/ojsde/dnb/blob/aa5b0e7fcc83e150b832908fb4b4909c2af180b1/DNBExportPlugin.php#L780 https://github.com/ojsde/dnb/blob/aa5b0e7fcc83e150b832908fb4b4909c2af180b1/locale/en/locale.po#L133

Would it be possible to supply error messages which indicate what the actual problem is? Thanks a lot.

ronste commented 4 months ago

Hi @BartChris ,

I completely agree with you that any kind of error should be reported indicating the actual problem. What kind of problem did you observe that was not reported or reported by a misleading error message?

Can you provide more information on the actual type of error you observed?

The lines of code you referenced above should only filter out galleys of file types the DNB does not accept (e.g. HTML, XML, exe, sh, ...). The message indicates the type of files the DNB accepts.

The only other reason for filtering out galleys may be the IP filtering of remote galleys. But this is handled by a separate exception and should be corretly reported.

Otherwise this code has nothing to do with other errors that might prevent deposition of an article.

I am happy to discuss any improvments but I need more information on the actual error. Do you use remote galleys or IP filtering? Do you export supplementary files?

Best wishes, Ronald

BartChris commented 4 months ago

In our system we stumbled over the problem that the submission had the wrong category, so that check here failed: https://github.com/ojsde/dnb/blob/aa5b0e7fcc83e150b832908fb4b4909c2af180b1/DNBExportPlugin.php#L1193

The error message indicated a missing PDF, but the articles had the wrong category assigned. Edit: So i am not sure wether wrong categories should just be silently ignored or wether a custom error message is necessary for them. The report of missing PDF files is just misleading.

ronste commented 4 months ago

OK, I understand. So would it be sufficient to modify the error message like this:

"The article {$param} cannot be exported because it has no galley labeled as document or supplementray galley, or the galley type is not PDF or EPUB." ?

BartChris commented 4 months ago

Thanks a lot. This sounds good. Would it maybe be more correct to say "The article {$param} cannot be exported because it has no galley labeled as document, or the galley type is not PDF or EPUB." ? because it is not enough to just provide a supplementray galley as return (count($galleys) > 0); will still evaluate to 0. Greetings Christoph

ronste commented 4 months ago

This error message is shown for both document galleys and supplementary galleys. What about:

"The article {$param} (or one of its supplementary galleys) cannot be exported because it has no galley file labeled as document or the file type of the galley or supplementary galley is not PDF or EPUB."

BartChris commented 4 months ago

Sounds good!

ronste commented 4 months ago

I updated the locales on the 3.3 and 3.4 branches. The changes will be distributed with the next releases.