Current date columns of items table which could apply for this enhancement:

replacementpricedate
datelastborrowed
datelastseen
damaged_on
itemlost_on
withdrawn_on

I was about to file a new bug for this as we discussed the use of this tool in context of 'quarantining' books. I think having the option to use datelastseeen (updated on checkin) would be very helpful to enable us to make the necessary status change.

But the other columns listed also create ideas for use cases.

+1, we definitely have libraries that would make use of this if it could key off of dates other than dateaccessioned.

Created attachment 129371 [details] [review]
Bug 22827: Add age dependency on other fields than dateaccessioned

A new agefield has been added to the 'Automatic item modifications by
age' tool. The options for the agefield are: replacementpricedate, datelastborrowed,
datelastseen, damaged_on, itemlost_on, withdrawn_on

If no option is selected then Koha will default to using
dateaccessioned.

Test plan:

1. Go to: Tools > Catalog > Automatic item modifications by age
2. Observe there is a new 'Age field' dropdown in the rule form.
3. Create a rule, set the values:
- 'Age in days' = 20
- 'Substitutions': 'items.barcode' = 'test'
fields
- 'Age field' = 'items.datelastseen'
- Save the rule
4. Confirm the 'List of rules' page displays 'items.datelastseen' in the 'Age field' column
5. Add another rule:
- 'Age in days' = 1
- 'Substitutions': 'items.barcode' = 'test2'
- Leave 'Age field' = 'Choose an age field'
- Save the rule
6. Confirm the 'List of rules' page displays 'items.dateaccesioned' in
the 'Age field' column for that second rule
7. Confirm you can delete rules
8. Edit a record:
- Make the items.dateaccessioned less than 20 days ago (so rule 1 is false)
- Make the items.datelastseen more than 1 day ago (so rule 2 is true)
9. Run the automatic_items_modification_by_age.pl:
- sudo koha-shell <instance>
- cd misc/cronjobs
- ./automatic_item_modification_by_age.pl -v
10. Confirm the item has it's barcode set to 'test2'
11. Run unit tests:
- sudo koha-shell <instance>
- cd t
- prove db_dependent/Item/AutomaticItemModificationByAge.t -v

Sponsored-By: Catalyst IT

Ready for testing. 

In my patchset the fields in comment #1 are hardcoded. Please let me know if there is a way to fetch datetime items columns without having to hardcode them?

I've not read the patch here yet.. but to answer your question.. you can introspect the Koha::Object.. see https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/Object.pm#L129 for an example.

But.. I'm not sure if it's worth the overhead/extra code lines here if I'm honest?

I followed the test plan successfully except:
- your directions are inconsistent. To make step 8 work, the rule in step 3 should use dateaccessioned and the rule in step 5 should use datelastseen
- I get a messy error on the test at the end. I don't have a clue what this means, so it could be my error:

Cannot detect source of 't/db_dependent/Item/AutomaticItemModificationByAge'! at /usr/share/perl/5.32/TAP/Parser/IteratorFactory.pm line 256.
	TAP::Parser::IteratorFactory::detect_source(TAP::Parser::IteratorFactory=HASH(0x55d2dc3c5c48), TAP::Parser::Source=HASH(0x55d2dc292438)) called at /usr/share/perl/5.32/TAP/Parser/IteratorFactory.pm line 211
	TAP::Parser::IteratorFactory::make_iterator(TAP::Parser::IteratorFactory=HASH(0x55d2dc3c5c48), TAP::Parser::Source=HASH(0x55d2dc292438)) called at /usr/share/perl/5.32/TAP/Parser.pm line 472
	TAP::Parser::_initialize(TAP::Parser=HASH(0x55d2dc030148), HASH(0x55d2dc03fee0)) called at /usr/share/perl/5.32/TAP/Object.pm line 55
	TAP::Object::new("TAP::Parser", HASH(0x55d2dc03fee0)) called at /usr/share/perl/5.32/TAP/Object.pm line 130
	TAP::Object::_construct(TAP::Harness=HASH(0x55d2dbad1678), "TAP::Parser", HASH(0x55d2dc03fee0)) called at /usr/share/perl/5.32/TAP/Harness.pm line 852
	TAP::Harness::make_parser(TAP::Harness=HASH(0x55d2dbad1678), TAP::Parser::Scheduler::Job=HASH(0x55d2dc268728)) called at /usr/share/perl/5.32/TAP/Harness.pm line 651
	TAP::Harness::_aggregate_single(TAP::Harness=HASH(0x55d2dbad1678), TAP::Parser::Aggregator=HASH(0x55d2dbad2128), TAP::Parser::Scheduler=HASH(0x55d2dc2686f8)) called at /usr/share/perl/5.32/TAP/Harness.pm line 743
	TAP::Harness::aggregate_tests(TAP::Harness=HASH(0x55d2dbad1678), TAP::Parser::Aggregator=HASH(0x55d2dbad2128), "t/db_dependent/Item/AutomaticItemModificationByAge.t") called at /usr/share/perl/5.32/TAP/Harness.pm line 558
	TAP::Harness::__ANON__() called at /usr/share/perl/5.32/TAP/Harness.pm line 571
	TAP::Harness::runtests(TAP::Harness=HASH(0x55d2dbad1678), "t/db_dependent/Item/AutomaticItemModificationByAge.t") called at /usr/share/perl/5.32/App/Prove.pm line 548
	App::Prove::_runtests(App::Prove=HASH(0x55d2dba9c538), HASH(0x55d2dbf17978), "t/db_dependent/Item/AutomaticItemModificationByAge.t") called at /usr/share/perl/5.32/App/Prove.pm line 506
	App::Prove::run(App::Prove=HASH(0x55d2dba9c538)) called at /usr/bin/prove line 13

