[Bug 27945] New: Limit the number of article requests per day per patron category
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Bug ID: 27945 Summary: Limit the number of article requests per day per patron category Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com We propose adding an "Article Request Limit" field to the patron category configuration. "Allow __ article requests per day" This limit would be enforced by querying the existing article requests in the database that have been placed during the day and allowing the patron to proceed or deny the request. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Change sponsored?|--- |Sponsored -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |agustinmoyano@theke.io |ity.org | CC| |tomascohen@gmail.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=26129 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27946 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27946 [Bug 27946] Add a charge per article request to patron categories configuration -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #1 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 123217 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123217&action=edit Bug 27945: Add tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #2 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 123218 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123218&action=edit Bug 27945: Add article_request_limit in categories table -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #3 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 123219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123219&action=edit Bug 27945: [DO NOT PUSH] Update DBIx schema -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #4 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 123220 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123220&action=edit Bug 27945: Add limit article request feature This patch makes it possible to limit article requests per patron per day. To test: 1. Apply patches 2. updatedatabase 3. Enable ArticleRequests preference 4. Edit a patron category and set an article request limit to 1 CHECK => if you set the limit to anything else but a positive number or empty string, a warning appears 5. In staff search biblios and request an article for a patron of the modified category 6. Repeat step 5 SUCCESS => if limit is reached, when you select the user to request an article a warning appears saying that the limit was reached 7. Repeat steps 5 and 6 but this time in opac SUCCESS => Patron is not allowed to request another article if limit is reached 8. prove t/db_dependent/ArticleRequests.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Maybe 'article_request_daily_limit'? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Base does not apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Needs Signoff |Failed QA --- Comment #7 from David Nind <david@davidnind.com> --- Hi Agustín. There are errors when running the database update: root@kohadevbox:koha(bz27945)$ updatedatabase DEV atomic update: bug_27945.perl Atomic update generated errors: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Unknown column 'article_request_fee' in 'categories' [for Statement " ALTER TABLE `categories` ADD COLUMN `article_request_limit` tinyint(4) DEFAULT NULL COMMENT 'Limit article scan requests per day' AFTER `article_request_fee` "] at /usr/share/perl5/DBIx/Class/Schema.pm line 1118. DBIx::Class::Schema::throw_exception(Koha::Schema=HASH(0x55c0aed443e8), "DBI Exception: DBD::mysql::db do failed: Unknown column 'arti"...) called at /usr/share/perl5/DBIx/Class/Storage.pm line 113 DBIx::Class::Storage::throw_exception(DBIx::Class::Storage::DBI::mysql=HASH(0x55c0b29ba4b0), "DBI Exception: DBD::mysql::db do failed: Unknown column 'arti"...) called at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1501 DBIx::Class::Storage::DBI::__ANON__("DBD::mysql::db do failed: Unknown column 'article_request_fee"..., DBI::db=HASH(0x55c0b4469f90), undef) called at (eval 1487) line 7 eval '$DBversion = \'XXX\'; # will be replaced by the RM if( CheckVersion( $DBversion ) ) { # you can use $dbh here like: # $dbh->do( "ALTER TABLE biblio ADD COLUMN badtaste int" ); # or perform some test and warn if( !column_exists( \'categories\', \'article_request_limit\' ) ) { $dbh->do(q{ ALTER TABLE `categories` ADD COLUMN `article_request_limit` tinyint(4) DEFAULT NULL COMMENT \'Limit article scan requests per day\' AFTER `article_request_fee` }); } # Always end with this (adjust the bug info) NewVersion( $DBversion, 27945, "Add article_request_limit to categories table"); } ' called at /kohadevbox/koha/installer/data/mysql/updatedatabase.pl line 24596 DEV atomic update: bug_27946.perl Upgrade to XXX done [20:29:34]: Bug 27946 - Add article_request_fee to categories table, add debit_line_id column to article_requests table and add ARTICLE_REQUEST as a debit type -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to David Nind from comment #7)
Hi Agustín.
There are errors when running the database update:
Hi David, thanks for taking the time to test this! I've just applied bug 27946 and then this one, and the database update ran smoothly. Please try again! It seems that you didn't have 27946 DB upgrade applied. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- On top of 27946: Applying: Bug 27945: Add tests Using index info to reconstruct a base tree... M t/db_dependent/ArticleRequests.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/ArticleRequests.t CONFLICT (content): Merge conflict in t/db_dependent/ArticleRequests.t error: Failed to merge in the changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #9)
On top of 27946:
Applying: Bug 27945: Add tests Using index info to reconstruct a base tree... M t/db_dependent/ArticleRequests.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/ArticleRequests.t CONFLICT (content): Merge conflict in t/db_dependent/ArticleRequests.t error: Failed to merge in the changes.
Wait thats not correct -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #10)
(In reply to Marcel de Rooy from comment #9)
On top of 27946:
Applying: Bug 27945: Add tests Using index info to reconstruct a base tree... M t/db_dependent/ArticleRequests.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/ArticleRequests.t CONFLICT (content): Merge conflict in t/db_dependent/ArticleRequests.t error: Failed to merge in the changes.
Wait thats not correct
It does apply correctly. Wrong base.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- FAIL opac/opac-request-article.pl OK critic FAIL forbidden patterns forbidden pattern: Use of Data::Printer, should only be used for devlopment purpose (line 85) OK git manipulation OK pod SKIP spelling FAIL valid Can't locate Data/Printer.pm in @INC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123220|0 |1 is obsolete| | --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 123714 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123714&action=edit Bug 27945: Add limit article request feature This patch makes it possible to limit article requests per patron per day. To test: 1. Apply patches 2. updatedatabase 3. Enable ArticleRequests preference 4. Edit a patron category and set an article request limit to 1 CHECK => if you set the limit to anything else but a positive number or empty string, a warning appears 5. In staff search biblios and request an article for a patron of the modified category 6. Repeat step 5 SUCCESS => if limit is reached, when you select the user to request an article a warning appears saying that the limit was reached 7. Repeat steps 5 and 6 but this time in opac SUCCESS => Patron is not allowed to request another article if limit is reached 8. prove t/db_dependent/ArticleRequests.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 123715 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123715&action=edit Bug 27945: (QA follow-up) Fix exception POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123217|0 |1 is obsolete| | --- Comment #15 from David Nind <david@davidnind.com> --- Created attachment 123774 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123774&action=edit Bug 27945: Add tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123218|0 |1 is obsolete| | --- Comment #16 from David Nind <david@davidnind.com> --- Created attachment 123775 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123775&action=edit Bug 27945: Add article_request_limit in categories table Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123219|0 |1 is obsolete| | --- Comment #17 from David Nind <david@davidnind.com> --- Created attachment 123776 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123776&action=edit Bug 27945: [DO NOT PUSH] Update DBIx schema Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123714|0 |1 is obsolete| | --- Comment #18 from David Nind <david@davidnind.com> --- Created attachment 123777 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123777&action=edit Bug 27945: Add limit article request feature This patch makes it possible to limit article requests per patron per day. To test: 1. Apply patches 2. updatedatabase 3. Enable ArticleRequests preference 4. Edit a patron category and set an article request limit to 1 CHECK => if you set the limit to anything else but a positive number or empty string, a warning appears 5. In staff search biblios and request an article for a patron of the modified category 6. Repeat step 5 SUCCESS => if limit is reached, when you select the user to request an article a warning appears saying that the limit was reached 7. Repeat steps 5 and 6 but this time in opac SUCCESS => Patron is not allowed to request another article if limit is reached 8. prove t/db_dependent/ArticleRequests.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123715|0 |1 is obsolete| | --- Comment #19 from David Nind <david@davidnind.com> --- Created attachment 123778 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123778&action=edit Bug 27945: (QA follow-up) Fix exception POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement lets you release notes| |limit the number of article | |requests a patron can make | |each day. Edit the patron | |category and enter the | |'Maximum article scan | |requests'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #20 from David Nind <david@davidnind.com> --- One minor thing I noted after signing off. The column heading in patron categories is 'Maximum request limit' and the edit form says 'Maximum article scan requests'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #21 from Agustín Moyano <agustinmoyano@theke.io> --- (In reply to David Nind from comment #20)
One minor thing I noted after signing off.
The column heading in patron categories is 'Maximum request limit' and the edit form says 'Maximum article scan requests'.
Thanks David, I'll attach a follow-up -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #22 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 123820 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123820&action=edit Bug 27945: (follow-up) Fix column header title and edit form label -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- FAIL Koha/Exceptions/ArticleRequest.pm FAIL pod coverage POD is missing for 'description' False negative -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Koha::Patron + my $count = Koha::ArticleRequests->search({ + borrowernumber => $self->borrowernumber, + status => {'!=' => 'CANCELED'}, + created_on => { + '>=' => $date->date.' 00:00:00', + '<=' => $date->date.' 23:59:59' + } + })->count; We should use Koha::Database->new->schema->storage->datetime_parser. Look in the same modules for more examples. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Side note Currently we use the following custom check to limit numbers: my $compdate = dt_from_string->add( days => -1 ); my $count = Koha::ArticleRequests->search([ { borrowernumber => $borrowernumber, status => ['PENDING','PROCESSING'] }, { borrowernumber => $borrowernumber, status => 'COMPLETED', updated_on => { '>', Koha::Database->new->schema->storage->datetime_parser->format_date($compdate) }}, ])->count; This does not take into consideration the new/requested and pending status. So pending is still new here. But it looks at the number of requests in the pipeline for the user PLUS the completed ones within the last 24 hours. Advantage illustrated by the example below: Proposed check: Say that the limit is 5. User submits 5 requests on Friday night, Saturday, Sunday and Monday morning. This makes that we need to process 20 requests coming in on Monday. "Custom" check: User may only submit 5 requests on Friday night if we did not complete requests during that day. He is not allowed any more requests during the weekend. Only when we complete say 1 request on Monday, he is allowed to submit another one on Tuesday (24 hours later). What do you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- diff --git a/Koha/ArticleRequest.pm b/Koha/ArticleRequest.pm index e79e01610c..078f00bc70 100644 --- a/Koha/ArticleRequest.pm +++ b/Koha/ArticleRequest.pm @@ -27,7 +27,9 @@ use Koha::Items; use Koha::Libraries; use Koha::DateUtils qw( dt_from_string ); use C4::Context; +use Koha::ArticleRequests; Why do you add it here? Note that bug 27944 adds the Status module. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- } elsif ($article_request_limit ne '' && $article_request_limit !~ /\d+/) { If I add 2a in the limit field (fool proof), the form says that I successfully updated the record. But actually I didnt. Your regex could include ^\d+$ ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #28 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Overall: Looks good to me. Discussion item in comment25. Needs a few small adjustments for comment24, comment26 and comment27. Note that the dependency on 27946 is unfortunate. This could reach master sooner than 27946 which has a dependency on unfinished Accounting work. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123774|0 |1 is obsolete| | Attachment #123775|0 |1 is obsolete| | Attachment #123776|0 |1 is obsolete| | Attachment #123777|0 |1 is obsolete| | Attachment #123778|0 |1 is obsolete| | Attachment #123820|0 |1 is obsolete| | --- Comment #29 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124066 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124066&action=edit Bug 27945: Add tests Signed-off-by: David Nind <david@davidnind.com> Current depends: -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #30 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124067 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124067&action=edit Bug 27945: Add article_request_limit in categories table Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #31 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124068 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124068&action=edit Bug 27945: [DO NOT PUSH] Update DBIx schema Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #32 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124069 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124069&action=edit Bug 27945: Add limit article request feature This patch makes it possible to limit article requests per patron per day. To test: 1. Apply patches 2. updatedatabase 3. Enable ArticleRequests preference 4. Edit a patron category and set an article request limit to 1 CHECK => if you set the limit to anything else but a positive number or empty string, a warning appears 5. In staff search biblios and request an article for a patron of the modified category 6. Repeat step 5 SUCCESS => if limit is reached, when you select the user to request an article a warning appears saying that the limit was reached 7. Repeat steps 5 and 6 but this time in opac SUCCESS => Patron is not allowed to request another article if limit is reached 8. prove t/db_dependent/ArticleRequests.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #33 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124070 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124070&action=edit Bug 27945: (QA follow-up) Fix exception POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #34 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124071 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124071&action=edit Bug 27945: (follow-up) Fix column header title and edit form label -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|27946 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27946 [Bug 27946] Add a charge per article request to patron categories configuration -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124066|0 |1 is obsolete| | Attachment #124067|0 |1 is obsolete| | Attachment #124068|0 |1 is obsolete| | Attachment #124069|0 |1 is obsolete| | Attachment #124070|0 |1 is obsolete| | Attachment #124071|0 |1 is obsolete| | --- Comment #35 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124072 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124072&action=edit Bug 27945: Add tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #36 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124073 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124073&action=edit Bug 27945: Add article_request_limit in categories table Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #37 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124074 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124074&action=edit Bug 27945: [DO NOT PUSH] Update DBIx schema Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #38 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124075 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124075&action=edit Bug 27945: Add limit article request feature This patch makes it possible to limit article requests per patron per day. To test: 1. Apply patches 2. updatedatabase 3. Enable ArticleRequests preference 4. Edit a patron category and set an article request limit to 1 CHECK => if you set the limit to anything else but a positive number or empty string, a warning appears 5. In staff search biblios and request an article for a patron of the modified category 6. Repeat step 5 SUCCESS => if limit is reached, when you select the user to request an article a warning appears saying that the limit was reached 7. Repeat steps 5 and 6 but this time in opac SUCCESS => Patron is not allowed to request another article if limit is reached 8. prove t/db_dependent/ArticleRequests.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #39 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124076 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124076&action=edit Bug 27945: (QA follow-up) Fix exception POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #40 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124077 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124077&action=edit Bug 27945: (follow-up) Fix column header title and edit form label -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #41 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124078 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124078&action=edit Bug 27945: (follow-up) fix limit check in Patron.pm, regexp in categories.pl and removed unwanted use Koha::ArticleRequests in ArticleRequest.pm -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124078|0 |1 is obsolete| | --- Comment #42 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124079 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124079&action=edit Bug 27945: (follow-up) fix limit check in Patron.pm, regexp in categories.pl and removed unwanted use Koha::ArticleRequests in ArticleRequest.pm -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #43 from Agustín Moyano <agustinmoyano@theke.io> --- Hi Marcel, I removed dependency on bug 27946, implemented the check in comment 25 (with a minor modification), and addressed issues in comment 26 (that I believe it was due to a merge when I initially rebased 27946) and comment 27 Could you check it again? Thanks PS: I believe we should change texts in category form and patron's alert, because we no longer limit requests per day, but we limit "concurrent" requests per day -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #44 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #45 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Agustín Moyano from comment #43)
Hi Marcel,
I removed dependency on bug 27946, implemented the check in comment 25 (with a minor modification), and addressed issues in comment 26 (that I believe it was due to a merge when I initially rebased 27946) and comment 27
Could you check it again?
Will do. Looks good at first glance.
PS: I believe we should change texts in category form and patron's alert, because we no longer limit requests per day, but we limit "concurrent" requests per day
Good point. The term "Maximum article request limit" could still be improved. Can understand that it is hard to get the complete meaning into a short phrase. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #46 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- There is a functional dependency between this report and 27944. See bug 27944#68 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #47 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + { borrowernumber => $self->borrowernumber, status => { '!=' => ['CANCELED','COMPLETED'] } }, [2021/08/31 12:59:03] [WARN] SQL::Abstract::belch(): [SQL::Abstract::_where_field_op_ARRAYREF] Warning: A multi-element arrayref as an argument to the inequality op '!=' is technically equivalent to an always-true 1=1 (you probably wanted to say ...{ $inequality_op => [ -and => @values ] }... instead) at /usr/share/koha/Koha/Objects.pm line 601 Will fix this in a follow-up. No worries. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This enhancement lets you |This enhancement lets you release notes|limit the number of article |limit the number of active |requests a patron can make |article requests a patron |each day. Edit the patron |can make each day. Edit the |category and enter the |patron category and enter |'Maximum article scan |the 'Maximum active article |requests'. |requests'. Summary|Limit the number of article |Limit the number of active |requests per day per patron |article requests per patron |category |category -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #48 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #45)
(In reply to Agustín Moyano from comment #43)
PS: I believe we should change texts in category form and patron's alert, because we no longer limit requests per day, but we limit "concurrent" requests per day
Good point. The term "Maximum article request limit" could still be improved. Can understand that it is hard to get the complete meaning into a short phrase.
What about "active article requests" ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #49 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124284 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124284&action=edit Bug 27945: (QA follow-up) Fix limit check Resolve: [WARN] SQL::Abstract::belch(): [SQL::Abstract::_where_field_op_ARRAYREF] Warning: A multi-element arrayref as an argument to the inequality op '!=' is technically equivalent to an always-true 1=1 (you probably wanted to say ...{ $inequality_op => [ -and => @values ] }... instead) at /usr/share/koha/Koha/Objects.pm line 601 In this case we can simply count the statuses PENDING and PROCESSING. Note that bug 27944 will change that; PENDING should be replaced by REQUESTED there. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #50 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124285 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124285&action=edit Bug 27945: (QA follow-up) Terminology The term "Maximum article request limit" could be improved. In this patch we use 'Article request limit' to describe it more generic. On the field we say 'Maximum active article requests'. In the manual we need to explain that it is the sum of requests in Pending (Requested) or Processing stage and completed requests within the last 24 hours. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Failed QA --- Comment #51 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- We still need one thing imo: Fix the added tests for the adjusted check. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Manual -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #52 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 124633 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124633&action=edit Bug 27945: (follow-up) Fix tests for the adjusted check -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124072|0 |1 is obsolete| | --- Comment #53 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124673 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124673&action=edit Bug 27945: Add tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124073|0 |1 is obsolete| | --- Comment #54 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124674 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124674&action=edit Bug 27945: Add article_request_limit in categories table Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124074|0 |1 is obsolete| | --- Comment #55 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124675 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124675&action=edit Bug 27945: [DO NOT PUSH] Update DBIx schema Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124075|0 |1 is obsolete| | --- Comment #56 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124676 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124676&action=edit Bug 27945: Add limit article request feature This patch makes it possible to limit article requests per patron per day. To test: 1. Apply patches 2. updatedatabase 3. Enable ArticleRequests preference 4. Edit a patron category and set an article request limit to 1 CHECK => if you set the limit to anything else but a positive number or empty string, a warning appears 5. In staff search biblios and request an article for a patron of the modified category 6. Repeat step 5 SUCCESS => if limit is reached, when you select the user to request an article a warning appears saying that the limit was reached 7. Repeat steps 5 and 6 but this time in opac SUCCESS => Patron is not allowed to request another article if limit is reached 8. prove t/db_dependent/ArticleRequests.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124076|0 |1 is obsolete| | --- Comment #57 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124677&action=edit Bug 27945: (QA follow-up) Fix exception POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124077|0 |1 is obsolete| | --- Comment #58 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124678 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124678&action=edit Bug 27945: (follow-up) Fix column header title and edit form label Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124079|0 |1 is obsolete| | --- Comment #59 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124679&action=edit Bug 27945: (follow-up) fix limit check in Patron.pm, regexp in categories.pl and removed unwanted use Koha::ArticleRequests in ArticleRequest.pm Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124284|0 |1 is obsolete| | --- Comment #60 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124680 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124680&action=edit Bug 27945: (QA follow-up) Fix limit check Resolve: [WARN] SQL::Abstract::belch(): [SQL::Abstract::_where_field_op_ARRAYREF] Warning: A multi-element arrayref as an argument to the inequality op '!=' is technically equivalent to an always-true 1=1 (you probably wanted to say ...{ $inequality_op => [ -and => @values ] }... instead) at /usr/share/koha/Koha/Objects.pm line 601 In this case we can simply count the statuses PENDING and PROCESSING. Note that bug 27944 will change that; PENDING should be replaced by REQUESTED there. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124285|0 |1 is obsolete| | --- Comment #61 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124681 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124681&action=edit Bug 27945: (QA follow-up) Terminology The term "Maximum article request limit" could be improved. In this patch we use 'Article request limit' to describe it more generic. On the field we say 'Maximum active article requests'. In the manual we need to explain that it is the sum of requests in Pending (Requested) or Processing stage and completed requests within the last 24 hours. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124633|0 |1 is obsolete| | --- Comment #62 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124682 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124682&action=edit Bug 27945: (follow-up) Fix tests for the adjusted check Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #63 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Agustín Moyano from comment #52)
Created attachment 124633 [details] [review] Bug 27945: (follow-up) Fix tests for the adjusted check
Thx Agustin. Looks good to me now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |jonathan.druart+koha@gmail. | |com --- Comment #64 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- If I apply patches from this bug (27945) and the ones from bug 27944 I am getting the following failures: t/db_dependent/ArticleRequests.t .. 22/55 # Failed test 'Limit is 1, so patron cannot request more articles' # at t/db_dependent/ArticleRequests.t line 292. # got: '1' # expected: '0' # Failed test 'There is still an AR, so patron cannot request more articles' # at t/db_dependent/ArticleRequests.t line 309. # got: '1' # expected: '0' # Failed test 'There is still one article request' # at t/db_dependent/ArticleRequests.t line 310. # got: '2' # expected: '1' # Failed test 'A new AR was created, so patron cannot request more articles' # at t/db_dependent/ArticleRequests.t line 329. # got: '1' # expected: '0' # Failed test 'There are 2 article requests' # at t/db_dependent/ArticleRequests.t line 330. # got: '3' # expected: '2' # Failed test 'There is an old AR but not completed or cancelled, so patron cannot request more articles' # at t/db_dependent/ArticleRequests.t line 347. # got: '1' # expected: '0' # Failed test 'There are 3 current article requests' # at t/db_dependent/ArticleRequests.t line 348. # got: '4' # expected: '3' # Looks like you planned 13 tests but ran 12. # Looks like you failed 7 tests of 12 run. t/db_dependent/ArticleRequests.t .. 55/55 # Failed test 'article request limit' # at t/db_dependent/ArticleRequests.t line 350. There are 3 PQA bugs related to article requests (bug 27944, bug 27945 and bug 27947). Please make sure they don't conflict with each others and that the tests pass. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27944 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27944 [Bug 27944] Add new stages to the article request process -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA --- Comment #65 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Tests pass on its own, I've now set bugs dependencies, so if other bug breaks this, we will treat it in the specific bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|27944 | Depends on| |27944 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27944 [Bug 27944] Add new stages to the article request process -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124673|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124674|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124675|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124676|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124677|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124678|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124679|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124680|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124681|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124682|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #66 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125140 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125140&action=edit Bug 27945: Add tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #67 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125142 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125142&action=edit Bug 27945: Add article_request_limit in categories table Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #68 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125143 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125143&action=edit Bug 27945: [DO NOT PUSH] Update DBIx schema Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #69 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125144 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125144&action=edit Bug 27945: Add limit article request feature This patch makes it possible to limit article requests per patron per day. To test: 1. Apply patches 2. updatedatabase 3. Enable ArticleRequests preference 4. Edit a patron category and set an article request limit to 1 CHECK => if you set the limit to anything else but a positive number or empty string, a warning appears 5. In staff search biblios and request an article for a patron of the modified category 6. Repeat step 5 SUCCESS => if limit is reached, when you select the user to request an article a warning appears saying that the limit was reached 7. Repeat steps 5 and 6 but this time in opac SUCCESS => Patron is not allowed to request another article if limit is reached 8. prove t/db_dependent/ArticleRequests.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #70 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125145 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125145&action=edit Bug 27945: (QA follow-up) Fix exception POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #71 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125146 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125146&action=edit Bug 27945: (follow-up) Fix column header title and edit form label Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #72 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125147 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125147&action=edit Bug 27945: (follow-up) fix limit check in Patron.pm, regexp in categories.pl and removed unwanted use Koha::ArticleRequests in ArticleRequest.pm Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #73 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125148 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125148&action=edit Bug 27945: (QA follow-up) Fix limit check Resolve: [WARN] SQL::Abstract::belch(): [SQL::Abstract::_where_field_op_ARRAYREF] Warning: A multi-element arrayref as an argument to the inequality op '!=' is technically equivalent to an always-true 1=1 (you probably wanted to say ...{ $inequality_op => [ -and => @values ] }... instead) at /usr/share/koha/Koha/Objects.pm line 601 In this case we can simply count the statuses PENDING and PROCESSING. Note that bug 27944 will change that; PENDING should be replaced by REQUESTED there. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #74 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125149 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125149&action=edit Bug 27945: (QA follow-up) Terminology The term "Maximum article request limit" could be improved. In this patch we use 'Article request limit' to describe it more generic. On the field we say 'Maximum active article requests'. In the manual we need to explain that it is the sum of requests in Pending (Requested) or Processing stage and completed requests within the last 24 hours. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #75 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125150 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125150&action=edit Bug 27945: (follow-up) Fix tests for the adjusted check Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #76 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125151 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125151&action=edit Bug 27945: Move atomicupdate to new format It also removes the implicit dependency on bug 27946 by changing the AFTER column. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #77 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125152 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125152&action=edit Bug 27945: (QA follow-up) Fix and move tests to proper file Koha::Patron methods tests belong to the t/db_dependent/Koha/Patron.t tests file. This patch moves the tests, and also acknowledges the fact that we can use Test::Exception instead of a try/catch block. It also fixes the tests so they actually trigger the ->request method, which is the one that raises the exception if the limit is reached. At some point, because of the dependency mess, this bug ended up with the exception throwing in the wrong method, and that's why tests were also failing. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/ArticleRequests.t => FAIL: Patron.t passes, ArticleRequests.t doesn't 2. Apply this patch 3. Repeat 1 => SUCCESS: Both pass! 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #78 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Rebased on top of bug 27944, fixed a problem introduced by a rebase, fixed the tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29083 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 [Bug 29083] Update article requests-related Koha::Patron methods to use relationships -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27947 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27947 [Bug 27947] Add default cancellation reasons to article requests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125151|0 |1 is obsolete| | --- Comment #79 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 125178 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125178&action=edit Bug 27945: Move atomicupdate to new format It also removes the implicit dependency on bug 27946 by changing the AFTER column. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125152|0 |1 is obsolete| | --- Comment #80 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 125179 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125179&action=edit Bug 27945: (QA follow-up) Fix and move tests to proper file Koha::Patron methods tests belong to the t/db_dependent/Koha/Patron.t tests file. This patch moves the tests, and also acknowledges the fact that we can use Test::Exception instead of a try/catch block. It also fixes the tests so they actually trigger the ->request method, which is the one that raises the exception if the limit is reached. At some point, because of the dependency mess, this bug ended up with the exception throwing in the wrong method, and that's why tests were also failing. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/ArticleRequests.t => FAIL: Patron.t passes, ArticleRequests.t doesn't 2. Apply this patch 3. Repeat 1 => SUCCESS: Both pass! 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|29083 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 [Bug 29083] Update article requests-related Koha::Patron methods to use relationships -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #81 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 1. +my $can_request = $borrower->can_request_article $patron is better now 2. + my $compdate = dt_from_string->add( days => -1 ); I don't understand, what are we doing here? Can you add a line in the POD about that? 3. + if ($article_request_limit ne '' && $article_request_limit !~ /^\d+$/) { + push @messages, {type => 'error', code => 'article_request_numeric_limit' }; + $op = 'add_form'; + } elsif ($article_request_limit ne '' && $article_request_limit < 0) { + push @messages, {type => 'error', code => 'article_request_negative_limit' }; + $op = 'add_form'; + } else { Why that? I am expecting to see a JS validation and an exception raised in Koha::Category->store. But not in the controller. 4. circ/request-article.pl + error_message => 'Patron cannot request more articles for today' <p>[% error_message | html %]</p> Hum? Non translatable message. Same for the exception actually: + error_message => $_->{message} And same in opac-request-article.pl 5. opac-request-article.pl + exit unless $_->[0] && $_->[0] eq 'EXIT'; This is very awkward. Why not simply having the print+exit outside of the try? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #82 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125569 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125569&action=edit Bug 27945: (QA follow-up) Clarify behavior in POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #83 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #81)
1. +my $can_request = $borrower->can_request_article $patron is better now
Done.
2. + my $compdate = dt_from_string->add( days => -1 ); I don't understand, what are we doing here? Can you add a line in the POD about that?
Done. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #84 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #81)
3.
+ if ($article_request_limit ne '' && $article_request_limit !~ /^\d+$/) { + push @messages, {type => 'error', code => 'article_request_numeric_limit' }; + $op = 'add_form'; + } elsif ($article_request_limit ne '' && $article_request_limit < 0) { + push @messages, {type => 'error', code => 'article_request_negative_limit' }; + $op = 'add_form'; + } else {
Why that? I am expecting to see a JS validation and an exception raised in Koha::Category->store. But not in the controller.
Is this a blocker for inclusion? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #85 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125571 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125571&action=edit Bug 27945: (QA follow-up) Validate the input in the form Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #86 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #84)
(In reply to Jonathan Druart from comment #81)
3.
+ if ($article_request_limit ne '' && $article_request_limit !~ /^\d+$/) { + push @messages, {type => 'error', code => 'article_request_numeric_limit' }; + $op = 'add_form'; + } elsif ($article_request_limit ne '' && $article_request_limit < 0) { + push @messages, {type => 'error', code => 'article_request_negative_limit' }; + $op = 'add_form'; + } else {
Why that? I am expecting to see a JS validation and an exception raised in Koha::Category->store. But not in the controller.
Is this a blocker for inclusion?
I think so, it does not make sense to have it there. It may be better to just remove this check as we have the JS validation. If it is really what you need, you must use a "int unsigned" at DB level. Btw why do you use tinyint(4) when other "limit" columns in the same table are smallint(6)? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #87 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I am not convinced by the COMPLETED, updated_on code: 965 Returns true if the patron can request articles. As limits apply for the patron 966 on on the same day, those completed the same day are considered as current. 976 my $dtf = Koha::Database->new->schema->storage->datetime_parser; 977 my $compdate = dt_from_string->add( days => -1 ); 978 my $count = Koha::ArticleRequests->search([ 979 { borrowernumber => $self->borrowernumber, status => ['REQUESTED','PENDING','PROCESSING'] }, 980 { borrowernumber => $self->borrowernumber, status => 'COMPLETED', updated_on => { '>', $dtf->format_date($compdate) }}, if today is Oct 1st, $dtf->format_date($compdate) will be Sept 30th and updated_on > 2021-09-30 will return the rows updated yesterday as well. There is no test coverage for that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #88 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #86)
I think so, it does not make sense to have it there. It may be better to just remove this check as we have the JS validation.
If it is really what you need, you must use a "int unsigned" at DB level.
Agustin added the test with \d+ before there was js validation. I agree to remove it now. Not sure if we should really specify unsigned in SQL; did we do that more often? We could just check !value < 0 in js as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #89 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #87)
if today is Oct 1st, $dtf->format_date($compdate) will be Sept 30th and updated_on > 2021-09-30 will return the rows updated yesterday as well.
Read 'same day' here as last 24 hours. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #90 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #89)
(In reply to Jonathan Druart from comment #87)
if today is Oct 1st, $dtf->format_date($compdate) will be Sept 30th and updated_on > 2021-09-30 will return the rows updated yesterday as well.
Read 'same day' here as last 24 hours.
Are you sure this is correct? Sept 30th 16pm is yesterday for me, and it's Oct 1st 11am now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #91 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #88)
(In reply to Jonathan Druart from comment #86)
I think so, it does not make sense to have it there. It may be better to just remove this check as we have the JS validation.
If it is really what you need, you must use a "int unsigned" at DB level.
Agustin added the test with \d+ before there was js validation. I agree to remove it now. Not sure if we should really specify unsigned in SQL; did we do that more often? We could just check !value < 0 in js as well.
The DB structure and data types should help us avoid useless manual validations. But as we don't use STRICT_TRANS_TABLES, even marking the column as UNSIGNED INT will allow setting even a string... This is one of the reasons we should push for the API instead of this controller scripts (for data entry at least), because we can add the rules to the spec, and the OpenAPI plugin validates all the things for us. That said, I believe we can either keep or remove the validation. Keeping it prevents passing bad data manually, removing it cleans the controller a tiny bit. I would say let's do whatever the RM is more comfortable with so we move on. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #92 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #87)
I am not convinced by the COMPLETED, updated_on code:
965 Returns true if the patron can request articles. As limits apply for the patron 966 on on the same day, those completed the same day are considered as current.
976 my $dtf = Koha::Database->new->schema->storage->datetime_parser; 977 my $compdate = dt_from_string->add( days => -1 ); 978 my $count = Koha::ArticleRequests->search([ 979 { borrowernumber => $self->borrowernumber, status => ['REQUESTED','PENDING','PROCESSING'] }, 980 { borrowernumber => $self->borrowernumber, status => 'COMPLETED', updated_on => { '>', $dtf->format_date($compdate) }},
if today is Oct 1st, $dtf->format_date($compdate) will be Sept 30th and updated_on > 2021-09-30 will return the rows updated yesterday as well.
There is no test coverage for that.
So this is the main blocker for this dev. I will add tests: Marcel: what is the right behavior? 24 hours timespan? same day? I feel like 'same day' is the right thing. But will implement it as you want, so: how? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #93 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #88)
do that more often? We could just check !value < 0 in js as well.
The JS validation I added already checks the min value is 1. So no negative values allowed (in the form) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #94 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #91)
The DB structure and data types should help us avoid useless manual validations. But as we don't use STRICT_TRANS_TABLES, even marking the column as UNSIGNED INT will allow setting even a string...
(nitpicky mode) It will actually raise a non-blocker warnings and the value will be ignored. The DBMS won't store a string into a int ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #95 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #92)
Marcel: what is the right behavior?
Thats a good question. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #96 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #95)
(In reply to Tomás Cohen Arazi from comment #92)
Marcel: what is the right behavior?
Thats a good question.
I'm about to resubmit this patchset with some requests from the RM, and I will make it 'same day' as it was at some point. Sand promise to work on another approach on another bug if it is filed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #97 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125638 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125638&action=edit Bug 27945: Clarify 'same day' behavior This patch introduces tests for the 'same day' check of the ability to place article requests for a patron. The limit goes against current requests, and those that have been completed on the same day. The tests cover this specific situation. The current behavior is that it takes into account a 24 hr timespan, but consensus on the QA step was that we should do it as 'same day' and use a separate feature request to change this, if required. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #98 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- But rhetorical questions dont need an answer ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #99 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 125690 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125690&action=edit Bug 27945: (follow-up) Clarify 'same day' behavior Don't use MySQLism -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #100 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Why is it a category's attribute and not a circ rule? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125569|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125179|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125638|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125690|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125178|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125571|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125140|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125142|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125143|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125144|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125145|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125146|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125147|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125148|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125149|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125150|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #101 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125702 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125702&action=edit Bug 27945: Add tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #102 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125703 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125703&action=edit Bug 27945: Add limit article request feature This patch makes it possible to limit article requests per patron per day. To test: 1. Apply patches 2. updatedatabase 3. Enable ArticleRequests preference 4. Edit a patron category and set an article request limit to 1 CHECK => if you set the limit to anything else but a positive number or empty string, a warning appears 5. In staff search biblios and request an article for a patron of the modified category 6. Repeat step 5 SUCCESS => if limit is reached, when you select the user to request an article a warning appears saying that the limit was reached 7. Repeat steps 5 and 6 but this time in opac SUCCESS => Patron is not allowed to request another article if limit is reached 8. prove t/db_dependent/ArticleRequests.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Edit: This patchset originally changed the 'categories' table structure and relied on that for limit calculation. I removed all that code and squashed into this one, as we moved everything to the circulation_rules table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #103 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125704 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125704&action=edit Bug 27945: Clarify 'same day' behavior This patch introduces tests for the 'same day' check of the ability to place article requests for a patron. The limit goes against current requests, and those that have been completed on the same day. The tests cover this specific situation. The current behavior is that it takes into account a 24 hr timespan, but consensus on the QA step was that we should do it as 'same day' and use a separate feature request to change this, if required. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #104 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125705 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125705&action=edit Bug 27945: Add max_daily_article_requests circulation rule Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #105 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125706 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125706&action=edit Bug 27945: Implement limits using circulation rules -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #106 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125707 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125707&action=edit Bug 27945: Fix error handling and translatability This patch adds better error handling and reporting when placing an article request fails. It also makes the error messages translatable. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29168 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29168 [Bug 29168] Disable "Request article" links when limit is reached -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29169 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29169 [Bug 29169] Wrong "daily limit" warning when article request is not available -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #107 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 125726 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125726&action=edit Bug 27945: Don't save rule if not defined The DB rev added a row with NULL (unlimited), and the UI a row with '' (unlimited as well) when saved without value. Better is to not have a row (still unlimited) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #108 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125741 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125741&action=edit Bug 27945: Terminology: max daily => open requests limit Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #109 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #110 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 125761 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125761&action=edit Bug 27945: (follow-up) Add tests Restore +x flag -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #111 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #110)
Created attachment 125761 [details] [review] Bug 27945: (follow-up) Add tests
Restore +x flag
Pushed to master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Bug 27945 depends on bug 27944, which changed state. Bug 27944 Summary: Add new stages to the article request process https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27944 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Resolution|--- |FIXED Status|Pushed to master |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com Keywords|Manual | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #112 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- I added an open_article_requests_limit without patron category by script. And it works fine. Disadvantage: the record is not shown on the Circulation rules interface.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32262 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32262 [Bug 32262] smart-rules: Some rules can be created but are not visible on interface -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |39624 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39624 [Bug 39624] Add open_article_requests_limit for patron category 'All' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 --- Comment #113 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #112)
I added an open_article_requests_limit without patron category by script. And it works fine. Disadvantage: the record is not shown on the Circulation rules interface..
See 39624 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org