https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38010
Jonathan Druart <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |39709 --- Comment #281 from Jonathan Druart <[email protected]> --- (In reply to Jonathan Druart from comment #276) > 1. Should we provide more rewrite rule to not break existing bookmarks? > For instance > http://localhost:8081/cgi-bin/koha/acqui/supplier.pl?booksellerid=1 now > returns 404 Still valid. > 3. breadcrumb: > http://localhost:8081/cgi-bin/koha/vendors?supplier=my+vendor > Click "Vendors" in the breadcrumb: > http://localhost:8081/cgi-bin/koha/vendors?supplier=my+vendor# > Expected: see all vendors > Not a regression, and not sure we can fix here. Still valid, opened bug 39709. > 4. Filtering: > http://localhost:8081/cgi-bin/koha/vendors?supplier=my+vendor > "Showing 1 to 1 of 1 entries (filtered from 3 total entries)" > But you have no way to remove the filtering and see all vendors > It is the current behaviour in main, but at least we had "Search for vendor > my vendor" in the breadcrumb Still valid, and this is confusing IMO. > 6. http://localhost:8081/cgi-bin/koha/vendors/2 > Warning in the console: > [Vue Router warn]: Discarded invalid param(s) "vendor_id" when navigating. Still valid. > 8. show vendor view: > a. website is a button: https://snipboard.io/16ZLPA.jpg (same for the > interfaces) Still a problem, the link is not clickable (missing href) > 9. edit vendor: > a. hit enter when you add an alias: the form is submitted. This is a > regression, we prevented that in the current code with: > acqui/supplier.tt:596 nodes.append("<input id='new_alias' > type='text' class='noEnterSubmit' />"); Fixed but now we cannot remove an alias, there is a JS error in the console: "Uncaught ReferenceError: e is not defined" > b. enter "foo" in discount: cannot save, no error on the UI, 400 in the > response: {"errors":[{"message":"Expected number\/null - got > string.","path":"\/body\/discount"}],"status":400} Better. There is a change in the behaviour however. Before we display %.1f, now we display what is in the DB. There is only one inconsistency: 1.234 is considered valid, 1.2345 is not and the error says "Please enter a decimal number in the format: 0.0". It's not clear why 3 decimals. If it's a float it should go as it. Not considering blocker. (In reply to Jonathan Druart from comment #277) > 11. We lost test on email and URI fields: > Account email: Please enter a valid email address. Email should be fixed with only `type="email"` however the URI check is still a problem. New ones: 12. http://localhost:8081/cgi-bin/koha/acquisition/vendors/1 * Missing "Receive shipment" * Missing space "+New" https://snipboard.io/WpbPck.jpg 13. Edit a vendor, open 2 contact blocks. Tick all options for first contact, then tick all for second contact: => Two from the first contact are unchecked: https://snipboard.io/SLNiJI.jpg 14. It's missing "%" for the discount display on the vendor show view: https://snipboard.io/hUw8dQ.jpg (In reply to Matt Blenkinsop from comment #280) > (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 Should have been done before this one, now it's kinda too late for 25.05. I let you decide what to do. Adding new uses of C4 from Koha should be avoided as much as possible. If you do not decide to move it before this patch set, then please open a follow-up bug report. > > 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 It's a regression caused by this change, not sure why we would need a separate bug. If you really think we should split then do but please provide a patch ASAP. > > 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? It was invalid indeed. You need it for the order, not the embed: _order_by=+me.name,+aliases.alias,+me.id > > 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 Can you open a bug report please? > > 7. DELETE /vendors/:id should use apiclient (acq.js) > > This feels like a follow-up as well, not directly related to the vendors code It's done now, thanks! Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39709 [Bug 39709] "Vendors" link in the breadcrumb should list all the vendors -- 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/
