Bug 34838 - The ILL module and tests generate warnings
Summary: The ILL module and tests generate warnings
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Pedro Amorim
QA Contact: Kyle M Hall (khall)
URL:
Keywords:
: 23040 34897 36918 (view as bug list)
Depends on:
Blocks: 8871 25515
  Show dependency treegraph
 
Reported: 2023-09-20 08:33 UTC by Jonathan Druart
Modified: 2024-07-25 23:07 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes the cause of several warnings generated by the tests for the inter-library loan (ILL) module.
Version(s) released in:
24.11.00,24.05.02,23.11.07
Circulation function:


Attachments
Bug 34838: Replace SUPER::AUTOLOAD with get_column (1.80 KB, patch)
2023-09-25 15:07 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 34838: Replace SUPER::AUTOLOAD with get_column (1.92 KB, patch)
2023-09-26 08:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle (1.28 KB, patch)
2023-09-26 09:35 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 34838: Only check for status in status graph if request has a status (1.45 KB, patch)
2023-09-26 09:35 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 34838: Tidy (2.74 KB, patch)
2023-09-26 09:42 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 34838: Replace SUPER::AUTOLOAD with get_column (1.98 KB, patch)
2023-09-26 11:20 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle (1.33 KB, patch)
2023-09-26 11:20 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 34838: Only check for status in status graph if request has a status (1.51 KB, patch)
2023-09-26 11:20 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 34838: Tidy (2.80 KB, patch)
2023-09-26 11:20 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
[23.11.x] Bug 34838: Replace SUPER::AUTOLOAD with get_column (1.99 KB, patch)
2024-05-16 20:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[23.11.x] Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle (1.34 KB, patch)
2024-05-16 20:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[23.11.x] Bug 34838: Only check for status in status graph if request has a status (1.52 KB, patch)
2024-05-16 20:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[23.11.x] Bug 34838: Tidy (2.81 KB, patch)
2024-05-16 20:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[23.11.x] Bug 34838: Use ->set to avoid method redefinition warnings (4.45 KB, patch)
2024-05-16 20:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[23.11.x] Bug 34838: Replace SUPER::AUTOLOAD with get_column (2.04 KB, patch)
2024-05-16 20:34 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[23.11.x] Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle (1.40 KB, patch)
2024-05-16 20:34 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[23.11.x] Bug 34838: Only check for status in status graph if request has a status (1.57 KB, patch)
2024-05-16 20:34 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[23.11.x] Bug 34838: Tidy (2.87 KB, patch)
2024-05-16 20:34 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[23.11.x] Bug 34838: Use ->set to avoid method redefinition warnings (4.45 KB, patch)
2024-05-16 20:34 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 34838: Replace SUPER::AUTOLOAD with get_column (2.04 KB, patch)
2024-05-16 20:35 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle (1.39 KB, patch)
2024-05-16 20:35 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 34838: Only check for status in status graph if request has a status (1.57 KB, patch)
2024-05-16 20:35 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 34838: Tidy (2.20 KB, patch)
2024-05-16 20:35 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 34838: Use ->set to avoid method redefinition warnings (3.56 KB, patch)
2024-05-16 20:35 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 34838: (QA follow-up): TypeDisclaimer tests - Add 'type' to request metadata (1.37 KB, patch)
2024-05-27 14:45 UTC, Pedro Amorim
Details | Diff | Splinter Review
[23.11.x] Bug 34838: (QA follow-up): TypeDisclaimer tests - Add 'type' to request metadata (1.37 KB, patch)
2024-05-27 15:05 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 34838: Replace SUPER::AUTOLOAD with get_column (2.11 KB, patch)
2024-05-28 10:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle (1.46 KB, patch)
2024-05-28 10:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34838: Only check for status in status graph if request has a status (1.64 KB, patch)
2024-05-28 10:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34838: Tidy (2.27 KB, patch)
2024-05-28 10:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34838: Use ->set to avoid method redefinition warnings (3.63 KB, patch)
2024-05-28 10:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34838: (QA follow-up): TypeDisclaimer tests - Add 'type' to request metadata (1.44 KB, patch)
2024-05-28 10:32 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 2023-09-20 08:33:56 UTC
https://jenkins.koha-community.org/job/Koha_Master/lastCompletedBuild/consoleText

