Skip to content

Instantly share code, notes, and snippets.

@marcofucci
Last active August 29, 2015 14:27
Show Gist options
  • Star 2 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save marcofucci/5148a4b22650dbd3d91f to your computer and use it in GitHub Desktop.
Save marcofucci/5148a4b22650dbd3d91f to your computer and use it in GitHub Desktop.

Cashbook transactions API

List of cashbook transactions api:

  • /cashbook/transactions/
  • /cashbook/transactions/actions/taken/
  • /cashbook/transactions/actions/release/

/cashbook/transactions/ -- GET

Returns only transactions for the prisons that the user can manage.

Filters

status

Filters by the status of the transactions.

Default: no status => all transactions

Possible values:

  • available: returns list of transactions that have not been taken by anyone
  • pending: returns list of transactions taken
  • credited: returns list of transactions credited

prison

Filters by list of prisons (in OR).

Default: all prison that the user can manage.

Note:

  • If the user can't manage a specified prison, the related returning value will be an empty list.

user

Filters by single user, list of users not allowed (to keep things simple).

Default:

  • all users that can manage the prisons that the logged-in user can manage if prison is not passed in
  • all users managing the related prisons if prison is passed in

Note:

  • if status is not passed in, the userfilter does not make sense at all and it won't do anything.
  • if status=available, the user filter does not make sense at all and it won't do anything.
  • if user is passed in but not prison and the specified user can't manage the prisons of the overall query, the endpoint will return an empty list.
  • if user and prison are passed in but the the specified user can't manage the specified prison, the endpoint will return an empty list.
  • if user and prison are passed in, the specified user can manage the specified prison but the logged-in user can't manage the specified prison, the endpoint will return an empty list.

/transactions/ -- PATCH

Marks/unmarks a list of transactions as credited.

Data

List of:

  • id: id of the transaction to be changed
  • credited: True if the transactions has to be marked as credited, False otherwise

Note:

  • returns 403 if at least one of the transactions have been taken by a different user. Only the user that took a transaction can mark/unmark it as credited.
  • returns 400 if at least one of the transactions is not taken. Transactions have to be taken before being able to get changed.

/transactions/actions/take/ -- POST

Takes (meaning locks) some transactions. There's no way to specify which transactions have to be taken as the user doesn't care which one they take.

No data has to be specified.

Params

count

Number of transactions to be taken.

Default: min(20, 20-count(taken transactions)). This is to make sure that the user can only take max 20 transactions at any given time.

prison (mandatory)

Id of the prison to be used when taking transactions. It has to be one of the prisons that the user can manage otherwise it will error.

Default: no defaults.

Note:

  • returns 403 if the specified prison is not one of the prisons that the user can manage.

for_user - we don't need this for now so maybe we don't need to implement it?

User the transactions will be assigned to.

Default: logged-in user

Note:

  • returns 403 if for_user is passed in but the logged-in user and the specified user do not belong to the same specified prison.

/transactions/actions/release/ -- POST

Releases (meaning unlocks) some transactions.

Data

transaction_ids (mandatory)

List of transactions to be released.

Note:

  • returns 403 if at least one of the transactions belongs to a prison not managed by the logged-in user
  • returns 400 if at least one of the transactions is credited. Available (meaning non-taken) transactions can be released without any problems as their state does not change anyway.
@ibrechin
Copy link

Does /take accept a list of prisons? If not, I think it should. Looking at the cashbook UI, there's no prison selection there, the user is just taking from the queue of all those available to them, in which case /take should just return the next batch of the given size across everything they can get.

@marcofucci
Copy link
Author

/take takes just one prison and it's a mandatory param. I was thinking that it's not worth supporting multiple prisons as it's a lot of work and it doesn't add anything really.

@SteveMarshall
Copy link

For the most part, I think this is great. A few small observations/thoughts:

  1. The filters on /cashbook/transactions/ are good, but I think status is missing a refunded value (and maybe refund_pending, too?). I'd also argue that available is the only status for which filtering by user isn't applicable: pending, credited, and refunded should all have assigned users, so filtering by user should return all of those. It would also be nice to allow multiple user IDs to be selected, long term.
  2. We should probably standardise the language we use for locking/taking transactions. I can see three different terms being used here: take, pending, and locked. My preference would be lock/locked throughout, but I'm open to other terms…
  3. The explanation of count on …/take/ isn't entirely clear to me. Do we have a hard limit of 20, or does specifying count=50 mean that I get a total of 50 locked to me? If the latter, what happens if I have 50 locked and then try to lock with count=20? Do we release the excess?
  4. Do we need prison on …/take/ to be mandatory? My instinct is not: if someone only has permission to process one prison, that's all they'll get. If they have more than one prison, chances are they don't care which prison they're crediting to when crediting (and, if they do, that's something we can handle in the cashbook).
  5. Leave out the for_user stuff on …/take/: we don't need it yet, as you say.
  6. Should the 400 returns in /transaction/ PATCH and in …/release/ be 409 Conflict, maybe?

@SteveMarshall
Copy link

Other than those few clarifications, though, I think this is a pretty neat API. 👍

@marcofucci
Copy link
Author

Let's start with point 2 and make a revision.

@SteveMarshall @ibrechin are we ok with renaming take, pending and locked to lock/locked ?

@SteveMarshall
Copy link

@marcofucci If, by that, you mean the action is lock and the status is locked, then yeah, works for me!

@ibrechin
Copy link

@marcofucci re: lock, sure, fine

@marcofucci
Copy link
Author

So available becomes unlocked ? does it make sense?

@SteveMarshall
Copy link

Nah, I think available still makes sense there. unlocked sounds like it comes after locked, so would be weird to have as an initial state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment