|
Description
Nicole C. Engard
2011-10-17 18:01:39 UTC
Hi Nicole, there is a system preference so that you can make renewals be added as suggestions in acquisitions - I think the note should appear there somewhere. I have not tested this workflow in while. I think last time I worked with it was before the new acq module got merged. But perhaps this explains a bit :) Okay I tested that preference and it still renews automatically and the note does not make it in to the suggestion ... so the data in this box (the notes box) is just lost as far as I can see. Created attachment 69474 [details] [review] Bug 7047 - Storing subscription renewal note in suggestion to be displayed on the suggestion page. Additionally I have added a library input field in case the librarian wants to set a library branch whilst renewing a subscription. With the use case being they may have ommitted to set the branchcode whilst creating the subscription. Test plan: 1. Create a subscription (if one does not already exist) 2. Set the RenewSerialAddsSuggestion syspref to 'Add' 3. Renew the item making sure to write in a value into the note field 3. Visit the suggestions page and notice that the note is not displayed for the newly created suggestion 4. Apply patch 5. Repeat step 3. Notice that there is now a new branchcode dropdown box. Select one of your libraries and write in the value into the note field 6. Visit suggestions and notice there is now a 'Suggestion note' column in the table containing the note. Also note that the suggestion has the correct branchcode associated with it Sponsored-By: Catalyst IT Comment on attachment 69474 [details] [review] Bug 7047 - Storing subscription renewal note in suggestion to be displayed on the suggestion page. Review of attachment 69474 [details] [review]: ----------------------------------------------------------------- ::: C4/Serials.pm @@ +1504,4 @@ > =cut > > sub ReNewSubscription { > + my ( $subscriptionid, $user, $startdate, $numberlength, $weeklength, $monthlength, $note, $branchcode ) = @_; This parameter list is huge. Rather than add another parameter, I would recommend converting it to a hashref. https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL16:_Hashrefs_should_be_used_as_arguments Thankfully, it is a very small refactor (git grep ReNewSubscription), and is much easier to maintain or expand in the future. Created attachment 71168 [details] [review] Bug 7047 - Storing subscription renewal note in suggestion to be displayed on the suggestion page. Additionally I have added a library input field in case the librarian wants to set a library branch whilst renewing a subscription. With the use case being they may have ommitted to set the branchcode whilst creating the subscription. Test plan: 1. Create a subscription (if one does not already exist) 2. Set the RenewSerialAddsSuggestion syspref to 'Add' 3. Renew the item making sure to write in a value into the note field 3. Visit the suggestions page and notice that the note is not displayed for the newly created suggestion 4. Apply patch 5. Repeat step 3. Notice that there is now a new branchcode dropdown box. Select one of your libraries and write in the value into the note field 6. Visit suggestions and notice there is now a 'Suggestion note' column in the table containing the note. Also note that the suggestion has the correct branchcode associated with it Sponsored-By: Catalyst IT Signed-off-by: Maksim Sen <maksim@inlibro.com> Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr> Created attachment 72639 [details] [review] Bug 7047 - Storing subscription renewal note in suggestion to be displayed on the suggestion page. Additionally I have added a library input field in case the librarian wants to set a library branch whilst renewing a subscription. With the use case being they may have ommitted to set the branchcode whilst creating the subscription. Test plan: 1. Create a subscription (if one does not already exist) 2. Set the RenewSerialAddsSuggestion syspref to 'Add' 3. Renew the item making sure to write in a value into the note field 3. Visit the suggestions page and notice that the note is not displayed for the newly created suggestion 4. Apply patch 5. Repeat step 3. Notice that there is now a new branchcode dropdown box. Select one of your libraries and write in the value into the note field 6. Visit suggestions and notice there is now a 'Suggestion note' column in the table containing the note. Also note that the suggestion has the correct branchcode associated with it Sponsored-By: Catalyst IT Signed-off-by: Maksim Sen <maksim@inlibro.com> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Hi Alex, Couple things here: 1 - There is a stray warn left in the patchset 2 - I would tend to agree with Mark, the subroutine should be converted to a hashref 3 - This should probably be two bugs - one to fix the note, one to add the branch option 4 - The span is lost when creating the new column, we should preserve this to retain any CSS that may affect display Created attachment 96309 [details] [review] Bug 7047: Storing subscription renewal note in suggestion to be displayed on the suggestion page. Additionally I have added a library input field in case the librarian wants to set a library branch whilst renewing a subscription. With the use case being they may have ommitted to set the branchcode whilst creating the subscription. Test plan: 1. Create a subscription (if one does not already exist) 2. Set the RenewSerialAddsSuggestion syspref to 'Add' 3. Renew the item making sure to write in a value into the note field 3. Visit the suggestions page and notice that the note is not displayed for the newly created suggestion 4. Apply patch 5. Repeat step 3. Notice that there is now a new branchcode dropdown box. Select one of your libraries and write in the value into the note field 6. Visit suggestions and notice there is now a 'Suggestion note' column in the table containing the note. Also note that the suggestion has the correct branchcode associated with it Sponsored-By: Catalyst IT Signed-off-by: Maksim Sen <maksim@inlibro.com> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Created attachment 96310 [details] [review] Bug 7047: Change ReNewSubscription prototype - use hashref It also removes a warn statement. Created attachment 96311 [details] [review] Bug 7047: Revert changes relative to the new column on suggestion list We should move that to its own bug report if needed, not sure we will all agree on that new column. *** Bug 16688 has been marked as a duplicate of this bug. *** (In reply to Jonathan Druart from comment #12) > *** Bug 16688 has been marked as a duplicate of this bug. *** Adjusting severity, setting to major. Created attachment 96317 [details] [review] Bug 7047: Storing subscription renewal note in suggestion to be displayed on the suggestion page. Additionally I have added a library input field in case the librarian wants to set a library branch whilst renewing a subscription. With the use case being they may have ommitted to set the branchcode whilst creating the subscription. Test plan: 1. Create a subscription (if one does not already exist) 2. Set the RenewSerialAddsSuggestion syspref to 'Add' 3. Renew the item making sure to write in a value into the note field 3. Visit the suggestions page and notice that the note is not displayed for the newly created suggestion 4. Apply patch 5. Repeat step 3. Notice that there is now a new branchcode dropdown box. Select one of your libraries and write in the value into the note field 6. Visit suggestions and notice there is now a 'Suggestion note' column in the table containing the note. Also note that the suggestion has the correct branchcode associated with it Sponsored-By: Catalyst IT Signed-off-by: Maksim Sen <maksim@inlibro.com> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 96318 [details] [review] Bug 7047: Change ReNewSubscription prototype - use hashref It also removes a warn statement. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 96319 [details] [review] Bug 7047: Revert changes relative to the new column on suggestion list We should move that to its own bug report if needed, not sure we will all agree on that new column. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 96320 [details] [review] Bug 7047: (QA follow-up) Add missing filters Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 96321 [details] [review] Bug 7047: (QA follow-up) Fix tests Tests fail on biblionumber constraint for suggestions if 'RenewSerialAddsSuggestion' is enabled Signed-off-by: Nick Clemens <nick@bywatersolutions.com> This works as described - thx! But I think the ordering process needs more work in another bug. As it stands I am not sure the feature itself is very useful. Still, this is a start! - The suggestion is not linked to the existing record, a new record is created during the ordering process. - The order is not "from a subscription" - which means it won't show up in the serials module. Created attachment 96585 [details] [review] Bug 7047: Storing subscription renewal note in suggestion to be displayed on the suggestion page. Additionally I have added a library input field in case the librarian wants to set a library branch whilst renewing a subscription. With the use case being they may have ommitted to set the branchcode whilst creating the subscription. Test plan: 1. Create a subscription (if one does not already exist) 2. Set the RenewSerialAddsSuggestion syspref to 'Add' 3. Renew the item making sure to write in a value into the note field 3. Visit the suggestions page and notice that the note is not displayed for the newly created suggestion 4. Apply patch 5. Repeat step 3. Notice that there is now a new branchcode dropdown box. Select one of your libraries and write in the value into the note field 6. Visit suggestions and notice there is now a 'Suggestion note' column in the table containing the note. Also note that the suggestion has the correct branchcode associated with it Sponsored-By: Catalyst IT Signed-off-by: Maksim Sen <maksim@inlibro.com> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 96586 [details] [review] Bug 7047: Change ReNewSubscription prototype - use hashref It also removes a warn statement. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 96587 [details] [review] Bug 7047: Revert changes relative to the new column on suggestion list We should move that to its own bug report if needed, not sure we will all agree on that new column. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 96588 [details] [review] Bug 7047: (QA follow-up) Add missing filters Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 96589 [details] [review] Bug 7047: (QA follow-up) Fix tests Tests fail on biblionumber constraint for suggestions if 'RenewSerialAddsSuggestion' is enabled Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> (In reply to Katrin Fischer from comment #19) > This works as described - thx! > > But I think the ordering process needs more work in another bug. As it > stands I am not sure the feature itself is very useful. Still, this is a > start! > > - The suggestion is not linked to the existing record, a new record is > created during the ordering process. > - The order is not "from a subscription" - which means it won't show up in > the serials module. Can you report these as distinct bugs before I push so we don't loose them? Nice work everyone! Pushed to master for 20.05 (In reply to Martin Renvoize from comment #25) > (In reply to Katrin Fischer from comment #19) > > This works as described - thx! > > > > But I think the ordering process needs more work in another bug. As it > > stands I am not sure the feature itself is very useful. Still, this is a > > start! > > > > - The suggestion is not linked to the existing record, a new record is > > created during the ordering process. > > - The order is not "from a subscription" - which means it won't show up in > > the serials module. > > Can you report these as distinct bugs before I push so we don't loose them? Reported: Bug 24319 - When RenewSerialAddsSuggestion is used, use existing record when ordering from suggestion Bug 24318 - When RenewSerialAddsSuggestion is used, treat suggestion as "order from subscription" Does not apply to 19.11.x branch. Not backported. |