It's full of:
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Comment 1 Jonathan Druart 2023-09-20 08:34:28 UTC
I think those ones are new.
Comment 2 Pedro Amorim 2023-09-25 13:22:00 UTC
I think this was introduced by bug 34223 but I'm not sure why. Investigating.
Comment 3 Pedro Amorim 2023-09-25 15:07:27 UTC
Created attachment 156178 [details] [review]
Bug 34838: Replace SUPER::AUTOLOAD with get_column

I'm not 100% happy with this but I'm out of ideas.

I think the problem warning happens because there is no method status or status_alias directly defined in Object.pm so it tries to define it, but it has already been defined by AUTOLOAD previously.
Comment 4 Jonathan Druart 2023-09-26 08:48:19 UTC
Created attachment 156200 [details] [review]
Bug 34838: Replace SUPER::AUTOLOAD with get_column

I'm not 100% happy with this but I'm out of ideas.

I think the problem warning happens because there is no method status or status_alias directly defined in Object.pm so it tries to define it, but it has already been defined by AUTOLOAD previously.

Test plan:
  prove t/db_dependent/Koha/Illbackend.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Jonathan Druart 2023-09-26 08:48:32 UTC
This fix seems correct to me.
Comment 6 Jonathan Druart 2023-09-26 08:50:15 UTC
Note that 4 warnings remain

t/db_dependent/Koha/Illbackend.t .. Use of uninitialized value in exists at /kohadevbox/koha/Koha/Illrequest.pm line 1956.
Use of uninitialized value in exists at /kohadevbox/koha/Koha/Illrequest.pm line 1961.
Use of uninitialized value in exists at /kohadevbox/koha/Koha/Illrequest.pm line 1956.
Use of uninitialized value in exists at /kohadevbox/koha/Koha/Illrequest.pm line 1961.
Comment 7 Pedro Amorim 2023-09-26 09:35:42 UTC
Created attachment 156201 [details] [review]
Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle

Because its highly likely that at least one request will have status_alias null in the database, the select MAX query will always return at least one result with NULL status_alias, throwing a warning, so we skip that
Comment 8 Pedro Amorim 2023-09-26 09:35:45 UTC
Created attachment 156202 [details] [review]
Bug 34838: Only check for status in status graph if request has a status

This check is required because when the existing_statuses method checks for the existing status_alias, it'll leave out the status from the query, so the status will be NULL for those particular query results when calling strings_map, throwing a warning
Comment 9 Pedro Amorim 2023-09-26 09:42:12 UTC
Created attachment 156204 [details] [review]
Bug 34838: Tidy
Comment 10 Nick Clemens (kidclamp) 2023-09-26 11:20:20 UTC
Created attachment 156217 [details] [review]
Bug 34838: Replace SUPER::AUTOLOAD with get_column

I'm not 100% happy with this but I'm out of ideas.

I think the problem warning happens because there is no method status or status_alias directly defined in Object.pm so it tries to define it, but it has already been defined by AUTOLOAD previously.

Test plan:
  prove t/db_dependent/Koha/Illbackend.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens (kidclamp) 2023-09-26 11:20:22 UTC
Created attachment 156218 [details] [review]
Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle

Because its highly likely that at least one request will have status_alias null in the database, the select MAX query will always return at least one result with NULL status_alias, throwing a warning, so we skip that

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Nick Clemens (kidclamp) 2023-09-26 11:20:24 UTC
Created attachment 156219 [details] [review]
Bug 34838: Only check for status in status graph if request has a status

