Bug 5788 - Move ReservesMaxPickupDelay system preference to the Circulation Matrix.
Summary: Move ReservesMaxPickupDelay system preference to the Circulation Matrix.
Status: CLOSED DUPLICATE of bug 5786
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: PATCH-Sent (DO NOT USE) enhancement (vote)
Assignee: Srdjan Jankovic
QA Contact: Bugs List
URL:
Keywords:
Depends on: 5786
Blocks: 5609
  Show dependency treegraph
 
Reported: 2011-02-21 20:08 UTC by Chris Cormack
Modified: 2018-02-08 08:52 UTC (History)
8 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
patch (6.82 KB, patch)
2011-07-22 07:24 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
patch (6.74 KB, patch)
2011-08-02 03:34 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
patch (7.62 KB, patch)
2012-03-23 04:47 UTC, Srdjan Jankovic
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2011-02-21 20:08:14 UTC

    
Comment 1 Srdjan Jankovic 2011-07-22 07:24:46 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2011-07-22 17:00:19 UTC
The same as bug 5786 - the new column is there, but I can't save or add any changes to the circ matrix anymore.

Also, if I read the code right, shouldn't there be an update database to remove the system preference?

Nicole
Comment 3 Nicole C. Engard 2011-07-22 17:02:31 UTC
Oops - I applied it wrong :)  Will try again.
Comment 4 Nicole C. Engard 2011-07-22 17:17:31 UTC
Okay, after applying properly I can't add a new rule anymore after this is applied. I also can't update existing rules the way I used to.

Nicole
Comment 5 Srdjan Jankovic 2011-07-27 06:17:38 UTC
Note: this has a database patch.
Be sure to edit the updatedatabase.pl version numbers to make it apply, or alternatively run
 ALTER TABLE issuingrules ADD reservesmaxpickupdelay INTEGER;
against the database
Comment 6 Nicole C. Engard 2011-08-01 13:14:40 UTC
I had done the database update the first time.  I tried again just now and I can edit rules, but I can't add new rules with this patch.
Comment 7 Srdjan Jankovic 2011-08-02 03:34:40 UTC Comment hidden (obsolete)
Comment 8 Paul Poulain 2011-08-02 03:54:25 UTC
this feature is also included in 5872 (Enhancements to circulation management)
5872 does a lot more than just ReservesMaxPickupDelay to the Circulation Matrix: it also manage how many holds can be placed, and hold rules (on shelf yes/no and hold for patron of this branch only).
Comment 9 Nicole C. Engard 2011-08-02 04:23:34 UTC
Srdjan,

The patch applies and the rules are added just fine now, but I'm not sure how to test that it's actually working. Can you give me instructions for testing? 

I set a rule and then I confirmed a hold on the item. I then went in to the DB and changed the waiting date to be after before the number of days I set the max pickup delay to. I then checked the holds waiting pick up report and it did not move the hold to the holds over tab - that to me says it's not working, but I might not be testing right.  A test plan would help me confirm that this works or doesn't work.

Nicole
Comment 10 Srdjan Jankovic 2011-08-02 04:44:55 UTC
My understanding is that 5872 is an umbrella for various circulation matrix changes. I am not sure how you want to manage it, test each feature separately or all together. I will try to find that out.
Comment 11 Paul Poulain 2011-08-02 08:07:42 UTC
(In reply to comment #10)
> My understanding is that 5872 is an umbrella for various circulation matrix
> changes. I am not sure how you want to manage it, test each feature separately
> or all together. I will try to find that out.

It's no more an umbrella (Ask chris for more details)
If you want to see what it does, look at comment 26, it describes everything
Comment 12 Paul Poulain 2011-08-02 08:09:30 UTC
oups comment 23, not 26
Comment 13 Paul Poulain 2011-10-24 11:38:12 UTC
Updating Version : This ENH will be for Koha 3.8
Comment 14 Paul Poulain 2011-10-25 15:05:47 UTC
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)
Comment 15 Katrin Fischer 2011-12-21 05:35:18 UTC
Patch still applies on current master.

Problems found:
- no change to kohastructure

Question:
- why was this line removed from an older update?

$dbh->do("UPDATE `systempreferences` SET type='Integer' WHERE variable='ReservesMaxPickupDelay'");
Comment 16 Srdjan Jankovic 2011-12-21 07:51:28 UTC
I understand the confusion, I was confused myself. The removed line is the only place in the code where ReservesMaxPickupDelay is mentioned. Hence nothing to change, and I removed the line.

