| Summary: | TestBuilder.pm uses incorrect method for checking if objects to be created exists | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Joonas Kylmälä <joonas.kylmala> |
| Component: | Test Suite | Assignee: | Joonas Kylmälä <joonas.kylmala> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | minor | ||
| Priority: | P5 - low | CC: | fridolin.somers, jonathan.druart, kyle, victor |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
21.11.00,21.05.02,20.11.08
|
|
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 22690 | ||
| Attachments: |
Bug 28479: Use primary keys to check object existence in TestBuilder
Bug 28479: Use primary keys to check object existence in TestBuilder Bug 28479: Use primary keys to check object existence in TestBuilder |
||
|
Description
Joonas Kylmälä
2021-05-28 10:17:46 UTC
Created attachment 121473 [details] [review] Bug 28479: Use primary keys to check object existence in TestBuilder The TestBuilder::build_object function used any foreign keys to check whether an object already exists or not. This brought incorrectly results of unrelated objects because using any other keys other than primary keys don't guarantee our results to point to one single object. For example, as is put here in the unit test, if you created two items with the same biblionumber and then tried to create a hold using build_object() we were using the biblionumber to check whether an item was linked to the hold already. Thus, we were checking whether a random item was already linked to the hold instead of the one we wanted either by passing it explicitly to build_object() or the one build_object() created implicitly. This also resulted in following warnings when there were more than one match: DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235 To test: $ prove t/db_dependent Created attachment 121501 [details] [review] Bug 28479: Use primary keys to check object existence in TestBuilder The TestBuilder::build_object function used any foreign keys to check whether an object already exists or not. This brought incorrectly results of unrelated objects because using any other keys other than primary keys don't guarantee our results to point to one single object. For example, as is put here in the unit test, if you created two items with the same biblionumber and then tried to create a hold using build_object() we were using the biblionumber to check whether an item was linked to the hold already. Thus, we were checking whether a random item was already linked to the hold instead of the one we wanted either by passing it explicitly to build_object() or the one build_object() created implicitly. This also resulted in following warnings when there were more than one match: DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235 To test: $ prove t/db_dependent Signed-off-by: David Nind <david@davidnind.com> Created attachment 121858 [details] [review] Bug 28479: Use primary keys to check object existence in TestBuilder The TestBuilder::build_object function used any foreign keys to check whether an object already exists or not. This brought incorrectly results of unrelated objects because using any other keys other than primary keys don't guarantee our results to point to one single object. For example, as is put here in the unit test, if you created two items with the same biblionumber and then tried to create a hold using build_object() we were using the biblionumber to check whether an item was linked to the hold already. Thus, we were checking whether a random item was already linked to the hold instead of the one we wanted either by passing it explicitly to build_object() or the one build_object() created implicitly. This also resulted in following warnings when there were more than one match: DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235 To test: $ prove t/db_dependent Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 21.11, thanks to everybody involved! Pushed to 21.05.x for 21.05.02 Pushed to 20.11.x for 20.11.08 Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed. |