Bug 26558 - Guarantor information is lost when an error occurs during new account creation
Summary: Guarantor information is lost when an error occurs during new account creation
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Emmi Takkinen
QA Contact: Aleisha Amohia
URL:
Keywords:
: 34750 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-09-28 22:26 UTC by George Williams (NEKLS)
Modified: 2024-07-04 20:37 UTC (History)
9 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:
23.11.00,23.05.05
Circulation function:


Attachments
Bug 26558: Make guarantor information persist despite an error (3.20 KB, patch)
2022-12-27 08:30 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 26558: Make guarantor information persist despite an error (3.26 KB, patch)
2022-12-27 17:01 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 26558: Make guarantor information persist despite an error (3.16 KB, patch)
2023-06-28 09:04 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 26558: Make guarantor information persist despite an error (3.20 KB, patch)
2023-06-28 17:51 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 26558: Make guarantor information persist despite an error (3.26 KB, patch)
2023-09-12 02:27 UTC, Aleisha Amohia
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description George Williams (NEKLS) 2020-09-28 22:26:38 UTC
To reproduce:

1. Create a new account but cause an error that will keep the account from saving (enter the wrong age for a category or give the patron a username that's already being used)
2. Search for and select a guarantor
3. Try to save the account and wait for the "The following fields are wrong. Please fix them." message
4. Note that the guarantor information is gone and you need to search for and select the guarantor again
Comment 1 Emmi Takkinen 2022-12-05 10:08:51 UTC
Tested, this is still an issue on current master.
Comment 2 Emmi Takkinen 2022-12-13 13:37:40 UTC
Readding guarantor parameters when error occurs should to the trick. This could be achived same way it is done in bug 24113, but at the end of following code block in memberentry.pl:

if ($nok or !$nodouble){
    $op="add" if ($op eq "insert");
    $op="modify" if ($op eq "save");
    %data=%newdata; 
    $template->param( updtype => ($op eq 'add' ?'I':'M'));	# used to check for $op eq "insert"... but we just changed $op!
    unless ($step){  
        $template->param( step_1 => 1,step_2 => 1,step_3 => 1, step_4 => 1, step_5 => 1, step_6 => 1, step_7 => 1 );
    }  
}

However this really feels like pointless repetition and there should probably be a better way to do this. Or way to make these parameters persist.
Comment 3 Emmi Takkinen 2022-12-27 08:30:43 UTC
Created attachment 144844 [details] [review]
Bug 26558: Make guarantor information persist despite an error

When one tries to create an account with patron guarantor and
error occurs (already used username, wrong age etc.), guarantor
information is lost. This patch always saves added patron
guarantor information to the template param new_guarantors.

To test:
1. Create a new account but cause an error that will keep the
account from saving (enter the wrong age for a category or
give the patron a username that's already being used).
2. Search for and select a guarantor.
3. Try to save the account and wait for the "The following
fields are wrong. Please fix them." message.
=> Note that the guarantor information is gone and you need
to search for and select the guarantor again.
4. Apply this patch.
5. Repeat steps 1.-3.
=> Note that guarantor information hasn't been lost.

This patch also removes code block from duplicate patron
check because we now save param new_guarantors even if
error doesn't occur.

To test:
1. Create a new account but cause a duplicate patron error.
2. Search for and select a guarantor.
3. Try to save the account.
=> Guarantor information should persist.

Sponsored-by: Koha-Suomi Oy
Comment 4 ByWater Sandboxes 2022-12-27 17:01:19 UTC
Created attachment 144852 [details] [review]
Bug 26558: Make guarantor information persist despite an error

When one tries to create an account with patron guarantor and
error occurs (already used username, wrong age etc.), guarantor
information is lost. This patch always saves added patron
guarantor information to the template param new_guarantors.

To test:
1. Create a new account but cause an error that will keep the
account from saving (enter the wrong age for a category or
give the patron a username that's already being used).
2. Search for and select a guarantor.
3. Try to save the account and wait for the "The following
fields are wrong. Please fix them." message.
=> Note that the guarantor information is gone and you need
to search for and select the guarantor again.
4. Apply this patch.
5. Repeat steps 1.-3.
=> Note that guarantor information hasn't been lost.

This patch also removes code block from duplicate patron
check because we now save param new_guarantors even if
error doesn't occur.

To test:
1. Create a new account but cause a duplicate patron error.
2. Search for and select a guarantor.
3. Try to save the account.
=> Guarantor information should persist.

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Barbara Johnson  <barbara.johnson@bedfordtx.gov>
Comment 5 Barbara Johnson 2022-12-27 17:02:12 UTC
Worked great and will be very helpful.
Comment 6 Jonathan Druart 2023-02-23 14:06:53 UTC
Can we move that in a condition? We don't need to do it if all went well.
Comment 7 Emmi Takkinen 2023-06-28 09:04:15 UTC
Created attachment 152782 [details] [review]
Bug 26558: Make guarantor information persist despite an error

When one tries to create an account with patron guarantor and
error occurs (already used username, wrong age etc.), guarantor
information is lost. This patch always saves added patron
guarantor information to the template param new_guarantors.

To test:
1. Create a new account but cause an error that will keep the
account from saving (enter the wrong age for a category or
give the patron a username that's already being used).
2. Search for and select a guarantor.
3. Try to save the account and wait for the "The following
fields are wrong. Please fix them." message.
=> Note that the guarantor information is gone and you need
to search for and select the guarantor again.
4. Apply this patch.
5. Repeat steps 1.-3.
=> Note that guarantor information hasn't been lost.

This patch also removes code block from duplicate patron
check because we now save param new_guarantors even if
error doesn't occur.

To test:
1. Create a new account but cause a duplicate patron error.
2. Search for and select a guarantor.
3. Try to save the account.
=> Guarantor information should persist.

Sponsored-by: Koha-Suomi Oy
Comment 8 Emmi Takkinen 2023-06-28 09:05:17 UTC
I moved the check in if ($nok) statement. Barbara, would you like to test and sign-off again?
Comment 9 Sam Lau 2023-06-28 17:51:25 UTC
Created attachment 152834 [details] [review]
Bug 26558: Make guarantor information persist despite an error

When one tries to create an account with patron guarantor and
error occurs (already used username, wrong age etc.), guarantor
information is lost. This patch always saves added patron
guarantor information to the template param new_guarantors.

To test:
1. Create a new account but cause an error that will keep the
account from saving (enter the wrong age for a category or
give the patron a username that's already being used).
2. Search for and select a guarantor.
3. Try to save the account and wait for the "The following
fields are wrong. Please fix them." message.
=> Note that the guarantor information is gone and you need
to search for and select the guarantor again.
4. Apply this patch.
5. Repeat steps 1.-3.
=> Note that guarantor information hasn't been lost.

This patch also removes code block from duplicate patron
check because we now save param new_guarantors even if
error doesn't occur.

To test:
1. Create a new account but cause a duplicate patron error.
2. Search for and select a guarantor.
3. Try to save the account.
=> Guarantor information should persist.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 10 Emmi Takkinen 2023-09-11 05:39:02 UTC
*** Bug 34750 has been marked as a duplicate of this bug. ***
Comment 11 Aleisha Amohia 2023-09-12 02:27:45 UTC
Created attachment 155522 [details] [review]
Bug 26558: Make guarantor information persist despite an error

When one tries to create an account with patron guarantor and
error occurs (already used username, wrong age etc.), guarantor
information is lost. This patch always saves added patron
guarantor information to the template param new_guarantors.

To test:
1. Create a new account but cause an error that will keep the
account from saving (enter the wrong age for a category or
give the patron a username that's already being used).
2. Search for and select a guarantor.
3. Try to save the account and wait for the "The following
fields are wrong. Please fix them." message.
=> Note that the guarantor information is gone and you need
to search for and select the guarantor again.
4. Apply this patch.
5. Repeat steps 1.-3.
=> Note that guarantor information hasn't been lost.

This patch also removes code block from duplicate patron
check because we now save param new_guarantors even if
error doesn't occur.

To test:
1. Create a new account but cause a duplicate patron error.
2. Search for and select a guarantor.
3. Try to save the account.
=> Guarantor information should persist.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 12 Aleisha Amohia 2023-09-12 21:03:24 UTC
Note we have opened Bug 34776 which is a similar problem, messaging preferences are wiped when there is an error.
Comment 13 Tomás Cohen Arazi (tcohen) 2023-10-11 15:14:01 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 14 Fridolin Somers 2023-10-13 00:14:07 UTC
Nice small enhancement, I choose to backport

Pushed to 23.05.x for 23.05.05
Comment 15 Matt Blenkinsop 2023-10-17 17:35:01 UTC
Enhancement - not backporting to 22.11.x