Bug 13622 - Add option to display datetimes as dates in notices
Summary: Add option to display datetimes as dates in notices
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
: 13536 (view as bug list)
Depends on:
Blocks: 16624
  Show dependency treegraph
 
Reported: 2015-01-26 16:26 UTC by Jonathan Druart
Modified: 2016-12-05 21:31 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13536: Display a datetime without time in a notice (8.24 KB, patch)
2015-01-26 16:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13622: Display a datetime without time in a notice (8.29 KB, patch)
2015-05-18 12:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 13622: [QA Follow-up] Add unit test descriptions (2.53 KB, patch)
2015-05-18 12:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 13622: Display a datetime without time in a notice (8.38 KB, patch)
2015-07-05 21:14 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Bug 13622: [QA Follow-up] Add unit test descriptions (2.56 KB, patch)
2015-07-05 21:18 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Bug 13622: Display a datetime without time in a notice (8.43 KB, patch)
2015-09-14 13:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 13622: [QA Follow-up] Add unit test descriptions (2.60 KB, patch)
2015-09-14 13:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2015-01-26 16:26:53 UTC
Bug 13536 fixes an issue for 1 specific datetime.
I would like to make this possible for all date fields.
Comment 1 Jonathan Druart 2015-01-26 16:30:33 UTC Comment hidden (obsolete)
Comment 2 David Cook 2015-01-27 22:26:20 UTC
Interesting idea! How might this factor in for <<items.content>> where you might have multiple dates in the content?
Comment 3 Jonathan Druart 2015-01-28 09:40:03 UTC
(In reply to David Cook from comment #2)
> Interesting idea! How might this factor in for <<items.content>> where you
> might have multiple dates in the content?

It won't work for items.content.
This is a special case managed in cronjob script (overdue_notices.pl and advance_notices.pl).
Comment 4 David Cook 2015-01-29 00:14:53 UTC
(In reply to Jonathan Druart from comment #3)
> (In reply to David Cook from comment #2)
> > Interesting idea! How might this factor in for <<items.content>> where you
> > might have multiple dates in the content?
> 
> It won't work for items.content.
> This is a special case managed in cronjob script (overdue_notices.pl and
> advance_notices.pl).

Ah, right. Of course. You and I were the people on those bugs too, so I should've remembered.

Cool beans :)
Comment 5 Kyle M Hall 2015-01-30 12:31:30 UTC
I think the best solution would be to automatically strip times from items due at 23:59

Would this not solve the issue completely without the need for any special markup?
Comment 6 Marcel de Rooy 2015-02-02 07:58:18 UTC
(In reply to Kyle M Hall from comment #5)
> I think the best solution would be to automatically strip times from items
> due at 23:59
> 
> Would this not solve the issue completely without the need for any special
> markup?

This patch offers more than the due date.
Comment 7 Jonathan Druart 2015-03-30 12:08:22 UTC
Reintroduce this one in the needs signoff queue, there is no discussion and I think this patch could bring a global solution to date formatting in notices.
Comment 8 Katrin Fischer 2015-05-15 06:06:21 UTC
We had another library asking for this - it would be nice to see it moving.
Comment 9 Marcel de Rooy 2015-05-18 12:27:38 UTC Comment hidden (obsolete)
Comment 10 Marcel de Rooy 2015-05-18 12:27:42 UTC Comment hidden (obsolete)
Comment 11 Marcel de Rooy 2015-05-18 12:31:16 UTC
Here we have it moving again :)
I will close report 13536 now; this is more global.
The discussion status did not bring us any news.

Point of attention: the feature needs to be documented somewhere..
Comment 12 Marcel de Rooy 2015-05-18 12:31:38 UTC
*** Bug 13536 has been marked as a duplicate of this bug. ***
Comment 13 Indranil Das Gupta 2015-07-05 14:06:55 UTC
this could prolly use a Passed QA status ;-)
Comment 14 Katrin Fischer 2015-07-05 14:37:14 UTC
... or another sign-off to give QA more incentive! ;)
Comment 15 Indranil Das Gupta 2015-07-05 21:14:02 UTC Comment hidden (obsolete)
Comment 16 Indranil Das Gupta 2015-07-05 21:18:17 UTC Comment hidden (obsolete)
Comment 17 Indranil Das Gupta 2015-07-05 21:21:35 UTC
(In reply to Katrin Fischer from comment #14)
> ... or another sign-off to give QA more incentive! ;)

heh! done... updated the tests count in t/db_dependent/Letters.t to 64 (it is already 61 in the current master). Hope I've not messed up the author this time.
Comment 18 Katrin Fischer 2015-07-09 12:17:52 UTC
Patches look ok to me on first glance, thx Indranil!
Comment 19 Marcel de Rooy 2015-09-03 12:52:33 UTC
(In reply to Katrin Fischer from comment #18)
> Patches look ok to me on first glance, thx Indranil!

They have a double signoff. What's holding you back?
Comment 20 Marcel de Rooy 2015-09-14 13:11:39 UTC
Created attachment 42519 [details] [review]
Bug 13622: Display a datetime without time in a notice

For some needs, a librarian would like to display a datetime or
timestamp field without the time.

This patch adds filter logic in the notice/letter parsing process.

Test plan:
1/ Defined a notice using a datetime or timestamp DB field
(biblio.timestamp for instance).
2/ Generate the notice
3/ Verify that the letter is generated with the time
4/ Use the "dateonly" filter like:
  <<your_table.your_field | dateonly>>
  <<biblio.timestamp | dateonly>>
5/ Generate the notice
6/ Confirm the the letter is generated without the time for this field.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>

Updated the count of tests to 64 for t/db_dependent/Letters.t to pass
Comment 21 Marcel de Rooy 2015-09-14 13:11:43 UTC
Created attachment 42520 [details] [review]
Bug 13622: [QA Follow-up] Add unit test descriptions

This report adds a few unit tests for datonly flag in notices.
This patch adds (very trivial) unit test descriptions.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Comment 22 Marcel de Rooy 2015-09-14 13:17:58 UTC
QA Comment:
I am taking the liberty to pass qa on this report. Since Indranil signed off on both patches, I gain my QA token again..
Works as described.
QA tests pass.
The feature is kind of undocumented now. It should be described somewhere (help page, manual, on the form??)
The code in Letters.pm is already less maintainable and this fix will not improve the situation. But it is a global solution and it is in written in a way that could[..] be extended with similar filters.
The three added tests are testing the same thing. Seems to be overkill. I could better understand your testing the same report with dateonly flag and without it to show the difference. But no big deal.

Passed QA
Comment 23 Jonathan Druart 2015-09-14 13:39:56 UTC
(In reply to Marcel de Rooy from comment #22)
> The three added tests are testing the same thing. Seems to be overkill.

No, they test different patterns:
table.field | filter
field | filter
field|filter
Comment 24 Tomás Cohen Arazi 2015-09-16 13:54:56 UTC
Patches pushed to master.

Thanks Jonathan and Marcel!