That said, I'm super excited to see a patch here and am eager to test again!

(In reply to Andrew Fuerste-Henry from comment #8)
> That said, I'm super excited to see a patch here and am eager to test again!

Thanks Andrew. Hmm I'll fix up those 2 problems and attach a new patch to this ASAP!

Created attachment 129460 [details] [review]
Bug 22827: Add age dependency on other fields than dateaccessioned

A new agefield has been added to the 'Automatic item modifications by
age' tool. The options for the agefield are: replacementpricedate, datelastborrowed,
datelastseen, damaged_on, itemlost_on, withdrawn_on

If no option is selected then Koha will default to saving 'agefield' =
items.dateaccessioned

Similarly, if a Koha instance has an old item rule without 'agefield'
defined then Koha will default to using 'items.dateaccessioned'.
This is confirmed by the AutomaticItemModificationByAge.t unit test.

Test plan:

1. Go to: Tools > Catalog > Automatic item modifications by age
2. Observe there is a new 'Age field' dropdown in the rule form.
3. Create a rule, set the values:
- 'Age in days' = 20
- Leave 'Age field' = 'Choose an age field'
- 'Substitutions': 'items.barcode' = 'test'
- Save the rule
4. Confirm the 'List of rules' page displays 'items.dateaccessioned in the 'Age field' column
5. Add another rule:
- 'Age in days' = 2
- 'Age field' = 'items.datelastseen'
- 'Substitutions': 'items.barcode' = 'test2'
- Save the rule
6. Confirm the 'List of rules' page displays 'items.datelastseen' in
the 'Age field' column for that second rule
7. Add some more rules and confirm you can delete them
8. Edit a record:
- Make the items.dateaccessioned = 3 day ago (so rule 1 is false)
- Make the items.datelastseen = 3 days ago (so rule 2 is true)
9. Run the automatic_items_modification_by_age.pl:
- sudo koha-shell <instance>
- cd misc/cronjobs
- ./automatic_item_modification_by_age.pl -v -c
10. Confirm the item has it's barcode set to 'test2'
11. Run unit tests:
- sudo koha-shell <instance>
- prove t/db_dependent/Items/AutomaticItemModificationByAge.t -v

Sponsored-By: Catalyst IT

Hi Andrew and Martin, 

Martin: Thank you for your reply in comment #6. My concerns with hard coding were:

1. Everytime a datetime column is added to the items table then the community would have to remember to update this hardcoded list of columns.

2. If an existing datetime column is removed/renamed then the hardcoded list would have to be updated otherwise my alterations to C4::Items->ToggleNewStatus() would throw an error. As the hardcoded list determines what column is queried.

But as you say doing an introspect of the koha::Object may not be worthwhile. So I have left the hardcoded list for now. I'm happy to add it later though if that is what QA wants.

Andrew: I have corrected the test plan. That error you encountered was because my test plan noted an incorrect unit test filepath. So it was not your error :)

Could you please re-test now?

Many thanks!
Alex

Created attachment 129481 [details] [review]
Bug 22827: Add age dependency on other fields than dateaccessioned

A new agefield has been added to the 'Automatic item modifications by
age' tool. The options for the agefield are: replacementpricedate, datelastborrowed,
datelastseen, damaged_on, itemlost_on, withdrawn_on

If no option is selected then Koha will default to saving 'agefield' =
items.dateaccessioned

Similarly, if a Koha instance has an old item rule without 'agefield'
defined then Koha will default to using 'items.dateaccessioned'.
This is confirmed by the AutomaticItemModificationByAge.t unit test.

Test plan:

