Ohhh this would be cool. Might also be easier to find people to test this one. I don't have anyone using Preservation or ERM, but Acquisitions... for sure.
Created attachment 174119 [details] [review] Bug 38010: Add vue app and list component This patch adds the base vue app as well as the component to list vendors
Created attachment 174120 [details] [review] Bug 38010: Add the component to display a vendor
Created attachment 174121 [details] [review] Bug 38010: Add permissions This patch adds a permissions store and utility methods for implementing permissions checks in a Vue app
Created attachment 174122 [details] [review] Bug 38010: Add authorised values
Created attachment 174123 [details] [review] Bug 38010: Rename routes to acquisitions This patch renames the app from vendors to acquisitions to allow further acquisitions pages to be migrated
Created attachment 174124 [details] [review] Bug 38010: Add the form to add a new vendor
Created attachment 174125 [details] [review] Bug 38010: Update navigation links
Created attachment 174126 [details] [review] Bug 38010: Add a nav link button component
Created attachment 174128 [details] [review] Bug 38010: Allow external breadcrumbs This patch allows breadcrumbs to navigate to pages outside the Vue application
Created attachment 174129 [details] [review] Bug 38010: Add a contracts table to the vendor show component
Created attachment 174130 [details] [review] Bug 38010: Remove drawCallback from contracts table
Created attachment 174131 [details] [review] Bug 38010: Add permissions to action buttons
Created attachment 174132 [details] [review] Bug 38010: Make the delete button conditional on whether a vendor has baskets or subscriptions
Created attachment 174133 [details] [review] Bug 38010: Add cypress tests
Created attachment 174134 [details] [review] Bug 38010: Adjust vendor update logic This patch handles the embedded data that might also need updating when editing a vendor
Created attachment 174135 [details] [review] Bug 38010: Fix vendors search bar
Created attachment 174136 [details] [review] Bug 38010: Update links to vendor module
Created attachment 174137 [details] [review] Bug 38010: Rebase for Button component change
Created attachment 174138 [details] [review] Bug 38010: Add basket page
Created attachment 174139 [details] [review] Bug 38010: Allow embedding for baskets table
Created attachment 174140 [details] [review] Bug 38010: Point basket action buttons to correct pages
Created attachment 174141 [details] [review] Bug 38010: Fix null basket names
Created attachment 174142 [details] [review] Bug 38010: Respect AcqViewBaskets syspref
Created attachment 174143 [details] [review] Bug 38010: Add delete contact button
Created attachment 174144 [details] [review] Bug 38010: Add menu components for vendor and acquisitions menus
Created attachment 174145 [details] [review] Bug 38010: Implement side menus in vendors app
Created attachment 174146 [details] [review] Bug 38010: Fix rspack error in VendorShow
Created attachment 174147 [details] [review] Bug 38010: Allow searching by alias in the vendors table
Created attachment 174148 [details] [review] Bug 38010: Form fixes
Created attachment 174151 [details] [review] Bug 38010: Add vue app and list component This patch adds the base vue app as well as the component to list vendors
Created attachment 174152 [details] [review] Bug 38010: Add the component to display a vendor
Created attachment 174153 [details] [review] Bug 38010: Add permissions This patch adds a permissions store and utility methods for implementing permissions checks in a Vue app
Created attachment 174154 [details] [review] Bug 38010: Add authorised values
Created attachment 174155 [details] [review] Bug 38010: Rename routes to acquisitions This patch renames the app from vendors to acquisitions to allow further acquisitions pages to be migrated
Created attachment 174156 [details] [review] Bug 38010: Add the form to add a new vendor
Created attachment 174157 [details] [review] Bug 38010: Update navigation links
Created attachment 174158 [details] [review] Bug 38010: Add a nav link button component
Created attachment 174159 [details] [review] Bug 38010: Allow external breadcrumbs This patch allows breadcrumbs to navigate to pages outside the Vue application
Created attachment 174160 [details] [review] Bug 38010: Add a contracts table to the vendor show component
Created attachment 174161 [details] [review] Bug 38010: Remove drawCallback from contracts table
Created attachment 174162 [details] [review] Bug 38010: Add permissions to action buttons
Created attachment 174163 [details] [review] Bug 38010: Make the delete button conditional on whether a vendor has baskets or subscriptions
Created attachment 174164 [details] [review] Bug 38010: Add cypress tests
Created attachment 174165 [details] [review] Bug 38010: Adjust vendor update logic This patch handles the embedded data that might also need updating when editing a vendor
Created attachment 174166 [details] [review] Bug 38010: Fix vendors search bar
Created attachment 174167 [details] [review] Bug 38010: Update links to vendor module
Created attachment 174168 [details] [review] Bug 38010: Rebase for Button component change
Created attachment 174169 [details] [review] Bug 38010: Add basket page
Created attachment 174170 [details] [review] Bug 38010: Allow embedding for baskets table
Created attachment 174171 [details] [review] Bug 38010: Point basket action buttons to correct pages
Created attachment 174172 [details] [review] Bug 38010: Fix null basket names
Created attachment 174173 [details] [review] Bug 38010: Respect AcqViewBaskets syspref
Created attachment 174174 [details] [review] Bug 38010: Add delete contact button
Created attachment 174175 [details] [review] Bug 38010: Add menu components for vendor and acquisitions menus
Created attachment 174176 [details] [review] Bug 38010: Implement side menus in vendors app
Created attachment 174177 [details] [review] Bug 38010: Fix rspack error in VendorShow
Created attachment 174178 [details] [review] Bug 38010: Allow searching by alias in the vendors table
Created attachment 174179 [details] [review] Bug 38010: Form fixes
Created attachment 174194 [details] [review] Bug 38010: Rebase for bug 38390
Created attachment 174196 [details] [review] Bug 38010: Add vue app and list component This patch adds the base vue app as well as the component to list vendors
Created attachment 174197 [details] [review] Bug 38010: Add the component to display a vendor
Created attachment 174198 [details] [review] Bug 38010: Add permissions This patch adds a permissions store and utility methods for implementing permissions checks in a Vue app
Created attachment 174199 [details] [review] Bug 38010: Add authorised values
Created attachment 174200 [details] [review] Bug 38010: Rename routes to acquisitions This patch renames the app from vendors to acquisitions to allow further acquisitions pages to be migrated
Created attachment 174201 [details] [review] Bug 38010: Add the form to add a new vendor
Created attachment 174202 [details] [review] Bug 38010: Update navigation links
Created attachment 174203 [details] [review] Bug 38010: Add a nav link button component
Created attachment 174204 [details] [review] Bug 38010: Allow external breadcrumbs This patch allows breadcrumbs to navigate to pages outside the Vue application
Created attachment 174205 [details] [review] Bug 38010: Add a contracts table to the vendor show component
Created attachment 174206 [details] [review] Bug 38010: Remove drawCallback from contracts table
Created attachment 174207 [details] [review] Bug 38010: Add permissions to action buttons
Created attachment 174208 [details] [review] Bug 38010: Make the delete button conditional on whether a vendor has baskets or subscriptions
Created attachment 174209 [details] [review] Bug 38010: Add cypress tests
Created attachment 174210 [details] [review] Bug 38010: Adjust vendor update logic This patch handles the embedded data that might also need updating when editing a vendor
Created attachment 174211 [details] [review] Bug 38010: Fix vendors search bar
Created attachment 174212 [details] [review] Bug 38010: Update links to vendor module
Created attachment 174213 [details] [review] Bug 38010: Rebase for Button component change
Created attachment 174214 [details] [review] Bug 38010: Add basket page
Created attachment 174215 [details] [review] Bug 38010: Allow embedding for baskets table
Created attachment 174216 [details] [review] Bug 38010: Point basket action buttons to correct pages
Created attachment 174217 [details] [review] Bug 38010: Fix null basket names
Created attachment 174218 [details] [review] Bug 38010: Respect AcqViewBaskets syspref
Created attachment 174220 [details] [review] Bug 38010: Add delete contact button
Created attachment 174221 [details] [review] Bug 38010: Add menu components for vendor and acquisitions menus
Created attachment 174222 [details] [review] Bug 38010: Implement side menus in vendors app
Created attachment 174223 [details] [review] Bug 38010: Fix rspack error in VendorShow
Created attachment 174224 [details] [review] Bug 38010: Allow searching by alias in the vendors table
Created attachment 174225 [details] [review] Bug 38010: Form fixes
Created attachment 174226 [details] [review] Bug 38010: Rebase for bug 38390
Created attachment 174248 [details] [review] Bug 38010: Add recevie shipments action to table
Created attachment 174266 [details] [review] Bug 38010: Add receive shipments action to table
Created attachment 174267 [details] [review] Bug 38010: Restore new basket/contract buttons
Created attachment 174269 [details] [review] Bug 38010: Add dropdown button component
Created attachment 174270 [details] [review] Bug 38010: Reset basket view
Created attachment 174273 [details] [review] Bug 38010: Reset basket view
Created attachment 174327 [details] [review] Bug 38010: Add vue app and list component This patch adds the base vue app as well as the component to list vendors
Created attachment 174328 [details] [review] Bug 38010: Add the component to display a vendor
Created attachment 174329 [details] [review] Bug 38010: Add permissions This patch adds a permissions store and utility methods for implementing permissions checks in a Vue app
Created attachment 174330 [details] [review] Bug 38010: Add authorised values
Created attachment 174331 [details] [review] Bug 38010: Rename routes to acquisitions This patch renames the app from vendors to acquisitions to allow further acquisitions pages to be migrated
Created attachment 174332 [details] [review] Bug 38010: Add the form to add a new vendor
Created attachment 174333 [details] [review] Bug 38010: Update navigation links
Created attachment 174334 [details] [review] Bug 38010: Add a nav link button component
Created attachment 174335 [details] [review] Bug 38010: Allow external breadcrumbs This patch allows breadcrumbs to navigate to pages outside the Vue application
Created attachment 174336 [details] [review] Bug 38010: Add a contracts table to the vendor show component
Created attachment 174337 [details] [review] Bug 38010: Remove drawCallback from contracts table
Created attachment 174338 [details] [review] Bug 38010: Add permissions to action buttons
Created attachment 174339 [details] [review] Bug 38010: Make the delete button conditional on whether a vendor has baskets or subscriptions
Created attachment 174340 [details] [review] Bug 38010: Add cypress tests
Created attachment 174341 [details] [review] Bug 38010: Adjust vendor update logic This patch handles the embedded data that might also need updating when editing a vendor
Created attachment 174342 [details] [review] Bug 38010: Fix vendors search bar
Created attachment 174343 [details] [review] Bug 38010: Update links to vendor module
Created attachment 174344 [details] [review] Bug 38010: Rebase for Button component change
Created attachment 174345 [details] [review] Bug 38010: Add basket page
Created attachment 174346 [details] [review] Bug 38010: Allow embedding for baskets table
Created attachment 174347 [details] [review] Bug 38010: Point basket action buttons to correct pages
Created attachment 174348 [details] [review] Bug 38010: Fix null basket names
Created attachment 174349 [details] [review] Bug 38010: Respect AcqViewBaskets syspref
Created attachment 174350 [details] [review] Bug 38010: Add delete contact button
Created attachment 174351 [details] [review] Bug 38010: Add menu components for vendor and acquisitions menus
Created attachment 174352 [details] [review] Bug 38010: Implement side menus in vendors app
Created attachment 174353 [details] [review] Bug 38010: Fix rspack error in VendorShow
Created attachment 174354 [details] [review] Bug 38010: Allow searching by alias in the vendors table
Created attachment 174355 [details] [review] Bug 38010: Form fixes
Created attachment 174356 [details] [review] Bug 38010: Rebase for bug 38390
Created attachment 174357 [details] [review] Bug 38010: Add receive shipments action to table
Created attachment 174358 [details] [review] Bug 38010: Restore new basket/contract buttons
Created attachment 174359 [details] [review] Bug 38010: Add dropdown button component
Created attachment 174360 [details] [review] Bug 38010: Reset basket view
Created attachment 174603 [details] [review] Bug 38010: Remove unnecessary navigation check
I get a 404 error when I try to search for a vendor or follow a link to a vendor.
Hi Matt, not sure if this is just a thing with the sandbox or not, but when I go to edit vendor Askews & Holt in the sandbox, the library group says NaN, 1, NaN https://staff-vendors.sandboxes.ptfs-europe.co.uk/cgi-bin/koha/vendors/edit/4 To get there : 1. Go to the sandbox https://staff-vendors.sandboxes.ptfs-europe.co.uk/cgi-bin/koha/vendors 2. Click Askews & Holt (#4) 3. Click Edit vendor --> Library group visiblity: NaN, 1, NaN Also, is it normal that when we are in the vendors, the left side menu only lists vendors?
(In reply to Caroline Cyr La Rose from comment #133) > > Also, is it normal that when we are in the vendors, the left side menu only > lists vendors? To get there: 1. Go to the sandbox https://staff-vendors.sandboxes.ptfs-europe.co.uk/cgi-bin/koha/vendors 2. Click Askews & Holt (#4) 3. Click Edit vendor --> The left side menu only contains Vendors
Created attachment 175098 [details] [review] Bug 38010: Add vue app and list component This patch adds the base vue app as well as the component to list vendors
Created attachment 175099 [details] [review] Bug 38010: Add the component to display a vendor
Created attachment 175100 [details] [review] Bug 38010: Add permissions This patch adds a permissions store and utility methods for implementing permissions checks in a Vue app
Created attachment 175101 [details] [review] Bug 38010: Add authorised values
Created attachment 175102 [details] [review] Bug 38010: Rename routes to acquisitions This patch renames the app from vendors to acquisitions to allow further acquisitions pages to be migrated
Created attachment 175103 [details] [review] Bug 38010: Add the form to add a new vendor
Created attachment 175104 [details] [review] Bug 38010: Update navigation links
Created attachment 175105 [details] [review] Bug 38010: Allow external breadcrumbs This patch allows breadcrumbs to navigate to pages outside the Vue application
Created attachment 175106 [details] [review] Bug 38010: Add a contracts table to the vendor show component
Created attachment 175107 [details] [review] Bug 38010: Remove drawCallback from contracts table
Created attachment 175108 [details] [review] Bug 38010: Add permissions to action buttons
Created attachment 175109 [details] [review] Bug 38010: Make the delete button conditional on whether a vendor has baskets or subscriptions
Created attachment 175110 [details] [review] Bug 38010: Add cypress tests
Created attachment 175111 [details] [review] Bug 38010: Adjust vendor update logic This patch handles the embedded data that might also need updating when editing a vendor
Created attachment 175112 [details] [review] Bug 38010: Fix vendors search bar
Created attachment 175113 [details] [review] Bug 38010: Update links to vendor module
Created attachment 175114 [details] [review] Bug 38010: Rebase for Button component change
Created attachment 175115 [details] [review] Bug 38010: Add basket page
Created attachment 175116 [details] [review] Bug 38010: Allow embedding for baskets table
Created attachment 175117 [details] [review] Bug 38010: Point basket action buttons to correct pages
Created attachment 175118 [details] [review] Bug 38010: Fix null basket names
Created attachment 175119 [details] [review] Bug 38010: Respect AcqViewBaskets syspref
Created attachment 175120 [details] [review] Bug 38010: Add delete contact button
Created attachment 175121 [details] [review] Bug 38010: Add menu components for vendor and acquisitions menus
Created attachment 175122 [details] [review] Bug 38010: Implement side menus in vendors app
Created attachment 175123 [details] [review] Bug 38010: Fix rspack error in VendorShow
Created attachment 175124 [details] [review] Bug 38010: Allow searching by alias in the vendors table
Created attachment 175125 [details] [review] Bug 38010: Form fixes
Created attachment 175126 [details] [review] Bug 38010: Rebase for bug 38390
Created attachment 175127 [details] [review] Bug 38010: Add receive shipments action to table
Created attachment 175128 [details] [review] Bug 38010: Restore new basket/contract buttons
Created attachment 175129 [details] [review] Bug 38010: Add dropdown button component
Created attachment 175130 [details] [review] Bug 38010: Reset basket view
Created attachment 175131 [details] [review] Bug 38010: Remove unnecessary navigation check
Created attachment 175133 [details] [review] Bug 38010: Rebase for bug 37301
Created attachment 175134 [details] [review] Bug 38010: Fix tax rate display
Created attachment 175135 [details] [review] Bug 38010: Add callbacks to Toolbarbutton
Created attachment 175136 [details] [review] Bug 38010: Fix acqui-home.pl breadcrumbs
Created attachment 175137 [details] [review] Bug 38010: Add permissions for additional fields (cherry picked from commit 0387e998a49b91a57c798504c28a097a011a9d77)
Created attachment 175167 [details] [review] Bug 38010: Fix programmatic navigation id attribute
(In reply to Owen Leonard from comment #132) > I get a 404 error when I try to search for a vendor or follow a link to a > vendor. This sounds like an apache issue - you need to run reset_all after applying to get the apache redirect to activate
(In reply to Caroline Cyr La Rose from comment #133) > Hi Matt, not sure if this is just a thing with the sandbox or not, but when > I go to edit vendor Askews & Holt in the sandbox, the library group says > NaN, 1, NaN This is sandbox specific, that includes lots of further enhancements that aren't part of this bug > > Also, is it normal that when we are in the vendors, the left side menu only > lists vendors? I've removed this so that there is no menu which is what we currently have in main
(In reply to Matt Blenkinsop from comment #175) > (In reply to Owen Leonard from comment #132) > > I get a 404 error when I try to search for a vendor or follow a link to a > > vendor. > > This sounds like an apache issue - you need to run reset_all after applying > to get the apache redirect to activate Wondering (in general) how we handle this for updating installations - I believe we can't just add to the Apache files, but maybe I am mistaken. Hint on about?
Created attachment 175168 [details] [review] Bug 38010: Remove unnecessary test
Created attachment 175170 [details] [review] Bug 38010: Delete old code
I started testing and found two bugs which you can recreate in this sandbox : 1.) Error "Something went wrong: Error: Expected integer - got string." Go to acq module search for vendors Go to record 1 = "My Vendor" Click on Baskets on the left Click on Button "Edit vendor" on the top. This leads you to the following URL: https://staff-vendorsvue.sandboxes.ptfs-europe.co.uk/cgi-bin/koha/vendors/1&op=enter --> Error: "Something went wrong: Error: Expected integer - got string." 2.) Error 404 Go to acq module search for vendors Go to record 1 = "My Vendor" Click on Baskets on the left Go to the existing basket "My Basket" (basketno=1) Click on the link to the vendor "My Vendor" on the top. This leads you to the following URL: https://staff-vendorsvue.sandboxes.ptfs-europe.co.uk/cgi-bin/koha/acqui/cgi-bin/koha/vendors/1 --> Error 404
Created attachment 175240 [details] [review] Bug 38010: Fix navigation links
Created attachment 175241 [details] [review] Bug 38010: Prevent vendor contacts from duplicating
Things I discovered that are new / different behavior as before: - New vendors are inactive (radio button default is inactive, before it was active) - There is a new Button "New basket", this opens a new browser tab (did not exist before) - The Button "Receive shipment" exists even if there is no basket for the vendor (before the botton was only there if there was at least one basket). This means you can enter an invoice, for example with invoice adjustments. That is ok, but then it should not be possible to delete the vendor. If you delete the vendor the invoice is deleted as well. This means your budget is not correct any more! If there are invoices in the database it should definitly not be possible to delete a vendor.
Created attachment 175242 [details] [review] Bug 38010: Check for baskets for receive shipments button
Created attachment 175243 [details] [review] Bug 38010: Check for invoices before allowing vendor deletion
Created attachment 175248 [details] [review] Bug 38010: Only allow one primary contact
Created attachment 175249 [details] [review] Bug 38010: Remove unnecessary buttons
Created attachment 175266 [details] [review] Bug 38010: Apply format_date to vendor contracts
Created attachment 175791 [details] [review] Bug 38010: Change how interface details are displayed
Created attachment 175799 [details] [review] Bug 38010: Apply format_date to vendor contracts Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Well done Matt! Ready for QA
I was asked to leave this comment confirming performance issues with the acquisition module searching for vendors with thousands of baskets. Hopefully it will be resolved with this work. "We have a 25 second waiting time when we search for a vendor in the acquisition module that had 5000+ baskets."
(In reply to Ola Andersson from comment #192) > I was asked to leave this comment confirming performance issues with the > acquisition module searching for vendors with thousands of baskets. > Hopefully it will be resolved with this work. > > "We have a 25 second waiting time when we search for a vendor in the > acquisition module that had 5000+ baskets." I thought it might make a good stress test for the new vendor search :)
Had a quick look at the UI, some notes: 1. "Save" => "Submit" 2. No more sticky toolbar 3. a. "Add vendor" => "New vendor" b. It's not h2 instead of h1 4. Alias a. can be added empty (was not possible before) b. "Remove" does not remove the correct alias 5. http://localhost:8081/cgi-bin/koha/acqui/booksellers.pl?booksellerid=2 "Edit" is http://localhost:8081/cgi-bin/koha/vendors/2 ("show" view) 6. Empty contact can be added (was not possible before) 7. "Delete contact" and "Remove this interface" is not consistent (in wording, in position, and in styling) 8. Discount and delivery time have type="number" https://wiki.koha-community.org/wiki/Coding_Guidelines#ACC2:_Input_type_.22number.22_should_be_avoided 9. Delete not working http://localhost:8081/cgi-bin/koha/acqui/booksellers.pl?booksellerid=2 "Delete vendor", yes => 404 and vendor is not deleted. 10. Breadcrumb a. Small glitch in the breadcrumb (spacing difference before 'Vendors'?) https://snipboard.io/GQ5XRs.jpg vs https://snipboard.io/fubCc5.jpg b. http://localhost:8081/cgi-bin/koha/vendors 'Vendors' is a link (http://localhost:8081/cgi-bin/koha/vendors#) 11. Interfaces a. On the "show" view, "Type" is displayed when not filled (was not before) b. The name of the interface is now in a `legend` tag, was h3 before. I don't think you can use `legend` for something else than `fieldset`. 12. We are missing tests. You should have a look at bug 38461 and bug 38503.
13. The vendor search result is really different from what we have currently, and it's missing a lot of info and actions (basket list, "add to basket", "show all baskets", "show active vendors only", the ability to basket or contract for a vendor). Has this been discussed before already?
The permissions store is a bit weird as you rely on a global JS variable. How could this be reusable from the ERM module for instance? It would really have been great to have such concepts/ideas isolated into their own bug instead of lost in this huge set...
Should not we move js/vue/components/Vendors to js/vue/components/Acquisitions/Vendors?
+ `${(vendor.tax_rate * 100).toFixed(2)}` + "{enter}", + label: `${(gv.option * 100).toFixed(2)}%`, I don't think this is correct, you should use format_price (from format_price.inc)
Wrong paste koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementResource.vue: //AgreementResource specific setup here koha-tmpl/intranet-tmpl/prog/js/vue/components/Vendors/VendorResource.vue: //AgreementResource specific setup here
What's the purpose of VendorDetails.vue? I don't see how it could be useful.
It seems that this file contains things that are not from this patch set. koha-tmpl/intranet-tmpl/prog/js/vue/components/Islands/AcquisitionsMenu.vue
Code duplication, the acquisition menu is now in koha-tmpl/intranet-tmpl/prog/en/includes/acquisitions-menu.inc and koha-tmpl/intranet-tmpl/prog/js/vue/components/Islands/AcquisitionsMenu.vue This should be on its own bug really. Replace the left menu everywhere before this and don't duplicate the code.
(In reply to Jonathan Druart from comment #194) > 12. We are missing tests. You should have a look at bug 38461 and bug 38503. Also missing tests for the pm changes.
Koha/Acquisition/Bookseller.pm + $interface->password( $interface->plain_text_password ); Do we really want to expose the password? Maybe not something for now, but we should maybe have the "****" trick here with a "show" that will trigger a special route for plain text password (just for discussion as I am reading this code)
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?
Created attachment 176547 [details] [review] Bug 38010: (QA follow-up) Template fixes
First of a few follow-up patches, this one is needed to make 38899 work. More to follow
Created attachment 178182 [details] [review] Bug 38010: Add vue app and list component This patch adds the base vue app as well as the component to list vendors
Created attachment 178183 [details] [review] Bug 38010: Add the component to display a vendor
Created attachment 178184 [details] [review] Bug 38010: Add permissions This patch adds a permissions store and utility methods for implementing permissions checks in a Vue app
Created attachment 178185 [details] [review] Bug 38010: Add authorised values
Created attachment 178186 [details] [review] Bug 38010: Rename routes to acquisitions This patch renames the app from vendors to acquisitions to allow further acquisitions pages to be migrated
Created attachment 178187 [details] [review] Bug 38010: Add the form to add a new vendor
Created attachment 178188 [details] [review] Bug 38010: Update navigation links
Created attachment 178189 [details] [review] Bug 38010: Allow external breadcrumbs This patch allows breadcrumbs to navigate to pages outside the Vue application
Created attachment 178190 [details] [review] Bug 38010: Add a contracts table to the vendor show component
Created attachment 178191 [details] [review] Bug 38010: Remove drawCallback from contracts table
Created attachment 178192 [details] [review] Bug 38010: Add permissions to action buttons
Created attachment 178193 [details] [review] Bug 38010: Make the delete button conditional on whether a vendor has baskets or subscriptions
Created attachment 178194 [details] [review] Bug 38010: Add cypress tests
Created attachment 178195 [details] [review] Bug 38010: Adjust vendor update logic This patch handles the embedded data that might also need updating when editing a vendor
Created attachment 178196 [details] [review] Bug 38010: Fix vendors search bar
Created attachment 178197 [details] [review] Bug 38010: Update links to vendor module
Created attachment 178198 [details] [review] Bug 38010: Rebase for Button component change
Created attachment 178199 [details] [review] Bug 38010: Add basket page
Created attachment 178200 [details] [review] Bug 38010: Allow embedding for baskets table
Created attachment 178201 [details] [review] Bug 38010: Point basket action buttons to correct pages
Created attachment 178202 [details] [review] Bug 38010: Fix null basket names
Created attachment 178203 [details] [review] Bug 38010: Respect AcqViewBaskets syspref
Created attachment 178204 [details] [review] Bug 38010: Add delete contact button
Created attachment 178205 [details] [review] Bug 38010: Add menu components for vendor and acquisitions menus
Created attachment 178206 [details] [review] Bug 38010: Implement side menus in vendors app
Created attachment 178207 [details] [review] Bug 38010: Fix rspack error in VendorShow
Created attachment 178208 [details] [review] Bug 38010: Allow searching by alias in the vendors table
Created attachment 178209 [details] [review] Bug 38010: Form fixes
Created attachment 178210 [details] [review] Bug 38010: Rebase for bug 38390
Created attachment 178211 [details] [review] Bug 38010: Add receive shipments action to table
Created attachment 178212 [details] [review] Bug 38010: Restore new basket/contract buttons
Created attachment 178213 [details] [review] Bug 38010: Add dropdown button component
Created attachment 178214 [details] [review] Bug 38010: Reset basket view
Created attachment 178215 [details] [review] Bug 38010: Remove unnecessary navigation check
Created attachment 178216 [details] [review] Bug 38010: Fix tax rate display
Created attachment 178217 [details] [review] Bug 38010: Add callbacks to Toolbarbutton
Created attachment 178218 [details] [review] Bug 38010: Fix acqui-home.pl breadcrumbs
Created attachment 178219 [details] [review] Bug 38010: Fix programmatic navigation id attribute
Created attachment 178220 [details] [review] Bug 38010: Remove unnecessary test
Created attachment 178221 [details] [review] Bug 38010: Delete old code
Created attachment 178222 [details] [review] Bug 38010: Fix navigation links
Created attachment 178223 [details] [review] Bug 38010: Prevent vendor contacts from duplicating
Created attachment 178224 [details] [review] Bug 38010: Check for baskets for receive shipments button
Created attachment 178225 [details] [review] Bug 38010: Check for invoices before allowing vendor deletion
Created attachment 178226 [details] [review] Bug 38010: Only allow one primary contact
Created attachment 178227 [details] [review] Bug 38010: Remove unnecessary buttons
Created attachment 178228 [details] [review] Bug 38010: Apply format_date to vendor contracts
Created attachment 178229 [details] [review] Bug 38010: Change how interface details are displayed
Created attachment 178230 [details] [review] Bug 38010: (QA follow-up) Template fixes
Created attachment 178231 [details] [review] Bug 38010: (QA follow-up) Fix new vendor form
Created attachment 178232 [details] [review] Bug 38010: (QA follow-up) Restore navigation links This patch restores any links that previously pointed to booksellers.pl as this script is still in use. All references to supplier.pl remain pointed at the new Vue app
Created attachment 178233 [details] [review] Bug 38010: (QA follow-up) Fix delete button
Created attachment 178234 [details] [review] Bug 38010: (QA follow-up) Fix vendor show component
Created attachment 178235 [details] [review] Bug 38010: (QA follow-up) Use format_price
Created attachment 178236 [details] [review] Bug 38010: (QA follow-up) Rebase acquisitions amd vendor menus
Created attachment 178237 [details] [review] Bug 38010: (QA follow-up) Remove unnecessary code from booksellers.pl
Created attachment 178238 [details] [review] Bug 38010: (QA follow-up) Rebase for bug 37930
Created attachment 178239 [details] [review] Bug 38010: (QA follow-up) Rebase for bug 38854
Created attachment 178240 [details] [review] Bug 38010: (QA follow-up) Convert vendor menu to Vue island
Created attachment 178241 [details] [review] Bug 38010: (QA follow-up) Add unit tests
Created attachment 178242 [details] [review] Bug 38010: (QA follow-up) Fix toolbar on VendorShow The delete button was missing from VendorShow and bug 37301 has been removed as a dependency so some small fixes were required for that
(In reply to Jonathan Druart from comment #194) > Had a quick look at the UI, some notes: > 1. "Save" => "Submit" > 2. No more sticky toolbar > 3. > a. "Add vendor" => "New vendor" > b. It's not h2 instead of h1 > 4. Alias > a. can be added empty (was not possible before) > b. "Remove" does not remove the correct alias > 5. http://localhost:8081/cgi-bin/koha/acqui/booksellers.pl?booksellerid=2 > "Edit" is http://localhost:8081/cgi-bin/koha/vendors/2 ("show" view) > 6. Empty contact can be added (was not possible before) > 7. "Delete contact" and "Remove this interface" is not consistent (in > wording, in position, and in styling) > 8. Discount and delivery time have type="number" > https://wiki.koha-community.org/wiki/Coding_Guidelines#ACC2:_Input_type_. > 22number.22_should_be_avoided > 9. Delete not working > http://localhost:8081/cgi-bin/koha/acqui/booksellers.pl?booksellerid=2 > "Delete vendor", yes > => 404 and vendor is not deleted. > 10. Breadcrumb > a. Small glitch in the breadcrumb (spacing difference before 'Vendors'?) > https://snipboard.io/GQ5XRs.jpg > vs > https://snipboard.io/fubCc5.jpg > b. http://localhost:8081/cgi-bin/koha/vendors > 'Vendors' is a link (http://localhost:8081/cgi-bin/koha/vendors#) > 11. Interfaces > a. On the "show" view, "Type" is displayed when not filled (was not before) > b. The name of the interface is now in a `legend` tag, was h3 before. I > don't think you can use `legend` for something else than `fieldset`. > 12. We are missing tests. You should have a look at bug 38461 and bug 38503. I've addressed all of the above as best I can, the only one I'm not sure on is point 12 - what specifically do you want to see tests for? The main functions are CRUD which there are cypress tests for. I've also added more unit tests for the .pm methods
(In reply to Jonathan Druart from comment #195) > 13. The vendor search result is really different from what we have > currently, and it's missing a lot of info and actions (basket list, "add to > basket", "show all baskets", "show active vendors only", the ability to > basket or contract for a vendor). Has this been discussed before already? There were issues for libraries with large numbers of vendors and baskets - the vendor search could take a long time to load so the plan was to use the migration to make use of the new datatables work to allow pagination and search. The table filters allow for easy searching and filtering, with much faster load time (In reply to Jonathan Druart from comment #196) > The permissions store is a bit weird as you rely on a global JS variable. > How could this be reusable from the ERM module for instance? > > It would really have been great to have such concepts/ideas isolated into > their own bug instead of lost in this huge set... I've moved this to bug 38930 (In reply to Jonathan Druart from comment #198) > + `${(vendor.tax_rate * 100).toFixed(2)}` + "{enter}", > > + label: `${(gv.option * 100).toFixed(2)}%`, > > I don't think this is correct, you should use format_price (from > format_price.inc) Done (In reply to Jonathan Druart from comment #202) > Code duplication, the acquisition menu is now in > koha-tmpl/intranet-tmpl/prog/en/includes/acquisitions-menu.inc and > koha-tmpl/intranet-tmpl/prog/js/vue/components/Islands/AcquisitionsMenu.vue > This should be on its own bug really. Replace the left menu everywhere > before this and don't duplicate the code. Bug 38941 (In reply to Jonathan Druart from comment #200) > What's the purpose of VendorDetails.vue? I don't see how it could be useful. The vendor form component was so long and bloated, same with VendorShow. I wanted to make it more readable by splitting the html out into separate components (In reply to Jonathan Druart from comment #204) > Koha/Acquisition/Bookseller.pm > + $interface->password( $interface->plain_text_password ); > > Do we really want to expose the password? Maybe not something for now, but > we should maybe have the "****" trick here with a "show" that will trigger a > special route for plain text password (just for discussion as I am reading > this code) This was discussed on Mattermost and we agreed this was the best way forward for now. The documentation has been updated to make it clear that Koha is not a password manager and that passwords will be visible through the API
Rebased onto the tidy work, same for dependencies. QA feedback comments above
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/js/vue/stores/vendors.js).
Soft feature freeze for 25.05 : 2025-04-29 means 30 working days left... What about a session in Marseille for testing and QA? @joubu, Matt, cait?
(In reply to Michaela Sieber from comment #273) > Soft feature freeze for 25.05 : 2025-04-29 > > means 30 working days left... > > What about a session in Marseille for testing and QA? > > @joubu, Matt, cait? I'd be happy to. Maybe Monday/Tuesday?
Sounds like a plan to me, I need to try and get it fully rebased before then now that i'm back
Testing on top of: c0fc58efa3b (openfifth/bug_38010) Bug 38010: (QA follow-up) Fix toolbar on VendorShow 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 2. Why do we have /cgi-bin/koha/vendors instead of /cgi-bin/koha/acquisition/vendors ? 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. 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 5. http://localhost:8081/cgi-bin/koha/acqui/vendors.pl interestingly shows: https://snipboard.io/5Pmeq2.jpg 6. http://localhost:8081/cgi-bin/koha/vendors/2 Warning in the console: [Vue Router warn]: Discarded invalid param(s) "vendor_id" when navigating. 7. http://localhost:8081/cgi-bin/koha/acqui/booksellers.pl?booksellerid=2 Errors in the console: Uncaught (in promise) DOMException: CustomElementRegistry.define: 'vendor-menu' has already been defined as a custom element Uncaught (in promise) DOMException: CustomElementRegistry.define: 'acquisitions-menu' has already been defined as a custom element 8. show vendor view: a. website is a button: https://snipboard.io/16ZLPA.jpg (same for the interfaces) b. contact options does not display correctly: https://snipboard.io/UI8Ko6.jpg c. "New" menu: the entries does not have the dropdown-item class => incorrect styling on mouse over: https://snipboard.io/aiB8zw.jpg c. Entries in the "New" menu open in new tab (same for "Receive shipments") 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' />"); 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} c. enter 111 in the discount, cannot save, no error on the UI, 500 in the response. Koha log has: Out of range value for column 'discount' d. delete an interface not working: "Uncaught TypeError: _ctx.deleteContact is not a function" 10. cannot edit from the list view: Uncaught Error: Missing required param "id"
11. We lost test on email and URI fields: Account email: Please enter a valid email address.
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. 2. I had the feeling there was something wrong in Koha::Acq::Bookseller::to_api, I think it causes this bug: Edit a vendor, add 2 interfaces, `select * from aqbookseller_interfaces` Edit again, don't change anything, save, select again MariaDB [koha_kohadev]> select interface_id, vendor_id, password from aqbookseller_interfaces\G *************************** 1. row *************************** interface_id: 8 vendor_id: 2 password: 53616c7465645f5fb2f929cb1abe15d15f9a6c54e9b38977a8055ece428f2c75 *************************** 2. row *************************** interface_id: 9 vendor_id: 2 password: 53616c7465645f5f2919e4566b6dacdaf29690ff2193f006bd55e843cb6ee0ae 2 rows in set (0.001 sec) MariaDB [koha_kohadev]> select interface_id, vendor_id, password from aqbookseller_interfaces\G *************************** 1. row *************************** interface_id: 10 vendor_id: 2 password: 53616c7465645f5f0b3f2ebce9124cd14317d4fe111dd0aeedaf9532460e9e43 *************************** 2. row *************************** interface_id: 11 vendor_id: 2 password: 53616c7465645f5ffd9ed9b565602e77db7510110fde8cfb43af971d46aefa3f No changes expected, we should not add another id. 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. 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. 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? 6. Manual link This needs to be adjusted Koha/Manual.pm: 'acqui/supplier' => '/acquisitions.html#vendors', 7. DELETE /vendors/:id should use apiclient (acq.js) 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> 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 %] 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) 10. Contracts table on the vendor show view is not correctly styled: https://snipboard.io/QHdc8N.jpg 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 + 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
(In reply to Jonathan Druart from comment #278) > QA review > 2. I had the feeling there was something wrong in > Koha::Acq::Bookseller::to_api, I think it causes this bug: > Edit a vendor, add 2 interfaces, `select * from aqbookseller_interfaces` > Edit again, don't change anything, save, select again > > MariaDB [koha_kohadev]> select interface_id, vendor_id, password from > aqbookseller_interfaces\G > *************************** 1. row *************************** > interface_id: 8 > vendor_id: 2 > password: > 53616c7465645f5fb2f929cb1abe15d15f9a6c54e9b38977a8055ece428f2c75 > *************************** 2. row *************************** > interface_id: 9 > vendor_id: 2 > password: > 53616c7465645f5f2919e4566b6dacdaf29690ff2193f006bd55e843cb6ee0ae > 2 rows in set (0.001 sec) > > MariaDB [koha_kohadev]> select interface_id, vendor_id, password from > aqbookseller_interfaces\G > *************************** 1. row *************************** > interface_id: 10 > vendor_id: 2 > password: > 53616c7465645f5f0b3f2ebce9124cd14317d4fe111dd0aeedaf9532460e9e43 > *************************** 2. row *************************** > interface_id: 11 > vendor_id: 2 > password: > 53616c7465645f5ffd9ed9b565602e77db7510110fde8cfb43af971d46aefa3f > > No changes expected, we should not add another id. Hum, retested this on main and it's the current behaviour, not blocker!
(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
(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!
(In reply to Jonathan Druart from comment #281) > (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. Fixed - redirect added to the new show page > > > 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. See 'Bug 38010: (QA follow-up) Stop breadcrumbs from keeping filters' for a concept - allows us to reuse it for future breadcrumbs as well > > 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. This is still happening in all Vue apps - I'm not entirely sure why, needs some deeper investigation > > 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) Typo - fixed > > 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" Fixed > (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. I've added the type - do you mean the URI field on the form for interfaces or the email more generally? > New ones: > 12. > http://localhost:8081/cgi-bin/koha/acquisition/vendors/1 > * Missing "Receive shipment" > * Missing space "+New" > https://snipboard.io/WpbPck.jpg Can't reproduce the first, Receive shipment shows for me - did you delete the baskets for the vendor? Space added to the "New" button > 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 This is the expected behaviour as we should only have one primary contact of each type - supplier.tt line 700 in main > 14. > It's missing "%" for the discount display on the vendor show view: > https://snipboard.io/hUw8dQ.jpg Fixed > (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. Follow-up opened (39711) - I've already started the code for it > > > 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. > I've added a patch to the branch which gives the 'Show password' functionality > > > 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? Done - bug 37912
Created attachment 181579 [details] [review] Bug 38010: (QA follow-up) Add a 'Show password' option
Attached the url now rather than having so many patches on the bug
Created attachment 181585 [details] [review] Bug 38010: (bugfix) Show contract of the basket if inactive If the contract is inactive it should be displayed on the basket edit view.
(In reply to Matt Blenkinsop from comment #282) > (In reply to Jonathan Druart from comment #281) > > > 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. > > See 'Bug 38010: (QA follow-up) Stop breadcrumbs from keeping filters' for a > concept - allows us to reuse it for future breadcrumbs as well Great. Still not perfect however, IMO we need a way to reset the filters near the table. But it's a good start. We will see what end-users say. > > > > 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. > > This is still happening in all Vue apps - I'm not entirely sure why, needs > some deeper investigation Opened bug 39764. > > (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. > > I've added the type - do you mean the URI field on the form for interfaces > or the email more generally? The 2 email fields (interface and contacts) are now solved with type="email". However we lost the URI check on "Company's details: Website". Not blocker, not considering it really important for now. > > New ones: > > 12. > > http://localhost:8081/cgi-bin/koha/acquisition/vendors/1 > > * Missing "Receive shipment" > > * Missing space "+New" > > https://snipboard.io/WpbPck.jpg > > Can't reproduce the first, Receive shipment shows for me - did you delete > the baskets for the vendor? Oops, yes I guess so! Forget that. > > 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 > > This is the expected behaviour as we should only have one primary contact of > each type - supplier.tt line 700 in main Indeed, it's confusing :D Sorry! Forget that as well. All good for me. I will do another round of code review and test tomorrow.
(In reply to Matt Blenkinsop from comment #282) > (In reply to Jonathan Druart from comment #281) > > > > 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? > > Done - bug 37912 Actually bug 39712.
tested and from an end-user perspective no blockers for the upcoming release. Minor enhancements could be solved as follow-up bugs
(In reply to Michaela Sieber from comment #288) > tested and from an end-user perspective no blockers for the upcoming release. > Minor enhancements could be solved as follow-up bugs Did you note the minor enhancements you would like to see?
Changes in behaviour: new a vendor cannot be deleted if it has invoices linked to it.
Created attachment 181727 [details] [review] Bug 38010: Add vue app and list component This patch adds the base vue app as well as the component to list vendors Amended-by: Jonathan Druart chmod 755 acqui/vendors.pl Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181728 [details] [review] Bug 38010: Add the component to display a vendor Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181729 [details] [review] Bug 38010: Add permissions This patch adds a permissions store and utility methods for implementing permissions checks in a Vue app Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181730 [details] [review] Bug 38010: Add authorised values Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181731 [details] [review] Bug 38010: Rename routes to acquisitions This patch renames the app from vendors to acquisitions to allow further acquisitions pages to be migrated Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181732 [details] [review] Bug 38010: Add the form to add a new vendor Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181733 [details] [review] Bug 38010: Update navigation links Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181734 [details] [review] Bug 38010: Allow external breadcrumbs This patch allows breadcrumbs to navigate to pages outside the Vue application Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181735 [details] [review] Bug 38010: Add a contracts table to the vendor show component Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181736 [details] [review] Bug 38010: Remove drawCallback from contracts table Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181737 [details] [review] Bug 38010: Add permissions to action buttons Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181738 [details] [review] Bug 38010: Make the delete button conditional on whether a vendor has baskets or subscriptions Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181739 [details] [review] Bug 38010: Add cypress tests Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181740 [details] [review] Bug 38010: Adjust vendor update logic This patch handles the embedded data that might also need updating when editing a vendor Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181741 [details] [review] Bug 38010: Fix vendors search bar Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181742 [details] [review] Bug 38010: Update links to vendor module Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181743 [details] [review] Bug 38010: Rebase for Button component change Bug 38010: Fix rspack error in VendorShow Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181744 [details] [review] Bug 38010: Add basket page Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181745 [details] [review] Bug 38010: Allow embedding for baskets table Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181746 [details] [review] Bug 38010: Point basket action buttons to correct pages Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181747 [details] [review] Bug 38010: Fix null basket names Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181748 [details] [review] Bug 38010: Respect AcqViewBaskets syspref Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181749 [details] [review] Bug 38010: Add delete contact button Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181750 [details] [review] Bug 38010: Add menu components for vendor and acquisitions menus Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181751 [details] [review] Bug 38010: Implement side menus in vendors app Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181752 [details] [review] Bug 38010: Allow searching by alias in the vendors table Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181753 [details] [review] Bug 38010: Form fixes Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181754 [details] [review] Bug 38010: Rebase for bug 38390 Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181755 [details] [review] Bug 38010: Add receive shipments action to table Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181756 [details] [review] Bug 38010: Restore new basket/contract buttons Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181757 [details] [review] Bug 38010: Add dropdown button component Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181758 [details] [review] Bug 38010: Reset basket view Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181759 [details] [review] Bug 38010: Remove unnecessary navigation check Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181760 [details] [review] Bug 38010: Fix tax rate display Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181761 [details] [review] Bug 38010: Add callbacks to Toolbarbutton Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181762 [details] [review] Bug 38010: Fix acqui-home.pl breadcrumbs Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181763 [details] [review] Bug 38010: Fix programmatic navigation id attribute Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181764 [details] [review] Bug 38010: Remove unneccessary test Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181765 [details] [review] Bug 38010: Delete old code Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181766 [details] [review] Bug 38010: Fix navigation links Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181767 [details] [review] Bug 38010: Prevent vendor contacts from duplicating Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181768 [details] [review] Bug 38010: Check for baskets for receive shipments button Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181769 [details] [review] Bug 38010: Check for invoices before allowing vendor deletion Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181770 [details] [review] Bug 38010: Only allow one primary contact Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181771 [details] [review] Bug 38010: Remove unnecessary buttons Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181772 [details] [review] Bug 38010: Apply format_date to vendor contracts Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181773 [details] [review] Bug 38010: Change how interface details are displayed Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181774 [details] [review] Bug 38010: (QA follow-up) Template fixes Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181775 [details] [review] Bug 38010: (QA follow-up) Fix new vendor form Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181776 [details] [review] Bug 38010: (QA follow-up) Restore navigation links This patch restores any links that previously pointed to booksellers.pl as this script is still in use. All references to supplier.pl remain pointed at the new Vue app Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181777 [details] [review] Bug 38010: (QA follow-up) Fix delete button Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181778 [details] [review] Bug 38010: (QA follow-up) Fix vendor show component Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181779 [details] [review] Bug 38010: (QA follow-up) Use format_price Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181780 [details] [review] Bug 38010: (QA follow-up) Rebase acquisitions amd vendor menus Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181781 [details] [review] Bug 38010: (QA follow-up) Remove unnecessary code from booksellers.pl Bug 38010: (QA follow-up) Fix QA tests Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181782 [details] [review] Bug 38010: (QA follow-up) Rebase for bug 37930 Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181783 [details] [review] Bug 38010: (QA follow-up) Rebase for bug 38854 Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181784 [details] [review] Bug 38010: (QA follow-up) Convert vendor menu to Vue island Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181785 [details] [review] Bug 38010: (QA follow-up) Add unit tests Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181786 [details] [review] Bug 38010: (QA follow-up) Fix toolbar on VendorShow The delete button was missing from VendorShow and bug 37301 has been removed as a dependency so some small fixes were required for that Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181787 [details] [review] Bug 38010: (QA follow-up) Update routes to include 'acquisition' Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181788 [details] [review] Bug 38010: (QA follow-up) Redirect Home component to List Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181789 [details] [review] Bug 38010: (QA follow-up) Move island hydration to container wrapper Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181790 [details] [review] Bug 38010: (QA follow-up) Fix issues in Show component This patch addresses the following: - website links were buttons so we now can pass a css class to the toolbar buttons again - contact options have been formatted to have the correct margin - the dropdown-item class has been added to the dropdown button list - entries in the dropdown were opening in a new tab, they are now a callback to avoid this Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181791 [details] [review] Bug 38010: (QA follow-up) Fix issues on vendor edit page This commit addresses the following: - Prevents form submission when hitting enter in the alias field - validation for the discount field - restores delete functionality to interfaces - fixes the broken edit button on the list view Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181792 [details] [review] Bug 38010: (QA follow-up) Adjust mapping for the manual Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181793 [details] [review] Bug 38010: (QA follow-up) UI fixes This patch addresses - Translation issues in VendorList - Removing the pagers from the contracts table - Removing redundant code from the vendors template file Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181794 [details] [review] Bug 38010: (QA follow-up) Handle form parameters in the browser Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181795 [details] [review] Bug 38010: (QA follow-up) Rebase on main Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181796 [details] [review] Bug 38010: (QA follow-up) Convert delete modal to use the API client Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181797 [details] [review] Bug 38010: (QA follow-up) Add a config endpoint for permissions Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181798 [details] [review] Bug 38010: (QA follow-up) Email type checking and UI fixes Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181799 [details] [review] Bug 38010: (QA follow-up) Stop breadcrumbs from keeping filters Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181800 [details] [review] Bug 38010: (QA follow-up) Add new apache redirect Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181801 [details] [review] Bug 38010: (QA follow-up) Add a 'Show password' option Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181802 [details] [review] Bug 38010: Improve show/hide password icon Use font-awesome-icon tag and add "hide" Also prevent the scrollbar to jump when the link is clicked (removing href) (cherry picked from commit 78876ee276cf0af6e307cd7a033bbbe2842e1dc1) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181803 [details] [review] Bug 38010: (QA follow-up) Add a Link component Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181804 [details] [review] Bug 38010: (QA follow-up) Add missing filter to main-container.inc Amended-by: Jonathan Druart -[% SET islands = Asset.js("js/vue/dist/islands.esm.js").match('(src="([^"]+)")').1 | $raw %] <script src="[% islands %]" type="module"> +[% SET islands = Asset.js("js/vue/dist/islands.esm.js").match('(src="([^"]+)")').1 %] <script src="[% islands | $raw %]" type="module"> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181805 [details] [review] Bug 38010: (QA follow-up) Remove extra sidebar_menu class Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181806 [details] [review] Bug 38010: Remove op=do_search from the 'Invoices' link So we don't see "no result for your search" (cherry picked from commit 1cc3dabaa25d503b6b6b7d49d81c9bf739013d10) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181807 [details] [review] Bug 38010: Code improvement (cherry picked from commit a14f1898672950be9d9097bc7b5dd56392a241b0) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181808 [details] [review] Bug 38010: Fix links to the manual The way we deal with routes for VueJS is messy and will need to be reconsidered, but it's not within the scope of this patch. (cherry picked from commit 2558583ef0fbf149664a3d9a2e3cec081ac505ff) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181809 [details] [review] Bug 38010: Fix wrong use of scalar in the REST API controller It was wrong, we need to use scalar on the array, not the arrayref (otherwise it will always be true) No change in behaviour expected here, just some code cleaning (cherry picked from commit 7abbcb7ba73da13d988202aaf06ba239e21692da) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181810 [details] [review] Bug 38010: Add tests for embed contacts,interfaces,aliases (cherry picked from commit cae75569e6e8f2bfe5a2e6d3acc111485f81cc1e) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181811 [details] [review] Bug 38010: (QA follow-up) Move syspref values to config endpoint Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181812 [details] [review] Bug 38010: Fix LeftMenu (cherry picked from commit a78af54eebf07a623779f4642bcdad28f35e662f) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181813 [details] [review] Bug 38010: Embed baskets+count and subscriptions+count We don't need to whole list of baskets and subscriptions (cherry picked from commit 391b4d3b32b57c97bfb7e66ab394020ae7eed0e5) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181814 [details] [review] Bug 38010: Migrate to_api logic to GET endpoint Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181815 [details] [review] Bug 38010: Translation fix Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181816 [details] [review] Bug 38010: Add protection logic to the DELETE endpoint Amended-by: Jonathan Druart typo: subsicriptions => subscriptions Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181817 [details] [review] Bug 38010: (QA follow-up) Add default settings to vendor store Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181818 [details] [review] Bug 38010: Fix vendor deletion tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181819 [details] [review] Bug 38010: Add tests for DELETE vendors/{vendor_id} Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181820 [details] [review] Bug 38010: Fix cypress tests - do not intercept /config We do not want to intercept /acquisitions/vendors/config Ideally we should mock it separately Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 181821 [details] [review] Bug 38010: Fix cypress tests - add _count to the mock object Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
95 patches... ugh. Did some testing, found some smaller and some odder things, no deal breakers, but I believe in quick follow-ups before release! 1) Terminology (follow-up) + description: List of GST values We removed GST as a term a while ago, I think this should probably read: List of tax rates + description: EDIFACT syspref value + description: MarcOrderingAutomation syspref value Please always spell out system preference instead of using the short form. (see Terminology list on wiki) 2) Vendor result list a) Currency takes a lot of width, not sure why. But it's also ambiguous. Can you please clarify if it's the list price currency or invoice price currency? (follow-up, string change) b) I think the vendor result table would greatly benefit from being column configurable (separate bug, can file later) c) The action column should be excluded from exports and print preview. (follow-up) d) Alias: if a vendor is found by searching for it's alias, it would be nice if there was some visual indicator. Maybe when adding column configuration we could add a column displaying the alias? (possible enhancement, can file later) 3) Managing vendors a) Encoding problem for the interface password (separate bug) - Create a new vendor - Add an interface with a password: KölnHamburgBerlinMünster - Save - Edit: The password umlauts appear mangled ... Now I was going to mark this blocker, but it already happens in 24.11... b) Deleting vendor only works from result list (follow-up or separate bug) In 24.11 we have a "Delete vendor" in the toolbar you can see on the vendor's detail page. With this patch set the "Delete" link only appears in the vendor result list. It works from there, so the vendor was "deletable" (no orders or subscriptions linked). 4) Managing contracts (follow-up or separate bug) a) Start and end date missing mark-up for sorting correctly with different date formats. This works in 24.11 (regression). The styling of the contacts table also appears a little odd, like dates should be aligned right etc.? X) Should not always complain, bonus points for: * Updating links to help files * Updating POD * Switching suppliers.pl to vendors.pl * Translatability :)
Pushed for 25.05! Well done everyone, thank you!
Created attachment 181891 [details] [review] Bug 38010: Fix xt/api.t
(In reply to Jonathan Druart from comment #388) > Created attachment 181891 [details] [review] [review] > Bug 38010: Fix xt/api.t Patch pushed.
(In reply to Katrin Fischer from comment #386) > 3) Managing vendors > > a) Encoding problem for the interface password (separate bug) > > - Create a new vendor > - Add an interface with a password: KölnHamburgBerlinMünster > - Save > - Edit: The password umlauts appear mangled > ... Now I was going to mark this blocker, but it already happens in 24.11... Opened bug 39826.
Created attachment 181898 [details] [review] Bug 38010: Embed invoices_count - fix patron deletion from show view Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Katrin Fischer from comment #386) > b) Deleting vendor only works from result list (follow-up or separate bug) > > In 24.11 we have a "Delete vendor" in the toolbar you can see on the > vendor's detail page. With this patch set the "Delete" link only appears in > the vendor result list. It works from there, so the vendor was "deletable" > (no orders or subscriptions linked). Done on the last patch. (In reply to Jonathan Druart from comment #391) > Created attachment 181898 [details] [review] [review] > Bug 38010: Embed invoices_count - fix patron deletion from show view > > Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to main.
Created attachment 181901 [details] [review] Bug 38010: Restore ordering on the contracts table
Created attachment 181963 [details] [review] Bug 38010: (follow-up) Fix API spec terminology
Created attachment 181964 [details] [review] Bug 38010: (follow-up) Make vendor table columns configurable
I could apply Matts 2 follow-ups, but I get an sha1 error on yours Joubu: Apply? [(y)es, (n)o, (i)nteractive] i Applying: Bug 38010: Restore ordering on the contracts table error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/js/vue/components/Vendors/VendorContracts.vue). error: could not build fake ancestor It doesn't appear to be pushed either.
Created attachment 182053 [details] [review] Bug 38010: Restore ordering on the contracts table
(In reply to Katrin Fischer from comment #396) > I could apply Matts 2 follow-ups, but I get an sha1 error on yours Joubu: > > Apply? [(y)es, (n)o, (i)nteractive] i > Applying: Bug 38010: Restore ordering on the contracts table > error: sha1 information is lacking or useless > (koha-tmpl/intranet-tmpl/prog/js/vue/components/Vendors/VendorContracts.vue). > error: could not build fake ancestor > > It doesn't appear to be pushed either. Oops yes, I had another patch on the branch, and both should have been squashed before attaching. Should be fixed now.
Pushed last follow-up. @Joubu: Your patch restores the sorting. But I am wondering: I don't see the mark-up for date sorting in the table. data-order with the sortable form of the date? Applied patch, ran yarn build.
Created attachment 182351 [details] [review] Bug 38010: (follow-up) Remove DT reference
(In reply to Katrin Fischer from comment #399) > Pushed last follow-up. > > @Joubu: Your patch restores the sorting. But I am wondering: I don't see the > mark-up for date sorting in the table. data-order with the sortable form of > the date? > > Applied patch, ran yarn build. Looks ok to me: https://snipboard.io/1D8hHo.jpg The trick is here: + return type == "sort" + ? row.contractenddate + : format_date(row.contractenddate);
(In reply to Matt Blenkinsop from comment #400) > Created attachment 182351 [details] [review] [review] > Bug 38010: (follow-up) Remove DT reference Patch pushed.
Created attachment 182438 [details] [review] Bug 38010: Fix cypress tests by mocking more API responses Jenkins runs the tests on top of the onboarding, there is no acq data
(In reply to Jonathan Druart from comment #403) > Created attachment 182438 [details] [review] [review] > Bug 38010: Fix cypress tests by mocking more API responses > > Jenkins runs the tests on top of the onboarding, there is no acq data This patch fixes the 2 remaining cypress failures <testcase name="Vendor CRUD operations should edit a vendor" time="0.000" classname="should edit a vendor"> <failure message="Timed out retrying after 10000ms: Expected to find element: `#vendors_list table tbody tr:first`, but never found it." type="AssertionError"><![CDATA[AssertionError: Timed out retrying after 10000ms: Expected to find element: `#vendors_list table tbody tr:first`, but never found it. at Context.eval (webpack://koha/./t/cypress/integration/Acquisitions/Vendors_spec.ts:193:11)]]></failure> </testcase> <testcase name="Vendor CRUD operations should show a vendor" time="0.000" classname="should show a vendor"> <failure message="Timed out retrying after 10000ms: Expected to find element: `#vendors_show h1`, but never found it." type="AssertionError"><![CDATA[AssertionError: Timed out retrying after 10000ms: Expected to find element: `#vendors_show h1`, but never found it. at Context.eval (webpack://koha/./t/cypress/integration/Acquisitions/Vendors_spec.ts:232:11)]]></failure> </testcase>
(In reply to Jonathan Druart from comment #403) > Created attachment 182438 [details] [review] [review] > Bug 38010: Fix cypress tests by mocking more API responses > > Jenkins runs the tests on top of the onboarding, there is no acq data Patch pushed.
I believe we have taken care of the lose ends here, please correct me if I missed something.