https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38010
--- Comment #280 from Matt Blenkinsop <[email protected]> --- (In reply to Jonathan Druart from comment #278) > QA review > > 1. We really should move C4::Contracts to Koha::Acquisition::Contract[s] > before. It does not make sense to use C4::Contracts from Koha::Acquisition. > The module is pretty trivial to replace. I can do this but I think it should be a separate follow-up bug so that the change isn't lost in this code > 3. We lost the "Show" password link > + <p><span>Password: <span class="password"><a > href="#" class="show_password" data-plain-text-password="[% > i.plain_text_password | html %]">Show</a></span></span></p> > > We discussed it previously, but I actually didn't notice that this is > something we have currently. Need to be re-discussed maybe. We also had bug > 34778 since then. I think we should have a Vue component for handling this - I can add one but again that should probably be a separate follow-up > 4. (In reply to Jonathan Druart from comment #205) > > Koha/Schema/Result/Aqbookseller.pm > > +__PACKAGE__->has_many( > > + "aliases", > > + "Koha::Schema::Result::AqbooksellerAlias", > > > > Why? > > > > We already have Koha::Acquisition::Bookseller->aliases, when is this used? > > Still valid. This was so that I could embed directly via the API - I couldn't seem to get it to work with just the method itself. Maybe I'm missing something? > 5. Koha/REST/V1/Acquisitions/Vendors.pm > 92 my $subscriptions = delete $vendor->{subscriptions}; > 93 my $baskets = delete $vendor->{baskets}; > 94 my $contracts = delete $vendor->{contracts}; > Why? They need to be deleted before we store otherwise the store operation fails - I've changed them to just be delete clauses with no variable declaration > 6. Manual link > This needs to be adjusted > Koha/Manual.pm: 'acqui/supplier' => > '/acquisitions.html#vendors', Updated - we might need a new bug for an issue here as any url parameters in the Vue modules break the links - same happens in ERM, you just get the manual homepage > 7. DELETE /vendors/:id should use apiclient (acq.js) This feels like a follow-up as well, not directly related to the vendors code > 8. acqui/booksellers | supplier > We pass the idea > 118 supplier => $booksellerid, > But consider it a vendor object in the template: > 12 [% tx("Search for vendor {vendor}", { vendor = supplier }) | html > %] > 35 <span>Search for vendor [% supplier | $HtmlTags tag='em' > %]</span> I've changed the variable to booksellerid - we don't use the supplier object now that it is just for baskets > 8. vendors.tt > a. +<div id="__vendors"> > is this "__" to avoid conflict? > b. I don't think we need that: > + [% SET columns = ['cardnumber','name','category','branch','action'] > %] > + [% PROCESS patron_search_modal columns => columns, modal_title => > t("Select user") %] > + [% PROCESS patron_search_js columns => columns, filter => > erm_users, actions => ["select"], preview_on_name_click => 1 %] a. Changed to just be vendors b. Deleted these as they're redundant > 9. store/vendors has config.edifact = false > But we actually have more in the config: userPermissions, > marcOrderAutomation, currencies, gstValues > I think this should be moved to the config route anyway (bug 38930) Will upload the config route for 38930 shortly and then rebase this > 10. Contracts table on the vendor show view is not correctly styled: > https://snipboard.io/QHdc8N.jpg Pager removed to match main > 11. i18n problems in VendorList: > + escape_str( > + `${row.baskets.length} basket(s)` > + ) + > > + escape_str( > + `${row.subscriptions_count} > subscription(s)` > + ) + > > and missing escape_str > + row.name + Fixed > 12: 2 QA failures: > FAIL acqui/booksellers.pl > > FAIL critic > > # Variables::ProhibitUnusedVariables: Got 1 violation(s). > > > > FAIL acqui/vendors.pl > > FAIL file permissions > > File must have the exec flag Fixed I've addressed the bugs in comments 276 and 277 as well - they're all on the openfifth/bug_38010 branch as the attachment list is getting too long -- 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/
