Bug 32610

Summary: Add ability to specify patron attribute as a date
Product: Koha Reporter: Lucas Gass (lukeg) <lucas>
Component: PatronsAssignee: Shi Yao Wang <shi-yao.wang>
Status: RESOLVED FIXED QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: andrew, caroline.cyr-la-rose, fridolin.somers, gmcharlt, indradg, jonathan.druart, kyle.m.hall, m.de.rooy, martin.renvoize, pedro.amorim, shi-yao.wang, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20958
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36738
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36816
Change sponsored?: Unsponsored Patch complexity: Medium patch
Documentation contact: Caroline Cyr La Rose Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/899
Text to go in the release notes:
This makes it possible to add patron attribute fields as dates. The field will then display as any other date field including the calendar widget.
Version(s) released in:
24.05.00
Circulation function:
Bug Depends on:    
Bug Blocks: 36882, 37329, 36738, 36922    
Attachments: Bug 32610: Add borrower_attribute_types.is_date to the database
Bug 32610: [DO NOT PUSH] DBIC Update
Bug 32610: Add date option to extended patron attributes administration page
Bug 32610: Add date extended patron attributes to patron module
Bug 32610: Add date extended patron attributes to the OPAC
Bug 32610: Add date extended patron attribute handling to overdues report
Bug 32610: Add borrower_attribute_types.is_date to the database
Bug 32610: [DO NOT PUSH] DBIC Update
Bug 32610: Add date option to extended patron attributes administration page
Bug 32610: Add date extended patron attributes to patron module
Bug 32610: Add date extended patron attributes to the OPAC
Bug 32610: Add date extended patron attribute handling to overdues report
Bug 32610: Add borrower_attribute_types.is_date to the database
Bug 32610: [DO NOT PUSH] DBIC Update
Bug 32610: Add date option to extended patron attributes administration page
Bug 32610: Add date extended patron attributes to patron module
Bug 32610: Add date extended patron attributes to the OPAC
Bug 32610: Add date extended patron attribute handling to overdues report
Bug 32610: Remove data-date_to and tweak comment
Bug 32610: Add borrower_attribute_types.is_date to the database
Bug 32610: [DO NOT PUSH] DBIC Update
Bug 32610: Add date option to extended patron attributes administration page
Bug 32610: Add date extended patron attributes to patron module
Bug 32610: Add date extended patron attributes to the OPAC
Bug 32610: Add date extended patron attribute handling to overdues report
Bug 32610: Remove data-date_to and tweak comment
Bug 32610: Patron attribute is_date error handling and tests
Bug 32610: make date attributes repeatable
Bug 32610: Add borrower_attribute_types.is_date to the database
Bug 32610: [DO NOT PUSH] DBIC Update
Bug 32610: Add date option to extended patron attributes administration page
Bug 32610: Add date extended patron attributes to patron module
Bug 32610: Add date extended patron attributes to the OPAC
Bug 32610: Add date extended patron attribute handling to overdues report
Bug 32610: Remove data-date_to and tweak comment
Bug 32610: Patron attribute is_date error handling and tests
Bug 32610: make date attributes repeatable
Bug 32610: (QA follow-up) Fix unit tests
Bug 32610: (QA follow-up) Tidy exceptions file
Bug 32610: (QA follow-up) Tidy Koha::Patron::Attribute
Bug 32610: (follow-up) Adjust TestBuilder
Bug 36738: Date attributes follow-ups

Description Lucas Gass (lukeg) 2023-01-11 17:51:04 UTC
Sometimes libraries use patron extended attributes that are dates. It would be handy if we could specify that a particular attribute is a date and then have it become a flatpickr instance.
Comment 1 Katrin Fischer 2023-03-18 09:24:19 UTC
*** Bug 6339 has been marked as a duplicate of this bug. ***
Comment 2 Katrin Fischer 2023-03-18 11:00:18 UTC
Created attachment 148400 [details] [review]
Bug 32610: Add borrower_attribute_types.is_date to the database

* Change to kohastructure.sql for new installations
* Atomic database update to add the column for updating
  installations
