Bug 18818 - Manage categories displayed in Quick add patron list
Summary: Manage categories displayed in Quick add patron list
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-19 10:01 UTC by Baptiste
Modified: 2018-05-24 14:03 UTC (History)
12 users (show)

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


Attachments
Bug 18818: Display in quick add list now depends on a field in database (6.22 KB, patch)
2017-06-19 10:13 UTC, Baptiste
Details | Diff | Splinter Review
Bug 18818: Display in quick add list now depends on a field in database (6.33 KB, patch)
2018-01-22 16:18 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: (followup) correct template (3.91 KB, patch)
2018-01-22 16:39 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: Display in quick add list now depends on a field in database (6.43 KB, patch)
2018-01-26 13:50 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: (followup) correct template (3.91 KB, patch)
2018-01-26 13:50 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: Display in quick add list now depends on a field in database (7.71 KB, patch)
2018-01-26 14:27 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: (followup) correct template (3.91 KB, patch)
2018-01-26 14:27 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: Display in quick add list now depends on a field in database (7.73 KB, patch)
2018-01-26 15:27 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: (followup) correct template (3.91 KB, patch)
2018-01-26 15:27 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: define new column as boolean in DBIC Schema (947 bytes, patch)
2018-01-26 15:28 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: Display in quick add list now depends on a field in database (7.81 KB, patch)
2018-02-05 13:07 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: (followup) correct template (3.91 KB, patch)
2018-02-05 13:08 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: define new column as boolean in DBIC Schema (947 bytes, patch)
2018-02-05 13:08 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: Display in quick add list now depends on a field in database (7.91 KB, patch)
2018-03-14 13:57 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 18818: (followup) correct template (3.97 KB, patch)
2018-03-14 13:58 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 18818: define new column as boolean in DBIC Schema (1008 bytes, patch)
2018-03-14 13:58 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 18818: DBIC Schema changes (1.32 KB, patch)
2018-04-04 10:41 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18818: Display in quick add list now depends on a field in database (7.97 KB, patch)
2018-04-25 06:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18818: (follow-up) Correct template (4.03 KB, patch)
2018-04-25 06:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18818: Define new column as boolean in DBIC Schema (1.04 KB, patch)
2018-04-25 06:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18818: DBIC Schema changes (1.37 KB, patch)
2018-04-25 06:05 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Baptiste 2017-06-19 10:01:40 UTC
Some librarian would like to be able to manage the list displayed by clicking on members/members-home.pl => Quick add new patron
Comment 1 Baptiste 2017-06-19 10:13:43 UTC
Created attachment 64422 [details] [review]
Bug 18818: Display in quick add list now depends on a field in database

test plan:
1 - Apply patch
2 - Run update misc/devel/update_dbix_class_files.pl
3 - Run installer/data/mysql/updatedatabase.pl
4 - Check in Administation>patron categories there is a field named "Display in Quick add patron" (and check everything is set to "Display")
5 - Try to edit and check that the field exists and edits well the field
6 - Go to Patrons => quick add patron and check that the list displayed only consists in wished categories
Comment 2 Marc Véron 2017-06-21 09:39:21 UTC
Works fine.

There is a small capitalization issue (List view and edit)

Display
Don't Display  (should be: Don't display)

For readability, what about a change to the following in both list end edit form?
Show
Hide
Comment 3 Caroline Cyr La Rose 2017-09-08 20:47:23 UTC
I'm stuck at step 4.

I applied the patch and ran the two scripts update_dbix_class_files.pl and updatedatabase.pl. I then went into admin>patron categories and got a software error : 

