Description
Matt Blenkinsop
2024-09-26 13:15:48 UTC
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? |