[CIRC-1112] Recalls not being honored when there has been a renew or change due date Created: 04/Mar/21 Updated: 06/Dec/23 Resolved: 19/Nov/21 |
|
| Status: | Closed |
| Project: | mod-circulation |
| Components: | None |
| Affects versions: | None |
| Fix versions: | 22.1.2, 22.1.3, 23.0.0 |
| Type: | Bug | Priority: | P2 |
| Reporter: | Anya | Assignee: | Kyle Felker |
| Resolution: | Done | Votes: | 0 |
| Labels: | support | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||
| Issue links: |
|
||||||||||||||||||||
| Sprint: | Prokopovych - Sprint 128, Prokopovych - Sprint 117, Prokopovych - Sprint 123, Prokopovych - Sprint 114, Prokopovych - Sprint 115, Prokopovych - Sprint 111, Prokopovych - Sprint 113, Prokopovych - Sprint 116, Prokopovych - Sprint 124, Prokopovych - Sprint 126, Prokopovych - Sprint 125, Prokopovych - Sprint 127 | ||||||||||||||||||||
| Story Points: | 5 | ||||||||||||||||||||
| Development Team: | Prokopovych | ||||||||||||||||||||
| Release: | R3 2021 Bug Fix | ||||||||||||||||||||
| Affected Institution: |
5 Colleges, BNCF, Caltech, Chalmers, Cornell, Duke, GBV, Grand Valley State University, Lehigh, MI State University/Library of Michigan, Middle Tennessee State University, MO State, Okanagan College, OTHER, SHL, Simmons, Skidmore College, Spokane Public Libraries, St. Michael's College, STU, TAMU, Trinity College, Universidad de Concepción, University of Alabama, University of Chicago, University of Leipzig, University of Tennessee Martin, University of Zaragoza, Warner, Washington-Jefferson
|
||||||||||||||||||||
| Description |
|
We’re having trouble with recalls on loans that have been renewed or have changed due date. Expected result: Actual result:
To test the flag clear for renewals: 1. Set up a loan policy in which the minimum guaranteed loan period for recalled items is less than the full check-out period and renewal check-out period (I've been using a 60-day checkout with a 14 day recall period and a 60-day renewal). Attach that to an item. 2. Check the item out to a patron. I've been using the checkout-by-barcode endpoint directly with postman so that I can set the checkout date far enough in the past to make renewals possible. 3. Place a recall on the item for a different patron 4. Verify that the due date is truncated and the loan object shows the 5. Cancel the recall 8. Place another recall on the item. 9. Verify that the due date of the loan is truncated and the "dueDateChangedByRecall" flag set to "true" To Test Flag Clear Using Change Due Date 1. Set up a loan policy similar to the one above and attach to an item. 2. Check the item out to a patron 3. Place a recall on the item for another patron. 4. Verify that the due date is truncated and the loan object shows the 5. Cancel the recall. 6. Manually change the due date of the checked-out item using the UI. The value you change it to shouldn't matter, but it will make it easier to test if you lengthen the check-out period, as it will make date truncation easier to see. 7. If you can, use postman to pull the loan object directly from the circulation/loans endpoint and verify the dueDateChangedByRecall flag is now set to "false." 8. Place another recall on the item. 9. Verify the due date is properly truncated, and the dueDateChangedByRecall flag is now set to "true."
Interested parties: Marie Widigson |
| Comments |
| Comment by Anya [ 04/Mar/21 ] |
|
Cheryl Malmborg/ Erin Nettifee awareness |
| Comment by Holly Mistlebauer [ 04/Mar/21 ] |
|
Anya: I noticed that this "Bug" isn't assigned to my team yet. Is that because more work needs to be done first? I just wanted to check... |
| Comment by Holly Mistlebauer [ 11/Mar/21 ] |
|
Cheryl Malmborg: Please review this bug to confirm that it needs to be fixed. When you are done, please let me know so we can groom it for a dev to start. Thanks! |
| Comment by Anya [ 15/Mar/21 ] |
|
Support - Cheryl Malmborg do you think you will be able to get to this ticket this week? |
| Comment by Cheryl Malmborg [ 15/Mar/21 ] |
|
AnyaI tried to reproduce the problem in bugfest using short term renewable loans. I was able to reproduce the problem only once. I am setting up some daily loans to try reproducing. I think it has to be very a specific circumstance. |
| Comment by Anya [ 22/Mar/21 ] |
|
Support: Cheryl Malmborg could you provide your reproduction steps in the honeysuckle environment / Bugfest... (what will be Iris) - if they are different from Chalmers? |
| Comment by Holly Mistlebauer [ 26/Mar/21 ] |
|
Cheryl Malmborg: Please let me know if/when a dev should take a look at this. Thanks... |
| Comment by Anya [ 29/Mar/21 ] |
|
Support sig agrees with Holly Mistlebauer |
| Comment by Cheryl Malmborg [ 29/Mar/21 ] |
|
Anya I can re-create the loan policies and recall sequences I tried in Honeysuckle in bugfest. However, I was not able to reproduce the problem. Can you share the loan policy that Chalmers is using? A screen shot of the loan policy would be helpful. I want to duplicate your policy as closely as possible – closed library management, number of renewals,etc. Thank you.
|
| Comment by Anya [ 05/Apr/21 ] |
|
Marie Widigson could you share the policy? |
| Comment by Johanna Nilsson [ 06/Apr/21 ] |
|
I've shared our loan policy Anya |
| Comment by Anya [ 12/Apr/21 ] |
|
Support: Cheryl Malmborg do you have enough information to look at this issue? Thanks |
| Comment by Holly Mistlebauer [ 12/Apr/21 ] |
|
Cheryl Malmborg: Is this bug ready to be worked on? It looks like it is, but the assigned dev noticed that it is still assigned to you. I have unassigned it from you, please let me know ASAP if I should not have done that. Thanks! |
| Comment by Holly Mistlebauer [ 12/Apr/21 ] |
|
I misread Cheryl's comment above. She was not able to re-create the error on Honeysuckle. I have asked Cheryl to test this scenario on Iris to make sure it works. |
| Comment by Holly Mistlebauer [ 12/Apr/21 ] |
|
Anya: We are unable to re-create this problem. It might just be happening on the Chalmers instance. We aren't sure. What is the next step? Will someone from the Support SIG ask for more clarify from Chalmers? It would be good if Chalmers could supply a video of the steps leading up to this and then show what happens. Thanks... |
| Comment by Anya [ 12/Apr/21 ] |
|
Marie Widigson Please see Holly Mistlebauer comment above - could you share any info that would allow for replication? Thanks |
| Comment by Johanna Nilsson [ 13/Apr/21 ] |
|
Anya Holly Mistlebauer We are happy to share anything from Chalmers. We realy don't understand what's happening and we need this to work. I've uploaded a video from our FOLIO to Google Drive, let us know what else you need. https://drive.google.com/file/d/1bSJVCQaFSNz26Q829P7QyvN61z0WJko2/view?usp=sharing |
| Comment by Oleksiy_Lemeshko [ 13/Apr/21 ] |
|
marking this ticket as blocked since it is not yet ready for dev |
| Comment by Marc Johnson [ 16/Apr/21 ] |
I've changed this to draft, as I think that better reflects that it isn't ready for developer rather than development has started and cannot be completed due to a dependency. Please let me know if you think that is inappropriate. |
| Comment by Holly Mistlebauer [ 16/Apr/21 ] |
|
Anya: We are unable to re-create this error. Please ask Chalmers to recreate this error and then send us the logs. Thanks! |
| Comment by Anya [ 19/Apr/21 ] |
|
Hi Marie Widigson could you try to recreate this issue? Thanks |
| Comment by Johanna Nilsson [ 20/Apr/21 ] |
|
Hi Anya. How do we do that? What logs and how do we get them? (And since it was I who put the issue in EBSCO Connect maybe you should direct your comments to me and not Marie |
| Comment by Anya [ 26/Apr/21 ] |
|
Sorry about that Johanna Nilsson. The POs are unable to recreate this issue. Is this issue still happening? |
| Comment by Johanna Nilsson [ 28/Apr/21 ] |
|
No worries Anya. We're still having trubble, but now when I test it seems to only happen when ther's been a cancelled request and then a renewal or a change of due date? |
| Comment by Holly Mistlebauer [ 29/Apr/21 ] |
|
Anya: Will you be confirming the scenario Johanna mentioned in her comment above? Then you can let us know if a dev should start working on this. Thanks... |
| Comment by Holly Mistlebauer [ 29/Apr/21 ] |
|
Anya: And we still need the logs...sorry...I forgot that...thanks... |
| Comment by Anya [ 03/May/21 ] |
|
Support: I (anya) will request the logs. |
| Comment by Holly Mistlebauer [ 05/May/21 ] |
|
Anya: We are attempting to fix all P1/P2 bugs before the code freeze on May 28. Hopefully we can move this out of blocked status before then. Thanks... |
| Comment by Debra Howell [ 10/May/21 ] |
|
From Support SIG: Holly Mistlebauer Can this be moved to the current sprint? |
| Comment by Holly Mistlebauer [ 11/May/21 ] |
|
Debra Howell: This ticket is in the current sprint. We cannot reproduce this, so we need to see the logs. Carole Godfrey has asked me which logs are needed, and I will ask the devs for help with this. |
| Comment by Holly Mistlebauer [ 11/May/21 ] |
|
Slack message from Carole Godfrey to Holly on Friday, May 7... |
| Comment by Holly Mistlebauer [ 11/May/21 ] |
|
Holly's Slack response today... |
| Comment by Holly Mistlebauer [ 11/May/21 ] |
|
Marc Johnson: This is the ticket we need the logs for. Thanks... |
| Comment by Carole Godfrey [ 12/May/21 ] |
|
Marc Johnson – I have the. date and timeframe for this – if you could please let me know (is it mod-circulation and mod-circulation-storage that logs are needed for?) |
| Comment by Marc Johnson [ 12/May/21 ] |
I don't really know, I doubt there is specific logging for this process. Those are likely a good starting point for further investigation. |
| Comment by Carole Godfrey [ 12/May/21 ] |
|
Attaching logs for mod-circulation and mod-circulation-storage from April 13th (11-12AM UTC). I see a 403 error in the logs – but checked pub-sub user and believe these were what was required at the time Audit data - post event A recall request was made 13:35. The loan period was shortened to the guaranteed 14 days. The request was then cancelled and the loan was renewed. A new recall request was made 13:45 and there were no changes to the due date. Assuming this timeframe is (GMT+2) |
| Comment by Cheryl Malmborg [ 14/May/21 ] |
|
Kyle FelkerHolly Mistlebauer I used patron barcode clmu412 as the borrower and clmu482 and clmu47 as recallers. I've attached the loan action detail screen and circ log entries for 108857216. I don't know if it is significant that the loan action detail does not show the second request. I tried making the request after cancellation and renewal for a different patron that the for the first request. the due date still was not changed. |
| Comment by Kyle Felker [ 19/May/21 ] |
|
I'm having some difficulty replicating this issue in bugfest. Here is what I have done: 1. Cleared all requests on item 68968705 and checked it in. 2. Checked it back out to clmu412. Item now has a due date 90 days in future. 3. Go to the loan through the user module and change the due date-the system will not allow me to renew the item, even if I override. I just get a modal or error pane telling me that "renewal will not change the due date of the item." i change the due date manually to a date thirty days from now. 3. Go to the requests module and place a recall on the item (the original issue notes recalls specifically) for user clmu482 4. check the loan due date in various places-it is properly truncated to 14 days. 4. Cancel the recall. I note that the due date remains truncated after the recall is cancelled-which seems wrong. 5. Place another recall on the item for clmu47 6. Due date does not change. Edit: I am unable to replicate this in automated fashion with renewals, either. Artificially aging the loan 90 days, then renewing it, then placing a recall truncates the due date properly to the two-week minimum.
|
| Comment by Cheryl Malmborg [ 19/May/21 ] |
|
Kyle Felker I just tested again using 108857216 and user clmu412 I completed the request on 1088673216 by checking it in, checking out to requester and checking in again. I think the difference is that the renewal is done after the first recall is cancelled. This is how I interpret the report from Chalmers A recall request was made 13:35. The loan period was shortened to the guaranteed 14 days. The request was then cancelled and the loan was renewed. A new recall request was made 13:45 and there were no changes to the due date. 4 |
| Comment by Kyle Felker [ 19/May/21 ] |
|
> I completed the request on 1088673216 by checking it in, checking out to requester and checking in again. I am very confused. Fulfilling the request is not on the list of steps for reproduction I can see. I do see a comment about placing a request and then cancelling it, although it's not entirely clear to me wether you do this before or after the renewal. So are you are saying the steps to reproduce this are: 1. Check out an item. 2. Place a recall on it. 3. Check it back in 4. Check the item out to the requestor. 5. Check it back in. 6. Check it out to another user 7. Place another recall from a different user 8. Cancel the new recall 9. Renew the item 10. Place another recall (for a new user?) Please can the description of the issue be updated with a list of all the steps required to reproduce? |
| Comment by Cheryl Malmborg [ 19/May/21 ] |
|
Kyle Felker Sorry for the confusion |
| Comment by Kyle Felker [ 20/May/21 ] |
|
Thank you, I was able to replicate with those steps. |
| Comment by Kyle Felker [ 21/May/21 ] |
|
Cheryl Malmborg Holly Mistlebauer It may (or may not) be important to understand that this is not a bug. There is considerable apparatus in the code set up explicitly to insure that once a loan's due date is truncated due to recall it can never again be truncated for that reason. There is a flag set on the loan to track this and multiple checks are carried out any time there's an attempt to truncate the loan due date again, with the code set to abort if the flag has been previously set. Someone went to a great deal of trouble to put this exact behavior in place-it's very much intentional. Because of that, it's going to take a while to figure out how to get the code to treat renewals as a special case. |
| Comment by Cheryl Malmborg [ 21/May/21 ] |
|
Thank you Kyle. I think the flag is set to prevent the due date from being altered more than once when more than one recall request is in the queue for a loan. Holly, the of recalling an item, cancelling the recall, renewing the item, and recalling it again seems like a edge case to me. Should we ask Chalmers if they do this often? |
| Comment by Kyle Felker [ 26/May/21 ] |
|
Holly Mistlebauer Cheryl Malmborg If I change this, then renewed loans will be subject to due date truncation-multiple times, if multiple recalls are put on the loan. There's no way to stop that unless I devise a flag that specifically tracks wether the truncation happened before or after the loan was renewed. |
| Comment by Marc Johnson [ 26/May/21 ] |
|
Kyle Felker Holly Mistlebauer Cheryl Malmborg
Do we need an answer to these questions prior to merging the currently proposed changes? |
| Comment by Kyle Felker [ 26/May/21 ] |
|
> Do we need an answer to these questions prior to merging the currently proposed changes? I think so. I'm holding off on a merge until I get a more definite "yes, we want you to do this." It didn't hit me until I was reading your comments that this re-creates the problem the flag was intended to solve with renewed loans-they'll be truncatable more than once. |
| Comment by Cheryl Malmborg [ 26/May/21 ] |
|
Kyle Felker Marc Johnson Holly Mistlebauer Shouldn't the removal of the flag just be tied to the cancellation of a recall that is the only recall in a request queue? The discussion was long ago, but as I remember, the RA SIG agreed that reverting to the original due date for a cancelled recall item was complicated. It involved saving or re-calculating the pre-recall due date and sending a notice to the patron from whom the item was recalled. I believe that the SIG agreed that staff could renew or change the due date of the cancelled recalled item and contact the patron by email if appropriate. I'm not sure what Chalmers intention was in renewing the item (which shortened the pre-recall due date) rather than using change due date to restore the original due date. |
| Comment by Kyle Felker [ 27/May/21 ] |
|
Marc Johnson Holly Mistlebauer Cheryl Malmborg >Shouldn't the removal of the flag just be tied to the cancellation of a recall that is the only recall in a request queue? I can't comment on wether it should or shouldn't be, but I can tell you that right now, this is not how the code works. Once the flag is set, it's set for the life of the loan. Based on the way the flag is used and named, my understanding is that its purpose is to track wether or not the loan due date has ever been truncated due to recall, not wether the due date is allowed to be truncated. That distinction is subtle but important. A flag designed to tell you if the loan due date has ever, at any time, been truncated due to recall, would never be reset once set. Either the loan due date has, at some point, been truncated due to recall, or it hasn't. There's a lot of code in mod-circ that basically uses this flag to see if the loan has ever had a recall placed on it. Again, the assumption the code seems to be built on is that once a loan's due date is truncated due to recall, it should never again be truncated for that reason. Ever. Hence my concern here-if I tell the date-truncation code to ignore the flag for loans that have been renewed, then all renewed loans become truncatable due to recall, at-will, at any time, any number of times, for the entire life of the loan, forever and ever, amen. **>The discussion was long ago, but as I remember, the RA SIG agreed that reverting to the original due date for a cancelled recall item >was complicated. It involved saving or re-calculating the pre-recall due date and sending a notice to the patron from whom the item >was recalled. I believe that the SIG agreed that staff could renew or change the due date of the cancelled recalled item and contact >the patron by email if appropriate. Ah, that explains why when a recall is cancelled the truncated due date does not change.
|
| Comment by Marc Johnson [ 27/May/21 ] |
|
Kyle Felker Holly Mistlebauer Cheryl Malmborg
I have a vague recollection about some conversations that Cate, the RA SIG and the reporting SIG had around this flag. I think it was decided that a request being cancelled should not reset that flag. And that despite the name dueDateChangedByRecall it was actually being set whenever a recall request had been made for a borrowed item (even if the due date wasn't changed) and that the Reporting SIG were going to interpret it that way to achieve some reports. I'll try to find more specifics on that conversation. For starters, Brooks Travis might have more context, as this topic is touched upon in
|
| Comment by Marc Johnson [ 27/May/21 ] |
That fits with my understanding of the purpose of the flag (and is how I believe the reporting folks are interpreting it). |
| Comment by Brooks Travis [ 29/May/21 ] |
|
Sorry I'm late to this party. I've not been following this as closely as I probably should have been. I'm going to put something out here, but let me know if I've misunderstood how things currently work. Marc Johnson Kyle Felker That is basic gist. The reporting SIG wants to be able to track when loans have been affected by recalls. That said, I would think that a lot of that information should be available in the circulation log going forward. This would probably require some discussion with the RA SIG, but could we not consider just clearing the flag whenever an item has no active recall requests AND is either successfully renewed or its due date is changed? /cc Cheryl Malmborg |
| Comment by Marc Johnson [ 03/Jun/21 ] |
|
Brooks Travis Cheryl Malmborg Holly Mistlebauer
Thanks Brooks Travis for the thoughts. I don't think the developers can make that decision. I think the PO's need to make that decision with the reporting and resource access folks. I don't know if the circulation log is being included in the reporting systems. |
| Comment by Cheryl Malmborg [ 03/Jun/21 ] |
|
Marc Johnson Brooks Travis Holly Mistlebauer I think we do need to explain the situation to the RA SIG and reporting. It seems to me that the flag is having unexpected consequences. |
| Comment by Anya [ 07/Jun/21 ] |
|
Support: what is the status on this ticket?
|
| Comment by Kyle Felker [ 07/Jun/21 ] |
|
The concern I'd raise about allowing the flag to be cleared is that now it means something different. Right now, what it means as far as the code is concerned is "has this item ever had recall placed on it?" If you allow it to be cleared, it now does not indicate that reliably-it means something more like "Should I be able to truncate the loan due date for recalls?". That flag is checked by more parts of the circulation code than just the loan date truncation code. For example, the fee/fine calculation code is looking at it. If the code is built on the assumption that this flag means "has this ever had a recall placed on it" and the meaning changes to "can I truncate the loan due date currently for recalls," I'm not sure what effect that will have on other parts of the code-for example, you may get inconsistent or buggy behaviors in fee/fine calculation. You'd really need to go through every part of the code that checks the flag and make sure it works as expected after making such a change. Hopefully that makes sense? |
| Comment by Brooks Travis [ 07/Jun/21 ] |
|
Kyle Felker When/why is fees/fines looking at this data? Do you happen to know the stories that define that behavior? For that matter, do you have the story that originally defined the flag in the first place? |
| Comment by Kyle Felker [ 07/Jun/21 ] |
|
If you'd like to see all the places this is being looked at in the code, do a search on "dueDateChangedByRecall" and "wasDueDateChangedByRecall" in the mod-circ code. I'm seeing it showing up in the fine calculation service, the mark overdues as aged to lost service, and there's some logic using it in relation to honoring grace periods. I'd need to poke around more in those sections of the code to tell you exactly how it's being used. Do you need me to do so? As an aside, the name and description of this flag in the schema are not really accurate: , >Do you happen to know the stories that define that behavior? For that matter, do you have the story that originally defined the flag in the first place? I don't have any of those things, which is why changing how the flag behaves makes me nervous. I think Marc Johnson was looking for information about the original addition of the flag-this all happened sometime before I came on board the project. |
| Comment by Brooks Travis [ 07/Jun/21 ] |
|
Kyle Felker No, I don't need you to poke around any further at the moment. I'll take a look. Thanks. |
| Comment by Marc Johnson [ 08/Jun/21 ] |
I haven't had chance to dig into this further. What I can recall, is that there were quite a few conversations about how this flag should be used and it's meaning (but not it's name) changed over time. |
| Comment by Anya [ 28/Jun/21 ] |
|
Brooks Travis any update on this ?
|
| Comment by Anya [ 06/Jul/21 ] |
|
Brooks Travis -/ support is asking is there is an update on this |
| Comment by Brooks Travis [ 08/Jul/21 ] |
|
Anya Sorry. Not at this time. I will try to follow up with Kyle Felker and Marc Johnson before the next support SIG meeting. |
| Comment by Brooks Travis [ 08/Jul/21 ] |
|
Anya According to Marc, they're waiting on logs from Chalmers, as they're not able to recreate the issue. |
| Comment by Marc Johnson [ 08/Jul/21 ] |
I think it was Holly Mistlebauer who stated that My understanding from the comments above is that this report and investigation by Kyle Felker led to questions about whether we generally wanted to change the behaviour of changing the due date for recalled items by changing how we manage the flag used to determine if the due date has been changed before. As I understand it, no decision has been made about that, given that it would change the meaning of the flag and reporting or other folks might be using it. |
| Comment by Brooks Travis [ 08/Jul/21 ] |
|
Thanks, Marc Johnson. Sorry about the confusion. Anya, please ignore my previous. At this point, I'm unsure how we should proceed with this. It's a functional issue that needs to be resolved, but it sounds like it will/may have some downstream repercussions if we apply a fix. Do we need to meet with reporting and discuss potential impacts for them and/or maybe try to re-think the way recalls are handled at a more basic level and keep that particular field in place to service the reporting requirements but use something else for transaction processing? |
| Comment by Holly Mistlebauer [ 09/Jul/21 ] |
|
I just removed the Release setting because we don't have all of the info we need to work on this... |
| Comment by Anya [ 12/Jul/21 ] |
|
Support: this is a P2 - should it not be or what is the priority of this being fixed? |
| Comment by Angela Zoss [ 13/Jul/21 ] |
|
Reporting (Angela) is happy to have a call on this if it's easier. Basically, we have a recalls report that needs to be able to find users impacted by a recall, inside a given date range. We can't do that just with the flag, so we requested an additional date field to be added to mark when the flag was placed. I'm not sure I'm following the full thread here. It sounds like the proposed change here is to be able to clear the flag if the recall is cancelled? If that happens, does the due date go back to the original due date, or does the original user have to renew to re-lengthen the loan period? In this edge case, we'd have to check with people who need the report to see if they want this loan included as a loan/user that is affected by a recall or not. My gut is saying that having the flag clear would be okay for our reporting needs if the behavior is clearly documented for us and the report users agree that these users can be excluded from a recalls report. I think where reporting finds recalls tricky is not this sort of case (someone having a single recall that ends up being cancelled) but the case when multiple recalls stack up on an item, thus truncating a series of loans. If there are multiple recalls on a loan, we assume that the user with the first recall gets the item first, but that person's loan period is affected by the second recall, and thus the loan for that first recall has the dueDate... flag flipped. If that behavior changes, we need an alternate way of determining that a loan period has been affected by a recall placed on the item before that loan started. Let me know if that's not clear and/or another meeting about this would help! |
| Comment by Kyle Felker [ 14/Jul/21 ] |
|
>It sounds like the proposed change here is to be able to clear the flag if the recall is cancelled? Either that, or to recode renewals so they just don't check the flag. Either solution has potential problems-if you clear the flag, then what the flag means changes-right now, it's basically being used to indicate wether a loan has ever had a recall placed on it. This flag is being used by other parts of the code than just due date calculation-so doing this may cause unexpected behaviors in other parts of the code. If you have renewals ignore the flag, then renewed loans can be subject to date truncation multiple times. Personally, I think what might be warranted here is a new flag that tracks wether the the due date on the loan is currently eligible for truncation or not. Then the existing flag can continue to track wether the loan item has ever been recalled-I believe Brooks Travis suggests something similar in one of the posts above. Part of what makes this so complicated is that, evidently, the way this specific flag has been used has changed over time, and may have morphed from it's originally-intended meaning (whatever that was) to it's current effective meaning: "Has this item ever, at any point, had a recall placed on it?" > If that happens, does the due date go back to the original due date, Due dates for loans are calculated when the checkout/renewal is done, or when the first recall is placed on an item. Clearing the flag will have no impact on due dates already calculated-those are set. What it will affect is wether the due date will or won't be truncated at time of checkout/renewal. > or does the original user have to renew to re-lengthen the loan period? Currently, once a loan's due date has been truncated due to recall, it can never again be truncated for that reason. Ever. So a renewal on an item that previously had a recall placed on it will get the full check-out period, because it will check the flag, see that the loan has already had a recall placed on it at some point in the past, and will not truncate the due date. Hence, the original reported issue: People renewing loans that had recalls on them were getting a longer loan period than expected.
|
| Comment by Angela Zoss [ 14/Jul/21 ] |
|
Thanks so much, Kyle Felker! That's very helpful! I don't have much to add, but I think any solution that generates the correct logic will likely work okay for reporting. We just need there to be enough information in the data to be able to report on users experiencing due date truncation, including dates for truncation events that were triggered by recalls. I guess if we're trying to exclude people whose recalls were quickly cancelled and who were thus able to renew the book and keep it indefinitely, we'd probably have an easier time if the original dueDateChanged... flag just went away, but that does change the meaning of that flag and doesn't seem like the right way to go. With a good understanding of how the data model works, I think we'll be able to write appropriate reports to fit our needs, but we may just end up deciding to accept some uncertainty about whether a user whose due date was changed actually ended up experiencing a completed recall. |
| Comment by Anya [ 26/Jul/21 ] |
|
Support Sig - what are the next steps and when will this fix be approved (by who). Thanks.
CC Kyle Felker and Angela Zoss |
| Comment by Anya [ 09/Aug/21 ] |
|
Support Sig: Kyle Felker/ Angela Zoss what are the next steps ? |
| Comment by Marc Johnson [ 09/Aug/21 ] |
The next step is for the relevant POs / SIGs (RA and reporting) to decide how they want this area to work. |
| Comment by Angela Zoss [ 09/Aug/21 ] |
|
Reporting doesn't have a preference, as long as the logic is clear to us. |
| Comment by Brooks Travis [ 13/Aug/21 ] |
|
Marc Johnson Kyle Felker Can we get together with Cheryl Malmborg to go over the fix that was proposed and discuss the downstream implications and try to come up with a path forward on this and see if we can get it into a Juniper hot fix (or, at least, Kiwi)? |
| Comment by Marc Johnson [ 13/Aug/21 ] |
Kyle Felker is off sick at the moment. By all means arrange a meeting when he is back to discuss this. |
| Comment by Kyle Felker [ 16/Aug/21 ] |
|
Marc Johnson Brooks Travis Cheryl Malmborg
I am back in the office-feel free to reach out to schedule a call. |
| Comment by Brooks Travis [ 23/Aug/21 ] |
|
Marc Johnson Kyle Felker Cheryl Malmborg The RA SIG gave it's go-ahead today to implement the fix for this bug that clears the "dueDateChangedByRecall" flag, with all its downstream effects in fees/fines and lost item processing. |
| Comment by Marc Johnson [ 23/Aug/21 ] |
Thank you for discussing it with the SIG. Please could you update the issue to describe the specific circumstances under which this flag should be removed (and what behaviour that should lead to) |
| Comment by Debra Howell [ 20/Sep/21 ] |
|
From SUPPORT SIG: Holly Mistlebauer we are escalating this to a P1 as it should have been in Iris or an Iris hot fix based on its priority |
| Comment by Kyle Felker [ 21/Sep/21 ] |
|
Marc Johnson Brooks Travis Cheryl Malmborg We still need a concrete set of circumstances under which the flag should be cleared. Currently: 1. The flag is set when the first recall is placed on an item. If it's checked out when that happens, the due date is truncated. Under what circumstances would you like the flag to be cleared? From previous discussions, I might guess it looks like: 1. The flag should be cleared when a recall is deleted, and there are no more recalls in the queue. However, if we only do that, it won't fix the original reported issue-the scenario for that is a checked out item, which has already had it's due date truncated, is being renewed while there is still a recall in the queue. So if you want to address that: 2. Also clear the flag when a renewal is done on an item that has the flag already set. do this before the code checks for due date truncation. If we do that, note that renewals can now have their due dates truncated any number of times. It might be better just to have renewals not check the flag at all when deciding wether to truncate the due date, since that would produce the same behavior with less code. |
| Comment by Brooks Travis [ 21/Sep/21 ] |
|
Sorry, Kyle Felker I think I owed you a write-up of the details of this. The flag should be cleared whenever the following criteria are met:
Cancelling the recall, alone, does not clear the flag. My understanding of the actual behavior here was that the loan being recalled had already been recalled once, but subsequently renewed after the original recall was cancelled or transferred to another item, so subsequent recalls were not truncating the renewed loan because the dueDateChangedByRecall flag was already set. Unsetting that flag under the conditions above should avoid that problem in the future. The renewed (or due date changed) loan would see its loan period truncated the next time it was recalled. Cheryl Malmborg does that seem right to you? |
| Comment by Cheryl Malmborg [ 21/Sep/21 ] |
|
Brooks Travis That sounds right. |
| Comment by Kyle Felker [ 22/Sep/21 ] |
|
Okay, I think i can work with that. I'll move this out of draft and start work now. |
| Comment by Kyle Felker [ 22/Sep/21 ] |
|
>the due date of a loan with the flag set is changed Clarification: Changed for ANY reason, or changed only due to a recall? It will make a big difference in how I code this. |
| Comment by Brooks Travis [ 22/Sep/21 ] |
|
Kyle Felker Sorry, I guess I left an important clause off that. Should be “…the due date of a loan with the flag set is changed by a staff member (i.e. using the “/circulation/loans/{id}/change-due-date” api)”. Does that clarify? |
| Comment by Kyle Felker [ 22/Sep/21 ] |
|
Yes, that is much clearer, thanks. |
| Comment by Kyle Felker [ 23/Sep/21 ] |
|
I'd like to confirm testing steps for this-the reporting info in the actual issue is not sufficiently detailed for testing and verification. If this looks correct, I'd like to replace or add this to the issue description. Since there are two different scenarios for clearing the flag, there should be two separate tests: To test the flag clear for renewals: 1. Set up a loan policy in which the minimum guaranteed loan period for recalled items is less than the full check-out period and renewal check-out period (I've been using a 60-day checkout with a 14 day recall period and a 60-day renewal). Attach that to an item. 2. Check the item out to a patron. I've been using the checkout-by-barcode endpoint directly with postman so that I can set the checkout date far enough in the past to make renewals possible. 3. Place a recall on the item for a different patron 4. Verify that the due date is truncated and the loan object shows the 5. Cancel the recall 8. Place another recall on the item. 9. Verify that the due date of the loan is truncated and the "dueDateChangedByRecall" flag set to "true" To Test Flag Clear Using Change Due Date 1. Set up a loan policy similar to the one above and attach to an item. 2. Check the item out to a patron 3. Place a recall on the item for another patron. 4. Verify that the due date is truncated and the loan object shows the 5. Cancel the recall. 6. Manually change the due date of the checked-out item using the UI. The value you change it to shouldn't matter, but it will make it easier to test if you lengthen the check-out period, as it will make date truncation easier to see. 7. If you can, use postman to pull the loan object directly from the circulation/loans endpoint and verify the dueDateChangedByRecall flag is now set to "false." 8. Place another recall on the item. 9. Verify the due date is properly truncated, and the dueDateChangedByRecall flag is now set to "true." |
| Comment by Brooks Travis [ 23/Sep/21 ] |
|
Kyle Felker Looks right to me. Cheryl Malmborg? |
| Comment by Cheryl Malmborg [ 23/Sep/21 ] |
|
Kyle Felker Brooks Travis Looks right to me too. |
| Comment by Kyle Felker [ 27/Sep/21 ] |
|
Parking this for now as we need to focus on release work for Kiwi. I will be out of the office for the first part of October, so this may not get looked at again until the 18th of that month. |
| Comment by Anya [ 04/Oct/21 ] |
|
SUPPORT - this has been a live bug since March ... |
| Comment by Marc Johnson [ 04/Oct/21 ] |
I understand that and that it's frustrating. It has been waiting for decisions about how folks want the system to work until the 21st September. Now, we are in the midst of the 2021 R3 release (of which we are significantly behind our responsibilities for, partly due to a different P1 being prioritised ahead of them and this). Kyle is unavailable for two weeks and the team has very limited back end capacity. |
| Comment by Jeremy Huff [ 05/Oct/21 ] |
|
In light of Kyle Felker's comments on September 23rd, and the responses from Brooks Travis and Cheryl Malmborg, I have updated the description of this issue with the testing strategies which Kyle outlined. |
| Comment by Marc Johnson [ 09/Nov/21 ] |
|
Will you be picking up testing of this (from tomorrow, when it's deployed) in the hosted reference environments (e.g. snapshot)? I'm asking because folks are hoping that this will be reviewed, back ported, released and re-reviewed by Friday as it is targeted for 2021 R3 Bug Fix and I don't want any confusion as to who is testing this to reduce the chances of that. |
| Comment by Marc Johnson [ 11/Nov/21 ] |
|
Given this is a P1 that folks have been especially concerned about getting into 2021 R3, I'd like folks help to find someone to test this (before it can be back ported and released). Brooks Travis Cheryl Malmborg Erin Nettifee Debra Howell Angela Zoss Stephanie Buck Holly Mistlebauer Charlotte Whitt (as deputy lead PO for Prokopovych), Khalilah Gambrell (as overall lead PO) Who can test this change? |
| Comment by Charlotte Whitt [ 11/Nov/21 ] |
|
Brooks Travis Cheryl Malmborg Erin Nettifee - this ticket probably needs a tester with deep knowledge about the loan rules and how request recalls works in the business logic. Holly Mistlebauer is out 11/11 - 11/17 because of her move to AZ. |
| Comment by Brooks Travis [ 11/Nov/21 ] |
|
I believe I can test it today (11/11) following the RA SIG meeting. I'll try to complete the testing by 1pm ET today. |
| Comment by Brooks Travis [ 11/Nov/21 ] |
|
Verified in snapshot. |
| Comment by Stephanie Buck [ 11/Nov/21 ] |
|
Thank you, Brooks Travis!! |
| Comment by Oleksii Petrenko [ 15/Nov/21 ] |
|
Deployed to Kiwi bugfest. Please proceed with verification |
| Comment by Brooks Travis [ 15/Nov/21 ] |
|
I'm going to work on verifying this in Bugfest now. |
| Comment by Brooks Travis [ 15/Nov/21 ] |
|
Kyle Felker The change due date part of the scenario doesn't seem to be working in Bugfest. The dueDateChangedByRecall is still true after changing the due date after the recall was cancelled. As far as I can recall, it was working in snapshot. Sorry if I missed something there. |
| Comment by Kyle Felker [ 15/Nov/21 ] |
|
Marc Johnson indicates this could be caused by the backport of the code-apparently getting it to work with the older code required a number of changes. I'll retest on snapshot. |
| Comment by Brooks Travis [ 15/Nov/21 ] |
|
Kyle Felker I'm going to take another pass at it on snapshot, as well. |
| Comment by Brooks Travis [ 15/Nov/21 ] |
|
Kyle Felker Ok, apparently I missed this in my review on snapshot. Sorry about that. I just reproduced the dueDateChangedByRecall=True issue there, as well. |
| Comment by Charlotte Whitt [ 15/Nov/21 ] |
|
Brooks Travis Kyle Felker- with these new findings, then do this ticket need to be changed to have the status as In progress and not listed as In Bugfix Review? CC: Oleksii Petrenko |
| Comment by Kyle Felker [ 15/Nov/21 ] |
|
I'm not sure the problem here is with the code. The elaborate test I wrote for this condition passes. At first I thought I must have botched the test somehow, but when I dump the loan object at different stages of the test, the due dates and flag settings are exactly as they should be. Local internal code echoes out valid and correct data at different stages of the test as well. I had difficulty testing this part of the changes on scratch, because no matter what I did to this area of the code, its behavior on scratch did not change. I could not get logging messages to show up in the logs, for example. At one point, I inserted code that should have allowed the module to compile but blown up the change due date process with a 500 error-it continued to work exactly as it had before. I knew I had the right area of code, because when I ran local tests that changed due dates, I saw the log messages I had inserted in my local output. I talked to the infrastructure folks (Aliaksei Luhavy and Hleb Surnovich), and they said to use a checkbox in rancher that forces overwriting-but I've tried that and this part of the code still behaved as if I've made no changes to it. So I assumed something was wrong with scratch and debugged this using the test. Is it possible there's an issue with this part of the code not being properly overwritten when changes migrate up to the testing environments? Because otherwise, I'm stumped. This code should work. Locally, it does work. But it does not work in the reference environments and I can't figure out why. |
| Comment by Charlotte Whitt [ 15/Nov/21 ] |
|
Marc Johnson - we'll wait on your response. I do understand that it's late evening your time now. |
| Comment by Marc Johnson [ 15/Nov/21 ] |
Is that the code on the mainline or the code on the release branch / tag?
I would work on the same assumption. I think that is a deployment or activation issue not a code issue.
Not really. We don't partially update the code, it's either all of nothing. Especially not in the reference environments as they are complete rebuilds of the whole environment.
Kyle Felker Brooks Travis Does this mean that this change is not working in snapshot (despite it previously having passed testing there)? |
| Comment by Brooks Travis [ 15/Nov/21 ] |
|
Marc Johnson See my earlier comment regarding snapshot. I thought I’d given it a pretty thorough going-over before moving it to awaiting deployment, but when I tested it again today it didn’t behave as-expected in either environment. |
| Comment by Marc Johnson [ 16/Nov/21 ] |
|
I've put this issue back in progress as the changes have not produced the expected behaviour. I suggest it be removed from the 2021 R3 Bug Fix release, unless folks are intending to block the entire release on this issue (which has existed for multiple releases). |
| Comment by Charlotte Whitt [ 16/Nov/21 ] |
|
Marc Johnson re.
I agree with your suggestion. This should then be solved as a Kiwi Hotfix #1 Any objection from you Anya (Reporter) or the Support SIG? |
| Comment by Charlotte Whitt [ 16/Nov/21 ] |
|
Marc Johnson Kyle Felker - In a conversation with Khalilah Gambrell, we're talking about if you two together could work on the code, then we can try to give it one more work day (today). Will you be able to help Kyle Felker, Marc Johnson? Thank you so much in advance. |
| Comment by Marc Johnson [ 16/Nov/21 ] |
|
Charlotte Whitt Khalilah Gambrell
I'm happy to help however I can. I don't currently know how I can help. (and timezones / working days mean there is limited overlap) I will say that I doubt this will get resolved today. Given that we need to be able to analyse it on environments (and we can't get deployments to work on the scratch environments) then we won't get feedback on any changes till at least tomorrow. |
| Comment by Kyle Felker [ 16/Nov/21 ] |
|
Agree-it's unlikely to get resolved today. Marc, do you have a local testing environment running mod-circulation? If so, it would be a help if you could pull down the master branch and run through the change due date scenario in the description and tell me what happens. I'm still unable to debug this in scratch, as no changes I make to that area of code seem to affect it's behavior. I've reported this problem again to the back end folks. > Is that the code on the mainline or the code on the release branch / tag? Both. I've done log dumping and run the test on this with both mainline and the backport release branch-as far as the test environment is concerned, it's working perfectly in both places. |
| Comment by Marc Johnson [ 16/Nov/21 ] |
I do not. When I do development on modules, I typically do it in isolation.
Thank you for reporting this again, I think we need to understand why this isn't working.
How have you run the test? Are you referring to a manual or automated test? What do you mean by it's working perfectly in both places? I thought that both you and Brooks Travis could recreate this not working on the snapshot environment. Have I got that wrong? |
| Comment by Kyle Felker [ 16/Nov/21 ] |
|
I'm sorry, that was confusing phrasing-I mean the automated test for this behavior is passing, and when I dump internal variables and processes to the log when it's running, the loan object looks as it should. This is both in the backported branch of the module, and in mainline. The loan object goes through the changes it should go through, and ends up with the flag set to false, as it should be. I cannot see any fault in the test itself-it's passing because the loan object is in the state it should be. Without the ability to debug this on scratch, I can't tell you anything more. Interestingly, only the changeDueDateResource file seems to be affected by whatever it is that's going on there-I can see the effects of changes I've made elsewhere in the code (like logging statements). It's very odd. |
| Comment by Marc Johnson [ 16/Nov/21 ] |
When is the loan object logged? Is it just before the request to storage? If not, could something else be getting in the way?
I think there are basically only a few things we can do:
I've not done this before, but could you try using the snapshot back-end and a local mod-circulation and invoke the change due date API locally? |
| Comment by Charlotte Whitt [ 17/Nov/21 ] |
|
Hi Marc Johnson Kyle Felker and Brooks Travis I looked through the old comments and then I noticed this one from Chalmers:
Does that comment help us to come a bit closer to where the issue is hidden? |
| Comment by Charlotte Whitt [ 17/Nov/21 ] |
|
Status as of 11/17/2021: Marc Johnson 12:08 PM |
| Comment by Charlotte Whitt [ 17/Nov/21 ] |
|
Notes from CPT 11/17/2021 (from Prokopyvych attended Charlotte Whitt and Marc Johnson ): The tickets priority was lowered to P2. The work is now planned for Lotus, but can be reconsidered to be a hotfix #1 for Kiwi.
|
| Comment by Kyle Felker [ 17/Nov/21 ] |
|
>When is the loan object logged? Is it just before the request to storage? If not, could something else be getting in the way? That was my thought as well, so I inserted code into the loanRepository file itself, right before it saves the loan, that dumps the state of the flag. I then ran the test that checks for this behavior. What I'm seeing is: initial save: no flag (it doesn't exist until it gets an explicit value) I can actually move the flag unset code closer to the loan save step than it is now-I should be able to do it anytime before the loan is saved, but that will make it less likely to be tampered with by other parts of the code. I'll do so. > check the test code is doing what we expect I've had the test dump what it's seeing as it executes, and have re-checked the conditions-I just can't see anything wrong. > add plenty of logging for when the code is next deployed to snapshot I will do this now. > I've not done this before, but could you try using the snapshot back-end and a local mod-circulation and invoke the change due date API locally? I can try. |
| Comment by Marc Johnson [ 17/Nov/21 ] |
I'd like to check my understanding of this. Immediately prior to the HTTP request to mod-circulation-storage to update to loan, the representation of the loan to be sent is logged. That representation includes the correct (false) value for the flag. Yet after the change due date process finishes, the flag is the wrong (true) value? |
| Comment by Kyle Felker [ 17/Nov/21 ] |
|
> Yet after the change due date process finishes, the flag is the wrong (true) value? No. When the loan is initially created and saved, it has no DueDateChangedByRecall flag. Just prior to the second save, which is triggered by placing the recall, I see a loan with the flag set to true, as it should be. Just prior to the third save, which is triggered by the due date change, shows the loan with the flag set to false, as it should be. The progression of the flag I would expect to see based on the events in the test is: no flag->true->false This is what the test checks for. That is what I am seeing when I dump the loan object. |
| Comment by Marc Johnson [ 17/Nov/21 ] |
I don't think I'm following, how is this different to what I said? |
| Comment by Kyle Felker [ 17/Nov/21 ] |
|
> I don't think I'm following, how is this different to what I said? Then I do not understand what you are saying, as it seems very different from what I'm describing. When the change due date process is (almost) completed, right before the loan is saved, the flag is set to false, not true. It is also set to false when the test retrieves it, as it should be. |
| Comment by Marc Johnson [ 17/Nov/21 ] |
I think we are saying the same thing. What I was trying to check was:
How wrong is that description of the current situation? |
| Comment by Kyle Felker [ 17/Nov/21 ] |
|
>During the change due date process, immediately prior to the PUT, the flag is set to false in the JSON representation This is true. >Yet, after the process finishes, the flag is still set to true (which is wrong) in storage. This is not, as far as I can tell, true. When I dump the loan object in the test, at the point after the changeduedate process is complete, the resulting loan object shows the relevant flag set to false-the correct value. if it were set to true, the test would fail, as it checks for a false value in the final loan representation. I make sure in the test to re-request the loan object from storage after the change due date operation, so I am getting the loan after that change has been applied and saved. |
| Comment by Kyle Felker [ 17/Nov/21 ] |
|
I have created a ticket for the problems I am having with the scratch environment and blocked this issue on them. I am still trying to get a local version of mod-circ to work with snapshot. |
| Comment by Marc Johnson [ 17/Nov/21 ] |
Ok. Then what is the state of the loan after the change due date process has finished during a manual test on the hosted reference environments?
I think this might be the source of the confusion. I was trying to understand what happens on the hosted environments (for this behaviour to not work). Apologies for the ambiguity. |
| Comment by Kyle Felker [ 17/Nov/21 ] |
|
>Ok. Then what is the state of the loan after the change due date process has finished during a manual test on the hosted reference environments? Okay, I get you now. In scratch, according to the logging code I just put in, the value for the flag just before saving the record during a due date change process, is true. The flag has not been set to false, as it should be. The saved value of the flag, after the due date change process is complete, is also true. So the value of the flag on scratch as it moves through the checkout->recall->due date change is: no flag->true->true I cannot see any of the output of the logging messages I put in the due date change code on scratch, so I don't know anything about the state of the loan while that operation is taking place. |
| Comment by Kyle Felker [ 18/Nov/21 ] |
|
Charlotte WhittMarc JohnsonZak Burke After much gnashing of teeth and confusion, Marc and I have finally figured out what is going on here. The front-end UI does not use the change due date endpoint to change due dates. Instead, it uses a put operation directly on the loans endpoint to modify the loan. This is why it looked like the change due date endpoint was dead code-it wasn't being run by the UI. It also explains why my tests run perfectly, but the code in the reference environments acted as if it wasn't there-the tests use the change due date endpoint, so it triggers all my logging, as well as the new code that checks and unsets the flag. Apparently there were plans to switch the UI over to using the change due date endpoint, but for whatever reason it never happened. Marc is porting my changes over to the the loans endpoint so that it does the same checking and flag clearing as the change due date endpoint. We should be done with that within the hour. |
| Comment by Kyle Felker [ 18/Nov/21 ] |
|
Fixes for this behavior have been merged to master. Brooks, I think you were doing the testing? If you can let me know once you've tested the changes in snapshot, I will backport the fix ASAP. |
| Comment by Brooks Travis [ 18/Nov/21 ] |
|
Kyle Felker yes, I will do so ASAP (and try to be more thorough about it this time 😣) |
| Comment by Brooks Travis [ 18/Nov/21 ] |
|
Kyle Felker Marc Johnson When should I anticipate seeing the fixes in Snapshot? |
| Comment by Marc Johnson [ 18/Nov/21 ] |
Not till tomorrow, unless a rebuild of the environment is requested. |
| Comment by Zak Burke [ 19/Nov/21 ] |
|
Brooks Travis, Marc Johnson, change-due-date was implemented in a shared component (update the loan object, send the PUT request) in the scope of
It is extremely unfortunate that we have overlapping functionality between an action-based endpoint and a record-based endpoint. When an action-based endpoint exists, the fields it changes should be changeable exclusively through said endpoint and not by any other means. Otherwise, the existence of the endpoint and the ability to manage permissions for accessing it are rendered pointless. |
| Comment by Brooks Travis [ 19/Nov/21 ] |
|
Verified Recall > Check /loan-storage/loans/<loanUUID> response > dueDateChangedByRecall: true > Cancel > Change due date (via UI) > Check /loan-storage/loans/<loanUUID> response > dueDateChangedByRecall: false Verified Recall > Check /loan-storage/loans/<loanUUID> response > dueDateChangedByRecall: true > Cancel > Renew loan > Check /loan-storage/loans/<loanUUID> response > dueDateChangedByRecall: false Verified Recall > Check /loan-storage/loans/<loanUUID> response > dueDateChangedByRecall: true |
| Comment by Holly Mistlebauer [ 19/Nov/21 ] |
|
Kyle Felker: I just learned that this was pulled from the Kiwi release. The Status is AWAITING RELEASE. It the correct version AWAITING RELEASE or is there re-work that needs to be done? Thanks for the info... |
| Comment by Kyle Felker [ 19/Nov/21 ] |
|
As of this moment, the status should be awaiting release. I am right in the middle of releasing the bugfix version of this for the last release (22.1.3). |
| Comment by Marc Johnson [ 19/Nov/21 ] |
I think it was primarily pulled from the Kiwi release out of concerns it might not make it in time. Given the extension that was granted due to other issues, I think we may want to include it back in now that we have re-tested after some new changes. Charlotte Whitt Brooks Travis What do you think?
I was working on the basis that folks would still want it in Kiwi and released as soon as possible. Please let me know if that is incorrect and we shouldn't back port the last third of this change (the other two thirds have already been back ported and released). |
| Comment by Marc Johnson [ 19/Nov/21 ] |
It is unfortunate. We have quite a few examples of this in FOLIO (especially as action based endpoints were a later design choice) I took the decision to have them side by side in order to allow for a graceful transition over. And then have a conversation about removing the old one (which has the potential to be contentious, as I believe folks may be using it for migration). The transition did not get completed and so we still have both.
With the current FOLIO architecture, that is challenging to accomplish. This is primarily down to the storage / business logic split and how storage APIs are as accessible as business logic ones are. We can plan to get rid of the business logic endpoint. I think that needs to come after a transition and needs agreement from the community. We have to have an endpoint in storage that allows for a client to change the due date of a loan with a PUT (or similar) because other operations e.g. renewal or recall change that information. We could stop due dates from being changed via that without an additional desired permission. |
| Comment by Brooks Travis [ 19/Nov/21 ] |
|
I agree with Marc Johnson that if we can include this in the Kiwi GA, we should try to do so. |
| Comment by Kyle Felker [ 19/Nov/21 ] |
|
>I agree with Marc Johnson that if we can include this in the Kiwi GA, we should try to do so. Good, cos it's 90% done. |
| Comment by Brooks Travis [ 22/Nov/21 ] |
|
Verified in Kiwi Bugfest |