fix: use tracker importer in EnrollmentSmsSubmission DHIS2-17729

[teleivo]

As a reference this is the EnrollmentSmsSubmission processing logic before we made our changes to sms processing. You can see that users were able to create and update a tracked entity, enroll it and create and update events.

This is how the EnrollmentSmsSubmission sms is encoded. If you navigate further down you can find out which fields are mandatory. Mandatory are (meaning you'll get an NPE if you don't provide them): ids and enrollmentStatus.

• Use the tracker importer in EnrollmentSMSListener.
• Create SmsImportMapper with pure functions that translate a smscompression/models to a TrackerObjects that can be passed to the tracker importer.
• Replace mock based test with SmsImportMapper unit tests and a couple integration tests.
• There seems to be an issue with our equals implementation of the TrackedEntityAttribute see bug below. I was not able to retrieve the attributes even though my test user had read access to the them. They were filtered out in DefaultTrackedEntityService due to them not being equal.

Mapping of attributes

An EnrollmentSmsSubmission can contain tracked entity attributes. The previous listener deleted any attribute value that existed on a TE/EN if not present in the SMS. We kept this logic.

Tracked entity attributes can be

1. tracked entity type attributes only (TETA)
2. program attributes only (PA)
3. tracked entity type and program attributes

Users can send TEA 3. on the tracked entity and/or the enrollment. This is what the Capture app does as the UI guarantees that a program will always have its tracked entity types' attributes. This is not guaranteed when directly working with the API. The tracker importer validates that attributes are correctly placed. Since 1. and 2. are valid and could be sent to us via SMS we have to handle them well.

This is how each of the above types of attribute values are mapped onto tracker importer objects:

The assumption is that if an attribute is in the programs attributes that it can be put onto the enrollment. It might very well also be a tracked entity type attribute but that does not matter. The tracker importer will reject the import if an attribute is neither a TETA or PA.

The program with its attributes is fetched from the DB. An attribute in this set is either of type 2. or 3. The tracked entity is fetched from the DB with its attributes which includes all types mentioned above!

Next

These are the tracker related compression based SMS we need to adapt so they use the importer:

SubmissionType => Listener class processing these types of SMS

• DELETE => DeleteEventListener ✅ (fix: processing sms and test tracker event deletion DHIS2-17729 #18473)
• ENROLLMENT => EnrollmentSMSListener ✅ (this PR)
• RELATIONSHIP => RelationshipSMSListener ✅ (fix: use tracker importer in RelationshipSMSListener DHIS2-17729 #18509)
• SIMPLE_EVENT => SimpleEventSMSListener
• TRACKER_EVENT => TrackerEventSMSListener ✅ (fix: use tracker importer in TrackerEventSMSListener DHIS2-17729 #18522)

The above can be used used by Android when users are offline.

Then there are command based SMS listeners we need to adapt so they use the importer:

• TRACKED_ENTITY_REGISTRATION_PARSER => TrackedEntityRegistrationSMSListener
• PROGRAM_STAGE_DATAENTRY_PARSER => ProgramStageDataEntrySMSListener
• EVENT_REGISTRATION_PARSER => SingleEventListener

Bug?

  @Test
  void equalsImplementationIsOdd( ) {
    // JUnit assert equals shows no diff, Intellij says they are identical but not considered equal
    // we should dig into its equals impl
    TrackedEntityAttribute tea1 = createTrackedEntityAttribute('1', ValueType.TEXT);
    TrackedEntityAttribute tea2 = createTrackedEntityAttribute('1', ValueType.TEXT);
    assertEquals(tea1, tea2);
  }

[enricocolasante]

🎉

[enricocolasante]

I can see there are different map and some mapTo. Is it on purpose or we should just stick with one naming standard?

[teleivo]

The return type is not part of the method signature. That is why there are cases where I had to use a different name. I did try to use map for the package private methods that turn an sms submission into tracker objects. Other private helpers then got other names.

[enricocolasante]

Was the forbidden exception just not thrown?

[teleivo]

yes, that was a linting error that I fixed.

[muilpp]

LGTM, the only thing I'm concerned about is regarding this statement The previous listener deleted any attribute value that existed on a TE/EN if not present in the SMS. We kept this logic.

[muilpp]

LGTM! The only concern I have is regarding this statement: "The previous listener deleted any attribute value that existed on a TE/EN if not present in the SMS. We kept this logic."

Where are we deleting the attribute values from?
Does enrolling someone through the UI follow the same behavior?

[sonarcloud]

Quality Gate failed

Failed conditions
2 New issues
2 New Code Smells (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint