DTOSS-10350: Record updates to Appointments from NBSS

[Harriethw]

Description

NBSS sends data about an Appointment according to certain event triggers - Booked, Cancel, Update - and includes a status indicator on an Appointment where:

‘B’ = Booked
‘C’ = cancelled
‘A’ = Attended
‘D’ = DNA

We need the ability to update statuses of existing Appointments from messages we've already ingested based on a new update coming in from NBSS - e.g. if an Appointment goes from booked to cancelled.

The Appointments coming from NBSS into the MESH inbox will have an ID from NBSS to identify them, and we will use that to match with Appointments already created in our db.

This PR

• Makes nbss_id a unique field to avoid any duplications
• rather than getting or creating the Appointment object, we update or create one based on the nbss_id
• add an nbss_updated_at field to capture the Action Timestamp from NBSS - in the documentation we have on the MESH inbox this field is described as Date/timestamp for triggering the data transfer.  we think data transfers run every 5 mins so this should be as close as we need to when the actual event happened?
  Not totally clear that we need this field in terms of user needs for reporting/auditing, as that is all presumable dealt with in NBSS, but it does seem right that as we are updating fields on an object we should record when that update happened.

Jira link

https://nhsd-jira.digital.nhs.uk/browse/DTOSS-10350

Review notes

[Harriethw]

here I'm potentially updating any field on the Appointment that isn't nbss_id - is that ok? Should we take whatever the latest data is on an Appointment, as presumable should match NBSS? or should we only update e.g. status so that we keep a snapshot of the original content that we stored?

[steventux]

In the case of a cancellation I think we should only update the fields relating to the cancellation - the status, cancelled_by and the related timestamp coming from the Action Timestamp column.
Probably out of scope for this PR but is there a chance that a Booked Appointment can be overwritten with new details? Probably something for the team to investigate and outside the scope of what we're addressing here.

[Harriethw]

In the case of a cancellation I think we should only update the fields relating to the cancellation - the status, cancelled_by and the related timestamp coming from the Action Timestamp column.

This does make sense, I guess better to be as specific as possible - but I'm getting a bit into the weeds thinking of all the possible complications around how to decide whether to create/update and what to create/update. E.g. what if the original Booked appointment had one of it's fields different to the Cancelled appointment coming in? Can we rely on all the data being exactly the same? If that were the case we can't match on all the fields, only the nbss_id so we need to check if it exists first and then do something slightly different for all the statuses like below.
I'm also imagining that we can't assume if a Cancellation is coming in that there will be a Booked one already there, especially when we first spin up.
I don't know if I'm getting way too complicated with this 😅

        status = row["Status"]
        nbss_id = row["Appointment ID"]
        appointment = Appointment.objects.filter(nbss_id=nbss_id).first()

        if status == "C":
            if appointment != None:
                appointment.status = status
                appointment.cancelled_by = row["Cancelled By"]
                appointment.save()
                return (appointment, False)
            else:
              return (Appointment.objects.create(
                  nbss_id=row["Appointment ID"],
                  nhs_number=row["NHS Num"],
                  clinic=clinic,
                  starts_at=self.appointment_date_and_time(row),
                  number=row["Sequence"],
                  status=status,
                  booked_by=row["Booked By"],
                  cancelled_by=row["Cancelled By"]
              ), True)
        elif status == "B":
            if appointment != None:
                appointment.status = status
                appointment.booked_by = row["Booked By"]
                appointment.save()
                return (appointment, False)
            else:
                return (Appointment.objects.create(
                    nbss_id=row["Appointment ID"],
                    nhs_number=row["NHS Num"],
                    clinic=clinic,
                    starts_at=self.appointment_date_and_time(row),
                    number=row["Sequence"],
                    status=status,
                    booked_by=row["Booked By"],
                ), True)
        else:
            if appointment != None:
                appointment.status = status
                appointment.save()
                return (appointment, False)
            else:
                return (Appointment.objects.create(
                    nbss_id=row["Appointment ID"],
                    nhs_number=row["NHS Num"],
                    clinic=clinic,
                    starts_at=self.appointment_date_and_time(row),
                    number=row["Sequence"],
                    status=row["Status"],
                    booked_by=row["Booked By"],
                    cancelled_by=row["Cancelled By"]
                ), True)

[Harriethw]

Thinking about the above... I think we can safely assume the nhs_number, starts_at will be the same, but clinic might have slightly different address fields? So we could condense to something like

        defaults = {
            "number": row["Sequence"],
            "status": row["Status"],
            "clinic": row["Clinic"],
            # Some logic to work out whether to updated cancelled or booked based on status
            "cancelled_by": row["Cancelled By"],
            "booked_by": row["Booked By"],
            "cancelled_at": row["Action Timestamp"]
        }

        return Appointment.objects.update_or_create(
            nbss_id=row["Appointment ID"],
            nhs_number=row["NHS Num"],
            starts_at=self.appointment_date_and_time(row),
            defaults=defaults
        )

[steventux]

Agree that nbss_id, nhs_number and starts_at are good query params for the update_or_create statement.
I think clinic is found/created by clinic code via a separate query, and in any case we probably aren't interested in whether clinic has changed in the case of a cancellation (I would err on the side of preserving the original value).
There is definitely a set of scenarios for updates to existing bookings we need to cover but for this PR let's just make sure status, cancelled_by and cancelled_at (or whatever timestamp we use) are updated here.

[Harriethw]

ok I've just focused on the cancel use case and tried to keep as simple as possible. a few gotchas the tests uncovered - the number field is from the sequence row which apparently is just the row number of the dat file? so it will change based on where the appointment is in the data. I'm not sure what the implications of that are...
I had to still account for the booked scenario in order to cover existing behaviour of updating booked_by.

[steventux]

Looks great, I've commented about what I think we should update.
I've also made a comment about the nbss_updated_at timestamp - happy to chat about this.

[steventux]

Hard to know what we'll actually need for reporting but I'm not sure this date field gives us the precision on when things change because of a cancellation.
Might just be my Rails/ActiveRecord bias but created_at is the timestamp NOW() from postgres - so transaction commit time.
nbss_updated_at is a past event that occurred in NBSS, perhaps less than a minute ago but not an event in Manage.
If we create a "booked" appointment and then it is cancelled we lose the information about the nbss booking time because nbss_updated_at is overwritten.
Comparing created_at to nbss_updated_at is not a great indicator of when something was booked vs. cancelled in this scenario because the timestamps refer to events on different systems.
Although a bit more involved, I wonder if we would preserve more accuracy if we had:
created_at and updated_at as Manage db timestamps and booked_at, cancelled_at as NBSS timestamps (coming from the Action Timestamp column of the dat file.

[Harriethw]

created_at and updated_at as Manage db timestamps and booked_at, cancelled_at as NBSS timestamps (coming from the Action Timestamp column of the dat file.

yep that makes a lot more sense!

[steventux]

In the case of a cancellation I think we should only update the fields relating to the cancellation - the status, cancelled_by and the related timestamp coming from the Action Timestamp column.
Probably out of scope for this PR but is there a chance that a Booked Appointment can be overwritten with new details? Probably something for the team to investigate and outside the scope of what we're addressing here.

[steventux]

💯

[steventux]

💾