Consistency check for mandatory fields concerning SORMAS UI and Data Dictionary

markusmann-vg commented 2 years ago

Feature Description

There are fields that in SORMAS are considered to be mandatory but do not have a according entry in the data dictionary.

Problem Description

This leads to confusion about what is mandatory and what's not - especially when using the ReST interface.

Proposed Change

If a field is considered to be mandatory in the UI, it should be also be mentioned as mandatory in the data dictionary - in other words: for those fields the @NotNull annotation needs to be added to the DTO class.

The following fields need to be required in the dto:

[ ] CaseData.outcome
[ ] CaseData.caseOrigin
[ ] PreviousHospitalization.region
[ ] PreviousHospitalization.district
[ ] PreviousHospitalization.healthFacility
[ ] Prescription.therapy
[ ] Prescription.prescriptionDate
[ ] Prescription.prescriptionType
[ ] Treatment.therapy
[ ] Treatment.treatmentDateTime
[ ] Treatment.treatmentType
[ ] ClinicalVisit.clinicalCourse
[ ] ClinicalVisit.visitDateTime
[ ] Contact.region
[ ] Contact.district
[ ] Sample.lab
[ ] AdditionalTest.sample
[ ] Task.taskStatus
[ ] Event.eventTitle
[ ] Action.actionStatus
[ ] TravelEntry.reportDate
[ ] TravelEntry.disease
[ ] TravelEntry.responsibleRegion
[ ] TravelEntry.responsibleDistrict
[ ] TravelEntry.pointOfEntry
[ ] ExternalMessage.status

The same goes for the following infrastructure entities:

[ ] Continent.defaultName
[ ] Subcontinent.defaultName
[ ] Subcontinent.continent
[ ] Country.defaultName
[ ] Country.isoCode
[ ] Country.subcontinent
[ ] Area.name
[ ] Region.name
[ ] Region.epidCode
[ ] District.name
[ ] District.epidCode
[ ] District.region
[ ] Community.name
[ ] Community.region
[ ] Community.district
[ ] Facility.name
[ ] Facility.region
[ ] Facility.district
[ ] Facility.type
[ ] PointOfEntry.pointOfEntryType
[ ] PointOfEntry.name
[ ] PointOfEntry.region
[ ] PointOfEntry.district
[ ] User.active
[ ] User.userName
[ ] User.firstName
[ ] User.lastName

Related to this we have to provide default values for the following fields, so ReST calls can rely on them. These assignments then no longer need to be part of the related dto build method

[ ] CaseData.outcome: CaseOutcome.NO_OUTCOME
[ ] CaseData.caseOrigin: CaseOrigin.IN_COUNTRY)
[ ] CaseData.caseClassification: CaseClassification.NOT_CLASSIFIED
[ ] CaseData.investigationStatus: InvestigationStatus.PENDING
[ ] CaseData.followUpStatus: FollowUpStatus.NO_FOLLOW_UP
[ ] Contact.contactClassification: ContactClassification.UNCONFIRMED
[ ] Contact.contactStatus: ContactStatus.ACTIVE
[ ] Event.eventStatus: EventStatus.SIGNAL
[ ] Event.eventInvestigationStatus: EventInvestigationStatus.PENDING
[ ] Task.taskStatus: TaskStatus.PENDING
[ ] Task.priority: TaskPriority.NORMAL
[ ] Action.actionStatus: ActionStatus.PENDING
[ ] Action.priority: ActionPriority.NORMAL
[ ] Sample.pathogenTestResult: PathogenTestResultType.PENDING
[ ] ExternalMessage.status: ExternalMessageStatus.UNPROCESSED

Also:

[ ] Make sure that the default value is visible in the swagger documentation.

The following fields are currently marked required in the dto, but actually aren't required, so the annotation should be removed:

[x] CaseData.region
[x] CaseData.district
[x] Contact.lastContactDate
[x] PathogenTest.testDateTime
[x] PathogenTest.testResultText
[x] Event.eventDesc
[x] Vaccination.healthConditions

Follow-up issues

[x] What about dependent fields like Action.event? (only required when context is EVENT) -> not now. Check how this is handled in schema defintion languages, swagger or openEHR
[x] Should all of those fields be required in the database? -> only for not "content" data fields and meta fields, e.g UUID -> document this in the wiki: https://github.com/hzi-braunschweig/SORMAS-Project/wiki/Adding-a-New-Field-to-an-Entity

Possible Alternatives

Additional Information

MartinWahnschaffe commented 2 years ago

@markusmann-vg I have checked this and can't confirm it. The investigation status of the case is marked as a required field int the CaseDataDto since 2018 and the data dictionary correctly reflects this. https://github.com/hzi-braunschweig/SORMAS-Project/blob/development/sormas-api/src/main/resources/doc/SORMAS_Data_Dictionary.xlsx

MartinWahnschaffe commented 2 years ago

Got feedback from Stefan Hoffmann, that one of the fields where this is missing, is the Case.caseOrigin field (required in CaseCreateForm). This would definitely have to be annotated accordingly. I'm note sure how to find out whether other fields are missing as-well. @MateStrysewske any idea?

MateStrysewske commented 2 years ago

@MartinWahnschaffe No, not really.

MartinWahnschaffe commented 2 years ago

@FredrikSchaeferVitagroup Could you please have a look at the LabMessage & TestReport and extend the issue description if those two entities have any required fields?

@MateStrysewske Please have a look at the updateded issues description. There are a few discussion points - I'd say that we should have a meeting to decide how to proceed with them. Do you think someone else should be involved?

MateStrysewske commented 2 years ago

@MartinWahnschaffe I agree that a meeting would make sense. This is also not only relevant for the data dictionary but for users who are using the ReST API. Would potentially make sense to involve someone who's familiar with lab messages and/or test reports.

FredrikSchaefer commented 2 years ago

I just checked the sormas_schema.sql, and there it seems like those columns are required for LabMessage and TestReports:

LabMessage and TestReport
- id
- uuid
- changedate
- creationdate
- sys_period
- deleted (not explicitly set to NOT NULL but with DEFAULT value)
LabMessage
- status
TestReport
- labmessage_id

As the more technical attributes are not mentioned for all other intities, I will add LabMessage.status to the description (TestReport.labMessage is already annotated as Required).

SORMAS-Foundation / SORMAS-Project