Bug 11023 - Automatic item modification by age (Was "Toggle new status for items")
Summary: Automatic item modification by age (Was "Toggle new status for items")
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 16004 16030 16437 16607
  Show dependency treegraph
 
Reported: 2013-10-09 15:28 UTC by Jonathan Druart
Modified: 2017-06-14 22:15 UTC (History)
13 users (show)

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


Attachments
Bug 11023: Toggle "new" status (28.22 KB, patch)
2013-10-09 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (28.15 KB, patch)
2013-10-10 07:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (28.11 KB, patch)
2013-10-14 13:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (28.14 KB, patch)
2013-10-14 13:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (30.48 KB, patch)
2013-10-15 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (30.82 KB, patch)
2013-10-25 07:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add the ability to specify fields from biblioitems table. (6.47 KB, patch)
2013-11-05 16:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add UT for C4::Items::ToggleNewStatus (8.42 KB, patch)
2013-11-05 16:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (30.89 KB, patch)
2014-01-09 12:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add the ability to specify fields from biblioitems table. (6.47 KB, patch)
2014-01-09 12:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add UT for C4::Items::ToggleNewStatus (8.42 KB, patch)
2014-01-09 12:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (31.04 KB, patch)
2014-01-10 15:33 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 11023: Add the ability to specify fields from biblioitems table. (6.62 KB, patch)
2014-01-10 15:33 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 11023: Add UT for C4::Items::ToggleNewStatus (8.58 KB, patch)
2014-01-10 15:33 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 11023: FIX - condition on biblioitems table does not work (2.56 KB, patch)
2014-03-03 15:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Use DBIx-Class to retrieve column names (2.31 KB, patch)
2014-03-05 10:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (31.02 KB, patch)
2014-04-04 14:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add the ability to specify fields from biblioitems table. (6.62 KB, patch)
2014-04-04 14:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add UT for C4::Items::ToggleNewStatus (8.57 KB, patch)
2014-04-04 14:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: FIX - condition on biblioitems table does not work (2.56 KB, patch)
2014-04-04 14:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Use DBIx-Class to retrieve column names (2.31 KB, patch)
2014-04-04 14:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Update DB field description (1.79 KB, patch)
2014-04-04 14:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Update DB field description (1.79 KB, patch)
2014-04-07 08:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Don't use the biblioitems fields for the subtitution (3.19 KB, patch)
2014-04-30 08:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (28.96 KB, patch)
2014-06-25 13:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add the ability to specify fields from biblioitems table. (6.62 KB, patch)
2014-06-25 13:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add UT for C4::Items::ToggleNewStatus (8.57 KB, patch)
2014-06-25 13:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: FIX - condition on biblioitems table does not work (2.56 KB, patch)
2014-06-25 13:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Use DBIx-Class to retrieve column names (2.31 KB, patch)
2014-06-25 13:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Don't use the biblioitems fields for the subtitution (3.19 KB, patch)
2014-06-25 13:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: DB changes (5.25 KB, patch)
2014-06-25 13:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Rename the duration parameter with 'age' (8.85 KB, patch)
2014-09-23 08:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: The age parameter should be a number (1.63 KB, patch)
2014-09-23 08:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Change the name of the feature (2.92 KB, patch)
2014-09-23 08:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (29.02 KB, patch)
2014-11-05 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add the ability to specify fields from biblioitems table. (6.62 KB, patch)
2014-11-05 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add UT for C4::Items::ToggleNewStatus (8.57 KB, patch)
2014-11-05 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: FIX - condition on biblioitems table does not work (2.55 KB, patch)
2014-11-05 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Use DBIx-Class to retrieve column names (2.30 KB, patch)
2014-11-05 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Don't use the biblioitems fields for the subtitution (3.19 KB, patch)
2014-11-05 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: DB changes (4.86 KB, patch)
2014-11-05 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Rename the duration parameter with 'age' (8.85 KB, patch)
2014-11-05 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: The age parameter should be a number (1.63 KB, patch)
2014-11-05 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Change the name of the feature (2.92 KB, patch)
2014-11-05 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Toggle "new" status (29.07 KB, patch)
2014-12-29 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add the ability to specify fields from biblioitems table. (6.62 KB, patch)
2014-12-29 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Add UT for C4::Items::ToggleNewStatus (8.57 KB, patch)
2014-12-29 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: FIX - condition on biblioitems table does not work (2.55 KB, patch)
2014-12-29 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Use DBIx-Class to retrieve column names (2.29 KB, patch)
2014-12-29 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Don't use the biblioitems fields for the subtitution (3.19 KB, patch)
2014-12-29 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: DB changes (4.83 KB, patch)
2014-12-29 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Rename the duration parameter with 'age' (8.85 KB, patch)
2014-12-29 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: The age parameter should be a number (1.63 KB, patch)
2014-12-29 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Change the name of the feature (2.92 KB, patch)
2014-12-29 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 11023: Toggle "new" status (29.05 KB, patch)
2015-01-02 14:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11023: Add the ability to specify fields from biblioitems table. (6.60 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11023: Add UT for C4::Items::ToggleNewStatus (8.56 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11023: FIX - condition on biblioitems table does not work (2.62 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11023: Use DBIx-Class to retrieve column names (2.36 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11023: Don't use the biblioitems fields for the subtitution (3.23 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11023: DB changes (4.90 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11023: Rename the duration parameter with 'age' (8.92 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11023: The age parameter should be a number (1.69 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11023: Change the name of the feature (2.96 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11023 [QA Followup] (2.09 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11023 [QA Followup] - Complete the renaming of "toggle new status" to "automatic item modification by age" (18.09 KB, patch)
2015-01-02 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11023 [QA Followup] - Complete the renaming of "toggle new status" to "automatic item modification by age" (9.83 KB, patch)
2015-01-02 14:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11023 [QA Followup] - Complete the renaming of "toggle new status" to "automatic item modification by age" (10.67 KB, patch)
2015-01-02 14:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11023: Automatic item modification by age (Was Toggle "new" status") (40.98 KB, patch)
2015-01-19 12:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: DB changes (4.91 KB, patch)
2015-01-19 12:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11023: Automatic item modification by age (Was Toggle "new" status") (41.16 KB, patch)
2015-07-24 10:06 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11023: DB changes (2.71 KB, patch)
2015-07-24 10:07 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2013-10-09 15:28:12 UTC
Some libraries would like to manage a "new" status for items.
This status should be removed depending on some conditions.
Comment 1 Jonathan Druart 2013-10-09 16:00:09 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-10-10 07:20:34 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2013-10-14 13:24:24 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2013-10-14 13:57:30 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2013-10-15 14:08:43 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2013-10-25 07:41:15 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2013-10-25 07:44:04 UTC
There were been several iterations with Laurence. She found some bugs I fixed.
This last patch is the final version.
Comment 8 Jonathan Druart 2013-11-05 16:23:28 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2013-11-05 16:23:56 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2014-01-09 12:11:08 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2014-01-09 12:11:13 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2014-01-09 12:11:18 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2014-01-09 12:11:34 UTC
rebased patches.
Comment 14 sandboxes@biblibre.com 2014-01-10 15:33:12 UTC
Patch tested with a sandbox, by juliette et rémy <juliette.levast@iepg.fr>
Comment 15 sandboxes@biblibre.com 2014-01-10 15:33:50 UTC Comment hidden (obsolete)
Comment 16 sandboxes@biblibre.com 2014-01-10 15:33:54 UTC Comment hidden (obsolete)
Comment 17 sandboxes@biblibre.com 2014-01-10 15:33:57 UTC Comment hidden (obsolete)
Comment 18 Kyle M Hall 2014-01-17 15:49:59 UTC
Is there any reason this couldn't be done using dateaccessioned rather than adding a new field to items? I imagine the test for items being 'new' is less than simple in some cases, and I can see a definite advantage in pre-setting items as new rather than determining that on the fly for every item.

QA Issue: biblioitems_columns doesn't seem like it belongs in Items.pm, you could easily use DBIx::Class to get the columns names:
http://search.cpan.org/~frew/DBIx-Class-0.08121/lib/DBIx/Class/ResultSource.pm#columns
so no need for a special subroutine!
Comment 19 Jonathan Druart 2014-01-17 16:04:20 UTC
(In reply to Kyle M Hall from comment #18)
> Is there any reason this couldn't be done using dateaccessioned rather than
> adding a new field to items? I imagine the test for items being 'new' is
> less than simple in some cases, and I can see a definite advantage in
> pre-setting items as new rather than determining that on the fly for every
> item.

I am not sure to understand: the goal of the cronjob script is to remove the 'new' flag.
Comment 20 Jonathan Druart 2014-01-17 16:08:20 UTC
And the script is based on the items.dateaccessioned field for the duration value.
Comment 21 Jonathan Druart 2014-03-03 15:00:38 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2014-03-05 10:09:20 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2014-03-05 10:10:35 UTC
(In reply to Kyle M Hall from comment #18)
> QA Issue: biblioitems_columns doesn't seem like it belongs in Items.pm, you
> could easily use DBIx::Class to get the columns names:
> http://search.cpan.org/~frew/DBIx-Class-0.08121/lib/DBIx/Class/ResultSource.
> pm#columns
> so no need for a special subroutine!

Done in the last patch. I kept the subroutines because I use the column names in 2 different places.
Comment 24 Olli-Antti Kivilahti 2014-04-04 13:32:27 UTC
Firstly two merge conflicts: (got it working tho)
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl

Cronjob seems to work as intended and is a welcome addition.

The purpose of the items.new-column is unclear. I didn't manage to get it populated, even if the comments here say that the cronjob's purpose is to empty the new-column.

Apparently the new-column is a field, but then again it is a flag, and in the CREATE TABLE directive it is a VARCHAR(32). It is also the (status == new).
It is extremely confusing.

Apparently we can use it as we want, but then shouldn't it be labeled as such.
My suggestion, replace the new-column definition with this:
CREATE TABLE...
 `system_note` VARCHAR(32) DEFAULT NULL, -- You can put whatever free-text information regarding this item here. This field is meant for local modifications only and should not be used in features intended for master
...
;
Comment 25 Jonathan Druart 2014-04-04 14:25:13 UTC Comment hidden (obsolete)
Comment 26 Jonathan Druart 2014-04-04 14:25:17 UTC Comment hidden (obsolete)
Comment 27 Jonathan Druart 2014-04-04 14:25:22 UTC Comment hidden (obsolete)
Comment 28 Jonathan Druart 2014-04-04 14:25:26 UTC Comment hidden (obsolete)
Comment 29 Jonathan Druart 2014-04-04 14:25:31 UTC Comment hidden (obsolete)
Comment 30 Jonathan Druart 2014-04-04 14:25:35 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2014-04-04 14:26:26 UTC
(In reply to Olli-Antti Kivilahti from comment #24)
I disagree, this field is created to manage a "new" status, so I think "new" is a good name :)
I provided a followup to change the description.
Last patches have been rebased against master.

Thank you for testing Olli!
Comment 32 Olli-Antti Kivilahti 2014-04-04 14:30:15 UTC
We can agree to that :)

Good work!
Comment 33 paxed 2014-04-07 08:10:06 UTC
(In reply to Jonathan Druart from comment #30)
> Created attachment 26822 [details] [review] [review]
> Bug 11023: Update DB field description

Typo: "indented" should be "intended"
Comment 34 Jonathan Druart 2014-04-07 08:29:18 UTC Comment hidden (obsolete)
Comment 35 Katrin Fischer 2014-04-27 13:05:34 UTC
Starting with some initial testing and notes on this:

1) I am not sure the 2 specialiced subroutines for getting the 
   columns from items and biblioitems are needed - couldn't this
   be done in ToggleNewStatus with DBIC? Right now it seems like
   the patch is using a mix of old and new (see comment 18 from Kyle).

+sub columns {
+sub ToggleNewStatus {
+sub biblioitems_columns {

2) The 'new' column
   In comment 19 you said the goal of the cronjob was to remove the
   new flag. 
   I am unsure if I see the use of the new field in the code and the
   feature. To me it seems just like a new 'notes' field, that is not
   really accessible without further configuration.
   
   a) In order to be able to set the field, you need to add a Koha-2-
      MARC-Mapping and change the bibliographic frameworks. Only then
      you will be able to catalog it. The field won't be searchable
      as there is no index on it. If we want to add an index later, this
      will be difficult, as we can't tell which mappings people will
      have used.
      How will libraries learn about the existance of the new
      column? 

   b) For a general purpose field varchar(32) seems a bit limited.
   
   c) It seems the feature is independend on the existance of the new
      column. The central column for the feature seems to be the
      existing dateaccessioned column instead.
      With the implications of a) I am not sure we should include the
      'new' from the beginning without having a clearer definition
      on how to use it.

4) Features in documentation
   I think the documentation lists features more like in an ad, than
   like in a help file. It's not clear to the reader, how to achieve
   the described functionality. That's what I have come up with:
   
+  <li>know easily what are the new items in the catalogue.</li>

   This still requires SQL, so doesn't seem to depend on the feature.
    
+  <li>display an icon in the search results for new items.</li>

    The only way to do this I can come up with is limited to libraries
    using biblio level itypes and by switching the itemtype with the
    script.

+  <li>configure issuing rules depending the 'new' status.</li>

    I think this would also require switching the itemtype?

+  <li>get a RSS/Atom feeds on these new items.</li>

    I think this would require using a special value in an item 
    column that is searchable with Zebra. So you can build a search in
    Zebra that can then be used for the RSS feed?

4) biblioitems

   a) I am worried about some columns that should not be substituted:
      biblionumber, itemnumber, barcode, totalissues, onloan ...
      I think those are potentially dangerous and should not be listed.

   b) To me it seems like the only goal of including biblioitems is
      the possibility to change the bibliotitems itemtype. As it is now,
      it also allows changing values like title, pages, etc. which 
      borders on a record batch edit. To me this seems a bit misplaced 
      here.
      Also, ToggleNewStatus seems to only do ModItem - but you can
      define substitutions for biblioitems? (see also 6)

5) GUI

   a) It's not possible to edit a single rule. In order to add a rule
      or edit a rule, you always have to use 'Edit' and then all rules
      will display, the form for adding the new rule is at the very 
      bottom. It is not very comfortable.
   
   b) There are no mandatory fields, duration is optional, as are 
      conditions and substitions. So you can define an empty rule.

   c) In all other item related forms we use the names defined for the
      columns in the default framework (at least that's where I think
      those come from). But this feature only shows the names of the
      database columns. I think this is because of 2 a)?

