|
Description
Jonathan Field
2020-04-23 09:38:53 UTC
Created attachment 103856 [details] [review] Bug 25261: Add system preference Created attachment 103857 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Use of uninitialized value $no_of_parts in numeric gt (>) at /C4/Circulation.pm line 848. You should replace: my $no_of_parts = $item_object->materials; with: my $no_of_parts = $item_object->materials || 0; The confirmation need is controlled by a new syspref. Why not a per user permission like? CAN_user_circulate_force_checkout_materials You enhancement works great for check in: playing with CircConfirmParts, the confirmation if the return is required or not. But for checkout, independently of CircConfirmParts, the confirmation is required. I will sign your patch and add a follow-up to fix that. Created attachment 104011 [details] [review] Bug 25261: Add system preference Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 104012 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 104013 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Is "Parts" meaningful enough? As a non-native English speaker it seems vague to me, "parts" of what? Maybe adding "Item" in the syspref's name? Please ignore if it does not make sense :) Also, from QA script:
OK C4/Circulation.pm
FAIL circ/returns.pl
FAIL spelling
recieved ==> received
OK installer/data/mysql/sysprefs.sql
FAIL koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
FAIL filters
missing_filter at line 203 ( Please confirm that the accompanying materials are present: [% additional_materials %])
And I spotted a 8 indentation level in returns.pl
304 $needs_confirm = 1;
Created attachment 104404 [details] [review] Bug 25261: Add system preference Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 104405 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 104406 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 104407 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Created attachment 104408 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. QA failures corrected and system preference updated to include 'Item' to clarify it's use. I've also added a followup to allow for arbitrary notes in the materials field.. they are not always numeric it appears. Created attachment 104409 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 104410 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 104411 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 104412 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Created attachment 104413 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Created attachment 104414 [details] [review] Bug 25261: (QA follow-up) Add unit tests Created attachment 107217 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 107218 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 107219 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Created attachment 107220 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Created attachment 107221 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Created attachment 107222 [details] [review] Bug 25261: (QA follow-up) Add unit tests Created attachment 107228 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 107229 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 107230 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 107231 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 107232 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 107233 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 107234 [details] [review] Bug 25261: (QA follow-up) Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Found 2 problems: 1. When checking in, "Please confirm checkin" click "No" will check the item in 2. If you checkin from the checkouts page there is no alert Created attachment 107285 [details] [review] Bug 25261: (QA follow-up) Simplify tests and conditions Use build_sample_item in tests Simplify tests for the confirmation Created attachment 107331 [details] [review] Bug 25261: (QA follow-up) Non-confirm on return This patch updates the confirmation from an alert to a dismissable modal which allows for optionally not checking the item in (In reply to Jonathan Druart from comment #36) > Found 2 problems: > 1. When checking in, "Please confirm checkin" click "No" will check the item in Fixed in followup, good catch, not sure how my brain missed that :( > 2. If you checkin from the checkouts page there is no alert Can you clarify how to achieve this.. I can't replicate it at present. (In reply to Martin Renvoize from comment #39) > (In reply to Jonathan Druart from comment #36) > > 2. If you checkin from the checkouts page there is no alert > > Can you clarify how to achieve this.. I can't replicate it at present. from the checkouts view: circ/circulation.pl?borrowernumber=42 (In reply to Jonathan Druart from comment #40) > (In reply to Martin Renvoize from comment #39) > > (In reply to Jonathan Druart from comment #36) > > > 2. If you checkin from the checkouts page there is no alert > > > > Can you clarify how to achieve this.. I can't replicate it at present. > > from the checkouts view: circ/circulation.pl?borrowernumber=42 Nope, still not sure what you mean. I checkout an item to person X.. and then try to checkout the same item to Person Y... the alert is triggered.. is that not what you mean? How else can you perform a check-in on the checkout page? https://snipboard.io/RsyS8W.jpg last column Found it.. the action was hidden by the checkout table column settings.. grr. Looking at how the code in the works, this is going to be rather challenging to add [U+1F611] Created attachment 107822 [details] [review] Bug 25261: (QA follow-up) Add confirmation to issues table The issues table allows for checkins and was not appropriately requireing confirmation for the multi-part confirmation added in this bug. Created attachment 107823 [details] [review] Bug 25261: (QA follow-up) Add confirmation to issues table The issues table allows for checkins and was not appropriately requireing confirmation for the multi-part confirmation added in this bug. Let me know if you're happy or not with that followup Jonathan.. was an interesting area to dig into :) What about self checkout? Has it been tested? (In reply to Jonathan Druart from comment #47) > What about self checkout? Has it been tested? Reading the code I think it's missing from opac/sco/sco-main.pl line 154. I wonder about making it visible in sco/sci, as we had a lengthy discussion about libraries using this field for information that should not be seen by users: bug 13547 I would suggest to deal with self-checkout/checkin separately, maybe with a pref. There is the option to just block in SCO by the looks.. 'Please see a member of the library staff.' I wonder if that's the approach to take.? (In reply to Martin Renvoize from comment #50) > There is the option to just block in SCO by the looks.. > > 'Please see a member of the library staff.' > > I wonder if that's the approach to take.? I think that's a common approach for multi-part items. Only problem might be that it causes a change in behaviour we should probably document clearly in the pref description (items with materials no will no longer be able to be returned on self check). Hate to ask, but if we go there... what about SIP? Created attachment 108196 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108197 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108198 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108199 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108200 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108201 [details] [review] Bug 25261: (QA follow-up) Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108202 [details] [review] Bug 25261: (QA follow-up) Simplify tests and conditions Use build_sample_item in tests Simplify tests for the confirmation Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108203 [details] [review] Bug 25261: (QA follow-up) Non-confirm on return This patch updates the confirmation from an alert to a dismissable modal which allows for optionally not checking the item in Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108204 [details] [review] Bug 25261: (QA follow-up) Add confirmation to issues table The issues table allows for checkins and was not appropriately requireing confirmation for the multi-part confirmation added in this bug. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108205 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkout Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108206 [details] [review] Bug 25261: (QA follow-up) Add handling to SCO Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108207 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkin Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108208 [details] [review] Bug 25261: (QA follow-up) Add handling to SCI Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Bumping back to the NQA queue as we had a number of QA followups and I feel like a final QA run would be a good idea. What is the expected behavior on SIP, SCO and SCI? I think updating the commit messages for the last 3 patches could be really helpful. Created attachment 108230 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108231 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108232 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108233 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108234 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108235 [details] [review] Bug 25261: (QA follow-up) Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108236 [details] [review] Bug 25261: (QA follow-up) Simplify tests and conditions Use build_sample_item in tests Simplify tests for the confirmation Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108237 [details] [review] Bug 25261: (QA follow-up) Non-confirm on return This patch updates the confirmation from an alert to a dismissable modal which allows for optionally not checking the item in Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108238 [details] [review] Bug 25261: (QA follow-up) Add confirmation to issues table The issues table allows for checkins and was not appropriately requireing confirmation for the multi-part confirmation added in this bug. Test plan 1/ Enable the feature as per previous patches 2/ Checkot an item with attached materials 3/ Navigate to a page that display your users issues table (the checkout page is a reasonable example) 4/ You should have the option to select items for return in the table (If not, use the column settings to enable the feature) 5/ Select at least the item with attached materials to return 6/ Upon clicking the return buttton you should find that items without additional materials are returned as expected, but rows with additional materials turn yellow and contain a message and additional checkbox for confirmation in the table. 7/ Ensure the checkbox is selected and click the return button again 8/ This item should have been returned. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108239 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkout Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via a SIP unit 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108240 [details] [review] Bug 25261: (QA follow-up) Add handling to SCO Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via the SCO 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108241 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkin Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via a SIP unit 3/ Note that the return is blocked with an alert type of '99' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108242 [details] [review] Bug 25261: (QA follow-up) Add handling to SCI Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via the SCI 3/ Note that the return is blocked with the message '(The item cannot be returned at a self service terminal, please see library staff for assistance)' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> (In reply to Katrin Fischer from comment #66) > What is the expected behavior on SIP, SCO and SCI? I think updating the > commit messages for the last 3 patches could be really helpful. As requested, I've added a test plan per follow-up patch to clarify behaviour. Could someone help me out here by at least testing the SIP bit? I can take care of the other parts. SIP checkin fails - the message for needsconfirmation of 'additional_materials' is lowercased, but the test is UPPER CASED. Additionally the sub _can_we_issue deletes any lowercased keys - there is half an explanation in a comment One more thing: If i check in an item that needs confirmation the checkin list is cleared after confirmation To recreate: 0 - Enable ShowAllCheckins 1 - Check in several regular items 2 - Check in an item with parts 3 - Confirm the checkin in 4 - The list of checkins contains only last item Created attachment 109478 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109479 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109480 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109481 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109482 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109483 [details] [review] Bug 25261: (QA follow-up) Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109484 [details] [review] Bug 25261: (QA follow-up) Simplify tests and conditions Use build_sample_item in tests Simplify tests for the confirmation Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109485 [details] [review] Bug 25261: (QA follow-up) Non-confirm on return This patch updates the confirmation from an alert to a dismissable modal which allows for optionally not checking the item in Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109486 [details] [review] Bug 25261: (QA follow-up) Add confirmation to issues table The issues table allows for checkins and was not appropriately requireing confirmation for the multi-part confirmation added in this bug. Test plan 1/ Enable the feature as per previous patches 2/ Checkot an item with attached materials 3/ Navigate to a page that display your users issues table (the checkout page is a reasonable example) 4/ You should have the option to select items for return in the table (If not, use the column settings to enable the feature) 5/ Select at least the item with attached materials to return 6/ Upon clicking the return buttton you should find that items without additional materials are returned as expected, but rows with additional materials turn yellow and contain a message and additional checkbox for confirmation in the table. 7/ Ensure the checkbox is selected and click the return button again 8/ This item should have been returned. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109487 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkout Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via a SIP unit 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109488 [details] [review] Bug 25261: (QA follow-up) Add handling to SCO Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via the SCO 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109489 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkin Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via a SIP unit 3/ Note that the return is blocked with an alert type of '99' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109490 [details] [review] Bug 25261: (QA follow-up) Add handling to SCI Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via the SCI 3/ Note that the return is blocked with the message '(The item cannot be returned at a self service terminal, please see library staff for assistance)' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109491 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109492 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109493 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109494 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109495 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109496 [details] [review] Bug 25261: (QA follow-up) Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109497 [details] [review] Bug 25261: (QA follow-up) Simplify tests and conditions Use build_sample_item in tests Simplify tests for the confirmation Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109498 [details] [review] Bug 25261: (QA follow-up) Non-confirm on return This patch updates the confirmation from an alert to a dismissable modal which allows for optionally not checking the item in Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109499 [details] [review] Bug 25261: (QA follow-up) Add confirmation to issues table The issues table allows for checkins and was not appropriately requireing confirmation for the multi-part confirmation added in this bug. Test plan 1/ Enable the feature as per previous patches 2/ Checkot an item with attached materials 3/ Navigate to a page that display your users issues table (the checkout page is a reasonable example) 4/ You should have the option to select items for return in the table (If not, use the column settings to enable the feature) 5/ Select at least the item with attached materials to return 6/ Upon clicking the return buttton you should find that items without additional materials are returned as expected, but rows with additional materials turn yellow and contain a message and additional checkbox for confirmation in the table. 7/ Ensure the checkbox is selected and click the return button again 8/ This item should have been returned. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109500 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkout Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via a SIP unit 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109501 [details] [review] Bug 25261: (QA follow-up) Add handling to SCO Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via the SCO 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109502 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkin Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via a SIP unit 3/ Note that the return is blocked with an alert type of '99' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109503 [details] [review] Bug 25261: (QA follow-up) Add handling to SCI Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via the SCI 3/ Note that the return is blocked with the message '(The item cannot be returned at a self service terminal, please see library staff for assistance)' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109504 [details] [review] Bug 25261: (QA follow-up) Capitalize return of needsconfirm This patch capitalizes the 'ADDITIONAL_MATERIALS' key in the needsconfirm hash return value for CanBookBeIssued to restore consistency with previous code and ensure SIP Checkout works as expected. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Final patch should restore consistency in the return value for CanBookBeIssued such that SIP Checkout will perform as expected. Created attachment 109508 [details] [review] Bug 25261: (QA follow-up) Restore previously returned list Test plan 0 - Enable ShowAllCheckins 1 - Check in several regular items 2 - Check in an item with parts 3 - Confirm the checkin in 4 - The list of checkins contains all the previous checkins and the multi-part checkin too. All issues highlighted in QA should now be resolved :), Thanks for testing Nick :) (In reply to Katrin Fischer from comment #81) > Could someone help me out here by at least testing the SIP bit? I can take > care of the other parts. SIP bits are all good with latest patches - do you plan to test the rest? Can you please rebase? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 25261: Add system preference Using index info to reconstruct a base tree... M installer/data/mysql/sysprefs.sql M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Auto-merging installer/data/mysql/sysprefs.sql Applying: Bug 25261: Require confirmation of multiple parts items Applying: Bug 25261: Checkout confirmation depending on syspref Applying: Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts error: sha1 information is lacking or useless (circ/returns.pl). error: could not build fake ancestor Patch failed at 0001 Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-25261-follow-up-Rename-CircConfirmParts-to-Cir-9FLtrb.patch Created attachment 110038 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110039 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110040 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110041 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110042 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110043 [details] [review] Bug 25261: (QA follow-up) Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110044 [details] [review] Bug 25261: (QA follow-up) Simplify tests and conditions Use build_sample_item in tests Simplify tests for the confirmation Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110045 [details] [review] Bug 25261: (QA follow-up) Non-confirm on return This patch updates the confirmation from an alert to a dismissable modal which allows for optionally not checking the item in Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110046 [details] [review] Bug 25261: (QA follow-up) Add confirmation to issues table The issues table allows for checkins and was not appropriately requireing confirmation for the multi-part confirmation added in this bug. Test plan 1/ Enable the feature as per previous patches 2/ Checkot an item with attached materials 3/ Navigate to a page that display your users issues table (the checkout page is a reasonable example) 4/ You should have the option to select items for return in the table (If not, use the column settings to enable the feature) 5/ Select at least the item with attached materials to return 6/ Upon clicking the return buttton you should find that items without additional materials are returned as expected, but rows with additional materials turn yellow and contain a message and additional checkbox for confirmation in the table. 7/ Ensure the checkbox is selected and click the return button again 8/ This item should have been returned. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110047 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkout Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via a SIP unit 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110048 [details] [review] Bug 25261: (QA follow-up) Add handling to SCO Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via the SCO 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110049 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkin Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via a SIP unit 3/ Note that the return is blocked with an alert type of '99' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110050 [details] [review] Bug 25261: (QA follow-up) Add handling to SCI Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via the SCI 3/ Note that the return is blocked with the message '(The item cannot be returned at a self service terminal, please see library staff for assistance)' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110051 [details] [review] Bug 25261: (QA follow-up) Capitalize return of needsconfirm This patch capitalizes the 'ADDITIONAL_MATERIALS' key in the needsconfirm hash return value for CanBookBeIssued to restore consistency with previous code and ensure SIP Checkout works as expected. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110052 [details] [review] Bug 25261: (QA follow-up) Restore previously returned list Test plan 0 - Enable ShowAllCheckins 1 - Check in several regular items 2 - Check in an item with parts 3 - Confirm the checkin in 4 - The list of checkins contains all the previous checkins and the multi-part checkin too. Created attachment 110133 [details] [review] Bug 25261: (QA follow-up) Terminology: checkin/checkout When checking out an item with $3 set and the syspref active, I see both the old blue 'information dialog' and the new yellow 'confirmation dialog'. The information shown is doubled up this way - I believe if we ask for confirmation the blue one should not appear? Created attachment 110184 [details] [review] Bug 25261: (QA follow-up) Prevent double dialogue This patch removes the blue information message box if we are using multi-part confirmations. Corrected in followup. 1) Check in via check in page With confirmation off: When returning an item with $3 the dialog is blue. With the patch, the dialog is yellow and has a list item. I believe that the previous formatting should be kept in this case, so that checkout and checkin match up. With confirmation on: There is a similar problem as before with the modal AND the normal message showing in the background. I think we should only show the modal in this case. Visually I would maybe make the $3 content in the message stand out a bit more, making it <strong> maybe? 2) Self check out (sco-main) On self check out with returns allowed (SCOAllowCheckin): When you click on "renew item", it asks if you want to renew or return it. You can then successfully return the item from this dialog. Note: There are some issues with the CSS there - the Renew item link is not obvious as a button - and I am not sure why the normal return option doesn't display :(
> 2) Self check out (sco-main)
I also had trouble with the 'renew' option from here - could you try that please?
sci-main is ok. Created attachment 110258 [details] [review] Bug 25261: (QA follow-up) Make issue and returns messages consistent This patch makes the confirmation dialogue handing and visuality consistent between issues and returns. Created attachment 110259 [details] [review] Bug 25261: (QA follow-up) Catch attepts to return via SCO This patch adds a block to capture attempts to return items with attached materials when checking in via the self checkout when the relevant system preference is set. (In reply to Katrin Fischer from comment #135) > > 2) Self check out (sco-main) > > I also had trouble with the 'renew' option from here - could you try that > please? This is a bit weird; but I don't think it's anything to do with this bug. The renewal goes through as expected if you look at the data, but I would expect the page to redirect back to the logged in SCO session rather than just leave you on the confirmation page. The behaviour pre-exists this bug though, so I'd prefer to fix it in it's own bug. (In reply to Katrin Fischer from comment #134) > Note: There are some issues with the CSS there - the Renew item link is not > obvious as a button - and I am not sure why the normal return option doesn't > display :( Not related to this bug; certainly one to raise on it's own bug. (In reply to Martin Renvoize from comment #140) > (In reply to Katrin Fischer from comment #134) > > Note: There are some issues with the CSS there - the Renew item link is not > > obvious as a button - and I am not sure why the normal return option doesn't > > display :( > > Not related to this bug; certainly one to raise on it's own bug. Forgot to update here, already filed 2 new bugs for this: Bug 26479 Bug 26478 > > I also had trouble with the 'renew' option from here - could you try that
> > please?
>
> This is a bit weird; but I don't think it's anything to do with this bug.
> The renewal goes through as expected if you look at the data, but I would
> expect the page to redirect back to the logged in SCO session rather than
> just leave you on the confirmation page. The behaviour pre-exists this bug
> though, so I'd prefer to fix it in it's own bug.
It worked for me without the patches - the page did then reload for me. Same item as with the confirmation. I can retest, but it might be related.
Created attachment 110268 [details] [review] Bug 25261: (QA follow-up) Prevent server error on bad barcode Prior to this patch if you had CircConfirmParts enabled and you attempted to checkin a deleted item then you would be met with a server error. I retested this: - Turn on multi part checking and edit 3999900000018 to have $3 - Check out 3999900000018 in staff - Go to sco/sco-main.pl - The checkout shows in the table of checkouts - The last column is "Renew item" - Click on it, click "Renew" - page refreshes, all is good - Apply patches, repeat - Renew doesn't direct back to the overview page anymore Right.. you were getting caught by bug 26301.. which got pushed to master last night! Created attachment 110322 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110323 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110324 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110325 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110326 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110327 [details] [review] Bug 25261: (QA follow-up) Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110328 [details] [review] Bug 25261: (QA follow-up) Simplify tests and conditions Use build_sample_item in tests Simplify tests for the confirmation Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110329 [details] [review] Bug 25261: (QA follow-up) Non-confirm on return This patch updates the confirmation from an alert to a dismissable modal which allows for optionally not checking the item in Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110330 [details] [review] Bug 25261: (QA follow-up) Add confirmation to issues table The issues table allows for checkins and was not appropriately requireing confirmation for the multi-part confirmation added in this bug. Test plan 1/ Enable the feature as per previous patches 2/ Checkot an item with attached materials 3/ Navigate to a page that display your users issues table (the checkout page is a reasonable example) 4/ You should have the option to select items for return in the table (If not, use the column settings to enable the feature) 5/ Select at least the item with attached materials to return 6/ Upon clicking the return buttton you should find that items without additional materials are returned as expected, but rows with additional materials turn yellow and contain a message and additional checkbox for confirmation in the table. 7/ Ensure the checkbox is selected and click the return button again 8/ This item should have been returned. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110331 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkout Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via a SIP unit 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110332 [details] [review] Bug 25261: (QA follow-up) Add handling to SCO Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via the SCO 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110333 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkin Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via a SIP unit 3/ Note that the return is blocked with an alert type of '99' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110334 [details] [review] Bug 25261: (QA follow-up) Add handling to SCI Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via the SCI 3/ Note that the return is blocked with the message '(The item cannot be returned at a self service terminal, please see library staff for assistance)' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110335 [details] [review] Bug 25261: (QA follow-up) Capitalize return of needsconfirm This patch capitalizes the 'ADDITIONAL_MATERIALS' key in the needsconfirm hash return value for CanBookBeIssued to restore consistency with previous code and ensure SIP Checkout works as expected. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110336 [details] [review] Bug 25261: (QA follow-up) Restore previously returned list Test plan 0 - Enable ShowAllCheckins 1 - Check in several regular items 2 - Check in an item with parts 3 - Confirm the checkin in 4 - The list of checkins contains all the previous checkins and the multi-part checkin too. Created attachment 110337 [details] [review] Bug 25261: (QA follow-up) Terminology: checkin/checkout Created attachment 110338 [details] [review] Bug 25261: (QA follow-up) Prevent double dialogue This patch removes the blue information message box if we are using multi-part confirmations. Created attachment 110339 [details] [review] Bug 25261: (QA follow-up) Make issue and returns messages consistent This patch makes the confirmation dialogue handing and visuality consistent between issues and returns. Created attachment 110340 [details] [review] Bug 25261: (QA follow-up) Catch attepts to return via SCO This patch adds a block to capture attempts to return items with attached materials when checking in via the self checkout when the relevant system preference is set. Created attachment 110341 [details] [review] Bug 25261: (QA follow-up) Prevent server error on bad barcode Prior to this patch if you had CircConfirmParts enabled and you attempted to checkin a deleted item then you would be met with a server error. (In reply to Katrin Fischer from comment #144) > I retested this: > > - Turn on multi part checking and edit 3999900000018 to have $3 > - Check out 3999900000018 in staff > - Go to sco/sco-main.pl > - The checkout shows in the table of checkouts > - The last column is "Renew item" > - Click on it, click "Renew" - page refreshes, all is good > - Apply patches, repeat > - Renew doesn't direct back to the overview page anymore Rebased on master should have fixed this.. you were seeing a manifestation of bug 26301 which was pushed to master over night. Created attachment 110356 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110357 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110358 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110359 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110360 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110361 [details] [review] Bug 25261: (QA follow-up) Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110362 [details] [review] Bug 25261: (QA follow-up) Simplify tests and conditions Use build_sample_item in tests Simplify tests for the confirmation Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110363 [details] [review] Bug 25261: (QA follow-up) Non-confirm on return This patch updates the confirmation from an alert to a dismissable modal which allows for optionally not checking the item in Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110364 [details] [review] Bug 25261: (QA follow-up) Add confirmation to issues table The issues table allows for checkins and was not appropriately requireing confirmation for the multi-part confirmation added in this bug. Test plan 1/ Enable the feature as per previous patches 2/ Checkot an item with attached materials 3/ Navigate to a page that display your users issues table (the checkout page is a reasonable example) 4/ You should have the option to select items for return in the table (If not, use the column settings to enable the feature) 5/ Select at least the item with attached materials to return 6/ Upon clicking the return buttton you should find that items without additional materials are returned as expected, but rows with additional materials turn yellow and contain a message and additional checkbox for confirmation in the table. 7/ Ensure the checkbox is selected and click the return button again 8/ This item should have been returned. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110365 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkout Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via a SIP unit 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110366 [details] [review] Bug 25261: (QA follow-up) Add handling to SCO Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via the SCO 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110367 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkin Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via a SIP unit 3/ Note that the return is blocked with an alert type of '99' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110368 [details] [review] Bug 25261: (QA follow-up) Add handling to SCI Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via the SCI 3/ Note that the return is blocked with the message '(The item cannot be returned at a self service terminal, please see library staff for assistance)' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110369 [details] [review] Bug 25261: (QA follow-up) Capitalize return of needsconfirm This patch capitalizes the 'ADDITIONAL_MATERIALS' key in the needsconfirm hash return value for CanBookBeIssued to restore consistency with previous code and ensure SIP Checkout works as expected. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110370 [details] [review] Bug 25261: (QA follow-up) Restore previously returned list Test plan 0 - Enable ShowAllCheckins 1 - Check in several regular items 2 - Check in an item with parts 3 - Confirm the checkin in 4 - The list of checkins contains all the previous checkins and the multi-part checkin too. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110371 [details] [review] Bug 25261: (QA follow-up) Terminology: checkin/checkout Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110372 [details] [review] Bug 25261: (QA follow-up) Prevent double dialogue This patch removes the blue information message box if we are using multi-part confirmations. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110373 [details] [review] Bug 25261: (QA follow-up) Make issue and returns messages consistent This patch makes the confirmation dialogue handing and visuality consistent between issues and returns. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110374 [details] [review] Bug 25261: (QA follow-up) Catch attepts to return via SCO This patch adds a block to capture attempts to return items with attached materials when checking in via the self checkout when the relevant system preference is set. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110375 [details] [review] Bug 25261: (QA follow-up) Prevent server error on bad barcode Prior to this patch if you had CircConfirmParts enabled and you attempted to checkin a deleted item then you would be met with a server error. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110915 [details] [review] Bug 25261: Add system preference Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110916 [details] [review] Bug 25261: Require confirmation of multiple parts items Test plan 1/ Catalogue an item to contain multiple parts by populating 'Materials specified (bound volume or other part)' 2/ Enable the new system preference 'CircConfirmParts' 3/ Attempt to checkout the item created in step 1 to a user and note that confirmation is now required. 4/ Checkout the item 5/ Attempt to checkin the item you have just checked out and note that confirmation is required. 6/ Signoff Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110917 [details] [review] Bug 25261: Checkout confirmation depending on syspref With this follow-up patch, the intended confirmation of checkin based on CircConfirmParts value is effective. Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/] Sponsored-by: PTFS Europe [https://ptfs-europe.com/] Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110918 [details] [review] Bug 25261: (follow-up) Rename CircConfirmParts to CircConfirmItemParts This clarifies the preference name to make it clear we are talking about the 'parts' that make up an 'item'. 'Part' is a well known term in british english libraries and I think perhaps 'Materials' may be confused with other terms? Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110919 [details] [review] Bug 25261: (follow-up) item.material may contain non-numerics Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field, and thus the item.materials field, may contain arbitrary notes about the material rather than just numeric values. As such we need to check for the field being defined as aposed to greater than '0'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110920 [details] [review] Bug 25261: (QA follow-up) Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110921 [details] [review] Bug 25261: (QA follow-up) Simplify tests and conditions Use build_sample_item in tests Simplify tests for the confirmation Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110922 [details] [review] Bug 25261: (QA follow-up) Non-confirm on return This patch updates the confirmation from an alert to a dismissable modal which allows for optionally not checking the item in Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110923 [details] [review] Bug 25261: (QA follow-up) Add confirmation to issues table The issues table allows for checkins and was not appropriately requireing confirmation for the multi-part confirmation added in this bug. Test plan 1/ Enable the feature as per previous patches 2/ Checkot an item with attached materials 3/ Navigate to a page that display your users issues table (the checkout page is a reasonable example) 4/ You should have the option to select items for return in the table (If not, use the column settings to enable the feature) 5/ Select at least the item with attached materials to return 6/ Upon clicking the return buttton you should find that items without additional materials are returned as expected, but rows with additional materials turn yellow and contain a message and additional checkbox for confirmation in the table. 7/ Ensure the checkbox is selected and click the return button again 8/ This item should have been returned. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110924 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkout Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via a SIP unit 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110925 [details] [review] Bug 25261: (QA follow-up) Add handling to SCO Test plan 1/ Enable the feature as per previous patches 2/ Attempt to checkout an item with materials attached via the SCO 3/ Note that the checkut is blocked with the message 'Item must be issued at a circulation desk' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110926 [details] [review] Bug 25261: (QA follow-up) Catch check in SIP Checkin Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via a SIP unit 3/ Note that the return is blocked with an alert type of '99' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110927 [details] [review] Bug 25261: (QA follow-up) Add handling to SCI Test plan 1/ Enable the feature as per previous patches 2/ Attempt to return an item with materials attached via the SCI 3/ Note that the return is blocked with the message '(The item cannot be returned at a self service terminal, please see library staff for assistance)' 4/ Signoff Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110928 [details] [review] Bug 25261: (QA follow-up) Capitalize return of needsconfirm This patch capitalizes the 'ADDITIONAL_MATERIALS' key in the needsconfirm hash return value for CanBookBeIssued to restore consistency with previous code and ensure SIP Checkout works as expected. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110929 [details] [review] Bug 25261: (QA follow-up) Restore previously returned list Test plan 0 - Enable ShowAllCheckins 1 - Check in several regular items 2 - Check in an item with parts 3 - Confirm the checkin in 4 - The list of checkins contains all the previous checkins and the multi-part checkin too. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110930 [details] [review] Bug 25261: (QA follow-up) Terminology: checkin/checkout Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110931 [details] [review] Bug 25261: (QA follow-up) Prevent double dialogue This patch removes the blue information message box if we are using multi-part confirmations. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110932 [details] [review] Bug 25261: (QA follow-up) Make issue and returns messages consistent This patch makes the confirmation dialogue handing and visuality consistent between issues and returns. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110933 [details] [review] Bug 25261: (QA follow-up) Catch attepts to return via SCO This patch adds a block to capture attempts to return items with attached materials when checking in via the self checkout when the relevant system preference is set. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110934 [details] [review] Bug 25261: (QA follow-up) Prevent server error on bad barcode Prior to this patch if you had CircConfirmParts enabled and you attempted to checkin a deleted item then you would be met with a server error. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Minor rebase for conflict caused by bug 25321. Created attachment 111027 [details] [review] Bug 25261: Prevent tests to fail if CircConfirmItemParts is on Setting items.materials to NULL at TestBuilder level will (certainly) prevent some tests to fail if the pref is on In t/db_dependent/selenium/basic_workflow.t we set 952$3 to an empty string. The tests will pass even if the pref is turned on. Pushed to master for 20.11, thanks to everybody involved! enhancement will not be backported to 20.05.x Bug 25261: (QA follow-up) Add confirmation to issues table I don't understand this patch. 1. The JS code is added in the "renew" function but we are talking about checkin 2. Why don't we simply don't remove the checkbox to prevent checkin (like we do for item on hold)? 3. The "Confirm" will no longer be displayed because of the table refresh, see bug 28653 and bug 36131. |
