Implement a mechanism for removing unwanted fields from the entry screens. It will be a syspref, BorrowerUnwantedField, that will have the same format as BorrowerMandatoryField
Created attachment 3919 [details] [review] patch
*** Bug 3656 has been marked as a duplicate of this bug. ***
Created attachment 4004 [details] [review] Patch fixed to apply to master I attach a patch that will apply to current master. I had to fix the updatedatabase and the escaping in updatedatabase (patron''s) to make it work. Tested with: cardnumber|surname|firstname|title|othernames|initials|streetnumber|streettype|address|address2|city|state|zipcode|country|email|phone|mobile|fax|emailpro|phonepro|B_streetnumber|B_streettype|B_address|B_address2|B_city|B_state|B_zipcode|B_country|B_email|B_phone|dateofbirth|branchcode|categorycode|dateenrolled|dateexpiry|gonenoaddress|lost|debarred|contactname|contactfirstname|contacttitle|borrowernotes|relationship|sex|password|userid|opacnote|contactnote|sort1|sort2|altcontactfirstname|altcontactsurname|altcontactaddress1|altcontactaddress2|altcontactaddress3|altcontactstate|altcontactzipcode|altcontactcountry|altcontactphone Problems I see: - Salutation (title) can not be hidden - The patch is missing the translated sql files Possible improvements: - It works only for the edit screen, not for the patron details tab. Fields shoud be hidden there too. - When all fields from a block like 'Alternate contact' or 'Alternate address' are hidden, the empty block should not show up. - Patron account flags can not be hidden Note: - Category (categorycode) can not be hidden (probably by design)
Created attachment 4062 [details] [review] patch Corrected title removal. Also added whole group removal when appropriate. Left borrower flags, removal can cause some unpleasant situations. Removal should be specified and scoped separately if needed. Please retest.
Hi Srdjan, thx for your work on this :) I agree hiding the flags might cause problems if libraries for example choose to debar patrons automatically. I still have 2 problems with your patch: 1) The translated sql files are missing the system preference. Until we make all languages use the en file this is still needed so people installing in another language are not missing system preferences. 2) The feature to hide whole groups seems not complete and a little inconsistent to me: The condition to hide alternate contact is: + [% UNLESS noaltcontactsurname && noaltcontactfirstname %] So if you choose to hide the names to make it just another address field, the whole block will disappear. And is seems there is no way to hide the main address group. I actually have libraries saving only firstname, surname and email address of a patron, because they rely on another database for address information.
Created attachment 4071 [details] [review] patch Added translations, better hiding
(In reply to comment #6) > Created attachment 4071 [details] [review] > patch > > Added translations, better hiding As far as I can see, this patch still misses installer/data/mysql/nb-NO/1-Obligatorisk/sysprefs.sql. It will be a great new feature, though!
Created attachment 4080 [details] [review] patch nb-NO sysprefs test
The first patch (attachment 4071 [details] [review]) did not apply for me: Bug 6190 - Custom field removal from patron entry screens patch Apply? [yn] y Applying: bug_6190: BorrowerUnwantedField syspref and removing fields from the patron entry /home/oleonard/kohaclone/.git/rebase-apply/patch:733: trailing whitespace. $template->param( "no$_" => 1); error: patch failed: koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt:887 error: koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt: patch does not apply Patch failed at 0001 bug_6190: BorrowerUnwantedField syspref and removing fields from the patron entry
Created attachment 4337 [details] [review] patch
Marking this as failed QA for two reasons: 1. Hiding all fields in a group does not hide the whole group - see attached screenshot. 2. There is a problem with escaping in updatedatabase.pl - it looks like this: $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('BorrowerUnwantedField','','Name the fields you don\'t need to store for a patron\'s account',NULL,'free')"); should be this: $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('BorrowerUnwantedField','','Name the fields you don''t need to store for a patron''s account',NULL,'free')"); ('' instead of \') Can't wait to have this feature in place, though!
Created attachment 4481 [details] Empty groups still showing title and box This is what the patron entry screen looks like when BorrowerUnwantedField is set to cardnumber|surname|firstname|title|othernames|initials|streetnumber|streettype|address|address2|city|state|zipcode|country|email|phone|mobile|fax|emailpro|phonepro|B_streetnumber|B_streettype|B_address|B_address2|B_city|B_state|B_zipcode|B_country|B_email|B_phone|dateofbirth|branchcode|categorycode|dateenrolled|dateexpiry|gonenoaddress|lost|debarred|contactname|contactfirstname|contacttitle|borrowernotes|relationship|sex|password|userid|opacnote|contactnote|sort1|sort2|altcontactfirstname|altcontactsurname|altcontactaddress1|altcontactaddress2|altcontactaddress3|altcontactstate|altcontactzipcode|altcontactcountry|altcontactphone
Created attachment 4799 [details] [review] Bug 6190: Followup fixing empty groups - fixes quoting in database update - fixes appearance/disappearance of groups with no fields to be shown To test: - Enter database field names separated by | into systempreference BorrowerUnwantedField - Check if group/ box and heading are not displayed when all fields from that group are hidden using the system preference Example: othernames|initials|title|surname|firstname|sex|dateofbirth = Patron identity should be hidden othernames|initials|title|surname|firstname|sex = Group heading and date of birth field are shown
Created attachment 4800 [details] [review] bug_6190: BorrowerUnwantedField syspref and removing fields from the patron entry
For testing: apply first: bug_6190: BorrowerUnwantedField syspref and removing fields from the patron entry This is the rebased patch from Srdjan. apply second: Bug 6190: Followup fixing empty groups This is a follow up fixing the groups and quoting in the updatedatabase.
Srdjan, Can you please put notes in your patches about what they do? See Katrin's follow up for an example. This will help get your patches tested properly and signed off on quicker. Thanks a bunch! Nicole
Hi Nicole, you can apply both patches together and test as I described on the bug. The feature will allow to hide fields on the patron entry screen without using jquery, but using a syspref instead = BorrowerUnwantedField.
first patch does not apply: Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y Applying: bug_6190: BorrowerUnwantedField syspref and removing fields from the patron entry Using index info to reconstruct a base tree... <stdin>:789: trailing whitespace. $template->param( "no$_" => 1); warning: 1 line adds whitespace errors. Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/de-DE/mandatory/sysprefs.sql Auto-merging installer/data/mysql/en/mandatory/sysprefs.sql Auto-merging installer/data/mysql/fr-FR/1-Obligatoire/unimarc_standard_systemprefs.sql Auto-merging installer/data/mysql/it-IT/necessari/sysprefs.sql Auto-merging installer/data/mysql/nb-NO/1-Obligatorisk/sysprefs.sql CONFLICT (content): Merge conflict in installer/data/mysql/nb-NO/1-Obligatorisk/sysprefs.sql Auto-merging installer/data/mysql/pl-PL/mandatory/sysprefs.sql Auto-merging installer/data/mysql/ru-RU/mandatory/system_preferences_full_optimal_for_install_only.sql Auto-merging installer/data/mysql/uk-UA/mandatory/system_preferences_full_optimal_for_install_only.sql Auto-merging installer/data/mysql/updatedatabase.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt Auto-merging members/memberentry.pl Auto-merging xt/syspref.t CONFLICT (content): Merge conflict in xt/syspref.t Failed to merge in the changes. Patch failed at 0001 bug_6190: BorrowerUnwantedField syspref and removing fields from the patron entry When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". nengard@debian:~/kohaclone$
PS. I assumed that the first patch was the one that didn't say 'followup'
I remembered a little to late that I should attach the rebased first patch. Seems another syspref has been pushed and it needs to rebased again. There is also one small thing that I think is a mistake - the change to xt/syspref.t.
Created attachment 4976 [details] [review] patch
Looks like Srdjan sqashed in my follow-up, but not entirely sure as it's not easy to compare all changes.
Yes, I thought it was easier to keep just one patch
So what does this mean - I test the one patch? or does Katrin have to resubmit her follow up?
Just one patch, this one contains all relevant changes.
Applying: bug_6190: BorrowerUnwantedField syspref and removing fields from the patron entry /home/nengard/kohaclone/.git/rebase-apply/patch:348: trailing whitespace. [% END # hide fieldset %] /home/nengard/kohaclone/.git/rebase-apply/patch:522: trailing whitespace. [%END # hide fieldset %] /home/nengard/kohaclone/.git/rebase-apply/patch:841: trailing whitespace. $template->param( "no$_" => 1); error: patch failed: installer/data/mysql/updatedatabase.pl:4399 error: installer/data/mysql/updatedatabase.pl: patch does not apply Patch failed at 0001 bug_6190: BorrowerUnwantedField syspref and removing fields from the patron entry When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Patch left in /tmp/patch-u_pyV4.patch nengard@debian:~/kohaclone$
Tested applying with using git bz and it still doesn't apply: -------------------------- Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y Applying: bug_6190: BorrowerUnwantedField syspref and removing fields from the patron entry Using index info to reconstruct a base tree... <stdin>:348: trailing whitespace. [% END # hide fieldset %] <stdin>:522: trailing whitespace. [%END # hide fieldset %] <stdin>:841: trailing whitespace. $template->param( "no$_" => 1); warning: 3 lines add whitespace errors. Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt Auto-merging members/memberentry.pl Failed to merge in the changes. Patch failed at 0001 bug_6190: BorrowerUnwantedField syspref and removing fields from the patron entry When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". nengard@debian:~/kohaclone$
Created attachment 5308 [details] [review] patch
Sorry, this patch needs to be rebased and resubmitted again :(
Created attachment 5818 [details] [review] patch
Created attachment 5837 [details] [review] [SIGNED-OFF] Bug 6190: BorrowerUnwantedField syspref and removing fields from the patron entry Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tested with example from bug and by deleting parts of it: cardnumber|surname|firstname|title|othernames|initials|streetnumber|streettype|address|address2|city|state|zipcode|country|email|phone|mobile|fax|emailpro|phonepro|B_streetnumber|B_streettype|B_address|B_address2|B_city|B_state|B_zipcode|B_country|B_email|B_phone|dateofbirth|branchcode|categorycode|dateenrolled|dateexpiry|gonenoaddress|lost|debarred|contactname|contactfirstname|contacttitle|borrowernotes|relationship|sex|password|userid|opacnote|contactnote|sort1|sort2|altcontactfirstname|altcontactsurname|altcontactaddress1|altcontactaddress2|altcontactaddress3|altcontactstate|altcontactzipcode|altcontactcountry|altcontactphone Note: This only affects the edit screen, not the patron detail tab. A nice enhancement would be to make this page follow the system preference too.
Updating version : This ENH could be in Koha 3.8
Bug versionned for master. entries will be made against rel_3_8 once the patch has been applied (see thread about that on koha-devel yesterday)
Created attachment 6303 [details] [review] Signed patch Rebased signoff
Some QA remarks (not blocking approval): systempreferences.pl This script is now only used for Local use. Adding a line for Patrons is no longer useful. patrons.pref Just a practical question: How does a user know the name of a field to exclude, e.g. B_address2 ? Could you show the fields in a popup when clicking on a link or so? I realize that the same holds for BorrowerMandatoryField. Maybe a small follow up? memberentry.pl regex, use of o modifier: Perl will not recompile the pattern unless an interpolated variable that it contains changes. You can force Perl to skip the test and never recompile by adding a /o (which stands for "once") after the trailing delimiter. The regex here does not contain a variable. Actually, I wonder if you need this regex at all here. Thank you for work and numerous rebasing!
Hi Marcel, thx for QA :) We have the same problem for borrower import - the user does not know which fields correspond to which description in the user interface. Translation makes it even harder. I think the best we can do now is link to the database documentation - http://schema.koha-community.org/tables/borrowers.html
Patch pushed, please test Marcel & Katrin: i've added a link to the syspref description: The following <a href:...>database columns</a> will not appear on the patron entry screen it's a great idea !