Bug 9296 - overduerules table needs restructuring to allow future extension
Summary: overduerules table needs restructuring to allow future extension
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Marcel de Rooy
URL: http://intranet/cgi-bin/koha/tools/ov...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-16 19:18 UTC by MJ Ray (software.coop)
Modified: 2018-07-16 20:47 UTC (History)
7 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 9296 - restructure overduerules table to allow future extension (18.24 KB, patch)
2012-12-17 02:39 UTC, MJ Ray (software.coop)
Details | Diff | Splinter Review
Bug 9296 - restructure overduerules table to allow future extension (19.48 KB, patch)
2013-02-28 16:18 UTC, MJ Ray (software.coop)
Details | Diff | Splinter Review
Bug 9296 - restructure overduerules table to allow future extension (19.53 KB, patch)
2013-03-06 13:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9296 - restructure overduerules table to allow future extension (19.55 KB, patch)
2013-06-03 07:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description MJ Ray (software.coop) 2012-12-16 19:18:08 UTC
The overduerules table almost hard-wired to three standard notices, with fields like delay1, delay2, delay3.  I want to expand it in the future, so I plan to change it to the following:

CREATE TABLE `overduerules` ( -- overdue notice status and triggers
  `overduerules_id` mediumint NOT NULL AUTO_INCREMENT,
  `branchcode` varchar(10) NOT NULL default '', -- foreign key from the branches table to define which branch this rule is for (if blank it's all libraries)
  `categorycode` varchar(10) NOT NULL default '', -- foreign key from the categories table to define which patron category this rule is for
  `delay` int(4) default NULL, -- number of days after the item is overdue that this notice is sent
  `letter` varchar(20) default NULL, -- foreign key from the letter table to define which notice should be sent
  `debarred` varchar(1) default 0, -- is the patron restricted when this notice is sent (1 for yes, 0 for no)
  `typecode` varchar(10) NOT NULL default '1', -- 1 2 3 for first second third, may be extended later
  PRIMARY KEY (`overduerules_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

I think the existing user interface and cronjob can be made to work with this structure pretty easily.  Patch to follow.
Comment 1 MJ Ray (software.coop) 2012-12-17 02:39:39 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2012-12-19 20:42:18 UTC
I attempted to fix the merge conflicts, but even after doing so I got the following errors when I ran misc/cronjobs/overdue_notices.pl

Global symbol "@overdue_rules" requires explicit package name at misc/cronjobs/overdue_notices.pl line 432.
Global symbol "@output_chunks" requires explicit package name at misc/cronjobs/overdue_notices.pl line 623.
Global symbol "$admin_email_address" requires explicit package name at misc/cronjobs/overdue_notices.pl line 641.

It also looks like some code is not treating it as an array of hashrefs, but as a hashref itself.
Comment 3 Kyle M Hall 2013-01-22 13:24:04 UTC
Ping! Any chance of a rebase for this one MJ?
Comment 4 MJ Ray (software.coop) 2013-02-28 16:18:52 UTC Comment hidden (obsolete)
Comment 5 MJ Ray (software.coop) 2013-02-28 16:21:50 UTC
rebased but I have not retested yet
Comment 6 Kyle M Hall 2013-03-06 13:37:01 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2013-06-03 07:22:54 UTC
Created attachment 18604 [details] [review]
Bug 9296 - restructure overduerules table to allow future extension

The overduerules table is rather hardcoded to three notices.  This
patch restructures it so we could have more in future, but should
not change anything noticeable yet.

To test:
1) edit Tools: Overdue notices
2) cause some notices to be sent
3) notice how everything behaves the same as before

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Marcel de Rooy 2013-06-03 07:23:22 UTC
Simple rebase for updatedatabase
Comment 9 Marcel de Rooy 2013-06-03 07:53:46 UTC
Like the idea of this patch!
A grep on delay1 shows me that there is still some work to do:

misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl:                max(overd
uerules.branchcode) as rulebranch, TO_DAYS(NOW())-TO_DAYS(date_due) as daysoverd
ue, delay1, delay2, delay3,
misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl:                AND ( (TO
_DAYS(NOW())-TO_DAYS(date_due) ) = delay1
misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl:        if ( $issue->{'da
ysoverdue'} == $issue->{'delay1'} ) {
t/db_dependent/lib/KohaTest/Overdues/GetBranchcodesWithOverdueRules.pm:delay1,
      letter1,       debarred1,

Probably same for delay2 etc.
Comment 10 Blou 2014-08-25 18:08:45 UTC
I'll continue MJ's work.
We have this feature coming in http://wiki.koha-community.org/wiki/Overdue_Notice_Enhancement and this fits nicely with #12771.

I see by the diff that there are still a lot of hardcoded 1s,2s and 3s, as well as 1..3.  The plan is to remove those constraints.
Comment 11 Blou 2015-07-31 18:36:31 UTC
Note to keep this alive:

The first step was to change the dependency with the mtt table.
Done in #13624

We have also have an overhaul of the UI and overduenotices.pl in #12772 (submitted soon after internal tests).
Comment 12 Katrin Fischer 2018-07-15 09:53:43 UTC
Are there some plans to continue here?
Comment 13 Blou 2018-07-16 12:45:29 UTC
We'd love to, and we'll certain support (with testing) anyone working on it or similar patch, but it won't be us (inLibro) doing the coding at this point I'm sorry to say.
Comment 14 Katrin Fischer 2018-07-16 20:47:43 UTC
Thx for the reply Philippe. Resetting assignee.