Description
Nick Clemens (kidclamp)
2022-06-22 16:24:01 UTC
Created attachment 136443 [details] [review] Bug 31017: DB Updates Created attachment 136444 [details] [review] Bug 31017: DO NOT PUSH - Schema updates Created attachment 136445 [details] [review] Bug 31017: Add new vendor_type field to edit screena dn display This patch adds a new vendor_type field when creating/editing vendors and displays the field on search and details for a vendor To test: 1 - Apply patch, update database 2 - Edit/create a vendor in acquisitions 3 - Note new 'Vendor type' field, free text in editor 4 - Save a value 5 - Confirm it displays in vendor search results and vendor main page 6 - In Authorised values add a new value to 'VENDOR_TYPE' category 7 - Add/Edit a vendor, note the vendor type is now a dropdown selection 8 - Save with a value 9 - Confirm the description shows in results and vendor page I couldn't see the vendor category on the vendor page. Is the expectation that libraries will choose either the free form text field, or use the authorized values? Hi Nick! 1) I second David in saying that the vendor type should appear on the vendor details page (supplier.pl). Right now, it only appears in booksellers.pl 2) I would add a description to the authorised value category. Maybe something like "Values that can be entered to fill in the 'Vendor type' field in the acquisitions module, that can be used for statistical purposes" (I copied from Bsort1, but feel free to put whatever description you think is best) To answer David' second comment, I think it is ok to have either free text or authorised value. It is the same about everywhere in Koha: patron sort fields, acquisitions statistical fields, drop downs in MARC frameworks, etc. In all those cases, if you add values in the AV category, the free text field will become a drop down and you cannot choose to have both free text and dropdown. The only exceptions AFAIK are in the MARC frameworks, where it is now possible to add values on the fly while cataloging; the suggestion approval/refusal reasons, where there is an 'Other' option that, when selected opens a free text field; and for the cities and towns in patron files, where you can choose from the drop down or manually enter a city. But that last one (cities) is not authorised values... (In reply to Caroline Cyr La Rose from comment #5) > To answer David' second comment, I think it is ok to have either free text > or authorised value. It is the same about everywhere in Koha: patron sort > fields, acquisitions statistical fields, drop downs in MARC frameworks, etc. > ... Thanks Caroline. I hadn't noticed this before, and this was the first time I'd come across it. (I've been living such a sheltered life in Kohaland!) Tester par Rachael Laritz Created attachment 136903 [details] [review] Bug 31017: Add new vendor_type field to edit screen and display This patch adds a new vendor_type field when creating/editing vendors and displays the field on search and details for a vendor To test: 1 - Apply patch, update database 2 - Edit/create a vendor in acquisitions 3 - Note new 'Vendor type' field, free text in editor 4 - Save a value 5 - Confirm it displays in vendor search results and vendor main page 6 - In Authorised values add a new value to 'VENDOR_TYPE' category 7 - Confirm the description of VENDOR_TYPE shows and makes sense 8 - Add/Edit a vendor, note the vendor type is now a dropdown selection 9 - Save with a value 10 - Confirm the description shows in results and vendor page and vendor details Created attachment 137134 [details] [review] Bug 31017: Add new vendor_type field to edit screen and display This patch adds a new vendor_type field when creating/editing vendors and displays the field on search and details for a vendor To test: 1 - Apply patch, update database 2 - Edit/create a vendor in acquisitions 3 - Note new 'Vendor type' field, free text in editor 4 - Save a value 5 - Confirm it displays in vendor search results and vendor main page 6 - In Authorised values add a new value to 'VENDOR_TYPE' category 7 - Confirm the description of VENDOR_TYPE shows and makes sense 8 - Add/Edit a vendor, note the vendor type is now a dropdown selection 9 - Save with a value 10 - Confirm the description shows in results and vendor page and vendor details Signed-off-by: Caroline <caroline.cyr-la-rose@inlibro.com> Created attachment 138116 [details] [review] Bug 31017: DB Updates Signed-off-by: KIT Library Germany <michaela.sieber@kit.edu> Created attachment 138117 [details] [review] Bug 31017: DO NOT PUSH - Schema updates Signed-off-by: KIT Library Germany <michaela.sieber@kit.edu> Created attachment 138118 [details] [review] Bug 31017: Add new vendor_type field to edit screen and display This patch adds a new vendor_type field when creating/editing vendors and displays the field on search and details for a vendor To test: 1 - Apply patch, update database 2 - Edit/create a vendor in acquisitions 3 - Note new 'Vendor type' field, free text in editor 4 - Save a value 5 - Confirm it displays in vendor search results and vendor main page 6 - In Authorised values add a new value to 'VENDOR_TYPE' category 7 - Confirm the description of VENDOR_TYPE shows and makes sense 8 - Add/Edit a vendor, note the vendor type is now a dropdown selection 9 - Save with a value 10 - Confirm the description shows in results and vendor page and vendor details Signed-off-by: Caroline <caroline.cyr-la-rose@inlibro.com> Signed-off-by: KIT Library Germany <michaela.sieber@kit.edu> Why "vendor_type"? I mean, why is the DB column named "vendor_type"? We are not supposed to repeat the table's name in the column's name (aqbooksellers must be named vendor). Yes, 'type' is a terrible naming, but having the repetition seems worst. (In reply to Jonathan Druart from comment #14) > I mean, why is the DB column named "vendor_type"? We are not supposed to > repeat the table's name in the column's name (aqbooksellers must be named > vendor). > Yes, 'type' is a terrible naming, but having the repetition seems worst. Happy to adjust if we have a better option: classification category organization_type Or I can just make it type What would be real-life values? (In reply to Jonathan Druart from comment #16) > What would be real-life values? I thought this was related to the ERM work tbh :) Examples could be: Consortium, Library, Platform, Vendor etc. (In reply to Katrin Fischer from comment #17) > (In reply to Jonathan Druart from comment #16) > > What would be real-life values? > > I thought this was related to the ERM work tbh :) > > Examples could be: Consortium, Library, Platform, Vendor etc. I thought examples would be: Print, Audio, Maps, etc. Like "what type of things we buy from that vendor" (that's how I classified my vendors when I used to do acquisitions). But I guess that's the beauty of it, that it can be whatever the user needs. Nick also mentioned other classifications in the bug description
> Some libraries have many vendors, and they serve different purposes e.g.
> booksellers vs donors vs electronic resource providers
(In reply to Jonathan Druart from comment #16) > What would be real-life values? For the library requesting this it was values like: Copyright holder Donor Book vendor etc. It's really a way to classify/organize vendors for searching/reporting/etc category or type feel most correct to me I've used 'type' a lot for ERM, and it always feel wrong/too generic. If you are happy with 'type', go for it :) Created attachment 138942 [details] [review] Bug 31017: DB Updates Signed-off-by: KIT Library Germany <michaela.sieber@kit.edu> Created attachment 138943 [details] [review] Bug 31017: Add new vendor_type field to edit screen and display This patch adds a new vendor_type field when creating/editing vendors and displays the field on search and details for a vendor To test: 1 - Apply patch, update database 2 - Edit/create a vendor in acquisitions 3 - Note new 'Vendor type' field, free text in editor 4 - Save a value 5 - Confirm it displays in vendor search results and vendor main page 6 - In Authorised values add a new value to 'VENDOR_TYPE' category 7 - Confirm the description of VENDOR_TYPE shows and makes sense 8 - Add/Edit a vendor, note the vendor type is now a dropdown selection 9 - Save with a value 10 - Confirm the description shows in results and vendor page and vendor details Signed-off-by: Caroline <caroline.cyr-la-rose@inlibro.com> Signed-off-by: KIT Library Germany <michaela.sieber@kit.edu> Created attachment 138944 [details] [review] Bug 31017: Update field name to 'type' Created attachment 138945 [details] [review] Bug 31017: DO NOT PUSH - Schema updates Created attachment 138964 [details] [review] Bug 31017: DB Updates Signed-off-by: KIT Library Germany <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 138965 [details] [review] Bug 31017: Add new vendor_type field to edit screen and display This patch adds a new vendor_type field when creating/editing vendors and displays the field on search and details for a vendor To test: 1 - Apply patch, update database 2 - Edit/create a vendor in acquisitions 3 - Note new 'Vendor type' field, free text in editor 4 - Save a value 5 - Confirm it displays in vendor search results and vendor main page 6 - In Authorised values add a new value to 'VENDOR_TYPE' category 7 - Confirm the description of VENDOR_TYPE shows and makes sense 8 - Add/Edit a vendor, note the vendor type is now a dropdown selection 9 - Save with a value 10 - Confirm the description shows in results and vendor page and vendor details Signed-off-by: Caroline <caroline.cyr-la-rose@inlibro.com> Signed-off-by: KIT Library Germany <michaela.sieber@kit.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 138966 [details] [review] Bug 31017: Update field name to 'type' Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 138967 [details] [review] Bug 31017: DO NOT PUSH - Schema updates Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Passing QA, but I don't think we should start with no value (and a free input text). I think it would be better to have few values (to show what could be possible values) that could even be translatable. Letting author and RM decide. This could be done separately (but before 21.11 ideally). (In reply to Jonathan Druart from comment #30) > Letting author and RM decide. This could be done separately (but before > 21.11 ideally). Ha, apparently we are (already!) working for 22.11.. :D Created attachment 139007 [details] [review] Bug 31017: (follow-up) Add type field to api (In reply to Jonathan Druart from comment #31) > (In reply to Jonathan Druart from comment #30) > > Letting author and RM decide. This could be done separately (but before > > 21.11 ideally). > > Ha, apparently we are (already!) working for 22.11.. :D LOL Pushed to master for 22.11. Nice work everyone, thanks! Enhancement, will not be backported to 22.05.x series (In reply to Jonathan Druart from comment #30) > Passing QA, but I don't think we should start with no value (and a free > input text). > I think it would be better to have few values (to show what could be > possible values) that could even be translatable. > Letting author and RM decide. This could be done separately (but before > 21.11 ideally). I should have Failed QA because of this. It adds unnecessary complications when done after... |