Description
Jesse Weaver
2015-08-06 19:01:50 UTC
Created attachment 41643 [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. Created attachment 42145 [details] [review] Bug 14658 - Split PatronSelfRegistrationBorrowerUnwantedField into two preferences for creating and editing Tested and signed off, still needs sign-off from non-ByWater community member. Created attachment 43041 [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> 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.
> ::: 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
Created attachment 45847 [details] [review] Bug 14658 - (QA followup) make it easier to grep for these syspref names Should not we display the value (readonly) if it exists? (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). Created attachment 46074 [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> Created attachment 46075 [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> Created attachment 46235 [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> Created attachment 46236 [details] [review] Bug 14658 - (QA followup) make it easier to grep for these syspref names Created attachment 46237 [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> Created attachment 46238 [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> Sorry for spam, accidentally obliterated Joubu's signoffs. Created attachment 46352 [details] [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). (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? (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. (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? (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. Created attachment 47830 [details] [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). Signed-off-by: Nick Clemens <nick@bywatersolutions.com> 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 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. 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> 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> 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> Pushed to master! Should be in Koha 3.24. Thanks Jesse! |