http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6906
--- Comment #12 from Martin Renvoize <[email protected]> --- Comment on attachment 27558 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27558 Patch to implement described functionality Review of attachment 27558: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=6906&attachment=27558) ----------------------------------------------------------------- Pretty happy with this in general. Just a few minor points regarding style.. but then, I could be missing something ;) ::: installer/data/mysql/kohastructure.sql @@ +265,4 @@ > `altcontactphone` varchar(50) default NULL, -- the phone number for the > alternate contact for the patron/borrower > `smsalertnumber` varchar(50) default NULL, -- the mobile phone number > where the patron/borrower would like to receive notices (if SNS turned on) > `privacy` integer(11) DEFAULT '1' NOT NULL, -- patron/borrower's privacy > settings related to their reading history > + `checkprevissuebyborrower` varchar(7) NOT NULL default 'inherit', -- > produce a warning for this borrower if this item has previously been issued > to this borrower if 'yes', not if 'no', defer to category setting if > 'inherit'. I think I would lose the 'byborrower' part of 'checkprevissuebyborrower' here, I think 'checkprevissue' is descriptive enough, and we already know it's in the borrower context due to the table it's found in. @@ +466,5 @@ > `issuelimit` smallint(6) default NULL, -- unused in Koha > `reservefee` decimal(28,6) default NULL, -- cost to place holds > `hidelostitems` tinyint(1) NOT NULL default '0', -- are lost items shown > to this category (1 for yes, 0 for no) > + `category_type` varchar(1) NOT NULL default 'A', -- type of Koha patron > (Adult, Child, Professional, Organizational, Statistical, Staff), > + `checkprevissuebycategory` varchar(7) NOT NULL default 'inherit', -- > produce a warning for this borrower category if this item has previously been > issued to this borrower if 'yes', not if 'no', defer to category setting if > 'inherit'. Do you mean "defer to 'syspref' setting if inherit." here? I think I would lose the 'bycategory' part of 'checkprevissuebycategory' here, I think 'checkprevissue' is descriptive enough, and we already know it's in the category context due to the table it's found in. ::: installer/data/mysql/updatedatabase.pl @@ +8202,4 @@ > SetVersion($DBversion); > } > > +# FIXME: change $DBversion The 'normal' way of doing this now is to put '$DBversion = "3.15.00.XXX";' These happen all the time, so the RM and QA people are used to these types of conflicts ;) ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref @@ +50,4 @@ > class: multi > - (separate multiple choices with |) > - > + - pref: CheckPrevIssueByDefault As you can probably tell by now, I'm a fan of shorter, but still descriptive, names. Do we really need the 'ByDefault' here? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