Comment 3 Katrin Fischer 2023-03-18 11:00:21 UTC
Created attachment 148401 [details] [review]
Bug 32610: [DO NOT PUSH] DBIC Update
Comment 4 Katrin Fischer 2023-03-18 11:00:24 UTC
Created attachment 148402 [details] [review]
Bug 32610: Add date option to extended patron attributes administration page

With this page it will be possible to configure patron attributes
to be a date.

To test:
* Go to administration > patron attribute types
* Add a new patron attribute of type date
  * Dates cannot be repeatable or linked to an AV category, so:
  * Verify, if you check repeatable, date is disabled
  * Verify, if you select an AV category, date disabled
  * Verify, if you check date, AV category and repeatable are disabled
* Save the new patron attribute
* Edit the patron attribute
  * Verify all settings have been kept/stored correctly
Comment 5 Katrin Fischer 2023-03-18 11:00:27 UTC
Created attachment 148403 [details] [review]
Bug 32610: Add date extended patron attributes to patron module

This makes the necessary changes in the patron module of
the staff interface, so the new patron attribute appers and
behaves correctly when editing a patron record.

To test:
* You will need to test different configuration options for
  extended patron attributes (PA) in combination with the date option:
  * PA is a date and not mandatory
    * Patron form should have the calendar widget to let you set the date.
  * PA is a date and mandatory
    * Patron form shoudl have calendar widget and check that the date is
      set for allowing you to save the record.
  * PA is a date and unique
    * For this set the date in one patron record and try to
      set the same date in another. You should not be able to save.
  * PA displays in brief patron information
    * Make sure the date displays on the left formatted correctly
  * When the date PAs are saved, they should display nicely formatted
    on the details tab.
Comment 6 Katrin Fischer 2023-03-18 11:00:30 UTC
Created attachment 148404 [details] [review]
Bug 32610: Add date extended patron attributes to the OPAC

This makes sure that the attributes are handled correctly
when displayed and edited in the OPAC during address changes
or self registration.

To test:
  * You will need to test different configuration options for
    extended patron attributes (PA) in combination with the date option:
    * PA displays in OPAC, but is not editable
      * The date will show nicely formatted on the personal details tab.
    * PA displays and is editable in the OPAC
      * The date can be edited using the calendar widget
    * PA displays, is editabe and mandatory
      * The date is marked as required and you can't save without
        it being set.
  * Test the form behaves correctly when requesting changes for an
    existing patron account and when self registering.
Comment 7 Katrin Fischer 2023-03-18 11:00:34 UTC
Created attachment 148405 [details] [review]
Bug 32610: Add date extended patron attribute handling to overdues report

This makes sure that the overdues report manages filters for
extended patron attributes of the type date correctly.

To test:
* Make sure you have at least one PA marked as 'searchable'
* Make sure your patron has at east one checked out item
* Go to circulation > overdues
* If you have no overdue items, check "Show any items currently checked out:"
* Verify the list of checkouts displays
* Verify the PA filter option displays with the calendar widget
* Limit to the date in your patron's record - list remains
* Limit to any other date - list empties
Comment 8 Philip Orr 2023-03-28 14:00:44 UTC
Created attachment 148825 [details] [review]
Bug 32610: Add borrower_attribute_types.is_date to the database

* Change to kohastructure.sql for new installations
* Atomic database update to add the column for updating
  installations

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 9 Philip Orr 2023-03-28 14:00:46 UTC
Created attachment 148826 [details] [review]
Bug 32610: [DO NOT PUSH] DBIC Update

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 10 Philip Orr 2023-03-28 14:00:50 UTC
Created attachment 148827 [details] [review]
Bug 32610: Add date option to extended patron attributes administration page

With this page it will be possible to configure patron attributes
to be a date.

To test:
* Go to administration > patron attribute types
* Add a new patron attribute of type date
  * Dates cannot be repeatable or linked to an AV category, so:
  * Verify, if you check repeatable, date is disabled
  * Verify, if you select an AV category, date disabled
  * Verify, if you check date, AV category and repeatable are disabled
* Save the new patron attribute
* Edit the patron attribute
  * Verify all settings have been kept/stored correctly

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 11 Philip Orr 2023-03-28 14:00:53 UTC
Created attachment 148828 [details] [review]
Bug 32610: Add date extended patron attributes to patron module

This makes the necessary changes in the patron module of
the staff interface, so the new patron attribute appers and
behaves correctly when editing a patron record.

To test:
* You will need to test different configuration options for
  extended patron attributes (PA) in combination with the date option:
  * PA is a date and not mandatory
    * Patron form should have the calendar widget to let you set the date.
  * PA is a date and mandatory
    * Patron form shoudl have calendar widget and check that the date is
      set for allowing you to save the record.
  * PA is a date and unique
    * For this set the date in one patron record and try to
      set the same date in another. You should not be able to save.
  * PA displays in brief patron information
    * Make sure the date displays on the left formatted correctly
  * When the date PAs are saved, they should display nicely formatted
    on the details tab.

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 12 Philip Orr 2023-03-28 14:00:55 UTC
Created attachment 148829 [details] [review]
Bug 32610: Add date extended patron attributes to the OPAC

This makes sure that the attributes are handled correctly
when displayed and edited in the OPAC during address changes
or self registration.

To test:
  * You will need to test different configuration options for
    extended patron attributes (PA) in combination with the date option:
    * PA displays in OPAC, but is not editable
      * The date will show nicely formatted on the personal details tab.
    * PA displays and is editable in the OPAC
      * The date can be edited using the calendar widget
    * PA displays, is editabe and mandatory
      * The date is marked as required and you can't save without
        it being set.
  * Test the form behaves correctly when requesting changes for an
    existing patron account and when self registering.

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 13 Philip Orr 2023-03-28 14:00:59 UTC
Created attachment 148830 [details] [review]
Bug 32610: Add date extended patron attribute handling to overdues report

This makes sure that the overdues report manages filters for
extended patron attributes of the type date correctly.

To test:
* Make sure you have at least one PA marked as 'searchable'
* Make sure your patron has at east one checked out item
* Go to circulation > overdues
* If you have no overdue items, check "Show any items currently checked out:"
* Verify the list of checkouts displays
* Verify the PA filter option displays with the calendar widget
* Limit to the date in your patron's record - list remains
* Limit to any other date - list empties

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 14 Jonathan Druart 2023-04-25 19:18:48 UTC
Why are you adding data-date_to="to" in memberentrygen?

Not blocker but it would have been more flexible (for future enhancements) to have a datatype='date' column instead of is_date.
Comment 15 Jonathan Druart 2023-04-25 19:20:34 UTC
I guess you should adjust the comment here

circ/overdue.pl:my @patron_attr_filter_loop;   # array of [ domid cgivalue ismany isclone ordinal code description repeatable is_date authorised_value_category ]

koha-tmpl/intranet-tmpl/prog/en/modules/circ/overdue.tt:        <!-- domid cgivalue ismany isclone ordinal code description repeatable authorised_value_category -->
Comment 16 Shi Yao Wang 2023-10-25 18:56:07 UTC
Created attachment 157862 [details] [review]
Bug 32610: Add borrower_attribute_types.is_date to the database

* Change to kohastructure.sql for new installations
* Atomic database update to add the column for updating
  installations

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 17 Shi Yao Wang 2023-10-25 18:56:09 UTC
Created attachment 157863 [details] [review]
Bug 32610: [DO NOT PUSH] DBIC Update

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 18 Shi Yao Wang 2023-10-25 18:56:12 UTC
Created attachment 157864 [details] [review]
Bug 32610: Add date option to extended patron attributes administration page

With this page it will be possible to configure patron attributes
to be a date.

To test:
* Go to administration > patron attribute types
* Add a new patron attribute of type date
  * Dates cannot be repeatable or linked to an AV category, so:
  * Verify, if you check repeatable, date is disabled
  * Verify, if you select an AV category, date disabled
  * Verify, if you check date, AV category and repeatable are disabled
* Save the new patron attribute
* Edit the patron attribute
  * Verify all settings have been kept/stored correctly

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 19 Shi Yao Wang 2023-10-25 18:56:14 UTC
Created attachment 157865 [details] [review]
Bug 32610: Add date extended patron attributes to patron module

