daisy / pipeline-ui

A user interface for the DAISY Pipeline 2
MIT License
6 stars 2 forks source link

Outputs that don't get a link in the "Results" box #44

Closed bertfrees closed 1 year ago

bertfrees commented 1 year ago

Outputs defined as px:output="result" px:type="anyDirURI"don't get a link in the "Results" box. The output is listed, but there is no link (only the label).

bertfrees commented 1 year ago

Hold it, I'm not sure the type of output is the reason why I have seen this happen with the result links. Need to investigate more.

bertfrees commented 1 year ago

OK I've got something reproducible. Try the "DAISY 3 to MP3 (beta)" script, e.g. with the "dontworrybehappy" book. The output is generated (you can verify that the files are there under ~/Library/Application\ Support/pipeline-ui/jobs) but there is no link in the UI.

marisademeglio commented 1 year ago

Can you paste what the web service returns for this job when it's finished?

bertfrees commented 1 year ago
<job xmlns="http://www.daisy.org/ns/pipeline/data" href="http://127.0.0.1:49152/ws/jobs/07688b2f-2421-403c-b316-72208a5e3a42" id="07688b2f-2421-403c-b316-72208a5e3a42" priority="medium" status="SUCCESS">
  <nicename>DAISY 3 to MP3 (beta)</nicename>
  <script href="http://127.0.0.1:49152/ws/scripts/daisy3-to-mp3" id="daisy3-to-mp3">
    <nicename>DAISY 3 to MP3 (beta)</nicename>
    <description>Transforms a DAISY 3 publication into a folder structure with MP3 files (beta).</description>
    <version>1.0.1</version>
  </script>
  <messages progress="1.0">
    <message content="Loading DAISY 3" level="INFO" portion="0.1" progress="1.0" sequence="8" timeStamp="1670953179760"/>
    <message content="Rearranging audio into folder structure" level="INFO" portion="0.8" progress="1.0" sequence="2126" timeStamp="1670953183820"/>
    <message content="Storing MP3 files" level="INFO" portion="0.1" progress="1.0" sequence="2879" timeStamp="1670953186734">
      <message content="Failed to load content types: /Users/bert/Desktop/DAISY Pipeline.app/Contents/Resources/app.asar.unpacked/resources/daisy-pipeline/jre/lib/content-types.properties" level="WARNING" sequence="3037" timeStamp="1670953186924"/>
    </message>
  </messages>
  <log href="http://127.0.0.1:49152/ws/jobs/07688b2f-2421-403c-b316-72208a5e3a42/log"/>
  <results href="http://127.0.0.1:49152/ws/jobs/07688b2f-2421-403c-b316-72208a5e3a42/result" mime-type="application/zip">
    <result from="option" href="http://127.0.0.1:49152/ws/jobs/07688b2f-2421-403c-b316-72208a5e3a42/result/option/output-dir" mime-type="application/zip" name="output-dir" nicename="MP3 files">
      <result file="file:/Users/bert/Library/Application%20Support/pipeline-ui/jobs/07688b2f-2421-403c-b316-72208a5e3a42/output/output-dir/001/001/001.mp3" href="http://127.0.0.1:49152/ws/jobs/07688b2f-2421-403c-b316-72208a5e3a42/result/option/output-dir/idx/output-dir/001/001/001.mp3" mime-type="text" size="75233"/>
      <result file="file:/Users/bert/Library/Application%20Support/pipeline-ui/jobs/07688b2f-2421-403c-b316-72208a5e3a42/output/output-dir/003/001/001.mp3" href="http://127.0.0.1:49152/ws/jobs/07688b2f-2421-403c-b316-72208a5e3a42/result/option/output-dir/idx/output-dir/003/001/001.mp3" mime-type="text" size="74083"/>
      <result file="file:/Users/bert/Library/Application%20Support/pipeline-ui/jobs/07688b2f-2421-403c-b316-72208a5e3a42/output/output-dir/003/002/001.mp3" href="http://127.0.0.1:49152/ws/jobs/07688b2f-2421-403c-b316-72208a5e3a42/result/option/output-dir/idx/output-dir/003/002/001.mp3" mime-type="text" size="78576"/>
      <result file="file:/Users/bert/Library/Application%20Support/pipeline-ui/jobs/07688b2f-2421-403c-b316-72208a5e3a42/output/output-dir/004/001/001.mp3" href="http://127.0.0.1:49152/ws/jobs/07688b2f-2421-403c-b316-72208a5e3a42/result/option/output-dir/idx/output-dir/004/001/001.mp3" mime-type="text" size="96131"/>
      <result file="file:/Users/bert/Library/Application%20Support/pipeline-ui/jobs/07688b2f-2421-403c-b316-72208a5e3a42/output/output-dir/002/001/001.mp3" href="http://127.0.0.1:49152/ws/jobs/07688b2f-2421-403c-b316-72208a5e3a42/result/option/output-dir/idx/output-dir/002/001/001.mp3" mime-type="text" size="124761"/>
      <result file="file:/Users/bert/Library/Application%20Support/pipeline-ui/jobs/07688b2f-2421-403c-b316-72208a5e3a42/output/output-dir/002/002/001.mp3" href="http://127.0.0.1:49152/ws/jobs/07688b2f-2421-403c-b316-72208a5e3a42/result/option/output-dir/idx/output-dir/002/002/001.mp3" mime-type="text" size="82233"/>
    </result>
  </results>
