http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 Maxime Pelletier <pelletiermaxime@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Change sponsored?|--- |Sponsored --- Comment #2 from Maxime Pelletier <pelletiermaxime@gmail.com> 2011-10-04 17:57:51 UTC --- Forgot to mention this was sponsored by our client http://www.ccsr.qc.ca/ -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Patch Status|--- |Needs Signoff --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> 2011-11-02 20:26:36 UTC --- There is a patch attached, I am changing the status to 'needs signoff'. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> 2011-12-07 06:45:53 UTC --- I think something is weird with my test case, but an additional note about the patch: You can't search only for "expired before today". You are always prompted to also enter a date. I am not sure how this is supposed to work. Shouldn't it be enough to enter tomorrow's date to find subscriptions that already have expired? -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 --- Comment #6 from Maxime Pelletier <pelletiermaxime@gmail.com> 2012-01-04 16:59:02 UTC --- Created attachment 7038 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7038 New patch to fix warnings and to make it work -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 Maxime Pelletier <pelletiermaxime@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Failed QA |Needs Signoff -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 Adrien SAURAT <adrien.saurat@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |adrien.saurat@biblibre.com -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |paul.poulain@biblibre.com --- Comment #10 from Paul Poulain <paul.poulain@biblibre.com> 2012-02-21 14:29:18 UTC --- (In reply to comment #9)

Where is "showexpiredtoday" getting the string value "on"? There is no explicit value passed by the template. Is "on" the value passed by a checkbox by default on all browsers? I think having an explicit value in the template here would be more solid and stable.

Not quite ready to mark this as "Failed QA", but I would like some explanation of the safety of this before I mark it passed.

Ian is right: you can't rely on the value "on", you should just test if there is something. Chrome returns 0/1 afaik while FF returns empty/on However, i mark failed QA for another reason: i've 2 subscriptions in my database. One that has expired in 2010, one that will expire in march 2012. If I set the date to 2012-12-12 I should see 1 or 2 subscriptions in my list, depending on the whether the checkbox has been checked or no. If checked: I get 2, that's correct If unchecked: I get none, that's not correct. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|PATCH-Sent (DO NOT USE) |P5 - low Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paola.rossi@cineca.it Status|Needs Signoff |Patch doesn't apply --- Comment #12 from Paola Rossi <paola.rossi@cineca.it> --- I've tried to apply the patch against master 3.19.00.005 But: Patch format detection failed. So I pass the patch to the "Patch doesn't' apply" status. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35314|this patch is exactly the |Bug 6968 - Check expiration description|same diff as the previous |in serials expired before |patch, but formatted |today.patch |differently | -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6968 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #14 from Paola Rossi <paola.rossi@cineca.it> --- (In reply to Charles Farmer from comment #13)

Created attachment 35314 [details] [review] Bug 6968 - Check expiration in serials expired before today

I'm resubmitting the last patch, which I could apply on master 3.19.00.006, in a slightly different format.

Hoping this solves your issue.

Kind Charles, I've applied the patch against master 3.19.00.006 head 13526. Soon after having applied, when I: git format-patch -s origin/master, 1-A) I got a file where "From" is: charles <charles@inlibro.com> instead of : charles <charles.farmer@inlibro.com> It could be an error. 1-B) In that file there was this: Subject: [PATCH] Patch sponsored by our client http://www.ccsr.qc.ca/ instead of the usual commit message: Bug 6968 - Show items expired before today in check expiration of serials page identifying the patch and describing the content of the patch. [The patches' commit messages will be seen in the master branch: <http://git.koha-community.org/gitweb/?p=koha.git;a=summary>] So I pass the patch to "Patch doesn't apply" again. ---------------------------------------------------------------------------- Anyway I've gone on and tested your patch, that is fixing a lasting bug. I'm not a QA, I don't know precisely the QA guidelines. I write some considerations of mine you can evaluate. The patch you wrote added a true difference between: subscriptions "already" expired until today [included] (added to the selection by the new checkbox) subscriptions that "will" expire (from tomorrow [included] to the input date [excluded], chosen by the calendar). But IMO in master this difference was not so present: as you can see, the calendar is not limited from tomorrow on. So I think that your patch could "match" this "error" not so well. Perhaps : A) extending the current selection to all the end-dates of the subscriptions, and B) adding a "From" and a "To" dates input fields (as an improvement), could be a more "matching" solution. The default values of "From" and "To" could be the MIN and MAX end-date of the subscriptions in DB, whilst "today" could be set as the default in the 2 calendars. C) moreover, the resulting table could be sortable, on end-date at least (as a further improvement). Otherwise, if the solution provided by the patch has been already considered as being the best one, consider that some inaccuracies occur: 1-A) when I checked the checkbox and I chose an year ago as input date, the selection is not about "before today", but "before an year ago": the result is shown (IMO right). 1-B) if I didn't check the checkbox and I chose an year ago as input date, the selection is null, as you know [this result is wrong IMO]. If you limit the calendar from tomorrow on, the user will not be able to get a selection limited like 1-A). If you don't limit the calendar from tomorrow on, B) is an available case of choice to the user, and it is unfriendly to the user (misleading). Anyway, if the solution provided by the patch has been already considered as being the best one, in the form "before" won't be used in the same meaning: once "<", and the other time as " <=". This difference is unfriendly to the user: the resulting table could miss some particular subscriptions. To solve, a better solution can choose one of the 2 cases: hypothesis 2-1) ("<") "before" excluding the upper limit: Adding "Show expired before tomorrow:" instead of adding "Show expired before today:" could be better. hypothesis 2-2) ("<=") "before" including the upper limit (better, IMO): The patch should keep "Show expired before today:". But the patch should also modify the "Expiring before: " behaviour, to include the chosen value too in the selection. Anyway, if the solution provided by the patch has been already considered as being the best one, being the form a "Filters" one, its default should be better set to select all the subscriptions. So I think that: 3-A) a default could be set in the input fields: checked checkbox (!), usefull upper limit [=MAX end-date+included (2-2), or greater than MAX end-date]. As a further inprovement, the sort could be add to the table. If the expiring' time is so important to the user, the default sorting could be set to end-data DESC [in spite of the current subscription code, not related]. I hope these considerations can be of some help. -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.