6) Exploding?
   I defined a mean rule:
     Duration: 1 (only 1 item in my database has an accessiondate from
                yesterday)
     Conditions: no condition
     Substitutions:
     biblioitems.publishercode = XXXX
     biblioitems.url = something
     biblioitems.itemtype = XXXX
     items.barcode = xxxx
     items.materials = xxxx 
     
     I didn't expect much to happen, but I had misinterpreted the duration.
     
     Results:

DBD::mysql::st execute failed: Unknown column 'biblioitems.publishercode' in 'field list' at /home/katrin/kohaclone/C4/Items.pm line 2324.
ERROR in _koha_modify_item UPDATE items SET biblioitems.publishercode=? WHERE itemnumber=?: Unknown column 'biblioitems.publishercode' in 'field list' at /home/katrin/kohaclone/C4/Items.pm line 2327.
DBD::mysql::st execute failed: Unknown column 'biblioitems.url' in 'field list' at /home/katrin/kohaclone/C4/Items.pm line 2324.
ERROR in _koha_modify_item UPDATE items SET biblioitems.url=? WHERE itemnumber=?: Unknown column 'biblioitems.url' in 'field list' at /home/katrin/kohaclone/C4/Items.pm line 2327.
DBD::mysql::st execute failed: Unknown column 'biblioitems.itemtype' in 'field list' at /home/katrin/kohaclone/C4/Items.pm line 2324.
ERROR in _koha_modify_item UPDATE items SET biblioitems.itemtype=? WHERE itemnumber=?: Unknown column 'biblioitems.itemtype' in 'field list' at /home/katrin/kohaclone/C4/Items.pm line 2327.
DBD::mysql::st execute failed: Duplicate entry 'xxxx' for key 'itembarcodeidx' at /home/katrin/kohaclone/C4/Items.pm line 2324.
ERROR in _koha_modify_item UPDATE items SET items.barcode=? WHERE itemnumber=?: Duplicate entry 'xxxx' for key 'itembarcodeidx' at /home/katrin/kohaclone/C4/Items.pm line 2327.
   
   Of course, no all my items have xxxx in materials. I thought initially
   only those items exactly 1 day old would be touched. But it will change
   all items older than 1 day. So this potentially will update a lot of items
   on each run.
   
   The SQL also shows that you can't actually update biblioitems
   columns, although the GUI offers the option.
   