Template process failed: undef error - The method quick_add_display is not covered by tests! at C4/Templates.pm line 121.
Comment 4 Fridolin Somers 2018-01-22 16:15:48 UTC
(In reply to Caroline Cyr La Rose from comment #3)
> I'm stuck at step 4.
> 
> I applied the patch and ran the two scripts update_dbix_class_files.pl and
> updatedatabase.pl. I then went into admin>patron categories and got a
> software error : 
> 
> Template process failed: undef error - The method quick_add_display is not
> covered by tests! at C4/Templates.pm line 121.

I add this message when dbix class files are not updated.
Be sure to have quick_add_display in Koha/Schema/Result/Category.pm.
If using Plack/Memcached, restart them.
Comment 5 Fridolin Somers 2018-01-22 16:18:30 UTC
Created attachment 70801 [details] [review]
Bug 18818: Display in quick add list now depends on a field in database

test plan:
1 - Apply patch
2 - Run update misc/devel/update_dbix_class_files.pl
3 - Run installer/data/mysql/updatedatabase.pl
4 - Check in Administation>patron categories there is a field named "Display in Quick add patron" (and check everything is set to "Display")
5 - Try to edit and check that the field exists and edits well the field
6 - Go to Patrons => quick add patron and check that the list displayed only consists in wished categories
Comment 6 Fridolin Somers 2018-01-22 16:39:18 UTC
Created attachment 70802 [details] [review]
Bug 18818: (followup) correct template

Main change is the use of Yes/No instead if Display/Don't display.
It is more compact and more explicit.

And fixes in categories.tt :
- Template Toolkit markup inside HTML in <select id="quick_add_display">
- Missing double-quotes in HTML attributes
- Missing value display in delete confirmation table
- Correct capitalization
Comment 7 Fridolin Somers 2018-01-22 16:44:06 UTC
(In reply to Marc Véron from comment #2)
> Works fine.
> 
> There is a small capitalization issue (List view and edit)
> 
> Display
> Don't Display  (should be: Don't display)
> 
> For readability, what about a change to the following in both list end edit
> form?
> Show
> Hide

I've chosen to be more simple with Display in quick add : Yes/No.

Some template fixes where needed.

See follow-up.
Comment 8 Fridolin Somers 2018-01-26 13:50:17 UTC
Created attachment 70955 [details] [review]
Bug 18818: Display in quick add list now depends on a field in database

This patch allows to manage the patron categories contained in 'Quick add new patron'.

Test plan:
1) Apply patch
2) Update database structure : installer/data/mysql/updatedatabase.pl
3) Update DBIx shema : misc/devel/update_dbix_class_files.pl
4) Check in 'Administation > Patron categories' there is a field named "Display in Quick add patron" (and check everything is set to "Display")
5) Try to edit and check that the field exists and edits well the field
6) Go to 'Patrons > quick add patron' and check that the list contains only wished categories
Comment 9 Fridolin Somers 2018-01-26 13:50:31 UTC
Created attachment 70956 [details] [review]
Bug 18818: (followup) correct template

Main change is the use of Yes/No instead if Display/Don't display.
It is more compact and more explicit.

And fixes in categories.tt :
- Template Toolkit markup inside HTML in <select id="quick_add_display">
- Missing double-quotes in HTML attributes
- Missing value display in delete confirmation table
- Correct capitalization
Comment 10 Fridolin Somers 2018-01-26 14:27:07 UTC
Created attachment 70959 [details] [review]
Bug 18818: Display in quick add list now depends on a field in database

This patch allows to manage the patron categories contained in 'Quick add new patron'.

Test plan:
1) Apply patch
2) Update database structure : installer/data/mysql/updatedatabase.pl
3) Update DBIx shema : misc/devel/update_dbix_class_files.pl
4) Check in 'Administation > Patron categories' there is a field named "Display in Quick add patron" (and check everything is set to "Display")
5) Try to edit and check that the field exists and edits well the field
6) Go to 'Patrons > quick add patron' and check that the list contains only wished categories
Comment 11 Fridolin Somers 2018-01-26 14:27:22 UTC
Created attachment 70960 [details] [review]
Bug 18818: (followup) correct template

Main change is the use of Yes/No instead if Display/Don't display.
It is more compact and more explicit.

And fixes in categories.tt :
- Template Toolkit markup inside HTML in <select id="quick_add_display">
- Missing double-quotes in HTML attributes
- Missing value display in delete confirmation table
- Correct capitalization
Comment 13 Fridolin Somers 2018-01-26 15:27:43 UTC
Created attachment 70962 [details] [review]
Bug 18818: Display in quick add list now depends on a field in database

This patch allows to manage the patron categories contained in 'Quick add new patron'.

Test plan:
1) Apply patch
2) Update database structure : installer/data/mysql/updatedatabase.pl
3) Update DBIx shema : misc/devel/update_dbix_class_files.pl
4) Check in 'Administation > Patron categories' there is a field named "Display in Quick add patron" (and check everything is set to "Display")
5) Try to edit and check that the field exists and edits well the field
6) Go to 'Patrons > quick add patron' and check that the list contains only wished categories
Comment 14 Fridolin Somers 2018-01-26 15:27:58 UTC
Created attachment 70963 [details] [review]
Bug 18818: (followup) correct template

Main change is the use of Yes/No instead if Display/Don't display.
It is more compact and more explicit.

And fixes in categories.tt :
- Template Toolkit markup inside HTML in <select id="quick_add_display">
- Missing double-quotes in HTML attributes
- Missing value display in delete confirmation table
- Correct capitalization
Comment 15 Fridolin Somers 2018-01-26 15:28:12 UTC
Created attachment 70964 [details] [review]
Bug 18818: define new column as boolean in DBIC Schema

See https://wiki.koha-community.org/wiki/Coding_Guidelines#SQL12:_Booleans
Comment 16 Owen Leonard 2018-01-26 17:54:41 UTC
I'm getting this error when I run updatedatabase.pl:


{UNKNOWN}: DBIx::Class::ResultSource::column_info(): No such column quick_add_display at /home/vagrant/kohaclone/Koha/Schema/Result/Category.pm line 273
Compilation failed in require at /usr/share/perl5/Class/C3/Componentised.pm line 150. at /usr/share/perl5/Class/C3/Componentised.pm line 155
Compilation failed in require at /home/vagrant/kohaclone/Koha/Database.pm line 50.
Comment 17 Fridolin Somers 2018-02-05 13:07:54 UTC
Created attachment 71227 [details] [review]
Bug 18818: Display in quick add list now depends on a field in database

This patch allows to manage the patron categories contained in 'Quick add new patron'.

Test plan:
1) Apply patches but not "define new column as boolean in DBIC Schema"
2) Update database structure : installer/data/mysql/updatedatabase.pl
3) Apply patch "define new column as boolean in DBIC Schema"
4) Update DBIx shema : misc/devel/update_dbix_class_files.pl
5) Go to Administation > Patron categories
6) Check there is a field named "Display in quick add" with "Yes"
7) Edit a category to set "No" and save
8) Check change is well saved
9) Go to "Patrons" and click on "Quick add patron"
10) Check the list contains only wanted categories
Comment 18 Fridolin Somers 2018-02-05 13:08:07 UTC
Created attachment 71228 [details] [review]
Bug 18818: (followup) correct template

Main change is the use of Yes/No instead if Display/Don't display.
It is more compact and more explicit.

And fixes in categories.tt :
- Template Toolkit markup inside HTML in <select id="quick_add_display">
- Missing double-quotes in HTML attributes
- Missing value display in delete confirmation table
- Correct capitalization
Comment 19 Fridolin Somers 2018-02-05 13:08:24 UTC
Created attachment 71229 [details] [review]
Bug 18818: define new column as boolean in DBIC Schema

See https://wiki.koha-community.org/wiki/Coding_Guidelines#SQL12:_Booleans
Comment 20 Fridolin Somers 2018-02-05 13:10:21 UTC
(In reply to Owen Leonard from comment #16)
> I'm getting this error when I run updatedatabase.pl:
> 
> 
> {UNKNOWN}: DBIx::Class::ResultSource::column_info(): No such column
> quick_add_display at /home/vagrant/kohaclone/Koha/Schema/Result/Category.pm
> line 273
> Compilation failed in require at /usr/share/perl5/Class/C3/Componentised.pm
> line 150. at /usr/share/perl5/Class/C3/Componentised.pm line 155
> Compilation failed in require at /home/vagrant/kohaclone/Koha/Database.pm
> line 50.

Indeed, I have changed test plan to apply last patch only after updatedatabase.
I could not find another bug dealing with this DBIx change.

This bug can then not be tested with a sandbox.
Comment 21 Fridolin Somers 2018-03-14 07:57:18 UTC
Because of test plan I think this can not be tested on a sandbox.
Comment 22 Biblibre Sandboxes 2018-03-14 13:57:49 UTC
Created attachment 72867 [details] [review]
Bug 18818: Display in quick add list now depends on a field in database

This patch allows to manage the patron categories contained in 'Quick add new patron'.

Test plan:
1) Apply patches but not "define new column as boolean in DBIC Schema"
2) Update database structure : installer/data/mysql/updatedatabase.pl
3) Apply patch "define new column as boolean in DBIC Schema"
4) Update DBIx shema : misc/devel/update_dbix_class_files.pl
5) Go to Administation > Patron categories
6) Check there is a field named "Display in quick add" with "Yes"
7) Edit a category to set "No" and save
8) Check change is well saved
9) Go to "Patrons" and click on "Quick add patron"
10) Check the list contains only wanted categories

Signed-off-by: JM Broust <jean-manuel.broust@univ-lyon2.fr>
Comment 23 Biblibre Sandboxes 2018-03-14 13:58:01 UTC
Created attachment 72868 [details] [review]
Bug 18818: (followup) correct template

Main change is the use of Yes/No instead if Display/Don't display.
It is more compact and more explicit.

And fixes in categories.tt :
- Template Toolkit markup inside HTML in <select id="quick_add_display">
- Missing double-quotes in HTML attributes
- Missing value display in delete confirmation table
- Correct capitalization

Signed-off-by: JM Broust <jean-manuel.broust@univ-lyon2.fr>
Comment 24 Biblibre Sandboxes 2018-03-14 13:58:14 UTC
Created attachment 72869 [details] [review]
Bug 18818: define new column as boolean in DBIC Schema

See https://wiki.koha-community.org/wiki/Coding_Guidelines#SQL12:_Booleans

