|
Description
Pedro Amorim
2023-12-11 16:33:42 UTC
Same goes for the finetype column. Same for issuelimit Good catch. Found this comment: commit 3aa00f63131fdccba4bb57870bc86c93329d5050 Author: Nicole C. Engard <nicole@bywatersolutions.com> Date: Mon Oct 10 06:50:23 2011 -0400 Bug 6716 : Categories table This patch documents the patron categories table in Koha. There are a couple fields marked unused and a couple that aren't documented because I wasn't sure: * dateofbirthrequired * bulk Really, if Nicole didn't know about 'bulk' and marked the other 2 as unused, I am sure we can safely remove them! I think it would be nice to check if the columns have 'content' (I imagine someone might have 'parked' data there from a migration) and not drop them in that case or force people to clean up before resuming the database update. But yes +1 for clean-up. Created attachment 160400 [details] [review] Bug 35539: [DONT PUSH] dbic Removal of bulk Removal of finetype Removal of issuelimit Created attachment 160401 [details] [review] Bug 35539: Remove 'bulk' column/field from categories table Created attachment 160402 [details] [review] Bug 35539: Remove 'finetype' column/field from categories table Created attachment 160403 [details] [review] Bug 35539: Remove 'issuelimit' column/field from categories table Created attachment 160404 [details] [review] Bug 35539: Atomicupdate Removal of 'bulk' column Removal of 'finetype' column Removal of 'issuelimit' column Test plan, k-t-d: 1) Add data to the relevant columns, run the following mysql: update categories set bulk = 1; update categories set finetype = 1; update categories set issuelimit = 1; 2) Run updatedatabase, notice it shows a warning and does not update the database 3) Clean up the data, run the folllowing mysql: update categories set bulk = null; update categories set finetype = null; update categories set issuelimit = null; 4) Run updatedatabase again, notice it updates the database as intended Run: t/db_dependent/Circulation/GetHardDueDate.t Comment on attachment 160404 [details] [review] Bug 35539: Atomicupdate Review of attachment 160404 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/atomicupdate/bug_35539.pl @@ +15,5 @@ > + SELECT bulk FROM categories; > + | > + ); > + if ($bulkdata) { > + say $out YELLOW, We'll need to test how this looks in the UI.. it's a nice addition for the console output, but not everyone runs their updates this way.. many use the web UI (In reply to Martin Renvoize from comment #10) > We'll need to test how this looks in the UI.. it's a nice addition for the > console output, but not everyone runs their updates this way.. many use the > web UI Fair point, after testing an upgrade through the browser, the ANSI codes are printed in the web UI and it doesn't look good. I gave a shot trying to grab the colors coming from the terminal and converting them to HTML (using HTML::FromANSI) but this seems to create more problems than it solves. The idea here was to highlight - in a different color - that some action is required, before or following this upgrade. I've posted my WIP at bug 35682 Created attachment 160413 [details] [review] Bug 35539: [DONT PUSH] dbic Removal of bulk Removal of finetype Removal of issuelimit Signed-off-by: David Nind <david@davidnind.com> Created attachment 160414 [details] [review] Bug 35539: Remove 'bulk' column/field from categories table Signed-off-by: David Nind <david@davidnind.com> Created attachment 160415 [details] [review] Bug 35539: Remove 'finetype' column/field from categories table Signed-off-by: David Nind <david@davidnind.com> Created attachment 160416 [details] [review] Bug 35539: Remove 'issuelimit' column/field from categories table Signed-off-by: David Nind <david@davidnind.com> Created attachment 160417 [details] [review] Bug 35539: Atomicupdate Removal of 'bulk' column Removal of 'finetype' column Removal of 'issuelimit' column Test plan, k-t-d: 1) Add data to the relevant columns, run the following mysql: update categories set bulk = 1; update categories set finetype = 1; update categories set issuelimit = 1; 2) Run updatedatabase, notice it shows a warning and does not update the database 3) Clean up the data, run the folllowing mysql: update categories set bulk = null; update categories set finetype = null; update categories set issuelimit = null; 4) Run updatedatabase again, notice it updates the database as intended Run: t/db_dependent/Circulation/GetHardDueDate.t Signed-off-by: David Nind <david@davidnind.com> Hmm, I'm not seeing the "reset" being called in the db update. As in, ANSIColor resets.. i'd suggest using the `colored()` method alternatively, as it automatically wrapped the strings with a reset at the end working well with `say`. Or you might use AUTORESET instead. Created attachment 160446 [details] [review] Bug 35539: Atomicupdate Removal of 'bulk' column Removal of 'finetype' column Removal of 'issuelimit' column Test plan, k-t-d: 1) Add data to the relevant columns, run the following mysql: update categories set bulk = 1; update categories set finetype = 1; update categories set issuelimit = 1; 2) Run updatedatabase, notice it shows a warning and does not update the database 3) Clean up the data, run the folllowing mysql: update categories set bulk = null; update categories set finetype = null; update categories set issuelimit = null; 4) Run updatedatabase again, notice it updates the database as intended Run: t/db_dependent/Circulation/GetHardDueDate.t Signed-off-by: David Nind <david@davidnind.com> I've updated the atomicupdate to use updatedatabase report colors work from bug 35681. Colors should now be correctly applied only to the lines they're meant to apply, and be correctly shown in update reports in both CLI and browser UI. 1. Why Koha/REST/V1/PatronCategories.pm and not under Koha/REST/V1/Patrons? 2. I think you should return a filtered list using Koha::Patron::Categories->search_with_library_limits instead. Sorry, wrong bug! Created attachment 165724 [details] [review] Bug 35539: Atomicupdate Removal of 'bulk' column Removal of 'finetype' column Removal of 'issuelimit' column Test plan, k-t-d: 1) Add data to the relevant columns, run the following mysql: update categories set bulk = 1; update categories set finetype = 1; update categories set issuelimit = 1; 2) Run updatedatabase, notice it shows a warning and does not update the database 3) Clean up the data, run the folllowing mysql: update categories set bulk = null; update categories set finetype = null; update categories set issuelimit = null; 4) Run updatedatabase again, notice it updates the database as intended Run: t/db_dependent/Circulation/GetHardDueDate.t Signed-off-by: David Nind <david@davidnind.com> Created attachment 169209 [details] [review] Bug 35539: Remove 'bulk' column/field from categories table Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 169210 [details] [review] Bug 35539: Remove 'finetype' column/field from categories table Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 169211 [details] [review] Bug 35539: Remove 'issuelimit' column/field from categories table Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 169212 [details] [review] Bug 35539: Atomicupdate Removal of 'bulk' column Removal of 'finetype' column Removal of 'issuelimit' column Test plan, k-t-d: 1) Add data to the relevant columns, run the following mysql: update categories set bulk = 1; update categories set finetype = 1; update categories set issuelimit = 1; 2) Run updatedatabase, notice it shows a warning and does not update the database 3) Clean up the data, run the folllowing mysql: update categories set bulk = null; update categories set finetype = null; update categories set issuelimit = null; 4) Run updatedatabase again, notice it updates the database as intended Run: t/db_dependent/Circulation/GetHardDueDate.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 169213 [details] [review] Bug 35539: [DONT PUSH] dbic Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 169214 [details] [review] Bug 35539: (QA follow-up) Fix atomic update to look for first non-null values Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Thanks for all the hard work! Pushed to main for the next 24.11.00 release as RM Assistant |