search

openactive / open-booking-api

Repository for the Open Booking API specification
Other
2 stars 3 forks source link

Clarify B robustness to C1 and C2 differences #202

Open lukehesluke opened 3 years ago

lukehesluke commented 3 years ago

https://github.com/openactive/openactive-test-suite/blob/master/packages/openactive-integration-tests/test/templates/c2-req.js#L112

vs

https://github.com/openactive/openactive-test-suite/blob/master/packages/openactive-integration-tests/test/templates/b-req.js#L116

Most tests use these templates for C2/B requests. Therefore, if ever there's a Booking System which mandates that the customer should be consistent between C2 & B (which they might well want to do if they support leasing), most tests will fail.

The spec implies that amendments to an OrderQuote must happen at C2 rather than at B (here). These requests constitute an amendment between C2 -> B.

If my understanding of the spec is correct, then this should also spawn:

  1. Spec: New error class to inform that Order has been amended since OrderQuote
  2. OpenActive.Server.NET: Return the above error
  3. openactive-test-suite: Test the above error
lukehesluke commented 3 years ago

@nickevansuk does the above seem correct?

It seems like it makes sense to disallow amendment between C2 & B because:

  1. If customer B would have a different price due to e.g. being a member / student / etc.
  2. If the Booking System supports named leases, then their internal model may have vital assumptions that the lease is assigned to the same person as the order
nickevansuk commented 3 years ago

Currently there's no limitation around changes to anything C2 and B, as if a lease at B is missing it should just pick one up and carry on - so hence C2 isn't a dependency to run B

Additionally, as it's a guest checkout, updates to the details should not have an impact to the overall price etc here. So although changing the name might drop a lease and create a new one (in the case of a named lease - if for example a particular email address is only allocated a certain number of leases) that case should still be handled.

So the different identifiers were there to check that everything doesn't blow up if the details change...

nickevansuk commented 3 years ago

Suggest that the behaviour of B in terms of just trying to make a booking regardless of the previous state - even if C1/C2 call doesn't happen first in the extreme case - should certainly be clarified in the spec