Bug 25311 - Better error handling when creating/updating a patron
Summary: Better error handling when creating/updating a patron
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 25309
  Show dependency treegraph
 
Reported: 2020-04-29 12:38 UTC by Jonathan Druart
Modified: 2021-12-13 21:08 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06
Circulation function:


Attachments
Bug 25311: Better error handling when creating a patron (6.55 KB, patch)
2020-04-29 12:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25311: Better error handling when updating a patron (7.91 KB, patch)
2020-04-29 12:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25311: Better error handling when creating a patron (6.62 KB, patch)
2020-04-29 13:33 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 25311: Better error handling when updating a patron (7.98 KB, patch)
2020-04-29 13:33 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 25311: Better error handling when updating a patron (8.04 KB, patch)
2020-05-01 22:25 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25311: (QA follow-up) Add misssing filter (1.28 KB, patch)
2020-05-01 22:25 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25311: Better error handling when creating a patron (6.69 KB, patch)
2020-05-01 22:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25311: Better error handling when updating a patron (8.04 KB, patch)
2020-05-01 22:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25311: (QA follow-up) Add misssing filter (1.28 KB, patch)
2020-05-01 22:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
[19.11.x] Bug 25311: Better error handling when creating a patron (7.19 KB, patch)
2020-05-12 08:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
[19.11.x] Bug 25311: Better error handling when updating a patron (7.95 KB, patch)
2020-05-12 08:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
[19.11.x] Bug 25311: (QA follow-up) Add misssing filter (1.24 KB, patch)
2020-05-12 08:48 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 Jonathan Druart 2020-04-29 12:38:33 UTC
When a new patron is created from the staff interface, the error is not correctly handled and the real cause of the issue is hidden in the logs.
Comment 1 Jonathan Druart 2020-04-29 12:49:53 UTC
Created attachment 103930 [details] [review]
Bug 25311: Better error handling when creating a patron

This is still not ideal but brings a bit of enhancement.

One possible problem is that the patron creation will fail if the
streetnumber field is too long (borrowers.streetnumber is varchar(10).

Test plan:
0. Don't apply this patch
1. Create a new patron with a streetnumber longer than 10 characters
2. Save
=> The patron has not been created and the app explodes
The error is about extended_attributes and not meaningful
Can't call method "extended_attributes" on an undefined value at /kohadevbox/koha/members/memberentry.pl line 560
3. Apply the patch
4. Repeat 1. and 2
=> You get a warning on the interface and you still see the creation
form
5. Check the logs
=> The error is meaningful
"Data too long for column 'streetnumber'"
Comment 2 Jonathan Druart 2020-04-29 12:49:57 UTC
Created attachment 103931 [details] [review]
Bug 25311: Better error handling when updating a patron

Same as the precedent patch for patron's modification

Test plan is identical but with an existing patron
Comment 3 Martin Renvoize (ashimema) 2020-04-29 13:27:14 UTC
Code looks good.. just running through testing it.
Comment 4 Martin Renvoize (ashimema) 2020-04-29 13:33:24 UTC
Created attachment 103939 [details] [review]
Bug 25311: Better error handling when creating a patron

This is still not ideal but brings a bit of enhancement.

One possible problem is that the patron creation will fail if the
streetnumber field is too long (borrowers.streetnumber is varchar(10).

Test plan:
0. Don't apply this patch
1. Create a new patron with a streetnumber longer than 10 characters
2. Save
=> The patron has not been created and the app explodes
The error is about extended_attributes and not meaningful
Can't call method "extended_attributes" on an undefined value at /kohadevbox/koha/members/memberentry.pl line 560
3. Apply the patch
4. Repeat 1. and 2
=> You get a warning on the interface and you still see the creation
form
5. Check the logs
=> The error is meaningful
"Data too long for column 'streetnumber'"

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize (ashimema) 2020-04-29 13:33:27 UTC
Created attachment 103940 [details] [review]
Bug 25311: Better error handling when updating a patron

Same as the precedent patch for patron's modification

Test plan is identical but with an existing patron

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize (ashimema) 2020-04-29 13:34:44 UTC
Works well for me following the test plan and is a clear improvement.

I also tested housebound modifications, patron messaging preference modifications too.

Signing off.
Comment 7 Katrin Fischer 2020-05-01 22:09:25 UTC
This doesn't work like described in the test plan for me. When I create or update a patron with a streetnumber longer than 10 charactres, it saves nicely, but the entry is cut off at 10 characters.

Am I missing something?
Comment 8 Katrin Fischer 2020-05-01 22:23:32 UTC
Ok, the message only shows when <strict_sql_modes>1</strict_sql_modes> is set.

I don't like the silent failure, but I think that's because the DBMS is silently being mean. Need to fix that elsewhere.
Comment 9 Katrin Fischer 2020-05-01 22:25:00 UTC
Created attachment 104154 [details] [review]
Bug 25311: Better error handling when updating a patron

Same as the precedent patch for patron's modification

Test plan is identical but with an existing patron

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2020-05-01 22:25:04 UTC
Created attachment 104155 [details] [review]
Bug 25311: (QA follow-up) Add misssing filter

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Katrin Fischer 2020-05-01 22:27:23 UTC
Created attachment 104156 [details] [review]
Bug 25311: Better error handling when creating a patron

This is still not ideal but brings a bit of enhancement.

One possible problem is that the patron creation will fail if the
streetnumber field is too long (borrowers.streetnumber is varchar(10).

Test plan:
0. Don't apply this patch
1. Create a new patron with a streetnumber longer than 10 characters
2. Save
=> The patron has not been created and the app explodes
The error is about extended_attributes and not meaningful
Can't call method "extended_attributes" on an undefined value at /kohadevbox/koha/members/memberentry.pl line 560
3. Apply the patch
4. Repeat 1. and 2
=> You get a warning on the interface and you still see the creation
form
5. Check the logs
=> The error is meaningful
"Data too long for column 'streetnumber'"

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2020-05-01 22:27:26 UTC
Created attachment 104157 [details] [review]
Bug 25311: Better error handling when updating a patron

Same as the precedent patch for patron's modification

Test plan is identical but with an existing patron

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2020-05-01 22:27:31 UTC
Created attachment 104158 [details] [review]
Bug 25311: (QA follow-up) Add misssing filter

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Martin Renvoize (ashimema) 2020-05-04 07:42:29 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 15 Joy Nelson 2020-05-11 16:32:37 UTC
the recent devs on memberentrygen.tt has made this too difficult for me to rebase to 19.11.x  
Can this be rebased to 19.11.x?

Thanks
Comment 16 Jonathan Druart 2020-05-12 08:48:27 UTC
Created attachment 104743 [details] [review]
[19.11.x] Bug 25311: Better error handling when creating a patron

This is still not ideal but brings a bit of enhancement.

One possible problem is that the patron creation will fail if the
streetnumber field is too long (borrowers.streetnumber is varchar(10).

Test plan:
0. Don't apply this patch
1. Create a new patron with a streetnumber longer than 10 characters
2. Save
=> The patron has not been created and the app explodes
The error is about extended_attributes and not meaningful
Can't call method "extended_attributes" on an undefined value at /kohadevbox/koha/members/memberentry.pl line 560
3. Apply the patch
4. Repeat 1. and 2
=> You get a warning on the interface and you still see the creation
form
5. Check the logs
=> The error is meaningful
"Data too long for column 'streetnumber'"

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Jonathan Druart 2020-05-12 08:48:30 UTC
Created attachment 104744 [details] [review]
[19.11.x] Bug 25311: Better error handling when updating a patron

Same as the precedent patch for patron's modification

Test plan is identical but with an existing patron

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Jonathan Druart 2020-05-12 08:48:33 UTC
Created attachment 104745 [details] [review]
[19.11.x] Bug 25311: (QA follow-up) Add misssing filter

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 19 Joy Nelson 2020-05-19 23:53:51 UTC
Backported to 19.11.x for 19.11.06
Comment 20 Victor Grousset/tuxayo 2020-06-03 15:52:18 UTC
It doesn't apply cleanly on 19.05.x and the conflict isn't simple to solve.
If there is an interest in having this backported, feel free to submit a patch for 19.05.