This check is required because when the existing_statuses method checks for the existing status_alias, it'll leave out the status from the query, so the status will be NULL for those particular query results when calling strings_map, throwing a warning

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Nick Clemens (kidclamp) 2023-09-26 11:20:26 UTC
Created attachment 156220 [details] [review]
Bug 34838: Tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 Pedro Amorim 2023-10-03 14:11:04 UTC
*** Bug 34897 has been marked as a duplicate of this bug. ***
Comment 15 Jonathan Druart 2023-10-03 14:45:22 UTC
I am still getting a lot of warnings

t/db_dependent/Illrequests.t .. 1/15 illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 101.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 111.
t/db_dependent/Illrequests.t .. 6/15 Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 884.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 884.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 884.
t/db_dependent/Illrequests.t .. 8/15 Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1013.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1013.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1013.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1033.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1033.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1033.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1071.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1071.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1071.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1114.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1114.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1114.
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 922.
t/db_dependent/Illrequests.t .. 11/15 Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 922.
t/db_dependent/Illrequests.t .. 12/15 Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 922.
t/db_dependent/Illrequests.t .. 14/15 Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 922.
Comment 16 Jonathan Druart 2023-10-03 14:47:29 UTC
(In reply to Jonathan Druart from comment #6)
> Note that 4 warnings remain
> 
> t/db_dependent/Koha/Illbackend.t .. Use of uninitialized value in exists at
> /kohadevbox/koha/Koha/Illrequest.pm line 1956.
> Use of uninitialized value in exists at /kohadevbox/koha/Koha/Illrequest.pm
> line 1961.
> Use of uninitialized value in exists at /kohadevbox/koha/Koha/Illrequest.pm
> line 1956.
> Use of uninitialized value in exists at /kohadevbox/koha/Koha/Illrequest.pm
> line 1961.

So this was t/db_dependent/Koha/Illbackend.t

But the problem remains for t/db_dependent/Illrequests.t
Comment 17 Katrin Fischer 2023-10-08 10:30:11 UTC
*** Bug 23040 has been marked as a duplicate of this bug. ***
Comment 18 Tomás Cohen Arazi (tcohen) 2024-05-16 20:24:41 UTC
Created attachment 166841 [details] [review]
[23.11.x] Bug 34838: Replace SUPER::AUTOLOAD with get_column

I'm not 100% happy with this but I'm out of ideas.

I think the problem warning happens because there is no method status or status_alias directly defined in Object.pm so it tries to define it, but it has already been defined by AUTOLOAD previously.

Test plan:
  prove t/db_dependent/Koha/Illbackend.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 Tomás Cohen Arazi (tcohen) 2024-05-16 20:24:44 UTC
Created attachment 166842 [details] [review]
[23.11.x] Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle

Because its highly likely that at least one request will have status_alias null in the database, the select MAX query will always return at least one result with NULL status_alias, throwing a warning, so we skip that

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Tomás Cohen Arazi (tcohen) 2024-05-16 20:24:47 UTC
Created attachment 166843 [details] [review]
[23.11.x] Bug 34838: Only check for status in status graph if request has a status

This check is required because when the existing_statuses method checks for the existing status_alias, it'll leave out the status from the query, so the status will be NULL for those particular query results when calling strings_map, throwing a warning

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Tomás Cohen Arazi (tcohen) 2024-05-16 20:24:50 UTC
Created attachment 166844 [details] [review]
[23.11.x] Bug 34838: Tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Tomás Cohen Arazi (tcohen) 2024-05-16 20:24:53 UTC
Created attachment 166845 [details] [review]
[23.11.x] Bug 34838: Use ->set to avoid method redefinition warnings

This patch silences more warnings. To test

1. Be on 23.11.x
2. Apply the patches before this one
3. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Illrequests.t
=> FAIL: Lots of warnings like this:
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 955.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 955.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1071.
4. Apply this patch
5. Repeat 3
=> SUCCESS: No more warnings!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 23 Tomás Cohen Arazi (tcohen) 2024-05-16 20:34:42 UTC
Created attachment 166846 [details] [review]
[23.11.x] Bug 34838: Replace SUPER::AUTOLOAD with get_column

I'm not 100% happy with this but I'm out of ideas.

I think the problem warning happens because there is no method status or status_alias directly defined in Object.pm so it tries to define it, but it has already been defined by AUTOLOAD previously.

Test plan:
  prove t/db_dependent/Koha/Illbackend.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 24 Tomás Cohen Arazi (tcohen) 2024-05-16 20:34:45 UTC
Created attachment 166847 [details] [review]
[23.11.x] Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle

Because its highly likely that at least one request will have status_alias null in the database, the select MAX query will always return at least one result with NULL status_alias, throwing a warning, so we skip that

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 25 Tomás Cohen Arazi (tcohen) 2024-05-16 20:34:48 UTC
Created attachment 166848 [details] [review]
[23.11.x] Bug 34838: Only check for status in status graph if request has a status

This check is required because when the existing_statuses method checks for the existing status_alias, it'll leave out the status from the query, so the status will be NULL for those particular query results when calling strings_map, throwing a warning

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 26 Tomás Cohen Arazi (tcohen) 2024-05-16 20:34:51 UTC
Created attachment 166849 [details] [review]
[23.11.x] Bug 34838: Tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 27 Tomás Cohen Arazi (tcohen) 2024-05-16 20:34:53 UTC
Created attachment 166850 [details] [review]
[23.11.x] Bug 34838: Use ->set to avoid method redefinition warnings

This patch silences more warnings. To test

1. Be on 23.11.x
2. Apply the patches before this one
3. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Illrequests.t
=> FAIL: Lots of warnings like this:
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 955.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 955.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1071.
4. Apply this patch
5. Repeat 3
=> SUCCESS: No more warnings!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 28 Tomás Cohen Arazi (tcohen) 2024-05-16 20:35:30 UTC
Created attachment 166851 [details] [review]
Bug 34838: Replace SUPER::AUTOLOAD with get_column

I'm not 100% happy with this but I'm out of ideas.

I think the problem warning happens because there is no method status or status_alias directly defined in Object.pm so it tries to define it, but it has already been defined by AUTOLOAD previously.

Test plan:
  prove t/db_dependent/Koha/Illbackend.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 29 Tomás Cohen Arazi (tcohen) 2024-05-16 20:35:33 UTC
Created attachment 166852 [details] [review]
Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle

Because its highly likely that at least one request will have status_alias null in the database, the select MAX query will always return at least one result with NULL status_alias, throwing a warning, so we skip that

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 30 Tomás Cohen Arazi (tcohen) 2024-05-16 20:35:36 UTC
Created attachment 166853 [details] [review]
Bug 34838: Only check for status in status graph if request has a status

This check is required because when the existing_statuses method checks for the existing status_alias, it'll leave out the status from the query, so the status will be NULL for those particular query results when calling strings_map, throwing a warning

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 31 Tomás Cohen Arazi (tcohen) 2024-05-16 20:35:39 UTC
Created attachment 166854 [details] [review]
Bug 34838: Tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 32 Tomás Cohen Arazi (tcohen) 2024-05-16 20:35:41 UTC
Created attachment 166855 [details] [review]
Bug 34838: Use ->set to avoid method redefinition warnings

This patch silences more warnings. To test

1. Be on 23.11.x
2. Apply the patches before this one
3. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Illrequests.t
=> FAIL: Lots of warnings like this:
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 955.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 955.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1071.
4. Apply this patch
5. Repeat 3
=> SUCCESS: No more warnings!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 33 Tomás Cohen Arazi (tcohen) 2024-05-16 20:43:36 UTC
(In reply to Jonathan Druart from comment #15)
> I am still getting a lot of warnings

Please check with my follow-up. There's a '23.11' and a 'main' version of the patch set. Besides the noise on the tests, this is a nightnmare on busy sites with some moderate ILL activity. as any action triggers many warnings per request that is displayed.
Comment 34 Pedro Amorim 2024-05-22 13:41:07 UTC
*** Bug 36918 has been marked as a duplicate of this bug. ***
Comment 35 Jonathan Druart 2024-05-23 07:57:27 UTC
As you have marked all the other bugs as duplicate of this one I am expecting all the warnings to be removed.

But still:

t/db_dependent/Illrequest/TypeDisclaimer.t ........... 1/5 Use of uninitialized value $type in string eq at /kohadevbox/koha/Koha/ILL/Request/Workflow/TypeDisclaimer.pm line 160.
Use of uninitialized value $type in string eq at /kohadevbox/koha/Koha/ILL/Request/Workflow/TypeDisclaimer.pm line 160.
Comment 36 Pedro Amorim 2024-05-27 14:45:23 UTC
Created attachment 167193 [details] [review]
Bug 34838: (QA follow-up): TypeDisclaimer tests - Add 'type' to request metadata

This adds a type to the request (mocking it as a new ILL request of type journal being submitted)
Comment 37 Pedro Amorim 2024-05-27 15:05:57 UTC
Created attachment 167194 [details] [review]
[23.11.x] Bug 34838: (QA follow-up): TypeDisclaimer tests - Add 'type' to request metadata

This adds a type to the request (mocking it as a new ILL request of type journal being submitted)
Comment 38 Jonathan Druart 2024-05-28 10:32:19 UTC
Created attachment 167211 [details] [review]
Bug 34838: Replace SUPER::AUTOLOAD with get_column

I'm not 100% happy with this but I'm out of ideas.

I think the problem warning happens because there is no method status or status_alias directly defined in Object.pm so it tries to define it, but it has already been defined by AUTOLOAD previously.

Test plan:
  prove t/db_dependent/Koha/Illbackend.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 39 Jonathan Druart 2024-05-28 10:32:21 UTC
Created attachment 167212 [details] [review]
Bug 34838: Only call strings_map if status_alias is not undef in status_alias cycle

Because its highly likely that at least one request will have status_alias null in the database, the select MAX query will always return at least one result with NULL status_alias, throwing a warning, so we skip that

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 40 Jonathan Druart 2024-05-28 10:32:24 UTC
Created attachment 167213 [details] [review]
Bug 34838: Only check for status in status graph if request has a status

This check is required because when the existing_statuses method checks for the existing status_alias, it'll leave out the status from the query, so the status will be NULL for those particular query results when calling strings_map, throwing a warning

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 41 Jonathan Druart 2024-05-28 10:32:27 UTC
Created attachment 167214 [details] [review]
Bug 34838: Tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 42 Jonathan Druart 2024-05-28 10:32:29 UTC
Created attachment 167215 [details] [review]
Bug 34838: Use ->set to avoid method redefinition warnings

This patch silences more warnings. To test

1. Be on 23.11.x
2. Apply the patches before this one
3. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Illrequests.t
=> FAIL: Lots of warnings like this:
Subroutine Koha::Illrequest::SUPER::status redefined at /kohadevbox/koha/Koha/Object.pm line 955.
Subroutine Koha::Illrequest::SUPER::status_alias redefined at /kohadevbox/koha/Koha/Object.pm line 955.
illrequestattributes is DEPRECATED in favor of extended_attributes at t/db_dependent/Illrequests.t line 1071.
4. Apply this patch
5. Repeat 3
=> SUCCESS: No more warnings!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 43 Jonathan Druart 2024-05-28 10:32:32 UTC
Created attachment 167216 [details] [review]
Bug 34838: (QA follow-up): TypeDisclaimer tests - Add 'type' to request metadata

This adds a type to the request (mocking it as a new ILL request of type journal being submitted)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 44 Martin Renvoize (ashimema) 2024-06-10 11:04:02 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant
Comment 45 Lucas Gass (lukeg) 2024-06-27 16:22:31 UTC
Backported to 24.05.x for 24.05.02
Comment 46 Fridolin Somers 2024-07-17 13:15:02 UTC
Pushed to 23.11.x for 23.11.07
Comment 47 Wainui Witika-Park 2024-07-19 03:14:29 UTC
not backporting to 23.05.x unless requested