Bug 30939 - remove_unused_authorities.pl is broken
Summary: remove_unused_authorities.pl is broken
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All Linux
: P5 - low major
Assignee: Katrin Fischer
QA Contact: Fridolin Somers
URL:
Keywords:
Depends on: 17600
Blocks:
  Show dependency treegraph
 
Reported: 2022-06-10 08:06 UTC by Fernando
Modified: 2023-06-08 22:26 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes the ./misc/migration_tools/remove_unused_authorities.pl script so that it now works and deletes unused authority records. Before this, it generated an error message at the first unused authority record and stopped (without deleting any unused authority records).
Version(s) released in:
22.11.00, 22.05.04, 21.11.11
Circulation function:


Attachments
Bug 30939: Fix use statement for DelAuthority (1.52 KB, patch)
2022-06-17 22:41 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 30939: Fix use statement for DelAuthority (1.56 KB, patch)
2022-06-18 21:07 UTC, David Nind
Details | Diff | Splinter Review
Bug 30939: Fix use statement for DelAuthority (1.62 KB, patch)
2022-06-27 21:30 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fernando 2022-06-10 08:06:35 UTC
When I try to remove unused authorithies I get:
authid=1 type=PERSO_NAME : used 6 time(s)
authid=2 type=TOPIC_TERM : used 23 time(s)
authid=3 type=PERSO_NAME : used 3 time(s)
authid=4 type=PERSO_NAMEUndefined subroutine &main::DelAuthority called at /usr/share/koha/bin/migration_tools/remove_unused_authorities.pl line 98.

When I try only a test works OK.
Thank you
Fernando
Comment 1 Katrin Fischer 2022-06-17 22:32:56 UTC
Confirmed on maser with the sample database:

authid=172 type=CORPO_NAME : used 2 time(s)
authid=173 type=PERSO_NAMEUndefined subroutine &main::DelAuthority called at ./misc/migration_tools/remove_unused_authorities.pl line 98.
Comment 2 Katrin Fischer 2022-06-17 22:41:28 UTC
Created attachment 136320 [details] [review]
Bug 30939: Fix use statement for DelAuthority

Without this patch, the script won't delete any
unused authorities, but gives an error instead
and dies:

Undefined subroutine &main::DelAuthority called at ./misc/migration_tools/remove_unused_authorities.pl line 98.

To test:
- Run from koha-shell:
  ./misc/migration_tools/remove_unused_authorities.pl -t
- Verify several authorities are reported as unused
- ./misc/migration_tools/remove_unused_authorities.pl -c
- Verify the error message is shown when the first unused
  authority is found and the script stops
- Apply patch and rerun:
  ./misc/migration_tools/remove_unused_authorities.pl -t
- Verify the error is gone, the script finishes and auhorities
  are deleted

https://bugs.koha-community.org/show_bug.cgi?id=30936
Comment 3 David Nind 2022-06-18 21:07:25 UTC
Created attachment 136323 [details] [review]
Bug 30939: Fix use statement for DelAuthority

Without this patch, the script won't delete any
unused authorities, but gives an error instead
and dies:

Undefined subroutine &main::DelAuthority called at ./misc/migration_tools/remove_unused_authorities.pl line 98.

To test:
- Run from koha-shell:
  ./misc/migration_tools/remove_unused_authorities.pl -t
- Verify several authorities are reported as unused
- ./misc/migration_tools/remove_unused_authorities.pl -c
- Verify the error message is shown when the first unused
  authority is found and the script stops
- Apply patch and rerun:
  ./misc/migration_tools/remove_unused_authorities.pl -t
- Verify the error is gone, the script finishes and auhorities
  are deleted

https://bugs.koha-community.org/show_bug.cgi?id=30936

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Fridolin Somers 2022-06-27 21:30:44 UTC
Created attachment 136661 [details] [review]
Bug 30939: Fix use statement for DelAuthority

Without this patch, the script won't delete any
unused authorities, but gives an error instead
and dies:

Undefined subroutine &main::DelAuthority called at ./misc/migration_tools/remove_unused_authorities.pl line 98.

To test:
- Run from koha-shell:
  ./misc/migration_tools/remove_unused_authorities.pl -t
- Verify several authorities are reported as unused
- ./misc/migration_tools/remove_unused_authorities.pl -c
- Verify the error message is shown when the first unused
  authority is found and the script stops
- Apply patch and rerun:
  ./misc/migration_tools/remove_unused_authorities.pl -t
- Verify the error is gone, the script finishes and auhorities
  are deleted

https://bugs.koha-community.org/show_bug.cgi?id=30936

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 5 Fridolin Somers 2022-06-27 21:37:55 UTC
How could be search for thoses ?

I try :
git grep 'use C4::.*;' | egrep -v '^(C4|Koha|t)/' | egrep -v 'qw ?(\(|/)' | grep -v Context

If found for example :
tools/batch_delete_records.pl:use C4::AuthoritiesMarc;

Is it a correct match ?
Comment 6 Tomás Cohen Arazi (tcohen) 2022-06-28 12:33:23 UTC
(In reply to Fridolin Somers from comment #5)
> How could be search for thoses ?
> 
> I try :
> git grep 'use C4::.*;' | egrep -v '^(C4|Koha|t)/' | egrep -v 'qw ?(\(|/)' |
> grep -v Context
> 
> If found for example :
> tools/batch_delete_records.pl:use C4::AuthoritiesMarc;
> 
> Is it a correct match ?

Nope. Because the script is using fully-qualified method names (e.g. C4::AuthoritiesMarc::GetAuthority( $record_id ) ) so not importing the function names into the package scope is not a real issue. We need to check that C4::AuthoritiesMarc defines a method that is used in a script.
Comment 7 Tomás Cohen Arazi (tcohen) 2022-06-28 12:41:27 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 8 Lucas Gass (lukeg) 2022-07-29 17:00:39 UTC
Backported to 22.05.x for 22.05.04
Comment 9 Arthur Suzuki 2022-08-11 13:06:30 UTC
applied to 21.11.x for 21.11.11
Comment 10 Victor Grousset/tuxayo 2022-08-13 16:30:51 UTC
Missing dependencies for 21.05.x, it shouldn't be affected, no backport.

Nothing to document, marking resolved.