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
Created attachment 45702 [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.
Created attachment 45703 [details] [review] Bug 15380: Introduce Koha::Authorit[y|ies] and Koha::Authority::Type[s] Based on Koha::Objects.
Created attachment 45704 [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.
Created attachment 45780 [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>
Created attachment 45781 [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>
Created attachment 45782 [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>
Tested all patches together, followed test plan, works as expected. Prevent deleting an authority type that is in use makes sense.
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.
(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.
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>
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>
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>
Another long needed foundations overhaul, well done Jonathan. Passing QA, no regressions found.
Pushed to master - pretty cool :) thanks Jonathan