Bug 20461 - New subscription form: "Item type" and "item type for older issues" fields are ignored
Summary: New subscription form: "Item type" and "item type for older issues" fields ar...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Victor Grousset/tuxayo
QA Contact: Testopia
URL:
Keywords:
Depends on: 7677
Blocks: 21264
  Show dependency treegraph
 
Reported: 2018-03-22 14:38 UTC by Victor Grousset/tuxayo
Modified: 2020-01-06 20:14 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20461 : Serials: fix ignored params in subscription creation form (1.52 KB, patch)
2018-03-22 15:06 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 20461 : Serials: fix ignored params in subscription creation form (1.57 KB, patch)
2018-03-23 10:12 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 20461 : Serials: fix ignored params in subscription creation form (1.72 KB, patch)
2018-03-23 11:36 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 20461 : Serials: fix ignored params in subscription creation form (1.79 KB, patch)
2018-03-23 15:04 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Grousset/tuxayo 2018-03-22 14:38:52 UTC
Impacts also 16.11.x

== Cause ==
In subscription-add.pl, the two params aren't passed to NewSubscription() but they are to ModSubscription()

== Workaround ==
Modify the newly created subscription to set these two values.
Comment 1 Victor Grousset/tuxayo 2018-03-22 15:06:42 UTC
Created attachment 73157 [details] [review]
Bug 20461 : Serials: fix ignored params in subscription creation form

In subscription-add.pl, the two params aren't passed to
NewSubscription() but they are to ModSubscription()

== Test plan ==
1. Enable syspref "makePreviousSerialAvailable"
2. Create a minimal subscription with a value in the "Item type" and
     "item type for older issues" fields.
3. Edit the subscription
4. You should see that the two fields are empty. This is the bug.
5. Apply this patch
6. Do step 2 and 3
7. You should see that the two fields have the right value
8. Express the joy of a successful and easy sign off.
   (this is important, otherwise the sign off spell won't work!)
Comment 2 Victor Grousset/tuxayo 2018-03-22 15:42:28 UTC
BTW, not other field are missing between the new and modify functions.
Comment 3 Victor Grousset/tuxayo 2018-03-22 15:42:52 UTC
*no other fields
Comment 4 Biblibre Sandboxes 2018-03-23 10:12:11 UTC
Patch tested with a sandbox, by delaye <stephane.delaye@biblibre.com>
Comment 5 Biblibre Sandboxes 2018-03-23 10:12:31 UTC
Created attachment 73168 [details] [review]
Bug 20461 : Serials: fix ignored params in subscription creation form

In subscription-add.pl, the two params aren't passed to
NewSubscription() but they are to ModSubscription()

== Test plan ==
1. Enable syspref "makePreviousSerialAvailable"
2. Create a minimal subscription with a value in the "Item type" and
     "item type for older issues" fields.
3. Edit the subscription
4. You should see that the two fields are empty. This is the bug.
5. Apply this patch
6. Do step 2 and 3
7. You should see that the two fields have the right value
8. Express the joy of a successful and easy sign off.
   (this is important, otherwise the sign off spell won't work!)

Signed-off-by: delaye <stephane.delaye@biblibre.com>
Comment 6 Victor Grousset/tuxayo 2018-03-23 11:11:56 UTC
Needs another sign off from someone not at BibLibre (current community rules)
Comment 7 Séverine Queune 2018-03-23 11:36:28 UTC
Created attachment 73174 [details] [review]
Bug 20461 : Serials: fix ignored params in subscription creation form

In subscription-add.pl, the two params aren't passed to
NewSubscription() but they are to ModSubscription()

== Test plan ==
1. Enable syspref "makePreviousSerialAvailable"
2. Create a minimal subscription with a value in the "Item type" and
     "item type for older issues" fields.
3. Edit the subscription
4. You should see that the two fields are empty. This is the bug.
5. Apply this patch
6. Do step 2 and 3
7. You should see that the two fields have the right value
8. Express the joy of a successful and easy sign off.
   (this is important, otherwise the sign off spell won't work!)

Signed-off-by: delaye <stephane.delaye@biblibre.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 8 Kyle M Hall 2018-03-23 15:04:25 UTC
Created attachment 73188 [details] [review]
Bug 20461 : Serials: fix ignored params in subscription creation form

In subscription-add.pl, the two params aren't passed to
NewSubscription() but they are to ModSubscription()

== Test plan ==
1. Enable syspref "makePreviousSerialAvailable"
2. Create a minimal subscription with a value in the "Item type" and
     "item type for older issues" fields.
3. Edit the subscription
4. You should see that the two fields are empty. This is the bug.
5. Apply this patch
6. Do step 2 and 3
7. You should see that the two fields have the right value
8. Express the joy of a successful and easy sign off.
   (this is important, otherwise the sign off spell won't work!)

Signed-off-by: delaye <stephane.delaye@biblibre.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Jonathan Druart 2018-03-23 15:09:36 UTC
Caused by:
  commit 99c25135599c99bb1f1c83d390eca4628a26f4cf
  BZ7677: New areas in subscriptions and new functions when receiving.
Comment 10 Jonathan Druart 2018-03-26 20:39:07 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 11 Nick Clemens 2018-04-24 01:35:24 UTC
Awesome work all, backported to stable for 17.11.05
Comment 12 Fridolin Somers 2018-04-24 15:11:04 UTC
Pushed to 17.05.x for 17.05.11
Comment 13 Victor Grousset/tuxayo 2018-08-23 12:34:39 UTC
== Going further: handling corrupted data ==
This leaves corrupted data that can break at least "transferstoreceive.pl" (circulation → Transfers to receive)

For reference, the error message is:
> item-level_itypes set but no itemtype set for item (12345) at /home/koha/src/Koha/Schema/Result/Item.pm line 698.
> Can't call method "description" on an undefined value at /home/koha/src/circ/transferstoreceive.pl line 99.

=== Manual fix ===
==== Subcriptions ====
So the problem is that subscriptions created before the fix have "Item type" and "item type for older issues" set to NULL.
This (subscriptions) is data creating corrupted data (items).
So one must fix them (get the subscriptionid list from the SQL bellow) one by one to set the correct types.
Or with an SQL query if for all the subscriptions you put the same "Item type" and "item type for older issues"

==== Items ====
One must also fix them (get itemnumber list from SQL bellow) one by one to set the correct types.
Or with batch item modification, for all the items you put the same "Item type"
But if you have a different item type for the last issue, them your dreams of mass fixing the issue will likely be crushed :( (hopefully there are still way depending on the cases)

=== SQL queries to find corrupted data ===
When your fixes are done, the following SQL queries should return nothing.

select itemnumber from items where itype is null;
subscriptionid FROM subscription WHERE previousitemtype is null or itemtype is null;
Comment 14 Victor Grousset/tuxayo 2018-08-23 14:12:18 UTC
Erratum:
missing SELECT at the beginning the last query:
select subscriptionid FROM subscription WHERE previousitemtype is null or itemtype is null;
Comment 15 Victor Grousset/tuxayo 2019-02-15 16:57:08 UTC
Just to be sure, if at the creation of the subscription, the previousitemtype is forgotten. Then invalid item will still be created right?

In that case (I'm not sure to have re-understood everything), is it relevant to:

Do a follow-up bugzilla with a patch that prevent from changing an item's itype to "" or NULL. That it to say, if previousitemtype is invalid, don't use it.
That would also deal will add the existing subscriptions that are still creating invalid items even after this patch.