tweak(adminPanel): RN-1274: Remove outdated survey responses and associated answers from DHIS via sync queue

[alexd-bes]

Issue RN-1274: Remove outdated survey responses and associated answers from DHIS via sync queue

Changes:

• Updated DHISChangeValidator to create delete entries in dhis_sync_queue for outdated survey responses and their associated answers

[rohan-bes]

A few changes requested, and maybe some testing around 2 areas come to mind:

1. Editing already outdated survey responses / answers
2. Un-outdating a survey response should push it to dhis2

I think the second might just work but worth double checking.

Also, could we get some unit tests for this 🙏

[rohan-bes]

Suggested change

	includeOutdated = false,
	outdated = false,

I felt the wording here was a little misleading, as includeOutdated suggests that we get both outdated and not outdated responses to me

[rohan-bes]

Suggested change

	excludeEventBased = false,
	includeOutdated = false,
	{ excludeEventBased = false, oudated = false } = {},

How about wrapping these parameters into a single options argument? That way callers can do:

this.queryValidSurveyResponseIds(surveyResponseIds, { excludeEventBased: true, outdated: true });
// instead of
this.queryValidSurveyResponseIds(surveyResponseIds, true, true);

Which reads a lot more clearly to me.

[rohan-bes]

Would suggest same change here

Suggested change

	excludeEventBased = false,
	includeOutdated = false,
	{ excludeEventBased = false, outdated = false } = {},

[rohan-bes]

Suggested change

	getOutdatedAnswers = async surveyResponseIds => {
	getDeletesForAssociatedAnswers = async surveyResponseIds => {

[rohan-bes]

Suggested change

	const deleteAnswers = await this.getOutdatedAnswers(surveyResponseUpdateIds);
	const outdatedAnswerDeletes = await this.getOutdatedAnswers(surveyResponseUpdateIds);

I know this one is super nitpicky but it feels in line with oudatedSurveyResponseDeletes

[rohan-bes]

We might want to test to check that this code doesn't lead to us attempting to delete the same answer twice if it the survey response changes. Admittedly we're moving away from changing survey responses in favour of outdating but it does still happen.

Looking at this code, it does seem like we'd attempt to delete it twice, and if so that might be a problem. A fix would be to not push a delete to the dhis sync queue if there's already one there?

[alexd-bes]

Hey @rohan-bes, any chance you can re-review this one? Georgia wants to get it into this next release ideally

[rohan-bes]

Thanks for the changes @alexd-bes 🙏

[rohan-bes]

Oh this was all part of the join previously?? Huh... I think this change is safe and definitely easier to reason about...

[alexd-bes]

Yep it was, my assumption is it should be filtering by dhis service types so I think it makes sense?

[rohan-bes]

So it will still filter actually when you add AND clauses in the join, but I'm not certain if the filtering would resolve in the same way.

For instance:

SELECT * FROM ancestor_descendant_relation adr
JOIN entity e ON adr.entity_id = e.id AND e.type = 'country';

# will resolve to the same set as:
SELECT * FROM ancestor_descendant_relation adr
JOIN entity e ON adr.entity_id = e.id 
WHERE e.type = 'country';

Could I ask you to just double check this one actually 🙏

[alexd-bes]

I just reverted it to the old way, just to make sure there are no regressions