</job>
NPavie commented 1 year ago

After a quick check, I think the problem is in https://github.com/daisy/pipeline-ui/blob/8cc5065f191db642ae4ac1b0bc648f25a8df370d/src/renderer/components/JobDetailsPane/Results.tsx#L55-L60

The slice operation needs a start and an end number, so we should have this to get the link displayed correctly

    let filename = fileHref
        ? fileHref.slice(
              fileHref.lastIndexOf('/'),
              fileHref.length
          )
        : ''

This make me think of a possible readability issue with the results panel : As we provide links for all files within a namedResults fileset, there can be a lot of files to display in that case. (For example when converting a sample to a daisy3, right now we have the list of all files within the produced daisy3, with a link to each file displayed in a list, but it might be more useful for a user to get only a link to open the results' folder)

marisademeglio commented 1 year ago

Ah yes good catch, it's "splice" that takes a count for its 2nd parameter where as "slice" wants an index.

The filename calculation that you pasted above is to get the label for the link, not the link itself -- but maybe that's the issue.

Let's look at readability issues too and if there is a problem we can open another issue. The "results" window should scroll like the messages and settings windows so if the list gets long, I think it's ok.

As for displaying every result vs just a link to the results folder, Pipeline UI version 0.1.0 just displayed a link to the results folder and we got feedback that the UI should list every result too. So that's why we may have this massive list but I agree in many cases, the user will just want to see the output folder, so that's why we are keeping the results folder link and placing it at the top of the job details view, so it's really obvious.

marisademeglio commented 1 year ago

@bertfrees can you send or link to the sample book "don't worry be happy" that you used to recreate this issue? I thought it was on the DAISY website under "samples" but I don't see it there now.

bertfrees commented 1 year ago

I used this one: https://github.com/daisy/pipeline-samples/tree/master/daisy3/dontworrybehappy

bertfrees commented 1 year ago

As we provide links for all files within a namedResults fileset, there can be a lot of files to display in that case.

When I suggested to have buttons/links for all results, I meant results as in "direct children of the results element". (These correspond with the Collection<JobResult> that are returned from the JobResultSet API for the port or option name in question.)

The child result elements are the individual files within each result. (These correspond with the JobResult in the Java API.) I don't think it makes that much sense to include them.

I can open a new issue for this if you want.

Note that the nicename attribute of the results is already used in the UI but I think there should also be a description attribute and that this should be used also by the UI, because now we're ignoring that useful information.

marisademeglio commented 1 year ago

Ok thanks, that was not clear from your earlier comment (in the email). Yes a new issue will be the way to go!

marisademeglio commented 1 year ago

@bertfrees I am trying to reproduce the error but I don't have the DAISY 3 to MP3 script in the Pipeline build that we're using. Does this error appear in other scripts too?

I suspect @NPavie is correct in that it's just an empty string bug but I'd like to test it out. Any script that is known to produce multiple result elements nested in a result element should have the same issue.

Aside: having these element names and nested structure (e.g. results>result>result) where they all mean different things isn't at all intuitive.

bertfrees commented 1 year ago

Ah yes, that's the version that includes v1.14.9 of the engine. It was built from the build-engine branch. There are various other scripts that produce multiple files in a result. E.g. anything that produces a DAISY 2.02 or DAISY 3.

NPavie commented 1 year ago

As for displaying every result vs just a link to the results folder, Pipeline UI version 0.1.0 just displayed a link to the results folder and we got feedback that the UI should list every result too.

Ha ok so forget about my recommandation ^^'

Sorry I was a bit quick in my explanations, but i tested and stated the issue using a daisy2 to daisy3 script. The problem was indeed that the list of links was correctly created (with each link having a # only href but having an onclick action used to open the file), but links were not displayed on screen, only empty lines, making the links not clickable.

NPavie commented 1 year ago

(i created a fix-44 branch with the fix i mentioned, i'll push it for tests)

marisademeglio commented 1 year ago

Oh we're good I have a fix here sandwiched in between some other pending commits (otherwise I would just take yours)! I almost pushed it yesterday but I was waiting to hear from Bert (yay async development!)

bertfrees commented 1 year ago

All outputs have a link now, but the files are not stored in the right place. The directory structure is lost. In case of the "DAISY 3 to MP3 (beta)" example above the result is a "output-dir" directory with a single file "001.mp3". All the links point to this same file.

bertfrees commented 1 year ago

The directory structure is still lost.

NPavie commented 1 year ago

Can you be more specific on what "directory structure" mean or provide an example of the expected structure with an example job ?

If it is the missmatch between the folder tree displayed in the UI versus the real file tree, it is because i use the named result name instead of the "nicename" when unzipping the result. If wanted, this can be easily fixed.

bertfrees commented 1 year ago

The whole structure of an output that has with files within directories, is lost. There are simply no directories. Everything is flat (and files with the same name are overwritten by each other). I gave an example in my comment above. All scripts that output a directory structure have this problem.

NPavie commented 1 year ago

The DAISY 3 or DAISY 2.02 to MP3 scripts seem to not work on windows: when testing with the "don't worry by happy" samples, the pipeline is returning ERROR - Audio file could not be read (unsupported file type): file:/C:/Users/npavie/Nextcloud/Devs/test-files/dontworrybehappy-daisy3/speechgen0001.mp3 (Please see detailed log for more info.)

Do you have another script that i could use for debugging this ? (I need to check if this is not linked to the decompress library i used in the backed ?)

bertfrees commented 1 year ago

The DAISY 3 or DAISY 2.02 to MP3 scripts seem to not work on windows

Hmm, that might indicate another issue then. I'll try it on macOS, and also with the CLI.

Do you have another script that i could use for debugging this ?

Just use the DAISY 3 to MP3 script (https://github.com/daisy/pipeline-ui/issues/44#issuecomment-1348496438).

NPavie commented 1 year ago

In the wait of a fix for the 2 scripts on windows machine, I'll try the 2 scripts on my mac machine for debugging this issue.

bertfrees commented 1 year ago

Aha, sorry, I misread. You already tested the DAISY TO MP3 script. You can also try it with scripts that output a DAISY, although I'm not sure exactly in which cases the output contains subfolders.

bertfrees commented 1 year ago

I've checked that DAISY 3 to MP3 works fine on macOS, also from the UI.