| Summary: | avoid AnonymousPatron in search_patrons_to_anonymise | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Fridolin Somers <fridolin.somers> |
| Component: | Patrons | Assignee: | Fridolin Somers <fridolin.somers> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | gmcharlt, jonathan.druart, kyle.m.hall, m.de.rooy |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
19.11.00,19.05.02
|
|
| Circulation function: | |||
| Bug Depends on: | 16966 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 22944: change unit tests
Bug 22944: avoid AnonymousPatron in search_patrons_to_anonymise Bug 22944: remove obsolete unit test Bug 22944: remove obsolete unit test Bug 22944: change unit tests Bug 22944: avoid AnonymousPatron in search_patrons_to_anonymise Bug 22944: remove obsolete unit test |
||
In theory the anymous patron should have privacy=0 but in case its not it should be explicitly avoided. Created attachment 89932 [details] [review] Bug 22944: change unit tests Add a dedicated UT and correct a test return value Created attachment 89933 [details] [review] Bug 22944: avoid AnonymousPatron in search_patrons_to_anonymise In Koha::Patrons::search_patrons_to_anonymise() old issues already affected to the anonymous patron should be avoided. It is useless and can impact performance. It is already working if AnymousPatron is not defined or 0 because of : 'old_issues.borrowernumber' => { 'not' => undef } In theory the anymous patron should have privacy=0 but in case its not it should be explicitly avoided. Test plan : Run t/db_dependent/Koha/Patrons.t You are not explaining in the test description what is really expected. We do not want the update to affect any rows because privacy is set to 0, right? (In reply to Jonathan Druart from comment #4) > You are not explaining in the test description what is really expected. We > do not want the update to affect any rows because privacy is set to 0, right? Actually it returns > 0 because of previous tests, so it should be adjusted (ideally tests must be independent from each others). I think this test must be removed as it is not testing anything useful: the previous statement will explode if something went wrong. (In reply to Jonathan Druart from comment #5) > I think this test must be removed as it is not testing anything useful: the > previous statement will explode if something went wrong. H is( $rows_affected, 0, 'AnonymiseIssueHistory should not return any error if success' ); (In reply to Jonathan Druart from comment #5) > I think this test must be removed as it is not testing anything useful: the > previous statement will explode if something went wrong. Hi, you mean this test ? is( $rows_affected, 0, 'AnonymiseIssueHistory should not return any error if success' ); (In reply to Fridolin SOMERS from comment #7) > (In reply to Jonathan Druart from comment #5) > > I think this test must be removed as it is not testing anything useful: the > > previous statement will explode if something went wrong. > > Hi, you mean this test ? > > is( $rows_affected, 0, 'AnonymiseIssueHistory should not return any error > if success' ); yes Created attachment 90488 [details] [review] Bug 22944: remove obsolete unit test Test 'AnonymiseIssueHistory should not return any error if success' is obsolete, previous statement will explode if something went wrong. Test plan : Run t/db_dependent/Koha/Patrons.t Created attachment 90628 [details] [review] Bug 22944: remove obsolete unit test Test 'AnonymiseIssueHistory should not return any error if success' is obsolete, previous statement will explode if something went wrong. Test plan : Run t/db_dependent/Koha/Patrons.t Signed-off-by: Arthur Bousquet <arthur.bousquet@inlibro.com> Current status: Signed Off Created attachment 90904 [details] [review] Bug 22944: change unit tests Add a dedicated UT and correct a test return value Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 90905 [details] [review] Bug 22944: avoid AnonymousPatron in search_patrons_to_anonymise In Koha::Patrons::search_patrons_to_anonymise() old issues already affected to the anonymous patron should be avoided. It is useless and can impact performance. It is already working if AnymousPatron is not defined or 0 because of : 'old_issues.borrowernumber' => { 'not' => undef } In theory the anymous patron should have privacy=0 but in case its not it should be explicitly avoided. Test plan : Run t/db_dependent/Koha/Patrons.t Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 90906 [details] [review] Bug 22944: remove obsolete unit test Test 'AnonymiseIssueHistory should not return any error if success' is obsolete, previous statement will explode if something went wrong. Test plan : Run t/db_dependent/Koha/Patrons.t Signed-off-by: Arthur Bousquet <arthur.bousquet@inlibro.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Nice work! Pushed to master for 19.11.00 Nice work! Pushed to master for 19.11.00 Pushed to 19.05.x for 19.05.02 |
In Koha::Patrons::search_patrons_to_anonymise() old issues already affected to the anonymous patron should be avoided. It is useless and can impact performance. It is already working if AnymousPatron is not defined or 0 because of : 'old_issues.borrowernumber' => { 'not' => undef }