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