https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38201
--- Comment #8 from Matt Blenkinsop <[email protected]> --- (In reply to Jonathan Druart from comment #4) > 1. I think this is problematic for translatability: > + {{ $__("Remove this %s").format(resourceName) }}</a > > + {{ $__("Add new %s").format(resourceName) }}</a > > + $__("There are no %s created yet").format(resourceName) We're now passing an i18n object with translated strings from the resource > 2. > http://localhost:8081/cgi-bin/koha/preservation/trains/1 > a. Toolbar button "Add items" is not correctly styled: > https://snipboard.io/W3Be1Y.jpg > b. warning in the console: > [Vue warn]: Property "class" was accessed during render but is not defined > on instance. > at <Toolbar key=0 to= > Object { name: "TrainsFormAddItem", params: {…} } > icon="plus" ... > This should be fixed now that the 'class' prop is gone from the toolbar > 3. Create a train, then delete it from the list view > a. The table is not refreshed and the train is still displayed > b. warning in the console: > [Vue Router warn]: Discarded invalid param(s) "train_item_id" when > navigating. See > https://github.com/vuejs/router/blob/main/packages/router/CHANGELOG.md#414- > 2022-08-22 for more details. This should also now be fixed, there was an issue with the reference to the datatable object > 4. AgreementResource.vue > + nameLC: __("user"), > + nameUC: __("Agreement user"), > + namePL: __("users"), > > > LC, UC, PL? Can it be verbose so we don't need to guess what it means? This is done > 5. Update users for an agreement: > https://snipboard.io/FywJhr.jpg > Change one field and all are updated, wrong binding here. > > Oops, it actually happens for other linked resources. > > See also the duplication of the error there: https://snipboard.io/jmvBbL.jpg This has been resolved, linked resources update independently > 6. > (In reply to Pedro Amorim from comment #3) > > Chatting with Matt, this should likely be based on bug 37930. > > It is currently not. > And bugs encountered on bug 37301 are fixed here. Can we have a clear tree? This isn't currently based on that work, 37930 is still in discussion so we could do with a clear direction on that bug before integrating it here, otherwise we may have to make a lot of changes depending on the outcome of 37930 > 7. Is there a TODO list? It seems that other components need adjustment to > use this change (the remaining FormAdd, List, Show components). What's > missing to make it ready for testing/QA and push? Nothing official, there are a few nice to haves that we have noted for the future but all the critical stuff is included now (as far as we've spotted, see if you have any other ideas). We still need to migrate Preservation to this work. There are also some components that we would like to remove in the future like Agreement*Display.vue and also transferring FormSelectVendors to FormRelationshipSelect but these are possibly out of scope for now and could be follow-ups. Usage Statistics is probably also out of scope for this as it has some very specific functionality, same with the EBSCO components > 8. With the additional abstractions everything is harder to read IMO, we > should add documentation, at least for the "Base" components. We've got JSDocs on the methods, we were considering adding a 'SkeletonResource.vue' component as an example like we have for atomic updates > 9. There is way too many changes for a single bug report, please split into > smaller chunks next time. It's really hard to follow. > > 10. Cypress tests are failing: > Spec Tests Passing > Failing > │ ✖ AdditionalFields_spec.ts 02:22 12 2 > 10 > │ ✖ InfiniteScrollSelect_spec.ts 00:56 4 - > 4 > │ ✖ Admin/RecordSources_spec.ts 00:51 5 1 > 4 > │ ✖ ERM/Agreements_spec.ts 01:06 5 - > 5 > │ ✖ ERM/Licenses_spec.ts 01:04 5 - > 5 > │ ✖ ERM/UserRoles_spec.ts 00:15 1 - > 1 We haven't updated the tests yet while its still WIP, I was considering writing new tests to check all the different abstracted features > 11. I cannot save an agreement with a license > 400 - Object { message: "Expected string - got null.", path: > "/body/agreement_licenses/0/status" } I believe this is also fixed > Overall I think the idea is good but it adds some "magic" that won't help > new developer to grasp our VueJS code. I feel like you have been too far, > and things won't be as easy to maintain as you think. > > This is something weird: > 310 { > 311 name: "ended_on", > 312 type: "component", > 313 label: __("End date"), > 314 componentPath: "./FlatPickrWrapper.vue", > 315 required: false, > 316 props: { > 317 id: { > 318 type: "string", > 319 value: "ended_on_", > 320 indexRequired: true, > 321 }, > 322 }, > 323 }, > > I expect more to see something like "type: date", and not a component path > to the flatpickr component. This is done, same for vendors > > The 500 lines of definition of the agreement (resourceAttrs in > AgreementResource.vue) are not trivial to understand, neither to know what > does what. > I think we could make it a bit more readable by moving some redundant code > that is in there: the relationshipWidget are defined in LicenseResource and > AgreementResource for instance. > > We should make sure the order of the attributes are always the same (how??), > otherwise it get messy, for instance: > 339 name: "notes", > 340 required: false, > 341 type: "text", > 342 label: __("Notes"), > > vs > > 421 name: "role", > 422 type: "select", > 423 label: __("Role"), > 424 avCat: "av_user_roles", > 425 required: true, > This would probably need a QA script check? > > This pattern looks wrong: > 482 resourceRelationships: { > 483 type: "resourceProperty", > 484 resourceProperty: "agreement_licenses", > 485 }, This has also been fixed, its is now just one line with a key of 'resourceProperty' -- You are receiving this mail because: You are the assignee for the bug. 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/
