Bug 13497 - The pref AddPatronLists should be removed
Summary: The pref AddPatronLists should be removed
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 9314 13970
  Show dependency treegraph
 
Reported: 2014-12-29 14:16 UTC by Jonathan Druart
Modified: 2015-12-03 22:11 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13497: Remove the pref AddPatronsLists (8.89 KB, patch)
2014-12-29 14:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13497: Remove the pref AddPatronsLists (8.88 KB, patch)
2015-02-19 11:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
[Signed-off] Bug 13497: Remove the pref AddPatronsLists (9.22 KB, patch)
2015-02-19 20:15 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 13497: Remove the pref AddPatronsLists (9.30 KB, patch)
2015-03-03 08:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13497: Remove the pref AddPatronsLists (7.91 KB, patch)
2015-04-13 14:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13497: (Maniac followup) spell category in english (1.36 KB, patch)
2015-04-13 14:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!