This makes the necessary changes in the patron module of
the staff interface, so the new patron attribute appers and
behaves correctly when editing a patron record.

To test:
* You will need to test different configuration options for
  extended patron attributes (PA) in combination with the date option:
  * PA is a date and not mandatory
    * Patron form should have the calendar widget to let you set the date.
  * PA is a date and mandatory
    * Patron form shoudl have calendar widget and check that the date is
      set for allowing you to save the record.
  * PA is a date and unique
    * For this set the date in one patron record and try to
      set the same date in another. You should not be able to save.
  * PA displays in brief patron information
    * Make sure the date displays on the left formatted correctly
  * When the date PAs are saved, they should display nicely formatted
    on the details tab.

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 20 Shi Yao Wang 2023-10-25 18:56:17 UTC
Created attachment 157866 [details] [review]
Bug 32610: Add date extended patron attributes to the OPAC

This makes sure that the attributes are handled correctly
when displayed and edited in the OPAC during address changes
or self registration.

To test:
  * You will need to test different configuration options for
    extended patron attributes (PA) in combination with the date option:
    * PA displays in OPAC, but is not editable
      * The date will show nicely formatted on the personal details tab.
    * PA displays and is editable in the OPAC
      * The date can be edited using the calendar widget
    * PA displays, is editabe and mandatory
      * The date is marked as required and you can't save without
        it being set.
  * Test the form behaves correctly when requesting changes for an
    existing patron account and when self registering.

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 21 Shi Yao Wang 2023-10-25 18:56:19 UTC
Created attachment 157867 [details] [review]
Bug 32610: Add date extended patron attribute handling to overdues report

This makes sure that the overdues report manages filters for
extended patron attributes of the type date correctly.

To test:
* Make sure you have at least one PA marked as 'searchable'
* Make sure your patron has at east one checked out item
* Go to circulation > overdues
* If you have no overdue items, check "Show any items currently checked out:"
* Verify the list of checkouts displays
* Verify the PA filter option displays with the calendar widget
* Limit to the date in your patron's record - list remains
* Limit to any other date - list empties

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 22 Shi Yao Wang 2023-10-25 18:57:18 UTC
rebase
Comment 23 Shi Yao Wang 2023-10-25 19:09:21 UTC
Created attachment 157868 [details] [review]
Bug 32610: Remove data-date_to and tweak comment
Comment 24 Katrin Fischer 2023-10-25 21:13:18 UTC
Thanks a lot Shi Yao!
Comment 25 Shi Yao Wang 2023-11-15 16:58:44 UTC
Created attachment 158994 [details] [review]
Bug 32610: Add borrower_attribute_types.is_date to the database

* Change to kohastructure.sql for new installations
* Atomic database update to add the column for updating
  installations

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 26 Shi Yao Wang 2023-11-15 16:58:47 UTC
Created attachment 158995 [details] [review]
Bug 32610: [DO NOT PUSH] DBIC Update

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 27 Shi Yao Wang 2023-11-15 16:58:49 UTC
Created attachment 158996 [details] [review]
Bug 32610: Add date option to extended patron attributes administration page

With this page it will be possible to configure patron attributes
to be a date.

To test:
* Go to administration > patron attribute types
* Add a new patron attribute of type date
  * Dates cannot be repeatable or linked to an AV category, so:
  * Verify, if you check repeatable, date is disabled
  * Verify, if you select an AV category, date disabled
  * Verify, if you check date, AV category and repeatable are disabled
* Save the new patron attribute
* Edit the patron attribute
  * Verify all settings have been kept/stored correctly

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 28 Shi Yao Wang 2023-11-15 16:58:52 UTC
Created attachment 158997 [details] [review]
Bug 32610: Add date extended patron attributes to patron module

This makes the necessary changes in the patron module of
the staff interface, so the new patron attribute appers and
behaves correctly when editing a patron record.

To test:
* You will need to test different configuration options for
  extended patron attributes (PA) in combination with the date option:
  * PA is a date and not mandatory
    * Patron form should have the calendar widget to let you set the date.
  * PA is a date and mandatory
    * Patron form shoudl have calendar widget and check that the date is
      set for allowing you to save the record.
  * PA is a date and unique
    * For this set the date in one patron record and try to
      set the same date in another. You should not be able to save.
  * PA displays in brief patron information
    * Make sure the date displays on the left formatted correctly
  * When the date PAs are saved, they should display nicely formatted
    on the details tab.

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 29 Shi Yao Wang 2023-11-15 16:58:54 UTC
Created attachment 158998 [details] [review]
Bug 32610: Add date extended patron attributes to the OPAC

This makes sure that the attributes are handled correctly
when displayed and edited in the OPAC during address changes
or self registration.

To test:
  * You will need to test different configuration options for
    extended patron attributes (PA) in combination with the date option:
    * PA displays in OPAC, but is not editable
      * The date will show nicely formatted on the personal details tab.
    * PA displays and is editable in the OPAC
      * The date can be edited using the calendar widget
    * PA displays, is editabe and mandatory
      * The date is marked as required and you can't save without
        it being set.
  * Test the form behaves correctly when requesting changes for an
    existing patron account and when self registering.

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 30 Shi Yao Wang 2023-11-15 16:58:56 UTC
Created attachment 158999 [details] [review]
Bug 32610: Add date extended patron attribute handling to overdues report

This makes sure that the overdues report manages filters for
extended patron attributes of the type date correctly.

To test:
* Make sure you have at least one PA marked as 'searchable'
* Make sure your patron has at east one checked out item
* Go to circulation > overdues
* If you have no overdue items, check "Show any items currently checked out:"
* Verify the list of checkouts displays
* Verify the PA filter option displays with the calendar widget
* Limit to the date in your patron's record - list remains
* Limit to any other date - list empties

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 31 Shi Yao Wang 2023-11-15 16:58:59 UTC
Created attachment 159000 [details] [review]
Bug 32610: Remove data-date_to and tweak comment
Comment 32 Shi Yao Wang 2023-11-15 16:59:10 UTC
rebase
Comment 33 Pedro Amorim 2023-12-22 14:43:02 UTC
Failing QA here:

- Missing tests
- import_borrowers.tt should check if a given patron_attribute that's being added is is_date and validate the data accordingly, else the following bug may occur:

1) Create a patron attribute type of is_date as per the original test plan, named 'date'
2) Import a patron, visit:
/cgi-bin/koha/tools/import_borrowers.pl
3) Click "Download a Starter CSV file with all the columns." and add a cardnumber, categorycode, branchcode and on the patron_attributes column put date:test
4) Import, notice the import is successful, now visit that patron edit page (do a search for the imported cardnumber), e.g. example for patron id 289:
/cgi-bin/koha/members/moremember.pl?borrowernumber=289
5) Notice it blows up with error:
Template process failed: undef error - The given date (teste) does not match the date format (iso) at /kohadevbox/koha/Koha/DateUtils.pm line 205.
 at /kohadevbox/koha/C4/Templates.pm line 127
Comment 34 Pedro Amorim 2023-12-22 14:48:02 UTC
Not a blocker, but why the design decision of restricting a date attribute type to not be able to also be repeatable?

