Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing
Summary: Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for cr...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jesse Weaver
QA Contact: Testopia
URL:
Keywords:
Depends on: 15813
Blocks: 14659 21192
  Show dependency treegraph
 
Reported: 2015-08-06 19:01 UTC by Jesse Weaver
Modified: 2018-08-13 16:01 UTC (History)
6 users (show)

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


Attachments
Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing (6.59 KB, patch)
2015-08-18 22:34 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing (6.61 KB, patch)
2015-09-01 11:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing (6.64 KB, patch)
2015-10-01 23:36 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14658 - (QA followup) make it easier to grep for these syspref names (1.02 KB, patch)
2015-12-18 17:21 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing (6.85 KB, patch)
2015-12-30 13:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14658 - (QA followup) make it easier to grep for these syspref names (1.09 KB, patch)
2015-12-30 13:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing (6.74 KB, patch)
2016-01-04 21:51 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14658 - (QA followup) make it easier to grep for these syspref names (1.02 KB, patch)
2016-01-04 21:51 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing (6.81 KB, patch)
2016-01-04 21:53 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14658 - (QA followup) make it easier to grep for these syspref names (1.09 KB, patch)
2016-01-04 21:53 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14658: (followup) fix issue with list context on DBIC call (1.05 KB, patch)
2016-01-06 20:21 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14658: (followup) fix issue with list context on DBIC call (1.10 KB, patch)
2016-02-09 20:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing (6.89 KB, patch)
2016-02-15 16:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14658 - (QA followup) make it easier to grep for these syspref names (1.16 KB, patch)
2016-02-15 16:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14658: Remove warning if prefs are not defined (986 bytes, patch)
2016-02-15 16:50 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Weaver 2015-08-06 19:01:50 UTC

    
Comment 1 Jesse Weaver 2015-08-18 22:34:31 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2015-09-01 11:42:13 UTC Comment hidden (obsolete)
Comment 3 Jesse Weaver 2015-10-01 23:36:25 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-10-02 09:13:53 UTC
Comment on attachment 43041 [details] [review]
Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing

Review of attachment 43041 [details] [review]:
-----------------------------------------------------------------

::: opac/opac-memberentry.pl
@@ +244,4 @@
>      my %hidden_fields;
>  
>      my $BorrowerUnwantedField =
> +      C4::Context->preference( "PatronSelf" . ucfirst($action) . "BorrowerUnwantedField" );

As a grep fan, I would prefer to see the entire name of the prefs.
Comment 5 Katrin Fischer 2015-10-03 18:03:48 UTC
> ::: opac/opac-memberentry.pl
> @@ +244,4 @@
> >      my %hidden_fields;
> >  
> >      my $BorrowerUnwantedField =
> > +      C4::Context->preference( "PatronSelf" . ucfirst($action) . "BorrowerUnwantedField" );
> 
> As a grep fan, I would prefer to see the entire name of the prefs.

