feat: documented deposits endpoint

[danjohnson95]

Documented the /booking/{bookingId}/deposits endpoint, but I have a few things to discuss:

• I'm not too keen on the term deposits. We're storing payments, deposits and auths in here. Do you think "payments" is clearer? Or can you think of anything better?

• How do you reckon we should output currency values - as a float or an integer of pence?

• What about if we always output any kind of monetary value as a PHP money object, which keeps the currency and amount together?:

"value": {
    "currency": "GBP",
    "amount": 1000
}

• Have I got the description for paid_type correct? Is there a better name we could use for that field?

• Is it weird behaviour that you can filter on deposits from the /bookings endpoint, but you have to go to the /bookings/{bookingId}/deposits endpoint to see those deposits?

[willtj]

Really good questions.

1. Deposits: Given the chance to re-do this I think I'd probably look to rename or restructure this, but I was encouraged to see that Google refers to reservation payments as deposits whether they're auths or actual payments, so it's not an awful generic term. I think I'd probably stick with it as is, where deposits is any requested payment along with any notes and status, and payments and refunds contain the details of actions taken against a customer's card.

If we do end up renaming fields, do we also need to make update endpoints that accept either the 'core' field name or the renamed one?

2. Currency values: I really like the thought of including the currency and the amount, and it probably does make sense to do the grownup thing and start treating currency amounts as integers.

Similar to the question above, if we change the return format should we also consider supporting that format in update requests?

3. Paid type: I'd say something like 'Where the type of deposit is 'paid' (i.e. payment received outside of Collins), this field contains the type of payment that was added'. Better values would have been type: 'external' and external_type: 'Cash' but I don't know if we want to take on updating the value of type? It's not a huge task if you think it's worth it to tidy things up though.

4. Filtering on deposits: I think it's probably still helpful to include deposits in the /bookings response. If we had to do an additional lookup to get the deposit details then I'd say we could just include an array of deposit IDs in the regular bookings request, but given that we already have them we may as well return them? I wouldn't object to returning just IDs if you think that's better.

[danjohnson95]

If we do end up renaming fields, do we also need to make update endpoints that accept either the 'core' field name or the renamed one?

I don't think this will be a problem - they should just follow the documentation for the API they're using. What do you think?

Similar to the question above, if we change the return format should we also consider supporting that format in update requests?

Ooh very interesting, I hadn't thought of that. The currency is set from the venue/venue group, right? If that's correct, then there's probably no point in getting them to pass through the currency - they won't be able to update it so it seems redundant. However making the request/response inconsistent is a bit vom. Maybe we don't even need to output the currency on the GET request? 😕 Thoughts?

Paid type: I'd say something like 'Where the type of deposit is 'paid' (i.e. payment received outside of Collins), this field contains the type of payment that was added'. Better values would have been type: 'external' and external_type: 'Cash' but I don't know if we want to take on updating the value of type? It's not a huge task if you think it's worth it to tidy things up though.

I think a tidy-up would be a good shout - it's going to be a lot harder to make these types of changes when third parties (and us) are using the API, so now is probably the best time.

Filtering on deposits: I think it's probably still helpful to include deposits in the /bookings response. If we had to do an additional lookup to get the deposit details then I'd say we could just include an array of deposit IDs in the regular bookings request, but given that we already have them we may as well return them? I wouldn't object to returning just IDs if you think that's better.

I'm -1 for outputting the entire deposits, but I'm happy to just output the IDs. Reasons against outputting all deposits within the booking:

• Presumably we'll be ignoring the field for update requests, so we're creating an inconsistency between the GET/POST requests.
• Where do we draw the line? A booking has lots of embedded documents, and outputting all of them would make requests huge and complicated.

[willtj]

I think probably inconsistency between GET/POST types is ok (eg including the currency in the GET, not expecting it in the POST). The idea of including the currency has definitely grown on me, but thinking about it now, maybe we should just return the amount as an int and start storing currency at a booking level. That would also prevent lookups to the venue each time we need to create a Money object for anything in the Booking model or embedded models.

Can you think of other fields you'd like to tidy up? For 'paid' deposits I think this would require a few changes in Collins UI to temporarily support both, then a bulk update in Mongo, possibly a change to the mapping for deposits in ES, and an ES reindex of all affected bookings. We should probably do the UI change this week and look to do the data change next week?

Including just embedded document IDs in the booking response doesn't feel too bad, but do you think we also want an option to return everything in one hit? And do we go all in and say that every embedded model should behave the same way, and have to deal with those like admin_notes that don't currently have an ID?

Given that we have everything available against the booking, I wonder if it might be taking a step backwards to start removing things from the GET response, and I think potentially inconsistency between GET and POST is ok. I wouldn't say that booking requests are particularly huge or complicated at the moment even though they include all embedded docs?