This document outlines issues with current fees/fines functionality implementation and proposes solutions to these issues.
Move fee/fine actions calculation to BE
Currently, calculations for fee/fine actions are happening on FE without any checks on the BE side. Instead, when a value is entered in the amount field ("Payment amount", "Waive amount" etc.) a call to one of the new endpoints should be made in order to check if this value is valid and if the action is allowed:
POST /accounts/{accountId}/check-pay
POST /accounts/{accountId}/check-waive
POST /accounts/{accountId}/check-transfer
POST /accounts/{accountId}/check-refund
Body:
{ "amount": "1.00" }
Reponse
In case of success:
Status code: 200
Response body:
{ "accountId": "e74d50c9-0c69-4f80-9e1b-a819719fc0c9" "amount": "1.00", "allowed": true, "remainingAmount": "9.00" }
In case if fee/fine was not found:
Status code: 422
Response body:
{ "accountId": "e74d50c9-0c69-4f80-9e1b-a819719fc0c9" "amount": "1.00", "allowed": false, "errorMessage": "Fee/fine was not found" }
In case if fee/fine is already closed:
Status code: 422
Response body:
{ "accountId": "e74d50c9-0c69-4f80-9e1b-a819719fc0c9" "amount": "1.00", "allowed": false, "errorMessage": "Fee/fine is already closed" }
In case if amount is zero or negative:
Status code: 422
Response body:
{ "accountId": "e74d50c9-0c69-4f80-9e1b-a819719fc0c9" "amount": "1.00", "allowed": false, "errorMessage": "Amount must be positive" }
In case if the amount is too high:
Status code: 422
Response body:
{ "accountId": "e74d50c9-0c69-4f80-9e1b-a819719fc0c9" "amount": "1.00", "allowed": false, "errorMessage": "Requested amount exceeds remaining amount" }
In case of invalid amount value (e.g. negative or not parsable):
Status code: 422
Response body:
{ "accountId": "e74d50c9-0c69-4f80-9e1b-a819719fc0c9" "amount": "abcdefg", "allowed": false, "errorMessage": "Invalid amount entered" }
After receiving a positive response from BE ("allowed": true), when a user proceeds with the fee/fine action, instead of updating Account and creating FeeFineAction objects directly UI needs to call one of the new endpoints with the same body:
POST /accounts/{accountId}/pay
POST /accounts/{accountId}/waive
POST /accounts/{accountId}/transfer
POST /accounts/{accountId}/refund
Pay
Body:
{ "amount": "1.00", "comments": "STAFF : comment for staff \n PATRON : comment for patron", "transactionInfo": "check #3456", "notifyPatron": true, "servicePointId": "7c5abc9f-f3d7-4856-b8d7-6712462ca007", "userName": "Folio, James", "paymentMethod": "Cash" }
Fields
comments
and transactionInfo
are optional
BE should take care of updating Account object and creating new FeeFineAction object.
Reponse
In case of success:
Status code: 201
Response body:
{ "accountId": "e74d50c9-0c69-4f80-9e1b-a819719fc0c9", "amount": "1.00" }
FE's responsibility is to (re)load Account and new FeeFineAction objects and update the page accordingly.
In case of a failure (which is still possible - for example, someone else can waive a fine that we're trying to pay between "check" and "perform" calls):
Status code: 422
Response body:
{ "accountId": "e74d50c9-0c69-4f80-9e1b-a819719fc0c9" "amount": "1.00" "errorMessage": "Requested amount exceeds remaining amount" }
15 Comments
Taras Spashchenko
Great start.
Indeed all the logic must be kept inside back-end modules since it is more robust, secure, flexible. Also, it eliminates all those unnecessary HTTP calls between UI and BE. Regarding end-point names, I would say that if this set of actions is fixed it would be better to use different end-points just to make API more expressive. Under the hood though, I suspect it will be handled by the same engine.
Zak_Burke
Is there any chance there will be separate permissions for separate actions, e.g. a user will have the ability to
pay
but not towaive
,transfer
, orrefund
? If so, we would need separate endpoints for each action. Maybe this a question for the PO, Holly Mistlebauer?Zak_Burke
... which is what I see Taras Spashchenko asked about just before me. I guess I should have refreshed this page after I read the original and before I came back from making coffee to add comments!
Alexander Kurash
Taras Spashchenko Zak_Burke Marc Johnson Updated with a separate endpoint for each action
Marc Johnson
Given that these actions are specific to a particular account, could an alternative API be something like
POST /accounts/{id}/pay
POST /accounts/{id}/waive
POST /accounts/{id}/transfer
POST /accounts/{id}/refund
That way the context of the account is part of the URL rather than the request body, which makes it easier to know which resource is being primarily affected.
Are there reservations about this design because of the concerns around the
account
terminology?Alexander Kurash
I like this idea. I used /feefineaction initially because we're trying to replace POSTs to this endpoint with something else. And yes, the actions are specific to a particular account (or whatever we may change its name to).
Zak_Burke
Sorry, here comes whiplash: my "separate endpoints for separate actions" point-of-view was derived from the fact that we don't support so-called "action-based" permissions in Folio because most endpoints are record-based: e.g. you get write-access to an item-record or you don't; if you can change one field you can change them all. But here, while a particular fine may correspond to a record, I think a payment is more like a single entry in a log of transactions. Transactions are typically not edited, ever. If somebody overpays, you don't update the original transaction; you issue a refund in a follow-up transaction.
There are different transaction types (payment, waive, transfer, refund) and there may be permissions associated with them, but maybe the original API with a single "transaction" endpoint makes sense. I'm struggling hard to think through this. I guess the real question is this: what system are we trying to represent with our API here? Are we modeling changes to an account-object (in which case we probably do need separate endpoints for separate transaction types) or are we modeling a transactions (in which case we can use a single endpoint because a transaction won't ever be updated). Maybe we are conflating the two and that's why I'm struggling?
Marc Johnson
Zak_Burke Thanks
I think I've lost some context. What are you reacting to, my previous comment?
Indeed. Whilst I don't think this part of FOLIO specifically follows such accounting model, as I understand it, these transactions are called
actions
. And there is a sequence ofactions
performed on anaccount
(although there are other names for this).What are you suggesting is being edited?
I think this might be where we hit some of the limitations of the language used by FOLIO at the moment.
I think each kind of activity is likely a different process. A payment is different in both intent and outcome to a refund.
A part of me wonders if the addition of a transaction to the log is a side-effect of a process, not the process itself?
Could it help if we had different names for the process (payment) and the side effect (results in a new log entry reducing the outstanding amount of fee or fine to pay)?
Excellent question.
I think activities or processes tend to happen in some form of context, e.g. a credit was made into account X.
I agree with you that we can choose between the context being expressed
Maybe I can flip this around a little.
Is there a single transaction log for all of the fees and fines in FOLIO together, or is there a log for each fee / fine?
Maybe an activity like
transfer
could help us explore this better.There are naturally two parties involved in a transfer (a single activity). Is there a single entry in the log or two?
This might be where FOLIO's model diverges from a transaction log, it is rather a history of activities not side-effects.
Alexander Kurash
We're doing two things:
1) changing Account object (updating it's balance)
2) creating FeeFineAction object (which is a transaction)
Taras Spashchenko
As far as I understand, all those (pay, waive, transfer, refund) are actions, not transaction representations. Requests to any of these endpoints do not contain the whole information about a transaction that should be performed. Conversely, it is necessary to perform some additional logic to collect all data needed to perform a transaction.
Marc Johnson
Taras Spashchenko Thanks, that is interesting.
What are they representations of?
What do you expect the side effects of these endpoints to be in that case, given they are insufficient to be a transaction?
How do you envisage transactions are performed and what are their side effects?
Taras Spashchenko
Ok, let me explain.
These new endpoints are not about manipulating with resources, rather they are requests to perform some actions regarding resources. Those requests must contain the minimum data required to perform such an action and they are always POST HTTP requests. For example, if we are talking about the “Pay” request. The body (payload) of such a request must contain only “AccountId” and “PayAmount”. All that is needed will be performed at the back-end module. It will check if this request can be fulfilled if the accountId exists if the Amount is correct and all other stuff. If everything is Ok, the back-end module will create a real transaction (a resource), and also it will return that transaction within the response body. If something is wrong, the response will contain a detailed explanation for the problem and an appropriate HTTP response code.
Marc Johnson
Taras Spashchenko
In this context, what do you mean by a resource or a transaction?
Alexander Kurash
I think Taras meant that current change makes it more RPC-style rather than REST. Previously we created FeeFineAction and updated Account objects with direct POST and PUT requests and now we're asking the server to create/update them for us (if our request passes validation of course).
Taras Spashchenko
And even more, since accountId can be a part of the request path, the only content for a payload could be the "PayAmount"