+1
Comment 6 Jesse Weaver 2015-12-18 17:21:02 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2015-12-23 10:55:18 UTC
Should not we display the value (readonly) if it exists?
Comment 8 Jesse Weaver 2015-12-29 20:49:13 UTC
(In reply to Jonathan Druart from comment #7)
> Should not we display the value (readonly) if it exists?

While this could be useful, I think it'd be surprising behavior given how the syspref currently works. Also, the bugs chaining off of this to allow patrons to enter their category or home library show up as readonly for existing patrons anyway (as you wouldn't want a patron to randomly change their library/category).
Comment 9 Jonathan Druart 2015-12-30 13:00:56 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2015-12-30 13:01:01 UTC Comment hidden (obsolete)
Comment 11 Jesse Weaver 2016-01-04 21:51:08 UTC Comment hidden (obsolete)
Comment 12 Jesse Weaver 2016-01-04 21:51:14 UTC Comment hidden (obsolete)
Comment 13 Jesse Weaver 2016-01-04 21:53:02 UTC Comment hidden (obsolete)
Comment 14 Jesse Weaver 2016-01-04 21:53:07 UTC Comment hidden (obsolete)
Comment 15 Jesse Weaver 2016-01-04 21:54:03 UTC
Sorry for spam, accidentally obliterated Joubu's signoffs.
Comment 16 Jesse Weaver 2016-01-06 20:21:25 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2016-01-08 10:08:30 UTC
(In reply to Jesse Weaver from comment #16)
> Created attachment 46352 [details] [review] [review]
> Bug 14658: (followup) fix issue with list context on DBIC call
> 
> While working on bug 14659, it was discovered that the call to
> ...->guarantor in list context will cause the hash to be misaligned
> (thus causing hidden => ... to not actually be sent to the template).

Could you move that to another bug report, to make it backportable to stable branches?
Comment 18 Jesse Weaver 2016-01-14 23:21:00 UTC
(In reply to Jonathan Druart from comment #17)
> (In reply to Jesse Weaver from comment #16)
> > Created attachment 46352 [details] [review] [review] [review]
> > Bug 14658: (followup) fix issue with list context on DBIC call
> > 
> > While working on bug 14659, it was discovered that the call to
> > ...->guarantor in list context will cause the hash to be misaligned
> > (thus causing hidden => ... to not actually be sent to the template).
> 
> Could you move that to another bug report, to make it backportable to stable
> branches?

I could, but this error doesn't really cause any issues unless you add something else to that $template->param call, which is only likely from an another enhancement. Not much benefit from backporting, in my opinion.
Comment 19 Owen Leonard 2016-01-15 17:13:03 UTC
(In reply to Jesse Weaver from comment #16)
> Created attachment 46352 [details] [review] [review]
> Bug 14658: (followup) fix issue with list context on DBIC call

Is this bug set to 'needs signoff' because of this follow-up? If so, is there a test plan for the follow-up?
Comment 20 Jesse Weaver 2016-01-15 22:47:54 UTC
(In reply to Owen Leonard from comment #19)
> (In reply to Jesse Weaver from comment #16)
> > Created attachment 46352 [details] [review] [review] [review]
> > Bug 14658: (followup) fix issue with list context on DBIC call
> 
> Is this bug set to 'needs signoff' because of this follow-up? If so, is
> there a test plan for the follow-up?

The followup was introduced to fix a bug caused by a change in master since this patch was originally submitted; the test plan should be the same as before.
Comment 21 Nick Clemens 2016-02-09 20:31:02 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2016-02-10 09:20:41 UTC
I would be happy to pass QA this one if the last patch is moved to another bug report (I will SO+QA it).
Comment 23 Mark Tompsett 2016-02-15 07:05:19 UTC
Comment on attachment 47830 [details] [review]
Bug 14658: (followup) fix issue with list context on DBIC call

This last patch was moved to a separate bug: bug 15813. Obsoleting this.
Comment 24 Jonathan Druart 2016-02-15 16:50:40 UTC
Created attachment 48069 [details] [review]
Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing

Test plan:
  1) Make sure there is at least one field in PatronSelfRegistrationBorrowerUnwantedField.
  2) Apply patch, and update database.
  3) Check to make sure that the new system preference
     PatronSelfModificationBorrowerUnwantedField has the same value as
     PatronSelfRegistrationBorrowerUnwantedField.
  4) Verify that the same fields are hidden for self-registering a new
     borrower and edting a new one (both on the OPAC).
  5) Change PatronSelfModificationBorrowerUnwantedField, and verify that
     the two preferences correctly apply to editing vs. creating.

Signed-off-by: Michael Sauers <msauers@dospace.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Jonathan Druart 2016-02-15 16:50:46 UTC
Created attachment 48070 [details] [review]
Bug 14658 - (QA followup) make it easier to grep for these syspref names

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 26 Jonathan Druart 2016-02-15 16:50:50 UTC
Created attachment 48071 [details] [review]
Bug 14658: Remove warning if prefs are not defined

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 27 Kyle M Hall 2016-02-26 13:53:36 UTC
Pushed to master! Should be in Koha 3.24. Thanks Jesse!