--------

I really like the idea of this, but to me it seems like there is some
sort of mismatch between the feature description and how it seems to
work. It feels like it needs some more thought.
I think maybe this feature 'grew' since its initial specification?

An example for a mismatch is also the description of the cronjob:

+./toggle_new_status.pl -h
+
+Toggle recent acquisitions status.
+Use this script to delete "new" status for items.

There is no mention of the new column in the script.

Another problem I see is that the feature as it is now is potentially
quite dangerous and substitiutions on biblioitems don't work. Maybe
biblioitems should be removed altogether from this.

I'd love to hear some more opinions on this.
Comment 36 Laurence Rault 2014-04-28 16:23:51 UTC
Just some more information on how the dev works before answering more in details to Katrin's notes

The goal of the cronjob is to remove a "new" information from items after a given period.

What could be a "new" information in the item : 

1) a 'new' status added just to inform about the new document : we suppose an item subfield mapped with items.new field.
This value can be used in search (if zebra index is configured), for custom rss feeds ; the subfield can be linked to an authorised value (authorised value=1, label=New). The cronjob will delete the value in the 'new' subfield.

2) a 'New' itype (itemtype) so that specific issuing rules can be defined for new items. With this configuration, final itemtype can be defined based on condition in another item subfield (ccode) or in a biblio subfield (mapped with biblioitems.itemtype) or in any items or biblioitems available field. The cronjob will delete the 'New' itype value and replace it by the suitable itype)