Signed-off-by: JM Broust <jean-manuel.broust@univ-lyon2.fr>
Comment 25 jmbroust 2018-03-14 16:18:07 UTC
tested on sandbox, git applied with Fridolyn because cannot be applied via sandbox.
Comment 26 Katrin Fischer 2018-04-03 21:39:53 UTC
There is an error in the QA test tools. please fix!

 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/patron-toolbar.inc
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   FAIL	  valid_template
		The method quick_add_display is not covered by tests!
Comment 27 Fridolin Somers 2018-04-04 10:36:44 UTC
Ah ok its missing a patch for DBIX class files
Comment 28 Fridolin Somers 2018-04-04 10:41:53 UTC
Created attachment 73604 [details] [review]
Bug 18818: DBIC Schema changes
Comment 29 Katrin Fischer 2018-04-06 06:02:34 UTC
Hi Frido, can you please check again?

- applied patches
- ran updatedatabase
- restart_all

And I still have the same error in the QA tools :(
Comment 30 Katrin Fischer 2018-04-21 11:40:57 UTC
I've tried again, same result:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/patron-toolbar.inc
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   FAIL	  valid_template
		The method quick_add_display is not covered by tests!
Comment 31 Jonathan Druart 2018-04-24 13:43:44 UTC
(In reply to Katrin Fischer from comment #30)
> I've tried again, same result:
> 
>  FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/patron-toolbar.inc
>    OK	  forbidden patterns
>    OK	  git manipulation
>    OK	  spelling
>    OK	  tt_valid
>    FAIL	  valid_template
> 		The method quick_add_display is not covered by tests!

Hum, this looks like a false positive. If you do not recreate the problem using the interface I'd ignore it.
Comment 32 Katrin Fischer 2018-04-25 06:05:00 UTC
Created attachment 74831 [details] [review]
Bug 18818: Display in quick add list now depends on a field in database

This patch allows to manage the patron categories contained in 'Quick add new patron'.

Test plan:
1) Apply patches but not "define new column as boolean in DBIC Schema"
2) Update database structure : installer/data/mysql/updatedatabase.pl
3) Apply patch "define new column as boolean in DBIC Schema"
4) Update DBIx shema : misc/devel/update_dbix_class_files.pl
5) Go to Administation > Patron categories
6) Check there is a field named "Display in quick add" with "Yes"
7) Edit a category to set "No" and save
8) Check change is well saved
9) Go to "Patrons" and click on "Quick add patron"
10) Check the list contains only wanted categories

Signed-off-by: JM Broust <jean-manuel.broust@univ-lyon2.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 33 Katrin Fischer 2018-04-25 06:05:05 UTC
Created attachment 74832 [details] [review]
Bug 18818: (follow-up) Correct template

Main change is the use of Yes/No instead if Display/Don't display.
It is more compact and more explicit.

And fixes in categories.tt :
- Template Toolkit markup inside HTML in <select id="quick_add_display">
- Missing double-quotes in HTML attributes
- Missing value display in delete confirmation table
- Correct capitalization

Signed-off-by: JM Broust <jean-manuel.broust@univ-lyon2.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 34 Katrin Fischer 2018-04-25 06:05:10 UTC
Created attachment 74833 [details] [review]
Bug 18818: Define new column as boolean in DBIC Schema

See https://wiki.koha-community.org/wiki/Coding_Guidelines#SQL12:_Booleans

Signed-off-by: JM Broust <jean-manuel.broust@univ-lyon2.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 35 Katrin Fischer 2018-04-25 06:05:15 UTC
Created attachment 74834 [details] [review]
Bug 18818: DBIC Schema changes

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 36 Jonathan Druart 2018-04-25 14:14:18 UTC
1. when adding/editing a patron category, "Display in quick add: " is a bit meaningless, I would add a quick description, like we have for "Default privacy:", "Block expired patrons:", etc.

2. There is a condition ([% IF Koha.Preference('PatronQuickAddFields') || Koha.Preference('BorrowerMandatoryField') %]) template-side to display the "Quick add new patron" button, should we do the same on the patron's category form?


And a question: what is the use case here? It seems that it would have been done in css or javascript easily... :)
Comment 37 Fridolin Somers 2018-05-24 14:03:03 UTC
(In reply to Jonathan Druart from comment #36)
> 1. when adding/editing a patron category, "Display in quick add: " is a bit
> meaningless, I would add a quick description, like we have for "Default
> privacy:", "Block expired patrons:", etc.
OK

> 
> 2. There is a condition ([% IF Koha.Preference('PatronQuickAddFields') ||
> Koha.Preference('BorrowerMandatoryField') %]) template-side to display the
> "Quick add new patron" button, should we do the same on the patron's
> category form?
Indeed

> 
> 
> And a question: what is the use case here? It seems that it would have been
> done in css or javascript easily... :)
It is a request from Aix-Marseille University which has 10 patron categories.
I think its more efficient for them to have only the most-used categories in quick-add.
The actually have a JS customisation but it would be better to have it for everybody in code.