As per bug 5786, if you want to test them one at the time, please decide on the order because first one to have patch pushed will invalidate testing of the other two.
Comment 17 Sophie MEYNIEUX 2012-02-03 16:40:18 UTC
The patch add the column "Holds max pickup delay" in issuing rules table.
I can add new rules but can't edit them
I didn't find how to test how this new value is used
Comment 18 Srdjan Jankovic 2012-02-06 00:50:38 UTC
Rules cannot be edited. They can only be deleted and reentered. It is a problem, and not the only one with the issuing rules if I nay add. But that is not part of this bug and should be raised as a separate bug.

As for the testing, this replaced "ReservesMaxPickupDelay" sys pref, so it should behave the same, only on the granular level.
Comment 19 Paul Poulain 2012-02-06 12:42:44 UTC
(In reply to comment #18)
> Rules cannot be edited. They can only be deleted and reentered. It is a
> problem, and not the only one with the issuing rules if I nay add. But that is
> not part of this bug and should be raised as a separate bug.
You're right.
There is something in biblibre/master that has been submitted in our very large patch about circulation rules, that is not outdated. In this patch, there is also something to modify a rule just by clicking on the number you want to modify.
For now, it's lost and only add/delete is possible
Comment 20 delaye 2012-03-13 14:03:58 UTC
I have the new column in the table of holds rules ==> ok
But when I return the copy who have been reserved
 this message
Software error:

Could not find a letter Called 'HOLD_PRINT' in the 'reserved' module at / home/koha/src/C4/Reserves.pm line 1740.

After  F5 the message disappears then the request is well recorded in table reserves but without expirationdate

select * from reserves where borrowernumber='7853'\G
*************************** 1. row ***************************
  borrowernumber: 7853
     reservedate: 2012-03-13
    biblionumber: 5
  constrainttype: a
      branchcode: MAURES
notificationdate: NULL
    reminderdate: NULL
cancellationdate: NULL
    reservenotes: 
        priority: 0
           found: W
       timestamp: 2012-03-13 14:47:03
      itemnumber: 67
     waitingdate: 2012-03-13
  expirationdate: NULL
  lowestPriority: 0


PS : HOLD_PRINT is in the table letter...
Comment 21 Srdjan Jankovic 2012-03-14 01:21:56 UTC
That's strange. can you please
* paste the output of "select * from letter where module='reserves' and code='HOLD_PRINT';"
* check what is in the apache error log after the first submission that throws an error

Because the notice is the last thing that gets done the hold is successful the first time, only the server error is thrown. reserves records are always created with expirationdate null
Comment 22 delaye 2012-03-14 12:58:10 UTC
log apache
[Tue Mar 13 14:44:35 2012] placerequest.pl: Use of uninitialized value in string eq at /home/koha/src/reserve/p
lacerequest.pl line 115.
[Tue Mar 13 14:44:35 2012] placerequest.pl: Use of uninitialized value $reqbib[0] in string ne at /home/koha/sr
c/reserve/placerequest.pl line 118.
[Tue Mar 13 14:44:36 2012] request.pl: Use of uninitialized value in string eq at /home/koha/src/reserve/reques
t.pl line 254.
[Tue Mar 13 14:44:36 2012] request.pl: Use of uninitialized value in string eq at /home/koha/src/reserve/reques
t.pl line 519.
[Tue Mar 13 14:44:48 2012] returns.pl: DBD::mysql::st execute failed: Unknown column 'branchcode' in 'where cla
use' at /home/koha/src/C4/Letters.pm line 133.
[Tue Mar 13 14:44:48 2012] returns.pl: DBD::mysql::st fetchrow_hashref failed: fetch() without execute() at /ho
me/koha/src/C4/Letters.pm line 134.
[Tue Mar 13 14:44:48 2012] returns.pl: No circulation CHECKIN letter at /home/koha/src/C4/Letters.pm line 436.
[Tue Mar 13 14:44:52 2012] returns.pl: DBD::mysql::st execute failed: Unknown column 'branchcode' in 'where cla
use' at /home/koha/src/C4/Letters.pm line 133.
[Tue Mar 13 14:44:52 2012] returns.pl: DBD::mysql::st fetchrow_hashref failed: fetch() without execute() at /ho
me/koha/src/C4/Letters.pm line 134.
[Tue Mar 13 14:44:52 2012] returns.pl: No reserves HOLD_PRINT letter at /home/koha/src/C4/Letters.pm line 436.
[Tue Mar 13 14:44:52 2012] returns.pl: Could not find a letter called 'HOLD_PRINT' in the 'reserves' module at 
/home/koha/src/C4/Reserves.pm line 1740.
[Tue Mar 13 14:46:38 2012] placerequest.pl: Use of uninitialized value in string eq at /home/koha/src/reserve/p
lacerequest.pl line 115.
[Tue Mar 13 14:46:38 2012] placerequest.pl: Use of uninitialized value $reqbib[0] in string ne at /home/koha/sr
c/reserve/placerequest.pl line 118.
[Tue Mar 13 14:46:39 2012] request.pl: Use of uninitialized value in string eq at /home/koha/src/reserve/reques

**************

output "select * from letter where module='reserves' and code='HOLD_PRINT';"

| reserves | HOLD_PRINT | Réservation disponible pour retrait (papier)  | Réservation disponible à la banque de retrait du site <<branches.branchname>>   | <<branches.branchname>>
<<branches.branchaddress1>>
<<branches.branchaddress2>>


Le service des réservations


<<borrowers.firstname>> <<borrowers.surname>>
<<borrowers.address>>
<<borrowers.city>> <<borrowers.zipcode>>


<<borrowers.firstname>> <<borrowers.surname>> <<borrowers.cardnumber>>

Votre réservation est disponible pour retrait depuis le <<reserves.waitingdate>>:

Titre: <<biblio.title>>
Auteur: <<biblio.author>>
Numéro de fascicule: <<items.copynumber>>
Comment 23 delaye 2012-03-15 13:11:33 UTC
after update database i test again the patch

i can't add a new rule in the table issuingrules (Administration koha/Rules Loan and hold)

log apach

[Thu Mar 15 14:04:51 2012] smart-rules.pl: DBD::mysql::st execute failed: Unknown column 'reservesmaxpickupdelay' in 'field list' at /home/koha/src/admin/smart-rules.pl line 132.
[Thu Mar 15 14:05:23 2012] smart-rules.pl: DBD::mysql::st execute failed: Unknown column 'reservesmaxpickupdelay' in 'field list' at /home/koha/src/admin/smart-rules.pl line 132.
Comment 24 delaye 2012-03-22 15:22:55 UTC
I can 't apply in a sandbox for test

i have this message

Applying: bug_5788: Moved ReservesMaxPickupDelay syspref to issue rules
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging admin/smart-rules.pl
Auto-merging installer/data/mysql/updatedatabase.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
Failed to merge in the changes.
Patch failed at 0001 bug_5788: Moved ReservesMaxPickupDelay syspref to issue rules
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, ins
Comment 25 Srdjan Jankovic 2012-03-23 04:47:06 UTC Comment hidden (obsolete)
Comment 26 delaye 2012-03-23 09:42:35 UTC
Falling back to patching base and 3-way merge...
Auto-merging admin/smart-rules.pl
CONFLICT (content): Merge conflict in admin/smart-rules.pl
Auto-merging installer/data/mysql/kohastructure.sql
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/admin/smart-rules.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
Failed to merge in the changes.
Comment 27 Srdjan Jankovic 2012-03-26 01:43:31 UTC
Can you please try again, I did not get any conflicts:

srdjan@ozzy:~/koha/dev ((88efb10...))$ git am attachment.cgi\?id\=8551
Applying: bug_5788: Added reservesmaxpickupdelay to circ matrix
Comment 28 delaye 2012-04-05 12:42:17 UTC
In the account of the patron who has reserved the book does not display the withdrawal deadline.
How do I know if the number of days specified in the holds rules is taken into account?
Comment 29 Srdjan Jankovic 2012-04-17 04:01:36 UTC
This bug was not for implementing the rule itself, but for moving ReservesMaxPickupDelay syspref to issuing_rules table. This rule has never been implemented.
Please correct me if this is wrong.
Comment 30 Owen Leonard 2012-07-18 19:55:26 UTC
Is this bug still valid? What does this patch do? Does it not work independently of some other enhancement?
Comment 31 Srdjan Jankovic 2012-07-18 23:50:01 UTC
It is sort of valid, probably needs rebasing, however this is being tested together with 5876 and 5787 by Bywater, and there will be a parch for all 3 of them together.
Comment 32 Melia Meggs 2012-07-24 23:33:23 UTC
A new patch coming soon for this!!  I want to provide test plans to facilitate community testing.

This patch moves ReservesMaxPickUpDelay from system preferences to the circ matrix.  It should behave the same way in the circ matrix as it currently behaves as a sys pref.

To test:

Set ReservesMaxPickUpDelay to 1
Place a hold
Check to make sure the item moves from the Holds Waiting tab to the Holds Over tab after 1 day
Comment 33 Srdjan Jankovic 2012-07-25 03:52:34 UTC
This bug will be resolved in unified patch on bug 5786
Comment 34 Srdjan Jankovic 2012-07-25 03:53:30 UTC
Comment on attachment 8551 [details] [review]
patch

Integrated with 5786, 5787
Comment 35 Julian Maurice 2012-08-06 14:28:30 UTC
As it was merged with bug 5786, I think the appropriate status is RESOLVED - DUPLICATE

*** This bug has been marked as a duplicate of bug 5786 ***