https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38010 --- Comment #280 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- (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.