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.
*** Bug 6339 has been marked as a duplicate of this bug. ***
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
Created attachment 148401 [details] [review] Bug 32610: [DO NOT PUSH] DBIC Update
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
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.
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.
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
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>
Created attachment 148826 [details] [review] Bug 32610: [DO NOT PUSH] DBIC Update Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
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>
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>
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>
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>
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.
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 -->
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>
Created attachment 157863 [details] [review] Bug 32610: [DO NOT PUSH] DBIC Update Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
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>
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>
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>
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>
rebase
Created attachment 157868 [details] [review] Bug 32610: Remove data-date_to and tweak comment
Thanks a lot Shi Yao!
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>
Created attachment 158995 [details] [review] Bug 32610: [DO NOT PUSH] DBIC Update Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
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>
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>
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>
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>
Created attachment 159000 [details] [review] Bug 32610: Remove data-date_to and tweak comment
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
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.
(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.
I cannot continue here on my own - happy if someone wants to take over or collaborate in some way.
Created attachment 165747 [details] [review] Bug 32610: Patron attribute is_date error handling and tests
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
(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.
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.
I'm upgrading this back to 'Signed off' and will encourage the repeatable nature to be tested properly during QA.
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>
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>
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>
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>
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>
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>
Created attachment 165860 [details] [review] Bug 32610: Remove data-date_to and tweak comment Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
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>
Created attachment 165864 [details] [review] Bug 32610: (QA follow-up) Tidy exceptions file Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 165865 [details] [review] Bug 32610: (QA follow-up) Tidy Koha::Patron::Attribute Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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.
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?
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.
(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?
Pushed for 24.05! Well done everyone, thank you!
(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?
Created attachment 165925 [details] [review] Bug 32610: (follow-up) Adjust TestBuilder Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(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.
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
Sorry, wrong bug
(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.
(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!
Thanks for the TestBuilder fix Tomas.. I had considered doing that but for some reason git distracted and missed it :(.
Not backported to 23.11.x