https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39320

--- Comment #47 from Jonathan Druart <[email protected]> ---
(In reply to Pedro Amorim from comment #46)

Thanks for the quick follow-ups!

> > 2. Do we really need the /erm/counts route. It needs tests (missing btw),
> > swagger spec, Mojo controller. When we could just hit /agreements count=1
> > and retrieve the X-Base-Total-Count headers.
> 
> Patch with tests added. Yes we need, otherwise it's 6 HTTP requests instead
> of 1 just to get counts. 

Not convince. I'd like Tomas's POV on /counts.

> > 5. From the "Counts" widget the links are "#", we should link to eg.
> > /agreements instead.
> > It also generates a vue warning:
> > """
> > [Vue Router warn]: history.state seems to have been manually replaced
> > without preserving the necessary values. Make sure to preserve existing
> > history state if you are manually calling history.replaceState:
> > 
> > history.replaceState(history.state, '', url)
> > """
> 
> Patch added. I've not seen the vue router warn before or after.

I've seen it again, then didn't recreate.
I also got "[Vue Router warn]: Discarded invalid param(s) "agreement_id" when
navigating." when moving from /agreements to /home.

Logging it here for the records, not blocker.

> > 7. ModuleDashboard/Widgets/ERMCounts.vue
> > {{ $__("There are") }}
> > It won't be correctly translatable, use placeholder (%s and format) instead.
> > By the way I don't think you should use definition.labelSingular and
> > definition.labelPlural here. It won't be translatable correctly in some
> > languages.
> > I don't have a suggestion to fix this and keep it in a loop however.
> 
> Patch added.

```
120             const joined = parts.join(", ");
121             return $__("There are %s.").format(joined);
```

this is still impossible to translate

I have no idea how to fix that, or... we cheat?

what about moving the list to ul/li?

```
There are:

1 agreement
0 license
etc.
```

> > 8. Widgets/ERMLatestSUSHIJobs.vue
> > b. The "View" job button: you are using a "viewJob" function. I think you
> > should use the callback instead, see Vendors/VendorResource.vue, "Receive
> > shipments"
> > 712                             callback: ({ id }, dt, event) => {
> > 713                                 event.preventDefault();
> > 714                                 window.location.href =
> > `/cgi-bin/koha/acqui/parcels.pl?booksellerid=${id}`;
> > 715                             },
> 
> Using viewJob is the same as using 'show' which is what we're using
> elsewhere in the framework for KohaTable action buttons.

Yes, but it's more verbose... Not blocker.

> > 9. Job status
> > Widgets/ERMLatestSUSHIJobs.vue
> >  44         const job_statuses = [
> > 
> >  53         function get_job_status(status) {
> >  54             const status_lib = job_statuses.find(s => s._id === status);
> >  55             return status_lib ? status_lib._str : status;
> >  56         }
> > 
> > If we add a new status we will obviously forget this place. I wouldn't
> > silently ignore an unknown status.
> 
> It's not silently ignoring an unknown status. It prints the untranslatable
> status code if it doesn't find a match.

Yes, so it's not visible, and for me it means it's ignored :)
We are supposed to have this list updated when we add a new status to the job
table. If we add "paused", devs will look at the regular pl/pm/tt, but for sure
forget this vue file. I would display "Status unknown" or "Status unknown:
${status}" (or even alert) so we clearly see there is something missing in the
code.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to