3) why not both of 1) and 2) or any item subfield the library wants to have some 'new' information.


Only items are modified, the modification of biblio (biblio or biblioitems) is not part of the dev. Biblioitems fields are used only for the condition criteria.
The items having dateaccessioned <= Now-duration will be modified by the job.
Comment 37 Jonathan Druart 2014-04-30 08:23:54 UTC
(In reply to Katrin Fischer from comment #35)

Helle Katrin,

> Starting with some initial testing and notes on this:
>
> 1) I am not sure the 2 specialiced subroutines for getting the
>    columns from items and biblioitems are needed - couldn't this
>    be done in ToggleNewStatus with DBIC? Right now it seems like
>    the patch is using a mix of old and new (see comment 18 from Kyle).

There are 2 calls, 1 in C4::Items::ToggleNewStatus and 1 in the pl script.
I don't think it is a good idea to call DBIC directly in pl files.

> 2) The 'new' column
>    In comment 19 you said the goal of the cronjob was to remove the
>    new flag.
>    I am unsure if I see the use of the new field in the code and the
>    feature. To me it seems just like a new 'notes' field, that is not
>    really accessible without further configuration.

As I tried to explain in the commit message, this feature allows to manage new items as you want.
There is no restriction or a "way todo". I listed test case examples, it is how we use this field at BibLibre.
This patch is an "introduction" to the idea of manage a "new" status.

>    a) In order to be able to set the field, you need to add a Koha-2-
>       MARC-Mapping and change the bibliographic frameworks. Only then
>       you will be able to catalog it. The field won't be searchable
>       as there is no index on it. If we want to add an index later, this
>       will be difficult, as we can't tell which mappings people will
>       have used.
>       How will libraries learn about the existance of the new
>       column?

Reading the changelog?

>    b) For a general purpose field varchar(32) seems a bit limited.

In a lot of cases, we used it as a boolean, so it is enough. What do you suggest, VARCHAR(64), TEXT,...?


>    c) It seems the feature is independend on the existance of the new
>       column. The central column for the feature seems to be the
>       existing dateaccessioned column instead.
>       With the implications of a) I am not sure we should include the
>       'new' from the beginning without having a clearer definition
>       on how to use it.

This field is important for the workflow of our users, it is part of the feature.

> 4) Features in documentation
>    I think the documentation lists features more like in an ad, than
>    like in a help file.

Sorry, it was not the expected goal.


> It's not clear to the reader, how to achieve
>    the described functionality. That's what I have come up with:
>    
> +  <li>know easily what are the new items in the catalogue.</li>
>
>    This still requires SQL, so doesn't seem to depend on the feature.

It depends on the "new" field, so it depends on this feature, no?


> +  <li>display an icon in the search results for new items.</li>
>
>     The only way to do this I can come up with is limited to libraries
>     using biblio level itypes and by switching the itemtype with the
>     script.

Yes, it is one of the purpose of it.


> +  <li>configure issuing rules depending the 'new' status.</li>
>
>     I think this would also require switching the itemtype?

Yes.


> +  <li>get a RSS/Atom feeds on these new items.</li>
>
>     I think this would require using a special value in an item
>     column that is searchable with Zebra. So you can build a search in
>     Zebra that can then be used for the RSS feed?

I don't know how they do that, maybe using a sql query.


> 4) biblioitems
>
>    a) I am worried about some columns that should not be substituted:
>       biblionumber, itemnumber, barcode, totalissues, onloan ...
>       I think those are potentially dangerous and should not be listed.

It is a cronjob script, all of them should be used with full knowledge of the facts.
I think it is easier/better to list all fields rather than to limit the possibilities.

>    b) To me it seems like the only goal of including biblioitems is
>       the possibility to change the bibliotitems itemtype. As it is now,
>       it also allows changing values like title, pages, etc. which
>       borders on a record batch edit. To me this seems a bit misplaced
>       here.
>       Also, ToggleNewStatus seems to only do ModItem - but you can
>       define substitutions for biblioitems? (see also 6)

It seems only conditions on biblioitems have been tested, we don't use biblioitems for substitutions. I will provide a patch to remove the biblioitems fields in the substitution list.


> 5) GUI
>
>    a) It's not possible to edit a single rule. In order to add a rule
>       or edit a rule, you always have to use 'Edit' and then all rules
>       will display, the form for adding the new rule is at the very
>       bottom. It is not very comfortable.

What do you suggest? I tried to do something ergonomic with a quite complicated form.
With the actual way, if you have to add several rules, it is useless to save after adding each one, but only once at the end.
I chose to edit all rules on the same screen, because they are stored in a syspref and I would prefer not to parse the pref to update the rules.

>    b) There are no mandatory fields, duration is optional, as are
>       conditions and substitions. So you can define an empty rule.

Yes, it could be possible if you want to mark all items as new.

I really want to avoid a long list of follow-ups.
I don't want to add all requests in this report. If some reports are opened and linked to this one, and they are relevant, I will provide a patch.

Originally, this feature was something like http://git.biblibre.com/?p=koha;a=blob;f=misc/cronjobs/toggle_new_status.pl;h=5acad013fac5bbbe22d17dab4369f99e28dd93e3;hb=8aa5f86906774da52bd317acef4131d9142c2431

We decided to submit something more flexible and more complete for the community, in order to open the possibilities.
Comment 38 Jonathan Druart 2014-04-30 08:31:44 UTC Comment hidden (obsolete)
Comment 39 Jonathan Druart 2014-06-20 10:07:52 UTC
Not sure why this is failed qa, I think it can be back to signoff.
Comment 40 Jonathan Druart 2014-06-25 13:17:26 UTC Comment hidden (obsolete)
Comment 41 Jonathan Druart 2014-06-25 13:17:31 UTC Comment hidden (obsolete)
Comment 42 Jonathan Druart 2014-06-25 13:17:36 UTC Comment hidden (obsolete)
Comment 43 Jonathan Druart 2014-06-25 13:17:40 UTC Comment hidden (obsolete)
Comment 44 Jonathan Druart 2014-06-25 13:17:44 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2014-06-25 13:17:48 UTC Comment hidden (obsolete)
Comment 46 Jonathan Druart 2014-06-25 13:17:52 UTC Comment hidden (obsolete)
Comment 47 Jonathan Druart 2014-06-25 13:19:39 UTC
I extracted the DB changes in a new patch. I also added the DBIx changes to it.
It fixes a bug: the items.new field was not list in the field list.
Comment 48 Jonathan Druart 2014-06-25 13:21:08 UTC
(In reply to Jonathan Druart from comment #46)
> Created attachment 29222 [details] [review] [review]
> Bug 11023: DB changes

Note: I don't know why this change has been generated:
-    on_delete     => "CASCADE",
+    on_delete     => "RESTRICT",
Comment 49 claire.hernandez@biblibre.com 2014-07-22 08:07:46 UTC
Just to support its integration. We use it a lot in production and for the moment we have to apply it manually, but patchs approved by BibLibre ;)
Comment 50 Kyle M Hall 2014-09-19 14:53:29 UTC
Issues:
1) Can enter an invalid days value ( for example 'test' rather than '5' ) in the editor

2) It would be better if the rules were listed in a table. The current display is very hard to read.

3) I think "Duration" should be "Age". Duration implies something taking place between now and the future, which isn't the case. I believe it's easier to think in terms of "When this item is 10 days old, do this".

3) I'm not sure if "Toggle new status" is a good name for this feature. Maybe something more along the lines of "Automatic item modifications by age". That's quite a mouthful, but I think it represents the feature better.

The feature seems quite powerful, and I think the current name belies that fact.
Comment 51 Jonathan Druart 2014-09-23 08:11:27 UTC Comment hidden (obsolete)
Comment 52 Jonathan Druart 2014-09-23 08:11:33 UTC Comment hidden (obsolete)
Comment 53 Jonathan Druart 2014-09-23 08:11:39 UTC Comment hidden (obsolete)
Comment 54 Jonathan Druart 2014-09-23 08:14:12 UTC
(In reply to Kyle M Hall from comment #50)

Kyle,
Thanks to QA this feature!

> Issues:
> 1) Can enter an invalid days value ( for example 'test' rather than '5' ) in
> the editor

Fixed, it's now a number.

> 2) It would be better if the rules were listed in a table. The current
> display is very hard to read.

I am not sure, you can have several conditions and substitutions for 1 rule.
I started with a table but I didn't manage to do something clear.

> 3) I think "Duration" should be "Age". Duration implies something taking
> place between now and the future, which isn't the case. I believe it's
> easier to think in terms of "When this item is 10 days old, do this".

Renamed.

> 3) I'm not sure if "Toggle new status" is a good name for this feature.
> Maybe something more along the lines of "Automatic item modifications by
> age". That's quite a mouthful, but I think it represents the feature better.

