https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39320 --- Comment #46 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- (In reply to Jonathan Druart from comment #33)
QA comments:
1. :deep CSS rules should be removed (or moved to css/vue.css)
Patch added
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.
3. Why store in cookie when you can use localstorage? (and don't need the new deps vue-cookies)
Patch added
4. drag&drop - you cannot select something in a box, it automatically drag the box. Cannot we have a dedicated zone/icon for that drag&drop behaviour?
Patch added
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.
6. Links from the "Licenses needed action" are correct (http://localhost:8081/cgi-bin/koha/erm/licenses/1), but it fully reloads the app
Patch added.
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.
8. Widgets/ERMLatestSUSHIJobs.vue a. 117 0: ["show"], What's that? I see that in the second column: <a href="/cgi-bin/koha/erm/eusage/usage_data_providers/undefined" class="show" draggable="false"></a>
Patch added.
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.
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.
Either we implement +string for /jobs or we raise an error. Also we are using the only occurrence of escapeHtml in all vue files, I would use escape_str (`return get_job_status(row.status).escapeHtml();`)
Patch added.
10. Cypress tests are failing: 1) ERM Module Dashboard
Run eUsage report: AssertionError: Timed out retrying after 10000ms: Expected to find element: `.widget#ERMRunUsageReport .widget-content`, but never found it.
Patch added. -- You are receiving this mail because: You are watching all bug changes.