Bug 13497

Summary: The pref AddPatronLists should be removed
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: katrin.fischer, tomascohen, veron
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13458
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9314
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 9314, 13970    
Attachments: Bug 13497: Remove the pref AddPatronsLists
Bug 13497: Remove the pref AddPatronsLists
[Signed-off] Bug 13497: Remove the pref AddPatronsLists
[PASSED QA] Bug 13497: Remove the pref AddPatronsLists
Bug 13497: Remove the pref AddPatronsLists
Bug 13497: (Maniac followup) spell category in english

Description Jonathan Druart 2014-12-29 14:16:15 UTC
The pref AddPatronLists does not work as expected if 'generic' is selected.
Indeed a patron should be linked to a valid patron category (see the constraint borrowers_ibfk_1 in the DB structure).
Comment 1 Jonathan Druart 2014-12-29 14:22:30 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2015-02-17 15:29:57 UTC
Does not apply:

CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/patron-toolbar.inc
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Comment 3 Jonathan Druart 2015-02-19 11:02:20 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2015-02-19 20:15:55 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2015-03-03 08:36:29 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2015-03-03 08:37:57 UTC
Hi Jonathan, 

I am happy with the patch, but I am having some doubts if we understand how this was meant to work correctly. Would you mind sending a short email to the Koha mailing list asking people about removing it/a use case where this is needed for?
Comment 7 Jonathan Druart 2015-03-03 08:47:05 UTC
(In reply to Katrin Fischer from comment #6)
> Hi Jonathan, 
> 
> I am happy with the patch, but I am having some doubts if we understand how
> this was meant to work correctly. Would you mind sending a short email to
> the Koha mailing list asking people about removing it/a use case where this
> is needed for?

I am not sure it's necessary, the pref has been introduced by bug 1782, the default value is categorycode, and it does not work if category_type if defined.
Actually I think this pref should have never been created, and it does not make sense since you need to set the borrowers.categorycode with a valid category code.
Comment 8 Katrin Fischer 2015-03-03 09:23:09 UTC
I guess I am just worried I misunderstood how it should work - looked for other related bugs, but had not much luck.
Comment 9 Jonathan Druart 2015-04-13 14:44:55 UTC
Created attachment 37755 [details] [review]
Bug 13497: Remove the pref AddPatronsLists

The pref AddPatronLists does not work as expected if 'generic' is
selected.
Indeed a patron should be linked to a valid patron category (see the
constraint borrowers_ibfk_1 in the DB structure).

Test plan:
0/ Confirm that the AddPatronLists does not work as expected if you choose
the generic patron types.
1/ Delete all you patron categories and check that the interface invites
you to create one.
2/ Confirm that you are able to create a patron and to link it to a patron
category you have created.

I could not verify step 1/, because at least one Patron category
can not be deleted (Staff, since I'm logged in as a Staff patron
and you can not delete categories that are in use).
Tested 0/ and 2/
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and tests.
Comment 10 Tomás Cohen Arazi 2015-04-13 14:52:55 UTC
Created attachment 37758 [details] [review]
Bug 13497: (Maniac followup) spell category in english

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 11 Tomás Cohen Arazi 2015-04-13 14:56:04 UTC
Patch pushed to master.

Thanks Jonathan!