Bug 31297 - Cannot add new subscription patterns from edit subscription page
Summary: Cannot add new subscription patterns from edit subscription page
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 17258
  Show dependency treegraph
 
Reported: 2022-08-05 13:16 UTC by Katrin Fischer
Modified: 2022-10-16 21:11 UTC (History)
3 users (show)

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


Attachments
Bug 31297: Fix creation of new subscription pattern from edit subscription (1.54 KB, patch)
2022-08-09 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2022-08-05 13:16:46 UTC
When adding a new subscription pattern from the edit subscription page, nothing happens when clicking on the bug. But we can see the following error from the API request:

XHRGEThttp://localhost:8081/cgi-bin/koha/serials/create-numberpattern.pl?patternname=Test season with month&numberingmethod={X} {Y} {Z}&label1=Season&label2=Year&label3=Month&add1=1&add2=1&add3=1&every1=1&every2=4&every3=4&setto1=0&setto2=1&setto3=0&numbering1=season&numbering3=monthname&whenmorethan1=3&whenmorethan2=99999&whenmorethan3=11&locale=spa
[HTTP/1.1 500 Internal Server Error 27ms]

C4::Serials::Numberpattern::AddSubscriptionNumberpattern(): DBI Exception: DBD::mysql::st execute failed: Field 'description' doesn't have a default value at /kohadevbox/koha/serials/create-numberpattern.pl line 58

description is a field in the database, that can only be filled from the administration page for numbering patterns, but not from this screen. This has always been the case.

Could this by a strict SQL error?
Comment 1 Jonathan Druart 2022-08-09 10:27:26 UTC
Looks like a duplicate of bug 28012.
Comment 2 Jonathan Druart 2022-08-09 10:29:07 UTC
(In reply to Jonathan Druart from comment #1)
> Looks like a duplicate of bug 28012.

Hum not really, related but not exactly the same.
Comment 3 Jonathan Druart 2022-08-09 10:35:22 UTC
Created attachment 138892 [details] [review]
Bug 31297: Fix creation of new subscription pattern from edit subscription

When editing a subscription there is the possibility to create a new
subscription pattern. But this form does not have a "description" for
this pattern, and the DBMS does not allow description to be NULLed.

Error is: DBI Exception: DBD::mysql::st execute failed: Field 'description' doesn't have a default value

This patch takes the shortest path to fix the 500 error (with no
feedback sent to the end user), but there are other better ways to fix
it:
1. Allow subscription_numberpatterns.description to be NULL, or
2. Add a description field on the form

Test plan:
Create a new subscription, on the second page click "show advanced
pattern", then "Modify pattern", enter a name and click "Save as new pattern"
Comment 4 Joonas Kylmälä 2022-09-03 11:39:48 UTC
Please let's go directly with the proper solution, number 1 or 2 that Jonathan suggested? Reviewing, merging and reverting this and then going for solution 1 or 2 is extra work.
Comment 5 Joonas Kylmälä 2022-09-03 11:41:13 UTC
(In reply to Joonas Kylmälä from comment #4)
> Please let's go directly with the proper solution, number 1 or 2 that
> Jonathan suggested? Reviewing, merging and reverting this and then going for
> solution 1 or 2 is extra work.

I personally prefer number 1, the user might not care about the description at all so having to fill fields that are not useful is not something we should force the users to do.
Comment 6 Katrin Fischer 2022-09-07 10:31:17 UTC
It's definitely 1)

1. Allow subscription_numberpatterns.description to be NULL

The description and hasn't even been shown in the past, so we should not make it mandatory suddenly. It could be nice to have on the form, but as an optional field.

Please keep in mind that we will probably want to backport (and that Jonathan is gone right now)