| Summary: | Purging suggestions test should not be on timestamp | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Didier Gautheron <didier.gautheron> |
| Component: | Acquisitions | Assignee: | Marion Durand <marion.durand> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | trivial | ||
| Priority: | P5 - low | CC: | andrew, david, jonathan.druart, kyle, marion.durand, victor |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: |
This changes the date field that cronjob misc/cronjobs/purge_suggestions.pl uses to calculate the number of days for deleting accepted or rejected suggestions. It now uses the managed on date, as the last updated date that was used can be changed by other database updates.
|
Version(s) released in: |
22.05.00,21.11.01,21.05.08
|
| Circulation function: | |||
| Bug Depends on: | 23590, 28408 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 28855: Purging suggestions test should not be on timestamp
Bug 28855: (follow-up) adapt specific test Bug 28855: Purging suggestions test should not be on timestamp Bug 28855: (follow-up) adapt specific test Bug 28855: Purging suggestions test should not be on timestamp Bug 28855: (follow-up) adapt specific test |
||
|
Description
Didier Gautheron
2021-08-13 10:44:06 UTC
Hi ! I'm not sure we can use lastmodificationdate. If we want to use lastmodificationdate, I think we have to initialise it with the suggestion date when creating a new suggestion It is currently initialised to NULL. So all suggestions that have never been modified will never be deleted. I'm not sure what to do for the existing suggestion that have not been modified yet (and so lastmodificationdate is still NULL) Another option would be to use suggesteddate that is always initialised with a date. Regards, Marion Created attachment 127482 [details] [review] Bug 28855: Purging suggestions test should not be on timestamp DelSuggestionsOlderThan is on "date", a timestamp. It can be a problem because "date" is updated when we update the database, in consequences all the suggestions take the same date. This patch use 'manageddate' instead. In order to test this patch, you will need to have some suggestions where "Managed on" (suggestions.manageddate in the database) is older from "Last updated" (suggestions.date in the database). This can append during some specific update. If you don't have such data, you will have to add them via SQL request. Example of sql for insertion (ID is the patron id of a librarian which can manage suggestions): INSERT INTO suggestions (suggestedby, suggesteddate, managedby, manageddate, acceptedby, accepteddate, STATUS, title) VALUES (1, '2020-10-13', 1, '2020-10-26', 1, '2020-10-26', 'ACCEPTED', 'title'); Test plan: 1- Choose a date and find (or add) a suggestion where: - "Last updated" is not older than this date - "Managed on" is older than this date - "Status" is different from "Pending" ("ASKED" in database) Calculate number_days = today's date - chosen date Example: (Today's date 28/10/2021, chosen date : 28/10/2020, number_days=365) Suggestion | .. | Managed on | .. | Last updated | .. | Status title | .. | 10/26/2020 | .. | 05/15/2021 | .. | Accepted 2- Run cron job /home/koha/src/misc/cronjobs/purge_suggestions.pl --confirm --days number_days 3- Check that this suggestion has not been deleted 4- Apply the patch 5- Repeat steps 1 and 2 6- Check that this suggestion has been deleted Created attachment 127483 [details] [review] Bug 28855: (follow-up) adapt specific test Adapt the test of function DelSuggestionsOlderThan to match the new behaviour of the function. Created attachment 128411 [details] [review] Bug 28855: Purging suggestions test should not be on timestamp DelSuggestionsOlderThan is on "date", a timestamp. It can be a problem because "date" is updated when we update the database, in consequences all the suggestions take the same date. This patch use 'manageddate' instead. In order to test this patch, you will need to have some suggestions where "Managed on" (suggestions.manageddate in the database) is older from "Last updated" (suggestions.date in the database). This can append during some specific update. If you don't have such data, you will have to add them via SQL request. Example of sql for insertion (ID is the patron id of a librarian which can manage suggestions): INSERT INTO suggestions (suggestedby, suggesteddate, managedby, manageddate, acceptedby, accepteddate, STATUS, title) VALUES (1, '2020-10-13', 1, '2020-10-26', 1, '2020-10-26', 'ACCEPTED', 'title'); Test plan: 1- Choose a date and find (or add) a suggestion where: - "Last updated" is not older than this date - "Managed on" is older than this date - "Status" is different from "Pending" ("ASKED" in database) Calculate number_days = today's date - chosen date Example: (Today's date 28/10/2021, chosen date : 28/10/2020, number_days=365) Suggestion | .. | Managed on | .. | Last updated | .. | Status title | .. | 10/26/2020 | .. | 05/15/2021 | .. | Accepted 2- Run cron job /home/koha/src/misc/cronjobs/purge_suggestions.pl --confirm --days number_days 3- Check that this suggestion has not been deleted 4- Apply the patch 5- Repeat steps 1 and 2 6- Check that this suggestion has been deleted Signed-off-by: David Nind <david@davidnind.com> Created attachment 128412 [details] [review] Bug 28855: (follow-up) adapt specific test Adapt the test of function DelSuggestionsOlderThan to match the new behaviour of the function. Signed-off-by: David Nind <david@davidnind.com> I reviewed the manual to see whether any changes are required - no change as required as it doesn't mention how the date is calculated (see acquisitions > suggestions section, acquisition system preferences > PurgeSuggestionsOlderThan, and cron jobs > clean up old suggestions). Andrew, does this make sense to you? It does to me but I would prefer to get your feedback on it before flagging it as PQA. Makes sense to me! Thanks, all. Created attachment 128464 [details] [review] Bug 28855: Purging suggestions test should not be on timestamp DelSuggestionsOlderThan is on "date", a timestamp. It can be a problem because "date" is updated when we update the database, in consequences all the suggestions take the same date. This patch use 'manageddate' instead. In order to test this patch, you will need to have some suggestions where "Managed on" (suggestions.manageddate in the database) is older from "Last updated" (suggestions.date in the database). This can append during some specific update. If you don't have such data, you will have to add them via SQL request. Example of sql for insertion (ID is the patron id of a librarian which can manage suggestions): INSERT INTO suggestions (suggestedby, suggesteddate, managedby, manageddate, acceptedby, accepteddate, STATUS, title) VALUES (1, '2020-10-13', 1, '2020-10-26', 1, '2020-10-26', 'ACCEPTED', 'title'); Test plan: 1- Choose a date and find (or add) a suggestion where: - "Last updated" is not older than this date - "Managed on" is older than this date - "Status" is different from "Pending" ("ASKED" in database) Calculate number_days = today's date - chosen date Example: (Today's date 28/10/2021, chosen date : 28/10/2020, number_days=365) Suggestion | .. | Managed on | .. | Last updated | .. | Status title | .. | 10/26/2020 | .. | 05/15/2021 | .. | Accepted 2- Run cron job /home/koha/src/misc/cronjobs/purge_suggestions.pl --confirm --days number_days 3- Check that this suggestion has not been deleted 4- Apply the patch 5- Repeat steps 1 and 2 6- Check that this suggestion has been deleted Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 128465 [details] [review] Bug 28855: (follow-up) adapt specific test Adapt the test of function DelSuggestionsOlderThan to match the new behaviour of the function. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 22.05, thanks to everybody involved [U+1F984] Pushed to 21.11.x for 21.11.01 Pushed to 21.05.x for 21.05.08 Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. |