1. Go to: Tools > Catalog > Automatic item modifications by age
2. Observe there is a new 'Age field' dropdown in the rule form.
3. Create a rule, set the values:
- 'Age in days' = 20
- Leave 'Age field' = 'Choose an age field'
- 'Substitutions': 'items.barcode' = 'test'
- Save the rule
4. Confirm the 'List of rules' page displays 'items.dateaccessioned in the 'Age field' column
5. Add another rule:
- 'Age in days' = 2
- 'Age field' = 'items.datelastseen'
- 'Substitutions': 'items.barcode' = 'test2'
- Save the rule
6. Confirm the 'List of rules' page displays 'items.datelastseen' in
the 'Age field' column for that second rule
7. Add some more rules and confirm you can delete them
8. Edit a record:
- Make the items.dateaccessioned = 3 day ago (so rule 1 is false)
- Make the items.datelastseen = 3 days ago (so rule 2 is true)
9. Run the automatic_items_modification_by_age.pl:
- sudo koha-shell <instance>
- cd misc/cronjobs
- ./automatic_item_modification_by_age.pl -v -c
10. Confirm the item has it's barcode set to 'test2'
11. Run unit tests:
- sudo koha-shell <instance>
- prove t/db_dependent/Items/AutomaticItemModificationByAge.t -v

Sponsored-By: Catalyst IT

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

I was wondering about the fallback instead of updating the rules, until I realized that the rules are stored in a system preference: automatic_item_modification_by_age_configuration 

Also filed a new bug for a warning that exists before and after the patches are applied: Bug 30366 - Warn when running automatic_item_modification_by_age.pl

Created attachment 132265 [details] [review]
Bug 22827: Add age dependency on other fields than dateaccessioned

A new agefield has been added to the 'Automatic item modifications by
age' tool. The options for the agefield are: replacementpricedate, datelastborrowed,
datelastseen, damaged_on, itemlost_on, withdrawn_on

If no option is selected then Koha will default to saving 'agefield' =
items.dateaccessioned

Similarly, if a Koha instance has an old item rule without 'agefield'
defined then Koha will default to using 'items.dateaccessioned'.
This is confirmed by the AutomaticItemModificationByAge.t unit test.

Test plan:

1. Go to: Tools > Catalog > Automatic item modifications by age
2. Observe there is a new 'Age field' dropdown in the rule form.
3. Create a rule, set the values:
- 'Age in days' = 20
- Leave 'Age field' = 'Choose an age field'
- 'Substitutions': 'items.barcode' = 'test'
- Save the rule
4. Confirm the 'List of rules' page displays 'items.dateaccessioned in the 'Age field' column
5. Add another rule:
- 'Age in days' = 2
- 'Age field' = 'items.datelastseen'
- 'Substitutions': 'items.barcode' = 'test2'
- Save the rule
6. Confirm the 'List of rules' page displays 'items.datelastseen' in
the 'Age field' column for that second rule
7. Add some more rules and confirm you can delete them
8. Edit a record:
- Make the items.dateaccessioned = 3 day ago (so rule 1 is false)
- Make the items.datelastseen = 3 days ago (so rule 2 is true)
9. Run the automatic_items_modification_by_age.pl:
- sudo koha-shell <instance>
- cd misc/cronjobs
- ./automatic_item_modification_by_age.pl -v -c
10. Confirm the item has it's barcode set to 'test2'
11. Run unit tests:
- sudo koha-shell <instance>
- prove t/db_dependent/Items/AutomaticItemModificationByAge.t -v

Sponsored-By: Catalyst IT

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Thanks for testing and QA Andrew and Katrin!

Nice enhancement.
Please add a release notes to explain this new option

Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑

(In reply to Fridolin Somers from comment #16)
> Nice enhancement.
> Please add a release notes to explain this new option

Thanks, Fridolin! I've added release notes, I hope they adequately explain this enhancement. 

Cheers,
Alex

<span class="hint">If not set then items.dateaccessioned will be used</span>

Instead of this we could just select this value in the dropdown list if none exist, and remove the empty option to make it mandatory.

(In reply to Jonathan Druart from comment #19)
> <span class="hint">If not set then items.dateaccessioned will be used</span>
> 
> Instead of this we could just select this value in the dropdown list if none
> exist, and remove the empty option to make it mandatory.

Note: We'd also need a database update for the old entries. I can try to file that later.

(In reply to Katrin Fischer from comment #20)
> (In reply to Jonathan Druart from comment #19)
> > <span class="hint">If not set then items.dateaccessioned will be used</span>
> > 
> > Instead of this we could just select this value in the dropdown list if none
> > exist, and remove the empty option to make it mandatory.
> 
> Note: We'd also need a database update for the old entries. I can try to
> file that later.

Not necessarily but yes that will be better ofc.

(In reply to Jonathan Druart from comment #21)
> (In reply to Katrin Fischer from comment #20)
> > (In reply to Jonathan Druart from comment #19)
> > > <span class="hint">If not set then items.dateaccessioned will be used</span>
> > > 
> > > Instead of this we could just select this value in the dropdown list if none
> > > exist, and remove the empty option to make it mandatory.
> > 
> > Note: We'd also need a database update for the old entries. I can try to
> > file that later.
> 
> Not necessarily but yes that will be better ofc.

Filed as:
Bug 33979 - Improve behaviour of source list on 'item modifications by age' tool