[Koha-bugs] [Bug 6906] show 'Borrower has previously issued $ITEM' alert on checkout

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6906

--- Comment #12 from Martin Renvoize <martin.renvoize at ptfs-europe.com> ---
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.