Bug 15380

Summary: Move the authority types related code to Koha::Authority::Type[s] - part 1
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: brendan, library, martin.renvoize, veron
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:
Bug Depends on:    
Bug Blocks: 15379, 15381, 15578, 16012    
Attachments: Bug 15380: Move Koha::Authority to Koha::MetadataRecord::Authority
Bug 15380: Introduce Koha::Authorit[y|ies] and Koha::Authority::Type[s]
Bug 15380: Use Koha::Authority::Type[s] in admin script
Bug 15380: Move Koha::Authority to Koha::MetadataRecord::Authority
Bug 15380: Introduce Koha::Authorit[y|ies] and Koha::Authority::Type[s]
Bug 15380: Use Koha::Authority::Type[s] in admin script
Bug 15380: Move Koha::Authority to Koha::MetadataRecord::Authority
Bug 15380: Introduce Koha::Authorit[y|ies] and Koha::Authority::Type[s]
Bug 15380: Use Koha::Authority::Type[s] in admin script

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