Renamed.
Comment 55 Jonathan Druart 2014-11-05 08:45:05 UTC Comment hidden (obsolete)
Comment 56 Jonathan Druart 2014-11-05 08:45:09 UTC Comment hidden (obsolete)
Comment 57 Jonathan Druart 2014-11-05 08:45:13 UTC Comment hidden (obsolete)
Comment 58 Jonathan Druart 2014-11-05 08:45:17 UTC Comment hidden (obsolete)
Comment 59 Jonathan Druart 2014-11-05 08:45:21 UTC Comment hidden (obsolete)
Comment 60 Jonathan Druart 2014-11-05 08:45:24 UTC Comment hidden (obsolete)
Comment 61 Jonathan Druart 2014-11-05 08:45:28 UTC Comment hidden (obsolete)
Comment 62 Jonathan Druart 2014-11-05 08:45:32 UTC Comment hidden (obsolete)
Comment 63 Jonathan Druart 2014-11-05 08:45:36 UTC Comment hidden (obsolete)
Comment 64 Jonathan Druart 2014-11-05 08:45:40 UTC Comment hidden (obsolete)
Comment 65 Katrin Fischer 2014-12-27 19:44:05 UTC
Fixed the first conflict on the tools home page, but then got stuck with a harder conflict:

Apply? [(y)es, (n)o, (i)nteractive] i
Applying: Bug 11023: Add the ability to specify fields from biblioitems table.
Applying: Bug 11023: Add UT for C4::Items::ToggleNewStatus
Applying: Bug 11023: FIX - condition on biblioitems table does not work
Applying: Bug 11023: Use DBIx-Class to retrieve column names
fatal: sha1 information is lacking or useless (C4/Items.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 11023: Use DBIx-Class to retrieve column names
The copy of the patch that failed is found in:
   /home/katrin/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-11023-Use-DBIx-Class-to-retrieve-column-names-J9vKlx.patch
Comment 66 Jonathan Druart 2014-12-29 10:22:12 UTC Comment hidden (obsolete)
Comment 67 Jonathan Druart 2014-12-29 10:22:16 UTC Comment hidden (obsolete)
Comment 68 Jonathan Druart 2014-12-29 10:22:20 UTC Comment hidden (obsolete)
Comment 69 Jonathan Druart 2014-12-29 10:22:25 UTC Comment hidden (obsolete)
Comment 70 Jonathan Druart 2014-12-29 10:22:28 UTC Comment hidden (obsolete)
Comment 71 Jonathan Druart 2014-12-29 10:22:32 UTC Comment hidden (obsolete)
Comment 72 Jonathan Druart 2014-12-29 10:22:36 UTC Comment hidden (obsolete)
Comment 73 Jonathan Druart 2014-12-29 10:22:40 UTC Comment hidden (obsolete)
Comment 74 Jonathan Druart 2014-12-29 10:22:44 UTC Comment hidden (obsolete)
Comment 75 Jonathan Druart 2014-12-29 10:22:48 UTC Comment hidden (obsolete)
Comment 76 Jonathan Druart 2014-12-29 10:23:10 UTC
Patches rebased.
Comment 77 Kyle M Hall 2015-01-02 14:00:46 UTC Comment hidden (obsolete)
Comment 78 Kyle M Hall 2015-01-02 14:01:01 UTC Comment hidden (obsolete)
Comment 79 Kyle M Hall 2015-01-02 14:01:04 UTC Comment hidden (obsolete)
Comment 80 Kyle M Hall 2015-01-02 14:01:08 UTC Comment hidden (obsolete)
Comment 81 Kyle M Hall 2015-01-02 14:01:11 UTC Comment hidden (obsolete)
Comment 82 Kyle M Hall 2015-01-02 14:01:14 UTC Comment hidden (obsolete)
Comment 83 Kyle M Hall 2015-01-02 14:01:17 UTC Comment hidden (obsolete)
Comment 84 Kyle M Hall 2015-01-02 14:01:21 UTC Comment hidden (obsolete)
Comment 85 Kyle M Hall 2015-01-02 14:01:24 UTC Comment hidden (obsolete)
Comment 86 Kyle M Hall 2015-01-02 14:01:27 UTC Comment hidden (obsolete)
Comment 87 Kyle M Hall 2015-01-02 14:01:31 UTC Comment hidden (obsolete)
Comment 88 Kyle M Hall 2015-01-02 14:01:34 UTC Comment hidden (obsolete)
Comment 89 Kyle M Hall 2015-01-02 14:09:49 UTC Comment hidden (obsolete)
Comment 90 Kyle M Hall 2015-01-02 14:14:37 UTC Comment hidden (obsolete)
Comment 91 Katrin Fischer 2015-01-04 22:00:24 UTC
No intention to block this, but still a bit worried about the possibility of people doing changes that might cause problems.

Would people be ok with adding a separate permission for this tool? Currently it's tied to the batch delete permission, but not working with the restricted batch delete:

flagsrequired => { tools => 'items_batchmod' },

Offering to write the patch :)
Comment 92 Jonathan Druart 2015-01-05 08:55:08 UTC
(In reply to Katrin Fischer from comment #91)
> No intention to block this, but still a bit worried about the possibility of
> people doing changes that might cause problems.
> 
> Would people be ok with adding a separate permission for this tool?
> Currently it's tied to the batch delete permission, but not working with the
> restricted batch delete:
> 
> flagsrequired => { tools => 'items_batchmod' },
> 
> Offering to write the patch :)