I can't find many good examples of how a repeatable date field would be useful, but I don't understand why we wouldn't allow it either.
Comment 35 Katrin Fischer 2023-12-23 11:31:02 UTC
(In reply to Pedro Amorim from comment #34)
> Not a blocker, but why the design decision of restricting a date attribute
> type to not be able to also be repeatable?
> 
> I can't find many good examples of how a repeatable date field would be
> useful, but I don't understand why we wouldn't allow it either.

Mostly because I wanted to keep it a bit more simple and thought that could still be done later if required. 

I had picked this up to work on in my free time, because it seemed something that people were interested in, but it grew quickly and got more complicated than I had first imagined. 

I think I'd definitely need some help to solve the import issue (good testing btw!) and probably the tests as well.

I am also a bit worried about the batch patron edit - at the time of me writing this patch set I had some trouble testing it because of a blocking bug.
Comment 36 Katrin Fischer 2024-04-01 13:11:41 UTC
I cannot continue here on my own - happy if someone wants to take over or collaborate in some way.
Comment 37 Shi Yao Wang 2024-04-29 14:17:35 UTC
Created attachment 165747 [details] [review]
Bug 32610: Patron attribute is_date error handling and tests
Comment 38 Shi Yao Wang 2024-04-29 16:32:46 UTC
Created attachment 165750 [details] [review]
Bug 32610: make date attributes repeatable

Test plan:
1- create a patron attribute type with is a date and repeatable checked
(administration -> patron attribute types)
2- add a couple of patron attribute of type date
3- Save
4- Edit to see if everything has been stored correctly
Comment 39 Shi Yao Wang 2024-04-29 16:34:31 UTC
(In reply to Pedro Amorim from comment #33)
> Failing QA here:
> 
> - Missing tests
> - import_borrowers.tt should check if a given patron_attribute that's being
> added is is_date and validate the data accordingly, else the following bug
> may occur:
> 
> 1) Create a patron attribute type of is_date as per the original test plan,
> named 'date'
> 2) Import a patron, visit:
> /cgi-bin/koha/tools/import_borrowers.pl
> 3) Click "Download a Starter CSV file with all the columns." and add a
> cardnumber, categorycode, branchcode and on the patron_attributes column put
> date:test
> 4) Import, notice the import is successful, now visit that patron edit page
> (do a search for the imported cardnumber), e.g. example for patron id 289:
> /cgi-bin/koha/members/moremember.pl?borrowernumber=289
> 5) Notice it blows up with error:
> Template process failed: undef error - The given date (teste) does not match
> the date format (iso) at /kohadevbox/koha/Koha/DateUtils.pm line 205.
>  at /kohadevbox/koha/C4/Templates.pm line 127

- Added some tests
- Added error handling for date type patron attribute. Borrower import should now skip patrons with invalid value of date type patron attribute.



