This string appears as is in Weblate to be translated, HTML tags and all <li>Job for uploaded file has been queued, <a href="/cgi-bin/koha/admin/background_jobs.pl?op=view&id=%s" target="_blank">click here</a> to check its progress.</li> Usually, HTML tags don't appear. I'm not sure what is different for this one. Probably something to do with JS or Vue? For reference, the line is koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersFileImport.vue:93 Furthermore, while we are there to correct it, we should also correct the "click here" link. I understand it is not an accessible way of making links. All other job queue messages look like this H1 Title: The job has been enqueued! Text: It will be processed as soon as possible. Link: View detail of the enqueued job See for example https://git.koha-community.org/Koha-community/Koha/src/branch/master/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt#L78
I found the same type of string again <li>Job for report type <strong>%s</strong> has been queued, <a href="/cgi-bin/koha/admin/background_jobs.pl?op=view&id=%s" target="_blank">click here</a> to check its progress.</li> at koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersList.vue:249
(In reply to Caroline Cyr La Rose from comment #1) > I found the same type of string again > > <li>Job for report type <strong>%s</strong> has been queued, <a > href="/cgi-bin/koha/admin/background_jobs.pl?op=view&id=%s" > target="_blank">click here</a> to check its progress.</li> > > at > > koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/ > UsageStatisticsDataProvidersList.vue:249 Yes, given the code that's expected: 248 message += this.$__( 249 '<li>Job for report type <strong>%s</strong> has been queued, <a href="/cgi-bin/koha/admin/background_jobs.pl?op=view&id=%s" target="_blank">click here</a> to check its progress.</li>' 250 ).format(job.report_type, job.job_id) Could be improved however.
Created attachment 159256 [details] [review] Bug 35392: Avoid HTML in translatable content
Created attachment 159263 [details] [review] Bug 35392: Move usage-reports store into erm store This is all ERM, no need to have it be separate store files
Created attachment 159264 [details] [review] Bug 35392: New getDialoJobEnqueueMessage DRY the dialog message shown when a background job is enqueued in ERM This takes Tomas work in UsageStatisticsDataProvidersList.vue, DRYs it and also applies it to UsageStatisticsDataProvidersFileImport.vue to address the issue mentioned by Caroline. To test: 1) Do a regular harvest, 2) Do a file upload 3) Notice in both cases the message is shown as before
Created attachment 159265 [details] [review] Bug 35392: Add missing file upload dialog cypress test
Pedro, I think those patches should be on another bug.
Well, maybe not, they seem related. I am getting a conflict on the second patch however Applying: Bug 35392: Move usage-reports store into erm store error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/js/vue/stores/usage-reports.js).
But really, it's too late for such changes. We should only fix the translation issue here.
Not tested, but maybe this is better: "Job for report type %s has been queued, %sclick here%s to check its progress".format('<strong>%s</strong>'.format(job.report_type), '<a href="/cgi-bin/koha/admin/background_jobs.pl?op=view&id=%s" target="_blank">'.format(job.job_id), '</a>') What do you think?
(In reply to Jonathan Druart from comment #8) > Well, maybe not, they seem related. > > I am getting a conflict on the second patch however > > Applying: Bug 35392: Move usage-reports store into erm store > error: sha1 information is lacking or useless > (koha-tmpl/intranet-tmpl/prog/js/vue/stores/usage-reports.js). It's actually on the third patch Applying: Bug 35392: New getDialoJobEnqueueMessage Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersList.vue Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersList.vue CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersList.vue
Created attachment 159279 [details] [review] Bug 35392: Avoid HTML in translatable content
Created attachment 159280 [details] [review] Bug 35392: Move usage-reports store into erm store This is all ERM, no need to have it be separate store files
Created attachment 159281 [details] [review] Bug 35392: New getDialoJobEnqueueMessage DRY the dialog message shown when a background job is enqueued in ERM This takes Tomas work in UsageStatisticsDataProvidersList.vue, DRYs it and also applies it to UsageStatisticsDataProvidersFileImport.vue to address the issue mentioned by Caroline. To test: 1) Do a regular harvest, 2) Do a file upload 3) Notice in both cases the message is shown as before
Created attachment 159283 [details] [review] Bug 35392: Add missing file upload dialog cypress test
(In reply to Jonathan Druart from comment #9) > But really, it's too late for such changes. We should only fix the > translation issue here. Hey, thanks for looking! Rebased. I wanted to make it easier to maintain going forward, whilst addressing Caroline's comment about the file upload, but I understand, please make obsolete if you don't agree!
@Joubu, are we able to move on here? I don't want my patches to be the cause of holding off this bug. Please advise. If you decide to obsolete my patches please note that they're also fixing Tomas' original patch: 1) Has a typo "$this." 2) Only addresses UsageStatisticsDataProvidersList.vue. Does not address UsageStatisticsDataProvidersFileImport.vue
The translation is one issue, but there is another: For usability and accessibility we should always put the link on meaningful words, so no links like 'here' or 'click here'. People know that they can click links and screen readers will be happier reading something that reveals the purpose of a link. Maybe this would work? '<li><span>Job for uploaded file has been queued.</span><a href="/cgi-bin/koha/admin/background_jobs.pl?op=view&id=%s" target="_blank">Check progress</a></li>' The span I think would break this into 2 strings for translation. Do we have another translation issue here? + { short: "Jan", description: "January", value: 1, active: true }, + { short: "Feb", description: "February", value: 2, active: true }, + { short: "Mar", description: "March", value: 3, active: true }, + { short: "Apr", description: "April", value: 4, active: true },
(In reply to Katrin Fischer from comment #18) > The translation is one issue, but there is another: > For usability and accessibility we should always put the link on meaningful > words, so no links like 'here' or 'click here'. People know that they can > click links and screen readers will be happier reading something that > reveals the purpose of a link. > > Maybe this would work? > > '<li><span>Job for uploaded file has been queued.</span><a > href="/cgi-bin/koha/admin/background_jobs.pl?op=view&id=%s" > target="_blank">Check progress</a></li>' > > The span I think would break this into 2 strings for translation. > > Do we have another translation issue here? > > + { short: "Jan", description: "January", value: 1, active: true > }, > + { short: "Feb", description: "February", value: 2, active: true > }, > + { short: "Mar", description: "March", value: 3, active: true }, > + { short: "Apr", description: "April", value: 4, active: true }, Katrin, these are very good points, however can we please open one (or two?) new bugs for these issues you mention? The original problem here has to do with HTML being present in a translatable string, whereas I believe what you're referring to are possibly different problems: 1) Usability: "Click here" link in ERM usage statistics background job dialog should be handled differently 2) Possible problem with month names translatability in ERM usage statistics reports (please provide a test plan)
I would need to test this, but I think it's not unproblematic for translations: + '" target="_blank">' + + __("click here") + + "</a> " + + __("to check its progress.") + + "</li>" This will make it 2 separate strings in the po file. This is often quite hard to translate, maybe nicer with Weblate that keeps more of the sequence, but they could end up in 2 different spots of a big file. That's why I was suggesting to rephrase AND get rid of the click here the same time by making the link something like "Check progress". This bug is about improving translatability for a specific string, so I think not totally out of scope. > + { short: "Mar", description: "March", value: 3, active: true }, > + { short: "Apr", description: "April", value: 4, active: true }, For the second: I have no idea where it appears in the ERM module, so hard to provide a test plan right now. The code just looked very suspicious. But totally agree it should be on a separate bug.
Created attachment 160450 [details] [review] Bug 35392: (QA follow-up): Fix 'click here' and 'to check its progress' being 2 seperate translatable strings
Created attachment 160451 [details] label demo (In reply to Katrin Fischer from comment #20) > This will make it 2 separate strings in the po file. This is often quite > hard to translate, maybe nicer with Weblate that keeps more of the sequence, > but they could end up in 2 different spots of a big file. That's why I was > suggesting to rephrase AND get rid of the click here the same time by making > the link something like "Check progress". Fair point, I've submitted a follow patch please take a look when convenient. Submitting a picture here of how it looks in the UI for both situations (new harvest job and file upload job).
Can we get a simple version of this fix for backport, please?
Created attachment 160512 [details] [review] Bug 35392: Fix HTML in translatable string
Created attachment 160513 [details] [review] Bug 35392: Update tests Run: yarn cypress run --spec "t/cypress/integration/ERM/DataProviders_spec.ts"
Created attachment 164466 [details] [review] Bug 35392: Fix HTML in translatable string Signed-off-by: David Nind <david@davidnind.com>
Created attachment 164467 [details] [review] Bug 35392: Update tests Run: yarn cypress run --spec "t/cypress/integration/ERM/DataProviders_spec.ts" Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. Apply patch 2. yarn build 3. Run tests: yarn cypress run --spec "t/cypress/integration/ERM/DataProviders_spec.ts" 4. Tests pass!
Created attachment 164473 [details] [review] Bug 35392: Fix HTML in translatable string Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 164474 [details] [review] Bug 35392: Update tests Run: yarn cypress run --spec "t/cypress/integration/ERM/DataProviders_spec.ts" Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 164475 [details] [review] Bug 35392: Use the same selector as everywhere else for .message Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed for 24.05! Well done everyone, thank you!
Pushed to 23.11.x for 23.11.06
Missing 23.05.x dependencies, no backport.