The items_batchmod permission make sense to me.
I would prefer to add the support for items_batchmod_restricted if it makes sense for you too.
Comment 93 Katrin Fischer 2015-01-05 09:21:31 UTC
Guess I am still a little worried about people breaking things with this new tool - so that's why I'd have preferred a separate permission. We have a separate permission for every other tool, so it would fit in well. 

But the combination with the restricted mode makes things more complicated here. 
Not sure what's best. If we leave it with item batchmod it would make sense to add the restricted behaviour, but also sounds more complicated to do :) Maybe we should open a separate bug?
Comment 94 Jonathan Druart 2015-01-05 09:29:27 UTC
(In reply to Katrin Fischer from comment #93)
> Maybe we should open a separate bug?

It's certainly the best.
While people will start to use it, we will get feedback to improve the existing behavior.
Comment 95 Jonathan Druart 2015-01-05 09:29:44 UTC
s/While/When
Comment 96 Jonathan Druart 2015-01-05 09:31:30 UTC
(In reply to Kyle M Hall from comment #90)
> Created attachment 34883 [details] [review] [review]
> Bug 11023 [QA Followup] - Complete the renaming of "toggle new status" to
> "automatic item modification by age"
> 
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

I am not sure it's a good idea to create files and rename them in the same patch set.
Maybe should we squash patches?
Comment 97 Jonathan Druart 2015-01-19 12:31:25 UTC Comment hidden (obsolete)
Comment 98 Jonathan Druart 2015-01-19 12:31:33 UTC Comment hidden (obsolete)
Comment 99 Jonathan Druart 2015-01-19 12:32:13 UTC
(In reply to Jonathan Druart from comment #96)
> (In reply to Kyle M Hall from comment #90)
> > Created attachment 34883 [details] [review] [review] [review]
> > Bug 11023 [QA Followup] - Complete the renaming of "toggle new status" to
> > "automatic item modification by age"
> > 
> > Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
> 
> I am not sure it's a good idea to create files and rename them in the same
> patch set.
> Maybe should we squash patches?

I squashed all patches, except the DB changes.
Comment 100 Tomás Cohen Arazi 2015-07-20 18:09:36 UTC
Can u please rebase it? I'm willing to test this ASAP.
Comment 101 claire.hernandez@biblibre.com 2015-07-24 09:45:19 UTC
Tomàs, Jajm should take it today. Thanks.
Comment 102 Julian Maurice 2015-07-24 10:06:41 UTC
Created attachment 41147 [details] [review]
Bug 11023: Automatic item modification by age (Was Toggle "new" status")

Rebased on master
Comment 103 Julian Maurice 2015-07-24 10:07:41 UTC
Created attachment 41148 [details] [review]
Bug 11023: DB changes

Rebased on master and Koha/Schema/* files removed from the patch to avoid conflicts
Comment 104 Pierre Angot 2015-09-24 13:24:56 UTC
Hello,

Can you tell me if this patch will be compatible with 3.18 ?

We are very interested ^^
Comment 105 Jonathan Druart 2015-09-24 15:35:07 UTC
(In reply to Pierre Angot from comment #104)
> Hello,
> 
> Can you tell me if this patch will be compatible with 3.18 ?
> 
> We are very interested ^^

At least one of the previous version was :)
It won't be backported, it's an enh.
Comment 106 Brendan Gallagher 2016-03-02 23:12:20 UTC
Pushed to Master - Should be in the May 2016 Release. Thanks!
Comment 107 Kyle M Hall 2016-03-04 19:37:23 UTC
I just had a thought about this. I think the db column 'new' is going to never be accessible via Koha::Item. I think a followup will be needed to change the column from 'new' to maybe 'new_status'? Any other column name suggestions would be appreciated!
Comment 108 Jonathan Druart 2016-03-08 09:53:22 UTC
(In reply to Kyle M Hall from comment #107)
> I just had a thought about this. I think the db column 'new' is going to
> never be accessible via Koha::Item. I think a followup will be needed to
> change the column from 'new' to maybe 'new_status'? Any other column name
> suggestions would be appreciated!

See bug 16004.