(In reply to Pedro Amorim from comment #34)
> Not a blocker, but why the design decision of restricting a date attribute
> type to not be able to also be repeatable?
> 
> I can't find many good examples of how a repeatable date field would be
> useful, but I don't understand why we wouldn't allow it either.

It should be repeatable now


Putting to Needs Signoff for someone to test repeatable date attribute.
Comment 40 Katrin Fischer 2024-04-29 16:52:06 UTC
Thank a lot for your help! Would you like to add yourself as assignee? I won't have time to continue working on this but would love to see it happen.
Comment 41 Martin Renvoize (ashimema) 2024-04-30 11:23:10 UTC
I'm upgrading this back to 'Signed off' and will encourage the repeatable nature to be tested properly during QA.
Comment 42 Martin Renvoize (ashimema) 2024-04-30 11:55:54 UTC
Created attachment 165854 [details] [review]
Bug 32610: Add borrower_attribute_types.is_date to the database

* Change to kohastructure.sql for new installations
* Atomic database update to add the column for updating
  installations

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 43 Martin Renvoize (ashimema) 2024-04-30 11:55:57 UTC
Created attachment 165855 [details] [review]
Bug 32610: [DO NOT PUSH] DBIC Update

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 44 Martin Renvoize (ashimema) 2024-04-30 11:56:01 UTC
Created attachment 165856 [details] [review]
Bug 32610: Add date option to extended patron attributes administration page

With this page it will be possible to configure patron attributes
to be a date.

To test:
* Go to administration > patron attribute types
* Add a new patron attribute of type date
  * Dates cannot be repeatable or linked to an AV category, so:
  * Verify, if you check repeatable, date is disabled
  * Verify, if you select an AV category, date disabled
  * Verify, if you check date, AV category and repeatable are disabled
* Save the new patron attribute
* Edit the patron attribute
  * Verify all settings have been kept/stored correctly

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 45 Martin Renvoize (ashimema) 2024-04-30 11:56:04 UTC
Created attachment 165857 [details] [review]
Bug 32610: Add date extended patron attributes to patron module

This makes the necessary changes in the patron module of
the staff interface, so the new patron attribute appers and
behaves correctly when editing a patron record.

To test:
* You will need to test different configuration options for
  extended patron attributes (PA) in combination with the date option:
  * PA is a date and not mandatory
    * Patron form should have the calendar widget to let you set the date.
  * PA is a date and mandatory
    * Patron form shoudl have calendar widget and check that the date is
      set for allowing you to save the record.
  * PA is a date and unique
    * For this set the date in one patron record and try to
      set the same date in another. You should not be able to save.
  * PA displays in brief patron information
    * Make sure the date displays on the left formatted correctly
  * When the date PAs are saved, they should display nicely formatted
    on the details tab.

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Martin Renvoize (ashimema) 2024-04-30 11:56:07 UTC
Created attachment 165858 [details] [review]
Bug 32610: Add date extended patron attributes to the OPAC

This makes sure that the attributes are handled correctly
when displayed and edited in the OPAC during address changes
or self registration.

To test:
  * You will need to test different configuration options for
    extended patron attributes (PA) in combination with the date option:
    * PA displays in OPAC, but is not editable
      * The date will show nicely formatted on the personal details tab.
    * PA displays and is editable in the OPAC
      * The date can be edited using the calendar widget
    * PA displays, is editabe and mandatory
      * The date is marked as required and you can't save without
        it being set.
  * Test the form behaves correctly when requesting changes for an
    existing patron account and when self registering.

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize (ashimema) 2024-04-30 11:56:11 UTC
Created attachment 165859 [details] [review]
Bug 32610: Add date extended patron attribute handling to overdues report

This makes sure that the overdues report manages filters for
extended patron attributes of the type date correctly.

To test:
* Make sure you have at least one PA marked as 'searchable'
* Make sure your patron has at east one checked out item
* Go to circulation > overdues
* If you have no overdue items, check "Show any items currently checked out:"
* Verify the list of checkouts displays
* Verify the PA filter option displays with the calendar widget
* Limit to the date in your patron's record - list remains
* Limit to any other date - list empties

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 48 Martin Renvoize (ashimema) 2024-04-30 11:56:14 UTC
Created attachment 165860 [details] [review]
Bug 32610: Remove data-date_to and tweak comment

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 49 Martin Renvoize (ashimema) 2024-04-30 11:56:17 UTC
Created attachment 165861 [details] [review]
Bug 32610: Patron attribute is_date error handling and tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 50 Martin Renvoize (ashimema) 2024-04-30 11:56:20 UTC
Created attachment 165862 [details] [review]
Bug 32610: make date attributes repeatable

Test plan:
1- create a patron attribute type with is a date and repeatable checked
(administration -> patron attribute types)
2- add a couple of patron attribute of type date
3- Save
4- Edit to see if everything has been stored correctly

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 51 Martin Renvoize (ashimema) 2024-04-30 11:56:23 UTC
Created attachment 165863 [details] [review]
Bug 32610: (QA follow-up) Fix unit tests

With the introduction of data type checking based on the is_date flag we
need to update the tests to ensure we're setting the attribute types
consistently to not date when we're testing for other flags.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 52 Martin Renvoize (ashimema) 2024-04-30 11:56:27 UTC
Created attachment 165864 [details] [review]
Bug 32610: (QA follow-up) Tidy exceptions file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 53 Martin Renvoize (ashimema) 2024-04-30 11:56:30 UTC
Created attachment 165865 [details] [review]
Bug 32610: (QA follow-up) Tidy Koha::Patron::Attribute

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 54 Katrin Fischer 2024-04-30 14:11:36 UTC
This requires a small follow-up: 

* Create a new repeatable date field
* Enter a date in the patron account
* Repeat the field: the data is repeated, but input field has wrong format:
  04/03/2024 turns into: 2024-04-03
  The wrong dates don't appear to save.

I believe the field should be empty/cleared when repeated.
Comment 55 Shi Yao Wang 2024-04-30 14:17:55 UTC
It would be really easy to add a new empty field everytime, but I noticed the contents of other non-date attributes are copied when we repeat them so I tried to do the same with date attribute.

Do we still want to add empty dates or do we want to fix the formatting and saving?
Comment 56 Katrin Fischer 2024-04-30 14:21:33 UTC
I am not sure there is a use case for the repeated text - maybe we could do a really quick poll on Mattermost. I'll ask.
Comment 57 Katrin Fischer 2024-04-30 14:35:15 UTC
(In reply to Katrin Fischer from comment #56)
> I am not sure there is a use case for the repeated text - maybe we could do
> a really quick poll on Mattermost. I'll ask.

Discussed with Caroline at least and I think clearing the data makes more sense and is (hopefully) also easy to do?
Comment 58 Katrin Fischer 2024-04-30 15:10:56 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 59 Shi Yao Wang 2024-04-30 16:37:40 UTC
(In reply to Katrin Fischer from comment #57)
> (In reply to Katrin Fischer from comment #56)
> > I am not sure there is a use case for the repeated text - maybe we could do
> > a really quick poll on Mattermost. I'll ask.
> 
> Discussed with Caroline at least and I think clearing the data makes more
> sense and is (hopefully) also easy to do?

Looks like I'm a bit late, I just saw the message.

Why is this already pushed?
Isn't there still formatting/save problems? Didn't we want to clear the date attributes when repeating it?
Comment 60 Tomás Cohen Arazi (tcohen) 2024-04-30 18:55:30 UTC
Created attachment 165925 [details] [review]
Bug 32610: (follow-up) Adjust TestBuilder

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 61 Tomás Cohen Arazi (tcohen) 2024-04-30 18:56:57 UTC
(In reply to Tomás Cohen Arazi from comment #60)
> Created attachment 165925 [details] [review] [review]
> Bug 32610: (follow-up) Adjust TestBuilder
> 
> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Pushed this trivial follow-up to clear the failing tests. Any place using testbuilder to generate attribute types would explode.
Comment 62 Shi Yao Wang 2024-04-30 19:07:31 UTC
Created attachment 165926 [details] [review]
Bug 36738: Date attributes follow-ups

Make the repeatable date patron attribute empty when repeating the
inputs to avoid date formatting problems (and keeping past date value
after repeating is not something we want)

Test plan:
1- create a patron attribute type with is a date and repeatable checked
(administration -> patron attribute types)
2- add a couple of patron attribute of type date, notice the repeated
inputs contain previous data
3- Apply the patch
4- redo step 2 and notice the repeated inputs are empty
Comment 63 Shi Yao Wang 2024-04-30 19:08:07 UTC
Sorry, wrong bug
Comment 64 Katrin Fischer 2024-05-01 10:47:14 UTC
(In reply to Shi Yao Wang from comment #59)
> (In reply to Katrin Fischer from comment #57)
> > (In reply to Katrin Fischer from comment #56)
> > > I am not sure there is a use case for the repeated text - maybe we could do
> > > a really quick poll on Mattermost. I'll ask.
> > 
> > Discussed with Caroline at least and I think clearing the data makes more
> > sense and is (hopefully) also easy to do?
> 
> Looks like I'm a bit late, I just saw the message.
> 
> Why is this already pushed?
> Isn't there still formatting/save problems? Didn't we want to clear the date
> attributes when repeating it?

Just to explain: I had the option not to push it, but it didn't make things explode badly and I trusted you to provide the follow-up as we had already talked about it :) So I set the "additional_work_needed" keyword and pushed it. The keyword indicates lose ends that we still need to take care of. You can also put those follow-ups on the original bug report as long we haven't released I will pick them from here. After we released we will always need separate bug reports so the fixes show in releases notes etc.
Comment 65 Katrin Fischer 2024-05-01 10:48:42 UTC
(In reply to Tomás Cohen Arazi from comment #61)
> (In reply to Tomás Cohen Arazi from comment #60)
> > Created attachment 165925 [details] [review] [review] [review]
> > Bug 32610: (follow-up) Adjust TestBuilder
> > 
> > Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
> 
> Pushed this trivial follow-up to clear the failing tests. Any place using
> testbuilder to generate attribute types would explode.

Thank you!
Comment 66 Martin Renvoize (ashimema) 2024-05-01 12:56:19 UTC
Thanks for the TestBuilder fix Tomas.. I had considered doing that but for some reason git distracted and missed it :(.
Comment 67 Fridolin Somers 2024-05-24 12:29:59 UTC
Not backported to 23.11.x