|
Description
Christopher Brannon
2017-12-28 18:39:47 UTC
Sponsored by CIN. # Work to be done: 1 - Add a new column to the items table - 'exclude_from_local_holds_priority' or similar 2 - This field should be settable on the edit item form, and bath modifications 3 - Modify existing local holds priority code to skip items with column flag 'exclude_from_local_holds_priority' set for checking local hold priorities 4 - Add a column to 'categories' => 'excluded_from_localholds_priority' 5 - Alter code to ignore local holds priority for these Created attachment 105715 [details] [review] Bug 19889: Add exclude_from_local_holds_priority columns to item, deleteditems and categories tables Sponsored-by: Cooperative Information Network (CIN) Created attachment 105716 [details] [review] Bug 19889: set exclude_from_local_holds_priority columns as boolean in schema Sponsored-by: Cooperative Information Network (CIN) Created attachment 105717 [details] [review] Bug 19889: Add tests Sponsored-by: Cooperative Information Network (CIN) Created attachment 105718 [details] [review] Bug 19889: Make it possible to exclude items and categories from local holds priority This patch adds the ability to exclude patrons (by category) from local holds, and items, by editing the item itself or by batch item modification tool. To test: 1. apply patches 2. updatedatabase 3. Enable LocalHoldsPriority preference, and leave LocalHoldsPriorityPatronControl in pickup library, and LocalHoldsPriorityItemControl in holding library. 4. Search for a biblio with one item. 5. Place a hold with a patron (patron1) and set pickup location to a different library of the item's home library 6. Place another hold with another patron (patron2) and set pickup location to be the same as the item's home library 7. ./misc/cronjobs/holds/build_holds_queue.pl 8. Go to circulation -> holds queue 9. Search by the item's home library CHECK => only the hold for patron2 (with the pickup location the same as the item's home library) appears in the table 10. Go back to the biblio details page and click on "Items" tab CHECK => There is a new section in the item's details between "Statuses" and "History" called "Priority" 11. Set exclude to "Yes" and update 12. repeat steps 7 to 9 SUCCESS => only the hold for patron1 now appears, even the other hold had local hold priority 13. Repeat step 10 and 11 but this time set exclude to "No" 14. repeat steps 7 to 9 CHECK => the hold for patron2 is back 15. Edit patron2's category and set exclude from local holds priority to "Yes" 16. Repeat steps 7 to 9 SUCCESS => the hold for patron1 is back 17. Go to tools -> Batch item modification and in barcode list place several (existing) barcodes and press continue CHECK => There is a new section in the bottom called "Priority" 18. Set exclude to "Yes" and save SUCCESS => all items in the list now have exclude setted to "Yes" 19. Try to checkout the first item to a patron3 SUCCESS => Alert message appears saying that patron1 has a hold on that item 20. Click on Yes and then checkin that item SUCCESS => There is a modal window saying that a hold was found for patron1 21. prove t/db_dependent/HoldsQueue.t t/db_dependent/Holds/LocalHoldsPriority.t 22. Sign off Sponsored-by: Cooperative Information Network (CIN) This is only one of four criteria that was specifically asked for in exclusion: patron categories, item types, formats, or specific items. I will test this patch, but it cannot be signed off as is. (In reply to Christopher Brannon from comment #7) > This is only one of four criteria that was specifically asked for in > exclusion: patron categories, item types, formats, or specific items. > > I will test this patch, but it cannot be signed off as is. Ignore this comment. The description of the test wasn't clear. I will test to see if all four criteria are met and works. Agustin, Kidclamp had to rebase a minor conflict. I don't know the details. After that, this is what I found: Everything works except the Batch Item Modification. It did not change the setting for the items specified. Might be because of the rebase. Please check. Also, the description on the patron category is confusing. It should read "If Yes, holds placed by patrons of this category will not be given priority." I am also unable to prove the test, step 21. I am testing from a sandbox. Created attachment 106435 [details] [review] Bug 19889: Add exclude_from_local_holds_priority columns to item, deleteditems and categories tables Sponsored-by: Cooperative Information Network (CIN) Created attachment 106436 [details] [review] Bug 19889: set exclude_from_local_holds_priority columns as boolean in schema Sponsored-by: Cooperative Information Network (CIN) Created attachment 106437 [details] [review] Bug 19889: Add tests Sponsored-by: Cooperative Information Network (CIN) Created attachment 106438 [details] [review] Bug 19889: Make it possible to exclude items and categories from local holds priority This patch adds the ability to exclude patrons (by category) from local holds, and items, by editing the item itself or by batch item modification tool. To test: 1. apply patches 2. updatedatabase 3. Enable LocalHoldsPriority preference, and leave LocalHoldsPriorityPatronControl in pickup library, and LocalHoldsPriorityItemControl in holding library. 4. Search for a biblio with one item. 5. Place a hold with a patron (patron1) and set pickup location to a different library of the item's home library 6. Place another hold with another patron (patron2) and set pickup location to be the same as the item's home library 7. ./misc/cronjobs/holds/build_holds_queue.pl 8. Go to circulation -> holds queue 9. Search by the item's home library CHECK => only the hold for patron2 (with the pickup location the same as the item's home library) appears in the table 10. Go back to the biblio details page and click on "Items" tab CHECK => There is a new section in the item's details between "Statuses" and "History" called "Priority" 11. Set exclude to "Yes" and update 12. repeat steps 7 to 9 SUCCESS => only the hold for patron1 now appears, even the other hold had local hold priority 13. Repeat step 10 and 11 but this time set exclude to "No" 14. repeat steps 7 to 9 CHECK => the hold for patron2 is back 15. Edit patron2's category and set exclude from local holds priority to "Yes" 16. Repeat steps 7 to 9 SUCCESS => the hold for patron1 is back 17. Go to tools -> Batch item modification and in barcode list place several (existing) barcodes and press continue CHECK => There is a new section in the bottom called "Priority" 18. Set exclude to "Yes" and save SUCCESS => all items in the list now have exclude setted to "Yes" 19. Try to checkout the first item to a patron3 SUCCESS => Alert message appears saying that patron1 has a hold on that item 20. Click on Yes and then checkin that item SUCCESS => There is a modal window saying that a hold was found for patron1 21. prove t/db_dependent/HoldsQueue.t t/db_dependent/Holds/LocalHoldsPriority.t 22. Sign off Sponsored-by: Cooperative Information Network (CIN) (In reply to Christopher Brannon from comment #9) > Agustin, Kidclamp had to rebase a minor conflict. I don't know the details. > > After that, this is what I found: > > Everything works except the Batch Item Modification. It did not change the > setting for the items specified. Might be because of the rebase. Please > check. > > Also, the description on the patron category is confusing. It should read > "If Yes, holds placed by patrons of this category will not be given > priority." > > I am also unable to prove the test, step 21. I am testing from a sandbox. Hello Christopher, I just updated and rebased this patches. About the description you're right.. at first I understood patron's category exclusion backwards (as if you would exclude the patron that couldn't get the item because of localholdspriority), but later coded things right, and that description remained wrong. Please test again. Thanks. Created attachment 106439 [details] [review] Bug 19889: Add exclude_from_local_holds_priority columns to item, deleteditems and categories tables Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 106440 [details] [review] Bug 19889: set exclude_from_local_holds_priority columns as boolean in schema Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 106441 [details] [review] Bug 19889: Add tests Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 106442 [details] [review] Bug 19889: Make it possible to exclude items and categories from local holds priority This patch adds the ability to exclude patrons (by category) from local holds, and items, by editing the item itself or by batch item modification tool. To test: 1. apply patches 2. updatedatabase 3. Enable LocalHoldsPriority preference, and leave LocalHoldsPriorityPatronControl in pickup library, and LocalHoldsPriorityItemControl in holding library. 4. Search for a biblio with one item. 5. Place a hold with a patron (patron1) and set pickup location to a different library of the item's home library 6. Place another hold with another patron (patron2) and set pickup location to be the same as the item's home library 7. ./misc/cronjobs/holds/build_holds_queue.pl 8. Go to circulation -> holds queue 9. Search by the item's home library CHECK => only the hold for patron2 (with the pickup location the same as the item's home library) appears in the table 10. Go back to the biblio details page and click on "Items" tab CHECK => There is a new section in the item's details between "Statuses" and "History" called "Priority" 11. Set exclude to "Yes" and update 12. repeat steps 7 to 9 SUCCESS => only the hold for patron1 now appears, even the other hold had local hold priority 13. Repeat step 10 and 11 but this time set exclude to "No" 14. repeat steps 7 to 9 CHECK => the hold for patron2 is back 15. Edit patron2's category and set exclude from local holds priority to "Yes" 16. Repeat steps 7 to 9 SUCCESS => the hold for patron1 is back 17. Go to tools -> Batch item modification and in barcode list place several (existing) barcodes and press continue CHECK => There is a new section in the bottom called "Priority" 18. Set exclude to "Yes" and save SUCCESS => all items in the list now have exclude setted to "Yes" 19. Try to checkout the first item to a patron3 SUCCESS => Alert message appears saying that patron1 has a hold on that item 20. Click on Yes and then checkin that item SUCCESS => There is a modal window saying that a hold was found for patron1 21. prove t/db_dependent/HoldsQueue.t t/db_dependent/Holds/LocalHoldsPriority.t 22. Sign off Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Test passes, and everything works according to the test. Let's discuss the following issues before I sign off, since we are funding this project: * When you update priority in the item (yes/no), there is nothing that indicates that it is done or set. It would be helpful if there was something that appeared to indicate that it took. Maybe (SET/UNSET) next to the dropbox? * The patron exclusion seems to be in an uncommon place. Technically, it makes sense to have this in the patron category, but usually these kinds of conditions are setup in the preferences.pl. I state this, because if we do some forward thinking here, if we want to do other exclusions in the future, it might be better to set this as a textbox pref, where you would set this exclusion with a patroncategory: PT, and then in the future we could exclude by itemtype, collectioncode, or other ways. It just makes sense to keep the exclusions all in one place rather than spread out everywhere. And if you do this, could biblio or barcode/itemnumber be set here as well, rather than in the item record itself? * A couple of the criteria we had asked for in the exclusions was item types and formats. Those were not addressed in these patches. I would be willing to forget about formats at this point, but would like to see item types. I am kicking myself for not including biblio in possible exclusions. As is, we have to exclude individual items. If that can be added easily, great. If not, at least the item types needs to be, since it was in the original request. Please the last paragraph in my comment about the criteria. I was going off the original description in this bug rather than the signed contract. This is what the contract called for: * Add a new column to the items table - 'ExcludeFromLoaclHoldsPriority' or similar * Addd the ability to mark the item from the items tab or the batch item modification page * Add a system preference 'CategoriesToExcludeFromLocalHoldsPriority' * This system preference would take a list of patron categories * Alter code to ignore local holds priority for these Talk to me about the reasoning for the placement of the patron exclusion in the patron category rather than a central preference that was outlined here. I just want to understand before you do anything. My comment about the priority for the item not showing as set still stands. That is confusing. (In reply to Christopher Brannon from comment #20) > * This system preference would take a list of patron categories > * Alter code to ignore local holds priority for these > > Talk to me about the reasoning for the placement of the patron exclusion in > the patron category rather than a central preference that was outlined here. > I just want to understand before you do anything. The original idea was to add a syspref, as you outline. I'm the one that decided it was better to make it a patron category attribute. This, because the syspref is basically a way to say something about the category, but in a place that is not tied to categories (and thus unnatural). This is the reasoning: From the coding point of view, we might need/want to ask if a patron belongs to a category that is excluded from localholdspriority, and we would do it like this: # Find the patron my $patron = Koha::Patrons->find($request->{borrowernumber}); # skip the patron if its category is excluded next if $patron->category->exclude_from_local_holds_priority; if we go the sysprefs way it would read like: my @excluded_categories = split(/\|/, C4::Context->preference("CategoriesToExcludeFromLocalHoldsPriority")) // (); my $patron = Koha::Patrons->find($request->{borrowernumber}); next if any { $_ eq $patron->categorycode } @excluded_categories; So the result is similar, but ugly-ish. And involves an error prone free-text field with pipe-separated values. So I don't see any advantage on it. If we were to expose this condition on the API also, we would need to add methods to extract the boolean with this kind of comparison, instead of just reading what's on the category table. That said, we can absolutely roll back to the syspref approach and come back to refactor this later. Once we have a more clear interface for setting all things related to local holds priorities in a single place. As I understand your point of the whole setting being sparse in different places being counter-intuitive. > My comment about the priority for the item not showing as set still stands. > That is confusing. (In reply to Tomás Cohen Arazi from comment #21) > So the result is similar, but ugly-ish. And involves an error prone > free-text field with pipe-separated values. So I don't see any advantage on > it. If we were to expose this condition on the API also, we would need to > add methods to extract the boolean with this kind of comparison, instead of > just reading what's on the category table. What if it wasn't a free-text field? What if we did something similar to what is being done in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22844? Select the categories from a list rather than free typing? I'd prefer to keep these settings together, but if you think it makes more sense to put exclusions in the patron categories, then that would be acceptable. It works. My biggest concern is trying to debug behavior. It's one thing to have the items with their own exclusions in the record. But trying to figure out why it excluding items in some cases and not others because of a patron category exclusion might be more challenging. If there were some indicator when the exclusion is skipped somewhere, then I would have no problem with it. If it is logged, then it would be easy to trace. I'm just trying to make sure that our enhancement doesn't become a headache for those using it to try and understand system behavior. Circulation rules alone are hard to follow when dealing with multiple branches. (In reply to Christopher Brannon from comment #22) > (In reply to Tomás Cohen Arazi from comment #21) > > > So the result is similar, but ugly-ish. And involves an error prone > > free-text field with pipe-separated values. So I don't see any advantage on > > it. If we were to expose this condition on the API also, we would need to > > add methods to extract the boolean with this kind of comparison, instead of > > just reading what's on the category table. > > What if it wasn't a free-text field? What if we did something similar to > what is being done in > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22844? Select the > categories from a list rather than free typing? > > I'd prefer to keep these settings together, but if you think it makes more > sense to put exclusions in the patron categories, then that would be > acceptable. It works. My biggest concern is trying to debug behavior. > It's one thing to have the items with their own exclusions in the record. > But trying to figure out why it excluding items in some cases and not others > because of a patron category exclusion might be more challenging. If there > were some indicator when the exclusion is skipped somewhere, then I would > have no problem with it. If it is logged, then it would be easy to trace. > > I'm just trying to make sure that our enhancement doesn't become a headache > for those using it to try and understand system behavior. Circulation rules > alone are hard to follow when dealing with multiple branches. Hi Christopher, great idea about logging when local hold is excluded. I'll get right into it. (In reply to Christopher Brannon from comment #22) > I'd prefer to keep these settings together, but if you think it makes more > sense to put exclusions in the patron categories, then that would be > acceptable. It works. My biggest concern is trying to debug behavior. > It's one thing to have the items with their own exclusions in the record. > But trying to figure out why it excluding items in some cases and not others > because of a patron category exclusion might be more challenging. If there > were some indicator when the exclusion is skipped somewhere, then I would > have no problem with it. If it is logged, then it would be easy to trace. > > I'm just trying to make sure that our enhancement doesn't become a headache > for those using it to try and understand system behavior. Circulation rules > alone are hard to follow when dealing with multiple branches. I think the feedback idea is both a great one, and also out of scope for this specific bug and development. What I'm imagining is inside the hold trapping code, every time we skip over a hold of higher priority, we add it to a list of skipped holds. This list then needs to be passed all the way back to the template where we can have some kind of 'reason' icon that when clicked will show the higher priority holds that were skipped, and the reason they were skipped. This would definitely be useful even excluding this specific development, and is only tangential to this bug report. I have filed bug 26027 for this new functionality. (In reply to Christopher Brannon from comment #22) > (In reply to Tomás Cohen Arazi from comment #21) > > > So the result is similar, but ugly-ish. And involves an error prone > > free-text field with pipe-separated values. So I don't see any advantage on > > it. If we were to expose this condition on the API also, we would need to > > add methods to extract the boolean with this kind of comparison, instead of > > just reading what's on the category table. > > What if it wasn't a free-text field? What if we did something similar to > what is being done in > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22844? Select the > categories from a list rather than free typing? I wasn't aware of that dev. Nice! (In reply to Tomás Cohen Arazi from comment #25) > (In reply to Christopher Brannon from comment #22) > > (In reply to Tomás Cohen Arazi from comment #21) > > > > > So the result is similar, but ugly-ish. And involves an error prone > > > free-text field with pipe-separated values. So I don't see any advantage on > > > it. If we were to expose this condition on the API also, we would need to > > > add methods to extract the boolean with this kind of comparison, instead of > > > just reading what's on the category table. > > > > What if it wasn't a free-text field? What if we did something similar to > > what is being done in > > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22844? Select the > > categories from a list rather than free typing? > > I wasn't aware of that dev. Nice! After reviewing the implementation I can say that we would still need a separate dev so we can make it handle data from the DB (the defined categories) as it is only using a hardcoded list of attributes... (In reply to Christopher Brannon from comment #19) > Test passes, and everything works according to the test. Let's discuss the > following issues before I sign off, since we are funding this project: > > * When you update priority in the item (yes/no), there is nothing that > indicates that it is done or set. It would be helpful if there was > something that appeared to indicate that it took. Maybe (SET/UNSET) next to > the dropbox? > Okay, so that just leaves this issue. Can this be addressed? (In reply to Christopher Brannon from comment #27) > (In reply to Christopher Brannon from comment #19) > > Test passes, and everything works according to the test. Let's discuss the > > following issues before I sign off, since we are funding this project: > > > > * When you update priority in the item (yes/no), there is nothing that > > indicates that it is done or set. It would be helpful if there was > > something that appeared to indicate that it took. Maybe (SET/UNSET) next to > > the dropbox? > > > Okay, so that just leaves this issue. Can this be addressed? That's what Kyle mentioned in comment 24. We are discussing this tomorrow. I've linked bug 26027 to this bugzilla ticket. That work is outside the scope of this original contracted dev. We will followup on that work after this development is completed. Thanks Joy (In reply to Tomás Cohen Arazi from comment #28) > (In reply to Christopher Brannon from comment #27) > > (In reply to Christopher Brannon from comment #19) > > > Test passes, and everything works according to the test. Let's discuss the > > > following issues before I sign off, since we are funding this project: > > > > > > * When you update priority in the item (yes/no), there is nothing that > > > indicates that it is done or set. It would be helpful if there was > > > something that appeared to indicate that it took. Maybe (SET/UNSET) next to > > > the dropbox? > > > > > Okay, so that just leaves this issue. Can this be addressed? > > That's what Kyle mentioned in comment 24. We are discussing this tomorrow. The issue in comment 27 still has not been addressed. I consider this incomplete without that indicator in place. Created attachment 107180 [details] [review] Bug 19889: Add feedback when exclusion is updated on moredetail.pl Whatever changed since the last sign-off is not working. This crashes the sandbox. Tested sandbox without patch, sandbox works. Added patch, sandbox crashes. RUNNING HANDLER [Run updatedatabase.pl in docker container] ******************** fatal: [localhost -> koha-bug19889]: FAILED! => {"changed": true, "cmd": "koha-shell -c '/kohadevbox/koha/installer/data/mysql/updatedatabase.pl' bug19889", "delta": "0:00:02.838032", "end": "2020-07-22 15:42:59.451294", "msg": "non-zero return code", "rc": 255, "start": "2020-07-22 15:42:56.613262", "stderr": "Number found where operator expected at /kohadevbox/koha/Koha/Schema/Result/Category.pm line 301, near \"Bug 19889\"\n\t(Do you need to predeclare Bug?)\n{UNKNOWN}: syntax error at /kohadevbox/koha/Koha/Schema/Result/Category.pm line 294, near \"==\"\nCompilation failed in require at /usr/share/perl5/Class/C3/Componentised.pm line 150. at /usr/share/perl5/Class/C3/Componentised.pm line 155\nCompilation failed in require at /kohadevbox/koha/Koha/Database.pm line 50.", "stderr_lines": ["Number found where operator expected at /kohadevbox/koha/Koha/Schema/Result/Category.pm line 301, near \"Bug 19889\"", "\t(Do you need to predeclare Bug?)", "{UNKNOWN}: syntax error at /kohadevbox/koha/Koha/Schema/Result/Category.pm line 294, near \"==\"", "Compilation failed in require at /usr/share/perl5/Class/C3/Componentised.pm line 150. at /usr/share/perl5/Class/C3/Componentised.pm line 155", "Compilation failed in require at /kohadevbox/koha/Koha/Database.pm line 50."], "stdout": "", "stdout_lines": []} NO MORE HOSTS LEFT ************************************************************* PLAY RECAP ********************************************************************* localhost : ok=31 changed=22 unreachable=0 failed=1 skipped=1 rescued=0 ignored=0 Whatever changed since the last sign-off is not working. This crashes the sandbox. Tested sandbox without patch, sandbox works. Added patch, sandbox crashes. RUNNING HANDLER [Run updatedatabase.pl in docker container] ******************** fatal: [localhost -> koha-bug19889]: FAILED! => {"changed": true, "cmd": "koha-shell -c '/kohadevbox/koha/installer/data/mysql/updatedatabase.pl' bug19889", "delta": "0:00:02.838032", "end": "2020-07-22 15:42:59.451294", "msg": "non-zero return code", "rc": 255, "start": "2020-07-22 15:42:56.613262", "stderr": "Number found where operator expected at /kohadevbox/koha/Koha/Schema/Result/Category.pm line 301, near \"Bug 19889\"\n\t(Do you need to predeclare Bug?)\n{UNKNOWN}: syntax error at /kohadevbox/koha/Koha/Schema/Result/Category.pm line 294, near \"==\"\nCompilation failed in require at /usr/share/perl5/Class/C3/Componentised.pm line 150. at /usr/share/perl5/Class/C3/Componentised.pm line 155\nCompilation failed in require at /kohadevbox/koha/Koha/Database.pm line 50.", "stderr_lines": ["Number found where operator expected at /kohadevbox/koha/Koha/Schema/Result/Category.pm line 301, near \"Bug 19889\"", "\t(Do you need to predeclare Bug?)", "{UNKNOWN}: syntax error at /kohadevbox/koha/Koha/Schema/Result/Category.pm line 294, near \"==\"", "Compilation failed in require at /usr/share/perl5/Class/C3/Componentised.pm line 150. at /usr/share/perl5/Class/C3/Componentised.pm line 155", "Compilation failed in require at /kohadevbox/koha/Koha/Database.pm line 50."], "stdout": "", "stdout_lines": []} NO MORE HOSTS LEFT ************************************************************* PLAY RECAP ********************************************************************* localhost : ok=31 changed=22 unreachable=0 failed=1 skipped=1 rescued=0 ignored=0 Created attachment 107187 [details] [review] Bug 19889: Add exclude_from_local_holds_priority columns to item, deleteditems and categories tables Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107188 [details] [review] Bug 19889: set exclude_from_local_holds_priority columns as boolean in schema Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107189 [details] [review] Bug 19889: Add tests Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107190 [details] [review] Bug 19889: Make it possible to exclude items and categories from local holds priority This patch adds the ability to exclude patrons (by category) from local holds, and items, by editing the item itself or by batch item modification tool. To test: 1. apply patches 2. updatedatabase 3. Enable LocalHoldsPriority preference, and leave LocalHoldsPriorityPatronControl in pickup library, and LocalHoldsPriorityItemControl in holding library. 4. Search for a biblio with one item. 5. Place a hold with a patron (patron1) and set pickup location to a different library of the item's home library 6. Place another hold with another patron (patron2) and set pickup location to be the same as the item's home library 7. ./misc/cronjobs/holds/build_holds_queue.pl 8. Go to circulation -> holds queue 9. Search by the item's home library CHECK => only the hold for patron2 (with the pickup location the same as the item's home library) appears in the table 10. Go back to the biblio details page and click on "Items" tab CHECK => There is a new section in the item's details between "Statuses" and "History" called "Priority" 11. Set exclude to "Yes" and update 12. repeat steps 7 to 9 SUCCESS => only the hold for patron1 now appears, even the other hold had local hold priority 13. Repeat step 10 and 11 but this time set exclude to "No" 14. repeat steps 7 to 9 CHECK => the hold for patron2 is back 15. Edit patron2's category and set exclude from local holds priority to "Yes" 16. Repeat steps 7 to 9 SUCCESS => the hold for patron1 is back 17. Go to tools -> Batch item modification and in barcode list place several (existing) barcodes and press continue CHECK => There is a new section in the bottom called "Priority" 18. Set exclude to "Yes" and save SUCCESS => all items in the list now have exclude setted to "Yes" 19. Try to checkout the first item to a patron3 SUCCESS => Alert message appears saying that patron1 has a hold on that item 20. Click on Yes and then checkin that item SUCCESS => There is a modal window saying that a hold was found for patron1 21. prove t/db_dependent/HoldsQueue.t t/db_dependent/Holds/LocalHoldsPriority.t 22. Sign off Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107191 [details] [review] Bug 19889: Add feedback when exclusion is updated on moredetail.pl You really like to trip us up on the testing. :) There is no info about HOW the last patch is supposed to show that a change has been made. I was expecting to see something similar to the statuses, where it puts a message next to the header of that section. Instead, it puts an alert at the top of the page. Had to look through the code to figure out what I was looking for. So, it does show staff that something happened. I think at some point we need to open a new bug and fix the consistency of the messages on this page. Statuses put a message next to the section header, this puts an alert at the top of the page, and the notes don't do anything. That's kind of wonky. I would have opted to be consistent with the way statuses show a change rather than do it a whole new way, but it works, and it doesn't do nothing like the notes. So, I'll work with that. :) Created attachment 107192 [details] [review] Bug 19889: Add exclude_from_local_holds_priority columns to item, deleteditems and categories tables Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107193 [details] [review] Bug 19889: set exclude_from_local_holds_priority columns as boolean in schema Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107194 [details] [review] Bug 19889: Add tests Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107195 [details] [review] Bug 19889: Make it possible to exclude items and categories from local holds priority This patch adds the ability to exclude patrons (by category) from local holds, and items, by editing the item itself or by batch item modification tool. To test: 1. apply patches 2. updatedatabase 3. Enable LocalHoldsPriority preference, and leave LocalHoldsPriorityPatronControl in pickup library, and LocalHoldsPriorityItemControl in holding library. 4. Search for a biblio with one item. 5. Place a hold with a patron (patron1) and set pickup location to a different library of the item's home library 6. Place another hold with another patron (patron2) and set pickup location to be the same as the item's home library 7. ./misc/cronjobs/holds/build_holds_queue.pl 8. Go to circulation -> holds queue 9. Search by the item's home library CHECK => only the hold for patron2 (with the pickup location the same as the item's home library) appears in the table 10. Go back to the biblio details page and click on "Items" tab CHECK => There is a new section in the item's details between "Statuses" and "History" called "Priority" 11. Set exclude to "Yes" and update 12. repeat steps 7 to 9 SUCCESS => only the hold for patron1 now appears, even the other hold had local hold priority 13. Repeat step 10 and 11 but this time set exclude to "No" 14. repeat steps 7 to 9 CHECK => the hold for patron2 is back 15. Edit patron2's category and set exclude from local holds priority to "Yes" 16. Repeat steps 7 to 9 SUCCESS => the hold for patron1 is back 17. Go to tools -> Batch item modification and in barcode list place several (existing) barcodes and press continue CHECK => There is a new section in the bottom called "Priority" 18. Set exclude to "Yes" and save SUCCESS => all items in the list now have exclude setted to "Yes" 19. Try to checkout the first item to a patron3 SUCCESS => Alert message appears saying that patron1 has a hold on that item 20. Click on Yes and then checkin that item SUCCESS => There is a modal window saying that a hold was found for patron1 21. prove t/db_dependent/HoldsQueue.t t/db_dependent/Holds/LocalHoldsPriority.t 22. Sign off Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107196 [details] [review] Bug 19889: Add feedback when exclusion is updated on moredetail.pl Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Everything works as expected, tests pass, qa tools green. Last patch needs a rebase - When updating an item on moredetail.pl the blue box can overlap with the title details https://snipboard.io/dKGf8y.jpg - When batch modifying items to change exclude... it registers no changes https://snipboard.io/i6GQ8x.jpg - I think we must display on moremember.pl and detail.pl the info on when a patron/item is excluded, otherwise these changes may not be visible to staff who cannot edit items/patrons/categories and they will not understand what is happening to the holds Created attachment 107908 [details] [review] Bug 19889: Add exclude_from_local_holds_priority columns to item, deleteditems and categories tables Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107909 [details] [review] Bug 19889: set exclude_from_local_holds_priority columns as boolean in schema Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107910 [details] [review] Bug 19889: Add tests Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107911 [details] [review] Bug 19889: Make it possible to exclude items and categories from local holds priority This patch adds the ability to exclude patrons (by category) from local holds, and items, by editing the item itself or by batch item modification tool. To test: 1. apply patches 2. updatedatabase 3. Enable LocalHoldsPriority preference, and leave LocalHoldsPriorityPatronControl in pickup library, and LocalHoldsPriorityItemControl in holding library. 4. Search for a biblio with one item. 5. Place a hold with a patron (patron1) and set pickup location to a different library of the item's home library 6. Place another hold with another patron (patron2) and set pickup location to be the same as the item's home library 7. ./misc/cronjobs/holds/build_holds_queue.pl 8. Go to circulation -> holds queue 9. Search by the item's home library CHECK => only the hold for patron2 (with the pickup location the same as the item's home library) appears in the table 10. Go back to the biblio details page and click on "Items" tab CHECK => There is a new section in the item's details between "Statuses" and "History" called "Priority" 11. Set exclude to "Yes" and update 12. repeat steps 7 to 9 SUCCESS => only the hold for patron1 now appears, even the other hold had local hold priority 13. Repeat step 10 and 11 but this time set exclude to "No" 14. repeat steps 7 to 9 CHECK => the hold for patron2 is back 15. Edit patron2's category and set exclude from local holds priority to "Yes" 16. Repeat steps 7 to 9 SUCCESS => the hold for patron1 is back 17. Go to tools -> Batch item modification and in barcode list place several (existing) barcodes and press continue CHECK => There is a new section in the bottom called "Priority" 18. Set exclude to "Yes" and save SUCCESS => all items in the list now have exclude setted to "Yes" 19. Try to checkout the first item to a patron3 SUCCESS => Alert message appears saying that patron1 has a hold on that item 20. Click on Yes and then checkin that item SUCCESS => There is a modal window saying that a hold was found for patron1 21. prove t/db_dependent/HoldsQueue.t t/db_dependent/Holds/LocalHoldsPriority.t 22. Sign off Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107912 [details] [review] Bug 19889: Add feedback when exclusion is updated on moredetail.pl Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Created attachment 107913 [details] [review] Bug 19889: (follow-up) Fix overlapping blue box and message in batch item modification Created attachment 107926 [details] [review] Bug 19889: Add exclude_from_local_holds_priority columns to item, deleteditems and categories tables Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 107927 [details] [review] Bug 19889: set exclude_from_local_holds_priority columns as boolean in schema Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 107928 [details] [review] Bug 19889: Add tests Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 107929 [details] [review] Bug 19889: Make it possible to exclude items and categories from local holds priority This patch adds the ability to exclude patrons (by category) from local holds, and items, by editing the item itself or by batch item modification tool. To test: 1. apply patches 2. updatedatabase 3. Enable LocalHoldsPriority preference, and leave LocalHoldsPriorityPatronControl in pickup library, and LocalHoldsPriorityItemControl in holding library. 4. Search for a biblio with one item. 5. Place a hold with a patron (patron1) and set pickup location to a different library of the item's home library 6. Place another hold with another patron (patron2) and set pickup location to be the same as the item's home library 7. ./misc/cronjobs/holds/build_holds_queue.pl 8. Go to circulation -> holds queue 9. Search by the item's home library CHECK => only the hold for patron2 (with the pickup location the same as the item's home library) appears in the table 10. Go back to the biblio details page and click on "Items" tab CHECK => There is a new section in the item's details between "Statuses" and "History" called "Priority" 11. Set exclude to "Yes" and update 12. repeat steps 7 to 9 SUCCESS => only the hold for patron1 now appears, even the other hold had local hold priority 13. Repeat step 10 and 11 but this time set exclude to "No" 14. repeat steps 7 to 9 CHECK => the hold for patron2 is back 15. Edit patron2's category and set exclude from local holds priority to "Yes" 16. Repeat steps 7 to 9 SUCCESS => the hold for patron1 is back 17. Go to tools -> Batch item modification and in barcode list place several (existing) barcodes and press continue CHECK => There is a new section in the bottom called "Priority" 18. Set exclude to "Yes" and save SUCCESS => all items in the list now have exclude setted to "Yes" 19. Try to checkout the first item to a patron3 SUCCESS => Alert message appears saying that patron1 has a hold on that item 20. Click on Yes and then checkin that item SUCCESS => There is a modal window saying that a hold was found for patron1 21. prove t/db_dependent/HoldsQueue.t t/db_dependent/Holds/LocalHoldsPriority.t 22. Sign off Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 107930 [details] [review] Bug 19889: Add feedback when exclusion is updated on moredetail.pl Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 107931 [details] [review] Bug 19889: (follow-up) Fix overlapping blue box and message in batch item modification Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Few minor things found: * I think "Batch item modification results" should display the value in the table if it has been modified * https://snipboard.io/NWPmMg.jpg Maybe we need a "legend" for the first fieldset and a less specific one for the one you add? What other field/attribute could we add to "Priority"? Double check with a English native speaker but maybe we would prefer "MARC fields" and "Other attributes"? * "Exclude from local holds priority" does not appear in the header of the "Patron categories" table * moredetails.pl + updated_exclude_from_local_holds_priority => $query->param('updated_exclude_from_local_holds_priority') => Force scalar context is missing for ->param * -print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber"); +print $cgi->redirect("moredetail.pl?" . $alerts . "biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber"); Put $alerts after existing params I'd say Also maybe "$messages" would be more appropriated * I think the following block is not translator friendly + <i>Exclude from local holds priority</i> updated to + [% IF updated_exclude_from_local_holds_priority %]<i>Yes</i>[% ELSE %]<i>No</i>[% END %] * Not blocker, in the tests you could have set the default value for exclude_from_local_holds_priority in t::lib::TestBuilder Created attachment 107948 [details] [review] Bug 19889: (follow-up) Add tests Scratching my head, is the following condition correct?
C4::HoldsQueue::MapItemsToHoldRequests
409 next
410 if ( !$item->{holdallowed} )
411 || ( $item->{holdallowed} == 1
412 && $item->{homebranch} ne $request->{borrowerbranch}
413 || $item->{_object}->exclude_from_local_holds_priority );
Should not we exclude it in any cases?
It's correct but read it is confusing, I would expect it to be more explicit:
409 next
410 if !$item->{holdallowed}
411 || ( $item->{holdallowed} == 1
412 && $item->{homebranch} ne $request->{borrowerbranch} )
413 || $item->{_object}->exclude_from_local_holds_priority
And, finally, why isn't it a circ rule actually? (In reply to Jonathan Druart from comment #61) > And, finally, why isn't it a circ rule actually? That's because we don't have a suitable user interface for this, until the revamp is pushed. (In reply to Jonathan Druart from comment #61) > And, finally, why isn't it a circ rule actually? There was a desire to be able to mark individual items as well. Marking it per item allows it to show in reports and other places as well. Created attachment 108213 [details] [review] Bug 19889: Add exclude_from_local_holds_priority columns to item, deleteditems and categories tables Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 108214 [details] [review] Bug 19889: set exclude_from_local_holds_priority columns as boolean in schema Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 108215 [details] [review] Bug 19889: Add tests Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 108216 [details] [review] Bug 19889: Make it possible to exclude items and categories from local holds priority This patch adds the ability to exclude patrons (by category) from local holds, and items, by editing the item itself or by batch item modification tool. To test: 1. apply patches 2. updatedatabase 3. Enable LocalHoldsPriority preference, and leave LocalHoldsPriorityPatronControl in pickup library, and LocalHoldsPriorityItemControl in holding library. 4. Search for a biblio with one item. 5. Place a hold with a patron (patron1) and set pickup location to a different library of the item's home library 6. Place another hold with another patron (patron2) and set pickup location to be the same as the item's home library 7. ./misc/cronjobs/holds/build_holds_queue.pl 8. Go to circulation -> holds queue 9. Search by the item's home library CHECK => only the hold for patron2 (with the pickup location the same as the item's home library) appears in the table 10. Go back to the biblio details page and click on "Items" tab CHECK => There is a new section in the item's details between "Statuses" and "History" called "Priority" 11. Set exclude to "Yes" and update 12. repeat steps 7 to 9 SUCCESS => only the hold for patron1 now appears, even the other hold had local hold priority 13. Repeat step 10 and 11 but this time set exclude to "No" 14. repeat steps 7 to 9 CHECK => the hold for patron2 is back 15. Edit patron2's category and set exclude from local holds priority to "Yes" 16. Repeat steps 7 to 9 SUCCESS => the hold for patron1 is back 17. Go to tools -> Batch item modification and in barcode list place several (existing) barcodes and press continue CHECK => There is a new section in the bottom called "Priority" 18. Set exclude to "Yes" and save SUCCESS => all items in the list now have exclude setted to "Yes" 19. Try to checkout the first item to a patron3 SUCCESS => Alert message appears saying that patron1 has a hold on that item 20. Click on Yes and then checkin that item SUCCESS => There is a modal window saying that a hold was found for patron1 21. prove t/db_dependent/HoldsQueue.t t/db_dependent/Holds/LocalHoldsPriority.t 22. Sign off Sponsored-by: Cooperative Information Network (CIN) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 108217 [details] [review] Bug 19889: Add feedback when exclusion is updated on moredetail.pl Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 108218 [details] [review] Bug 19889: (follow-up) Fix overlapping blue box and message in batch item modification Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 108219 [details] [review] Bug 19889: (follow-up) Add tests Created attachment 108220 [details] [review] Bug 19889: (follow-up) Fix few minor things Hi Jonathan, fixed most things but..
> *
> -print
> $cgi->redirect("moredetail.
> pl?biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber");
> +print $cgi->redirect("moredetail.pl?" . $alerts .
> "biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber");
>
> Put $alerts after existing params I'd say
changed name to messages, but this cannot be placed at the end of the string. It needs to be before #item$itemnumber
Created attachment 108593 [details] [review] Bug 19889: (follow-up) Add tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 108594 [details] [review] Bug 19889: (follow-up) Fix few minor things Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 108595 [details] [review] Bug 19889: (QA follow-up) use em over i and update field if unset em is better for accessibility We should set the field to 0 when it was previously unset Signed-off-by: Nick Clemens <nick@bywatersolutions.com> (In reply to Nick Clemens from comment #74) > Created attachment 108594 [details] [review] [review] > Bug 19889: (follow-up) Fix few minor things > > Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Agustin, please do not introduce unnecessary indentation changes. Created attachment 108725 [details] [review] Bug 19889: (follow-up) Fix few minor things Signed-off-by: Nick Clemens <nick@bywatersolutions.com> JD amended patch: remove unecessary indentation changes Created attachment 108726 [details] [review] Bug 19889: (QA follow-up) use em over i and update field if unset em is better for accessibility We should set the field to 0 when it was previously unset Signed-off-by: Nick Clemens <nick@bywatersolutions.com> I think there is something wrong in the batch mod tool, the value is set to 0 even if we don't select "Yes" or "No" To recreate have item bc_1 with value set to Yes Edit in batch, don't modify anything, submit => bc_1 has now "No" (0) (In reply to Jonathan Druart from comment #58) > * I think the following block is not translator friendly > + <i>Exclude from local holds priority</i> updated to > + [% IF updated_exclude_from_local_holds_priority %]<i>Yes</i>[% ELSE > %]<i>No</i>[% END %] You replaced it with: - [% IF updated_exclude_from_local_holds_priority %]<i>Yes</i>[% ELSE %]<i>No</i>[% END %] + [% IF updated_exclude_from_local_holds_priority %] + <i>Yes</i> + [% ELSE %] + <i>No</i> + [% END %] What I meant is that it will be displayed with the TT block in the po files. I think it would be better: [% IF updated_exclude_from_local_holds_priority %] <i>Exclude from local holds priority</i> updated to <i>Yes</i> [% ELSE %] <i>Exclude from local holds priority</i> updated to <i>No</i> [% END %] Asking confirmation to Katrin. Created attachment 108728 [details] [review] Bug 19889: (QA follow-up) Check that exclude from local holds priority is set and not a blank string (In reply to Nick Clemens from comment #81) > Created attachment 108728 [details] [review] [review] > Bug 19889: (QA follow-up) Check that exclude from local holds priority is > set and not a blank string I thought it was my follow-up that messed things up so followed up here. Seems we need to test if defined and set to something. Agustin feel free to comment or update my follow-ups if needed (In reply to Nick Clemens from comment #82) > (In reply to Nick Clemens from comment #81) > > Created attachment 108728 [details] [review] [review] [review] > > Bug 19889: (QA follow-up) Check that exclude from local holds priority is > > set and not a blank string > > I thought it was my follow-up that messed things up so followed up here. > Seems we need to test if defined and set to something. Agustin feel free to > comment or update my follow-ups if needed Thanks Nick for catching this up.. I'll check it out, but I don't think I will need to update your follow-up. It seems ok. (In reply to Jonathan Druart from comment #80) > (In reply to Jonathan Druart from comment #58) > > * I think the following block is not translator friendly > > + <i>Exclude from local holds priority</i> updated to > > + [% IF updated_exclude_from_local_holds_priority %]<i>Yes</i>[% ELSE > > %]<i>No</i>[% END %] > > You replaced it with: > > - [% IF updated_exclude_from_local_holds_priority %]<i>Yes</i>[% ELSE > %]<i>No</i>[% END %] > + [% IF updated_exclude_from_local_holds_priority %] > + <i>Yes</i> > + [% ELSE %] > + <i>No</i> > + [% END %] > > What I meant is that it will be displayed with the TT block in the po files. > I think it would be better: > [% IF updated_exclude_from_local_holds_priority %] > <i>Exclude from local holds priority</i> updated to <i>Yes</i> > [% ELSE %] > <i>Exclude from local holds priority</i> updated to <i>No</i> > [% END %] > > Asking confirmation to Katrin. I think I'd avoid the <i> - as HTML tags they break up the strings. I haven't tested this, but with your suggestion I think we could end up with: Exclude from local holds priority updated to Yes (Best is always to confirm by updating the po files if unsure) Better, I think, would be something like: Exclude from local holds priority updated to ... [IF ELSE for yes and no, maybe with apostropes instad of italic? 'Yes'] (In reply to Jonathan Druart from comment #76) > (In reply to Nick Clemens from comment #74) > > Created attachment 108594 [details] [review] [review] [review] > > Bug 19889: (follow-up) Fix few minor things > > > > Signed-off-by: Nick Clemens <nick@bywatersolutions.com> > > Agustin, please do not introduce unnecessary indentation changes. Ups.. Sorry Jonathan.. seems my perltidy is too eager. I'll check him out next time :P Created attachment 109131 [details] [review] Bug 19889: (QA follow-up) Check that exclude from local holds priority is set and not a blank string Signed-off-by: Agustin Moyano <agustinmoyano@theke.io> Created attachment 109132 [details] [review] Bug 19889: (follow-up) Fix text in moredetail.tt Created attachment 109285 [details] [review] Bug 19889: (follow-up) Fix text in moredetail.tt Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Katrin said the last patch should be okay for translations, moving to PQA Created attachment 109364 [details] [review] Bug 19889: Add select2 class to the new dropdown list This is needed since bug 25727. Pushed to master for 20.11, thanks to everybody involved! Created attachment 109419 [details] [review] Bug 19889: Add new attribute to the REST API definition It fixes t/db_dependent/api/v1/items.t (In reply to Jonathan Druart from comment #92) > Created attachment 109419 [details] [review] [review] > Bug 19889: Add new attribute to the REST API definition > > It fixes t/db_dependent/api/v1/items.t Pushed to master for 20.11. enhancement will not be backported to 20.05.x Cannot upgrade to master from earlier versions DBRev 20.06.00.032: ALTER TABLE `categories` ADD COLUMN `exclude_from_local_holds_priority` tinyint(1) default NULL AFTER `require_strong_password` DBRev 20.06.00.037: ALTER TABLE categories ADD COLUMN `require_strong_password` TINYINT(1) NULL DEFAULT NULL AFTER `min_password_length` -- set required password strength for patrons in this category We cannot add a column after another before we add that column, just need to swap these revisions (In reply to Nick Clemens from comment #95) > Cannot upgrade to master from earlier versions > > DBRev 20.06.00.032: > ALTER TABLE `categories` ADD COLUMN `exclude_from_local_holds_priority` > tinyint(1) default NULL AFTER `require_strong_password` > > DBRev 20.06.00.037: > ALTER TABLE categories ADD COLUMN `require_strong_password` TINYINT(1) NULL > DEFAULT NULL AFTER `min_password_length` -- set required password strength > for patrons in this category > > We cannot add a column after another before we add that column, just need to > swap these revisions Bug 26513 Should be fixed now commit d45a699975b9857990e19bb9ddc12267ebd05334 Bug 19889: (follow-up 2) update DB adjustments enhancement will not be backported to 20.05.x |
