Bug 7047 - Renewing serials note not visible
Summary: Renewing serials note not visible
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Alex Buckley
QA Contact: Bugs List
URL:
Keywords:
: 16688 (view as bug list)
Depends on: 24244
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-17 18:01 UTC by Nicole C. Engard
Modified: 2020-01-12 19:32 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00


Attachments
Bug 7047 - Storing subscription renewal note in suggestion to be displayed on the suggestion page. (9.99 KB, patch)
2017-12-04 00:59 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 7047 - Storing subscription renewal note in suggestion to be displayed on the suggestion page. (9.88 KB, patch)
2018-02-02 22:22 UTC, Maksim Sen
Details | Diff | Splinter Review
Bug 7047 - Storing subscription renewal note in suggestion to be displayed on the suggestion page. (10.08 KB, patch)
2018-03-12 10:37 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 7047: Storing subscription renewal note in suggestion to be displayed on the suggestion page. (9.58 KB, patch)
2019-12-16 11:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7047: Change ReNewSubscription prototype - use hashref (4.50 KB, patch)
2019-12-16 11:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7047: Revert changes relative to the new column on suggestion list (3.95 KB, patch)
2019-12-16 11:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7047: Storing subscription renewal note in suggestion to be displayed on the suggestion page. (9.64 KB, patch)
2019-12-16 11:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 7047: Change ReNewSubscription prototype - use hashref (4.55 KB, patch)
2019-12-16 11:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 7047: Revert changes relative to the new column on suggestion list (4.01 KB, patch)
2019-12-16 11:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 7047: (QA follow-up) Add missing filters (1.12 KB, patch)
2019-12-16 11:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 7047: (QA follow-up) Fix tests (1.70 KB, patch)
2019-12-16 11:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 7047: Storing subscription renewal note in suggestion to be displayed on the suggestion page. (9.70 KB, patch)
2019-12-22 21:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7047: Change ReNewSubscription prototype - use hashref (4.61 KB, patch)
2019-12-22 21:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7047: Revert changes relative to the new column on suggestion list (4.06 KB, patch)
2019-12-22 21:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7047: (QA follow-up) Add missing filters (1.17 KB, patch)
2019-12-22 21:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7047: (QA follow-up) Fix tests (1.75 KB, patch)
2019-12-22 21:55 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2011-10-17 18:01:39 UTC
There is a field on the serial renew form that says "Note for the librarian that will manage your renewal request: " but it doesn't display anywhere - and what does it mean anyway? When you renew it renews your subscription automatically and doesn't go in to review.
Comment 1 Katrin Fischer 2011-10-18 07:23:44 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 :)
Comment 2 Nicole C. Engard 2011-10-18 12:38:14 UTC
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.
Comment 3 Alex Buckley 2017-12-04 00:59:10 UTC
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 4 M. Tompsett 2018-01-31 00:05:08 UTC
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.
Comment 5 Maksim Sen 2018-02-02 22:22:45 UTC
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>
Comment 6 sandboxes@biblibre.com 2018-03-12 10:37:12 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 7 sandboxes@biblibre.com 2018-03-12 10:37:35 UTC
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>
Comment 8 Nick Clemens 2018-03-30 15:10:36 UTC
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
Comment 9 Jonathan Druart 2019-12-16 11:03:30 UTC
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>
Comment 10 Jonathan Druart 2019-12-16 11:03:34 UTC
Created attachment 96310 [details] [review]
Bug 7047: Change ReNewSubscription prototype - use hashref

It also removes a warn statement.
Comment 11 Jonathan Druart 2019-12-16 11:03:38 UTC
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.
Comment 12 Jonathan Druart 2019-12-16 11:08:22 UTC
*** Bug 16688 has been marked as a duplicate of this bug. ***
Comment 13 Jonathan Druart 2019-12-16 11:09:01 UTC
(In reply to Jonathan Druart from comment #12)
> *** Bug 16688 has been marked as a duplicate of this bug. ***

Adjusting severity, setting to major.
Comment 14 Nick Clemens 2019-12-16 11:47:12 UTC
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>
Comment 15 Nick Clemens 2019-12-16 11:47:15 UTC
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>
Comment 16 Nick Clemens 2019-12-16 11:47:18 UTC
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>
Comment 17 Nick Clemens 2019-12-16 11:47:21 UTC
Created attachment 96320 [details] [review]
Bug 7047: (QA follow-up) Add missing filters

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Nick Clemens 2019-12-16 11:47:25 UTC
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>
Comment 19 Katrin Fischer 2019-12-22 21:53:33 UTC
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.
Comment 20 Katrin Fischer 2019-12-22 21:54:47 UTC
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>
Comment 21 Katrin Fischer 2019-12-22 21:54:51 UTC
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>
Comment 22 Katrin Fischer 2019-12-22 21:54:56 UTC
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>
Comment 23 Katrin Fischer 2019-12-22 21:55:00 UTC
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>
Comment 24 Katrin Fischer 2019-12-22 21:55:04 UTC
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>
Comment 25 Martin Renvoize 2019-12-23 11:14:20 UTC
(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?
Comment 26 Martin Renvoize 2019-12-23 12:09:17 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 27 Katrin Fischer 2019-12-31 09:52:16 UTC
(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"
Comment 28 Joy Nelson 2020-01-12 19:32:13 UTC
Does not apply to 19.11.x branch.  Not backported.