Skip to content
This repository has been archived by the owner on Nov 7, 2023. It is now read-only.

Refactor: Rewrite and simplify the data model for booking #154

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nt-gt
Copy link
Contributor

@nt-gt nt-gt commented Nov 2, 2023

The carrier SMEs have decided that there should be one and only one
endpoint for bookings. This means the distinguish between a
BookingRequest and ConfirmedBooking should no longer be entity but
only status level changes.

To prepare for that, I have rewritten the data model with this focus
in mind. In this rewrite, BookingRequest and ConfirmedBooking are
replaced by Booking and BookingData.

The BookingData contains the combined set of attributes from
BookingRequest and ConfirmedBooking with a very few "meta data"
attributes (such as carrierBookingRequestReference,
carrierBookingReference, and the bookingStatus). Everything else
is in the BookingData entity.

Because this is a massive internal rewrite, I made it a goal to keep
the postman test passing running without any changes to the postman
collection. This should give us confidence that this is purely an
internal "implementation-detail"-level rewrite. No externally visible
contracts have chanced.

Other notes:

  • Henrik is still working on the updated Swagger specs for the new
    design. It is currently a private draft at DRAFT-SWAGGER

  • I made no changes to TOss. Again to avoid changing the postman
    tests. I expect we will able to regnerate those once Henrik is
    done with the swagger rewrite.

    • There are also no externally visisble changes to controllers
      for the same reason. The only change is a rename of a method in a
      service used by a controller).
  • Most of the SQL changes is removing now redundant SQL test data.
    Most of it became redundant with the removal of the summaries
    endpoint, but was not removed at the time. I removed it in this
    commit because the data was not used anyway and in some cases
    violated "1-1" constraints (we had multiple ConfirmedBookings
    linking to the BookingRequest, which is no longer possible in
    the new data model design).

  • I had to keep some "dead" attributes around (such as the "create"
    and "update" timestamps) because they are required by a JSON schema
    in the postman collection. The reference implementation does not
    maintain these field as strictly as before as they are scheduled
    for removal (DT-389). Just enough to satisify the JSON schema
    requirement to avoid having to touch the postman collection. I
    have spinkled TODO around with a refrence to DT-389 where I thought
    it was relevant.

Signed-off-by: Niels Thykier [email protected]

@nt-gt nt-gt force-pushed the booking-internal-data-model-rewrite branch from db1a4b7 to 55b3776 Compare November 2, 2023 11:12
@nt-gt nt-gt marked this pull request as ready for review November 2, 2023 11:17
The carrier SMEs have decided that there should be one and only one
endpoint for bookings.  This means the distinguish between a
`BookingRequest` and `ConfirmedBooking` should no longer be entity but
only status level changes.

To prepare for that, I have rewritten the data model with this focus
in mind.  In this rewrite, `BookingRequest` and `ConfirmedBooking` are
replaced by `Booking` and `BookingData`.

The `BookingData` contains the combined set of attributes from
`BookingRequest` and `ConfirmedBooking` with a very few "meta data"
attributes (such as `carrierBookingRequestReference`,
`carrierBookingReference`, and the `bookingStatus`). Everything else
is in the `BookingData` entity.

Because this is a massive internal rewrite, I made it a goal to keep
the postman test passing running without any changes to the postman
collection.  This should give us confidence that this is purely an
internal "implementation-detail"-level rewrite. No externally visible
contracts have chanced.

Other notes:
 * Henrik is still working on the updated Swagger specs for the new
   design. It is currently a private draft at [DRAFT-SWAGGER]

 * I made *no* changes to TOss. Again to avoid changing the postman
   tests.  I expect we will able to regnerate those once Henrik is
   done with the swagger rewrite.
   - There are also no *externally* visisble changes to controllers
     for the same reason. The only change is a rename of a method in a
     service used by a controller).

 * Most of the SQL changes is removing now redundant SQL test data.
   Most of it became redundant with the removal of the summaries
   endpoint, but was not removed at the time. I removed it in this
   commit because the data was not used anyway and in some cases
   violated "1-1" constraints (we had multiple ConfirmedBookings
   linking to the BookingRequest, which is no longer possible in
   the new data model design).

 * I had to keep some "dead" attributes around (such as the "create"
   and "update" timestamps) because they are required by a JSON schema
   in the postman collection.  The reference implementation does not
   maintain these field as strictly as before as they are scheduled
   for removal (DT-389). Just enough to satisify the JSON schema
   requirement to avoid having to touch the postman collection.  I
   have spinkled TODO around with a refrence to DT-389 where I thought
   it was relevant.

[DRAFT-SWAGGER]: https://app.swaggerhub.com/apis/dcsaorg/DCSA_BKG/2.0.0-Beta-1-Henrik
Signed-off-by: Niels Thykier <[email protected]>
@nt-gt nt-gt force-pushed the booking-internal-data-model-rewrite branch from 55b3776 to e797cfc Compare November 2, 2023 11:20
…main/persistence/entity/BookingData.java

Co-authored-by: Henrik Helmø Larsen <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants