Skip to content

Comments

[DTOSS-10724] symptoms#368

Merged
MatMoore merged 5 commits intomainfrom
dtoss-10724-symptoms
Sep 10, 2025
Merged

[DTOSS-10724] symptoms#368
MatMoore merged 5 commits intomainfrom
dtoss-10724-symptoms

Conversation

@MatMoore
Copy link
Contributor

@MatMoore MatMoore commented Sep 4, 2025

Description

Add some models so we can store participant-reported symptom information in a generic way.

Diagram

(note ParticipantRecordSymptom has been shortened to just Symptom)
diagram showing how symptoms fit into the rest of the data model

Jira link

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

Review notes

  1. I've split up the participants app's models.py into multiple files to make it somewhat easier to work on.

    Django doesn't exactly encourage this, and arguably we should split up the participants app instead, but I'm not feeling very confident that I could come up with a good split criteria right now, so I'd rather postpone app-level restructuring unless anyone else has a good idea of what it should look like. Maybe we should have another look at the app structure once we reach the point where we have most of the core functionality built out.

  2. I decided not to try and model symptoms and other kinds of medical informations (like implants, treatments, pregnancy and breast feeding) as one generic thing. They're different enough to be hard to model generically, and I expect this would create headaches later down the line. However, symptoms should be modelled in such a way that we can easily extend the specific symptoms we ask about.

  3. There are also some commonalities between this and participant reported mammograms which we have already implemented (see inline comments)

  4. There are no codes associated with the symptoms yet - but I'm envisioning that we will add them to the SystemType model at a later date.


class SymptomSubType(models.Model):
symptom_id = models.ForeignKey(SymptomType, on_delete=models.CASCADE)
name = models.CharField(null=False, blank=False)
Copy link
Contributor Author

@MatMoore MatMoore Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This 2-step hierarchy should be enough to store the symptom name, and also an optional secondary categorisation, e.g. in the case of Skin changes, this would be "How has the skin changed?"

@MatMoore MatMoore force-pushed the dtoss-10724-symptoms branch from d0f38bb to 4048375 Compare September 4, 2025 14:22
investigated = models.BooleanField()

# Onset information
when_started = models.CharField(blank=True, null=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some overlap here with how we model participant reported mammograms. In both cases we support a more specific date entry and a free text option.

I considered maybe extracting a specific model for this, like ParticipantReportedDate, but am thinking it's better to leave this until we have a couple more usages of this, and wait and see if the patterns evolve.

The two usages are slightly different because reported mammograms collect Year/Month/Day, whereas symptoms collect only Year/Month.

ParticipantReportedMammogram also has a few fields for capturing place in different ways, which also seems potentially reusable, but for symptoms we are not capturing this at the moment, only a boolean "yes this was investigated"

# Onset information
when_started = models.CharField(blank=True, null=False)
year_started = models.IntegerField(null=True, blank=True)
month_started = models.IntegerField(null=True, blank=True)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we could use a DateRange for this. But it seems a bit more complex.

)
participant_id = models.ForeignKey(Participant, on_delete=models.PROTECT)
appointment_id = models.ForeignKey(Appointment, on_delete=models.PROTECT)
reported_at = models.DateField()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this field might be implied by the appointment date, but in the future it could be collected pre-appointment so explicitly setting a date seems like a good idea

@MatMoore MatMoore force-pushed the dtoss-10724-symptoms branch from 476a806 to 7c6cb99 Compare September 8, 2025 10:39
@MatMoore MatMoore changed the title [WIP] [DTOSS-10724] symptoms [DTOSS-10724] symptoms Sep 8, 2025
@MatMoore MatMoore marked this pull request as ready for review September 8, 2025 11:06
@MatMoore MatMoore requested a review from a team September 8, 2025 11:06
SymptomSubType, on_delete=models.PROTECT, null=True, blank=True
)
participant = models.ForeignKey(Participant, on_delete=models.PROTECT)
appointment = models.ForeignKey(Appointment, on_delete=models.PROTECT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see why we'd need to associate with appointment, but if that's the case, can we simply infer the participant via the appointment record rather than also storing participant foreign key?

Copy link
Contributor Author

@MatMoore MatMoore Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we can. I've dropped the column.

Only downside is it will require multiple joins if we want to access participant data and symptom data together, e.g. 'appointment__screening_episode__participant`

I also added some methods just to delegate down to the participant so you don't have to do thing.appointment.screening_episode.participant everywhere

NIPPLE_CHANGE = "nipple-change"
OTHER = "other"

name = models.CharField(null=False, blank=False)
Copy link
Contributor

@malcolmbaig malcolmbaig Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I see an example of what kind of values you see name and id fields storing, for a both a SymptomType and SymptomSubType record?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name would just be a human readable version of the ID.

type: skin-change / "Skin change"

subtype: sores-or-cysts / "Sores or cysts"

It's not important that the participant reported the symptom, just that
it was captured during/in preparation for the appointment.

It's possible the mammographer could also notice symptoms such as lumps
during an appointment that the participant hasn't reported. We may later
support mammographers recording that, in which case we would use the
same model, but perhaps add an extra field to specify the provenance.

Features captured as part of image reading will be modelled separately
later.
@MatMoore MatMoore force-pushed the dtoss-10724-symptoms branch from 8f28047 to af3f6a8 Compare September 9, 2025 14:11
@MatMoore MatMoore force-pushed the dtoss-10724-symptoms branch from 6abaab3 to 3312a2b Compare September 10, 2025 10:41
Copy link
Contributor

@malcolmbaig malcolmbaig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍🏼

@MatMoore MatMoore merged commit 411e75d into main Sep 10, 2025
11 checks passed
@MatMoore MatMoore deleted the dtoss-10724-symptoms branch September 10, 2025 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants