Bug 27554 - Clarify and add tests for Koha::Patrons->update_category_to child to adult
Summary: Clarify and add tests for Koha::Patrons->update_category_to child to adult
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on: 17168
Blocks: 26307
  Show dependency treegraph
 
Reported: 2021-01-27 08:41 UTC by Fridolin Somers
Modified: 2021-06-14 21:34 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.03,20.05.09,19.11.15


Attachments
Bug 27554: Add a test for Koha::Patrons->update_category_to child to adult (2.77 KB, patch)
2021-01-27 08:50 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 27554: Fix test for Koha::Patrons->update_category_to child to adult (1.73 KB, patch)
2021-01-27 08:51 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 27554: Add a test for Koha::Patrons->update_category_to child to adult (2.82 KB, patch)
2021-01-27 09:55 UTC, David Nind
Details | Diff | Splinter Review
Bug 27554: Fix test for Koha::Patrons->update_category_to child to adult (1.77 KB, patch)
2021-01-27 09:55 UTC, David Nind
Details | Diff | Splinter Review
Bug 27554: Clarify and add tests (3.44 KB, patch)
2021-01-28 14:00 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27554: Clarify and add tests (3.50 KB, patch)
2021-01-28 14:25 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 27554: Clarify and add tests (3.56 KB, patch)
2021-01-31 13:00 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27554: Clarify and add tests for update_category_to (3.66 KB, patch)
2021-02-01 15:22 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 Fridolin Somers 2021-01-27 08:41:15 UTC
One unit test for Koha::Patrons->update_category_to seems wrong :
  is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode_2,too_young=>1})->update_category_to({category=>$a_categorycode}),1,'One child patron updated to adult category');

Child is updated to adult because he his too old, not too young.
Comment 1 Fridolin Somers 2021-01-27 08:50:44 UTC
Created attachment 115849 [details] [review]
Bug 27554: Add a test for Koha::Patrons->update_category_to child to adult

One unit test for Koha::Patrons->update_category_to seems wrong :
  is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode_2,too_young=>1})->update_category_to({category=>$a_categorycode}),1,'One child patron updated to adult category');

Child is updated to adult because he his too old, not too young.

This patch adds a test on category of patron that should be an
adult after update.

Test plan :
1) Without patch, run t/db_dependent/Patrons.t
2) It does not fail
3) Apply patch, run t/db_dependent/Patrons.t
4) It fails on test 'The over age patron has been updated to adult category'
Comment 2 Fridolin Somers 2021-01-27 08:51:00 UTC
Created attachment 115850 [details] [review]
Bug 27554: Fix test for Koha::Patrons->update_category_to child to adult

This patch fixes the test by using too_old instead of too_young

Test plan :
Apply patch, run t/db_dependent/Patrons.t, tests are OK
Comment 3 David Nind 2021-01-27 09:55:56 UTC
Created attachment 115860 [details] [review]
Bug 27554: Add a test for Koha::Patrons->update_category_to child to adult

One unit test for Koha::Patrons->update_category_to seems wrong :
  is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode_2,too_young=>1})->update_category_to({category=>$a_categorycode}),1,'One child patron updated to adult category');

Child is updated to adult because he his too old, not too young.

This patch adds a test on category of patron that should be an
adult after update.

Test plan :
1) Without patch, run t/db_dependent/Patrons.t
2) It does not fail
3) Apply patch, run t/db_dependent/Patrons.t
4) It fails on test 'The over age patron has been updated to adult category'

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2021-01-27 09:55:58 UTC
Created attachment 115861 [details] [review]
Bug 27554: Fix test for Koha::Patrons->update_category_to child to adult

This patch fixes the test by using too_old instead of too_young

Test plan :
Apply patch, run t/db_dependent/Patrons.t, tests are OK

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Nick Clemens 2021-01-28 12:42:33 UTC
I see what you thought here, but actually the patron was updated because they were too young

child1 is 4 years old - too young for child account

The script will move you to the next categry without checking the age limits for that category - i believe this is how the past script worked too. It is not a logical test, but valid

child3 is 18 - you make your tests affect this patron instead - maybe we want your test in addition to the existing tests?
Comment 6 Fridolin Somers 2021-01-28 13:22:07 UTC
(In reply to Nick Clemens from comment #5)
> I see what you thought here, but actually the patron was updated because
> they were too young
> 
> child1 is 4 years old - too young for child account
> 
> The script will move you to the next category without checking the age limits
> for that category - i believe this is how the past script worked too. It is
> not a logical test, but valid
OK but I'm working on adding a check on age limits so it breaks this test.

> 
> child3 is 18 - you make your tests affect this patron instead - maybe we
> want your test in addition to the existing tests?
Yes, in my opinion test should verify which patron is impacted.

But the test also checks that a child looses its guarantor so it must be from child to adult.

Why not fix the test ?
It would me more clear in case it fails.
Comment 7 Nick Clemens 2021-01-28 14:00:28 UTC
Created attachment 115933 [details] [review]
Bug 27554: Clarify and add tests

This patch adds a comment to the existing tests, clarifying what they test, and verifying the expected patron is affected. The
original test should be broken by 26307 when the behvaiour is prevented

I additionally add a test for 'too old'

To test:
1 - Apply patch
2 - Prove tests, they should pass
3 - Verify the comments make sense
Comment 8 Fridolin Somers 2021-01-28 14:25:38 UTC
Created attachment 115934 [details] [review]
Bug 27554: Clarify and add tests

This patch adds a comment to the existing tests, clarifying what they test, and verifying the expected patron is affected. The
original test should be broken by 26307 when the behvaiour is prevented

I additionally add a test for 'too old'

To test:
1 - Apply patch
2 - Prove tests, they should pass
3 - Verify the comments make sense

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 9 Katrin Fischer 2021-01-31 13:00:38 UTC
Created attachment 116097 [details] [review]
Bug 27554: Clarify and add tests

This patch adds a comment to the existing tests, clarifying what they test, and verifying the expected patron is affected. The
original test should be broken by 26307 when the behvaiour is prevented

I additionally add a test for 'too old'

To test:
1 - Apply patch
2 - Prove tests, they should pass
3 - Verify the comments make sense

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2021-01-31 13:01:09 UTC
Nice cooperation here :)
Comment 11 Jonathan Druart 2021-02-01 15:22:41 UTC
Created attachment 116171 [details] [review]
Bug 27554: Clarify and add tests for update_category_to

This patch adds a comment to the existing tests, clarifying what they test, and verifying the expected patron is affected. The
original test should be broken by 26307 when the behvaiour is prevented

I additionally add a test for 'too old'

To test:
1 - Apply patch
2 - Prove tests, they should pass
3 - Verify the comments make sense

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

JD amended patch: behvaiour ==> behaviour
and add more info to the commit title
Comment 12 Jonathan Druart 2021-02-01 16:27:19 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 13 Fridolin Somers 2021-02-05 15:37:34 UTC
Pushed to 20.11.x for 20.11.03
Comment 14 Andrew Fuerste-Henry 2021-02-08 20:36:15 UTC
Pushed to 20.05.x for 20.05.09
Comment 15 Victor Grousset/tuxayo 2021-02-10 23:11:43 UTC
Backported: Pushed to 19.11.x branch for 19.11.15