Bug 15380 - Move the authority types related code to Koha::Authority::Type[s] - part 1
Summary: Move the authority types related code to Koha::Authority::Type[s] - part 1
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 15379 15381 15578 16012
  Show dependency treegraph
 
Reported: 2015-12-15 14:35 UTC by Jonathan Druart
Modified: 2017-06-14 22:10 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15380: Move Koha::Authority to Koha::MetadataRecord::Authority (14.17 KB, patch)
2015-12-15 15:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15380: Introduce Koha::Authorit[y|ies] and Koha::Authority::Type[s] (8.19 KB, patch)
2015-12-15 15:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15380: Use Koha::Authority::Type[s] in admin script (20.92 KB, patch)
2015-12-15 15:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15380: Move Koha::Authority to Koha::MetadataRecord::Authority (14.31 KB, patch)
2015-12-17 15:15 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15380: Introduce Koha::Authorit[y|ies] and Koha::Authority::Type[s] (8.33 KB, patch)
2015-12-17 15:16 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15380: Use Koha::Authority::Type[s] in admin script (21.05 KB, patch)
2015-12-17 15:17 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15380: Move Koha::Authority to Koha::MetadataRecord::Authority (14.37 KB, patch)
2015-12-23 05:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15380: Introduce Koha::Authorit[y|ies] and Koha::Authority::Type[s] (8.39 KB, patch)
2015-12-23 05:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15380: Use Koha::Authority::Type[s] in admin script (21.11 KB, patch)
2015-12-23 05:48 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2015-12-15 14:35:18 UTC
This first part will include:
- move the existing Koha::Authority to Koha::MetadataRecord::Authority
- Introduce 4 new modules based on Koha::Object[s]:
  Koha::Authority & Koha::Authorities
  Koha::Authority::Type & Koha::Authority::Types
- use them from admin/branches.pl
Comment 1 Jonathan Druart 2015-12-15 15:00:22 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-12-15 15:00:26 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-12-15 15:00:31 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2015-12-17 15:15:39 UTC Comment hidden (obsolete)
Comment 5 Marc Véron 2015-12-17 15:16:33 UTC Comment hidden (obsolete)
Comment 6 Marc Véron 2015-12-17 15:17:25 UTC Comment hidden (obsolete)
Comment 7 Marc Véron 2015-12-17 15:19:34 UTC
Tested all patches together, followed test plan, works as expected.
Prevent deleting an authority type that is in use makes sense.
Comment 8 Ian Palko 2015-12-19 17:03:38 UTC
I am wondering if changes with Koha::Authority here will help solve the problem over in Bug 15358.

When Bernardo was doing some testing to try to resolve it, he was having issues after reverting the patch from Bug 8064 with authorities merging failing due to :

"In authorities/merge.pl, $recordObj1->authtype method returns 'undef',
it's a Koha::MetadataRecord object."

Perhaps before these changes to Koha::MetadataRecord::Authority are pushed the necessary changes can be made to harmonize it with repairing of the broken authorities merge.
Comment 9 Jonathan Druart 2015-12-21 09:58:30 UTC
(In reply to Ian Palko from comment #8)
> I am wondering if changes with Koha::Authority here will help solve the
> problem over in Bug 15358.

No, I did not change this module, just rename it.

> Perhaps before these changes to Koha::MetadataRecord::Authority are pushed
> the necessary changes can be made to harmonize it with repairing of the
> broken authorities merge.

It will be easy to fix this conflict, no need to block this patch or the other one.
Comment 10 Martin Renvoize 2015-12-23 05:48:15 UTC
Created attachment 45932 [details] [review]
Bug 15380: Move Koha::Authority to Koha::MetadataRecord::Authority

The existing Koha::Authority does not use Koha::Objects and does not
exclusively deal with DB objects.
It makes much sense to move it to let the space free for a
Koha::Authority and Koha::Authorities modules based on Koha::Object.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2015-12-23 05:48:18 UTC
Created attachment 45933 [details] [review]
Bug 15380: Introduce Koha::Authorit[y|ies] and Koha::Authority::Type[s]

Based on Koha::Objects.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2015-12-23 05:48:20 UTC
Created attachment 45934 [details] [review]
Bug 15380: Use Koha::Authority::Type[s] in admin script

This path rewrites the admin/authtypes.pl script to use the new modules
instead of executing the SQL queries from the script.

Test plan:
1/ Create 1+ authority types
2/ Update one
3/ Delete it
4/ Create an authority record using this authority type.
5/ Try to delete the authority type
Here there is a change in the behavior.
Prior to this patch, the user was able to delete the authority type, now
it's not.
I think it was a bug, feel free to give your opinion if you disagree on
this change.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2015-12-23 05:51:46 UTC
Another long needed foundations overhaul, well done Jonathan.

Passing QA, no regressions found.
Comment 14 Brendan Gallagher 2015-12-31 18:58:00 UTC
Pushed to master - pretty cool :)  thanks Jonathan