Event tracker repository implementation #6

9sneha-n · 2024-07-10T09:27:36Z

📌 References

Issue: Closes #? https://app.clickup.com/t/8694tp2rg, #8694tp2rg

📝 Implementation

Implement DiseaseOutbreakEvent Repo
Implement partially OrgUnit Repo
Implement partially Options Repo
Implement partially Team Member Repo

📹 Screenshots/Screen capture

🔥 Notes to the tester

src/data/repositories/OptionsD2Repository.ts

anagperal · 2024-08-01T11:39:58Z

src/data/repositories/TeamMemberD2Repository.ts

+            this.api.metadata.get({
+                users: {
+                    fields: { id: true, name: true, email: true, phoneNumber: true },
+                    filter: { username: { eq: id } },


Maybe for consistency add username: true to fields and to TeamMember because incidentManagerName is the username instead of the id. Also it's better to change to incidentManagerUsername ?

Added username: true , however do not understand the second part of the comment. Incident Manager is a type of role in the Team Member. Other Team Members could have other roles, so having the name incidentManagerUsername would not make sense, right?

But I was thinking that incidentManagerName in DiseaseOutbreakEventBaseAttrs was the incident manager username selected in the form so I was populating this property with the username (tha's also why I need username in TeamMember entity, and also needed to populate the options in te incident manager selector)

By the way, I tried to do it in the other PR but I had an error saying: Property 'username' does not exist on type 'D2User'

ahh ok, i get it now. You are right, i get the same error. Lets discuss in the next tech decisions meeting

src/data/repositories/utils/DiseaseOutbreakMapper.ts

anagperal

thanks @9sneha-n!! The idea of the mappers is very good! I have added some comments in the code

delemaf

So for it looks good tome, mapping is a very nice idead 👏

anagperal

thanks @9sneha-n!! All good!! we'll wait to comment the username error in the tech meeting and just one more thing related to how is modeled the na fields in DHIS2 and how are in the app: https://github.com/EyeSeeTea/zebra-dev/pull/6/files#r1705539592

tokland

[code-only review] I've added some comments, but 99% very minor things, code looks good.

src/CompositionRoot.ts

tokland · 2024-08-08T10:40:01Z

src/data/repositories/DiseaseOutbreakEventD2Repository.ts

+                fields: { attributes: true, trackedEntity: true },
+            })
+        ).map(trackedEntity => {
+            if (!trackedEntity.instances[0]) throw new Error("Tracked entity not found");


As the method is returning a Future, the natural way to signal an error is returning a Future.error (as you know, it's a typical pattern using flatMap)

(I'd apply the same idea for all the code base).

You can think of some abstraction to make it as DRY as possible. Proposal (just an idea):

.flatMap(res => assertOrError(res.instances[0])) .map(mapTrackedEntityAttributesToDiseaseOutbreak);

tokland · 2024-08-08T10:43:41Z

src/data/repositories/DiseaseOutbreakEventD2Repository.ts

+            });
+        });
+    }
+    save(diseaseOutbreak: DiseaseOutbreakEventBaseAttrs): FutureData<void> {


[nit-picking] blank lines between methods. A pity there is no way for prettier to do that automatically!

tokland · 2024-08-08T10:46:03Z

src/data/repositories/OptionsD2Repository.ts

+    constructor(private api: D2Api) {}
+
+    get(code: Code): FutureData<NamedRef> {
+        if (!code) return Future.success({ id: "", name: "" });


I am very weary of data repositories returning empty objects like this. It's going to be used as en empty option for dropdowns? that should be handled at the presentation level.

noted for future

tokland · 2024-08-08T10:46:49Z

src/data/repositories/OptionsD2Repository.ts

+            const option: NamedRef = {
+                id: response.options[0].code,
+                name: response.options[0].name,
+            };


options[0] referenced 3 times -> local variable

tokland · 2024-08-08T11:28:37Z

src/data/repositories/utils/DiseaseOutbreakMapper.ts

+        case "RTSL_ZEB_TEA_AREAS_AFFECTED_PROVINCES":
+            return diseaseOutbreak.areasAffectedProvinceIds[0] ?? ""; //TO DO : Handle multiple provinces once metadata change is done
+        case "RTSL_ZEB_TEA_AREAS_AFFECTED_DISTRICTS":
+            return diseaseOutbreak.areasAffectedDistrictIds[0] ?? ""; //TO DO : Handle multiple provinces once metadata change is done


abstract processing patterns

tokland · 2024-08-08T11:33:04Z

src/data/repositories/utils/MetadataHelper.ts

@@ -0,0 +1,21 @@
+import { D2Api } from "@eyeseetea/d2-api/2.36";
+
+export function getProgramTEAsMetadata(api: D2Api, programId: string) {


Minor, but typically you'd transform to future here, so the caller has less work to do.

tokland · 2024-08-08T11:33:46Z

src/data/repositories/utils/MetadataHelper.ts

@@ -0,0 +1,21 @@
+import { D2Api } from "@eyeseetea/d2-api/2.36";


We should avoid these imports. Instead, import from a wrapper module (src/types/d2-api.ts)

tokland · 2024-08-08T11:53:39Z

src/data/repositories/utils/DiseaseOutbreakMapper.ts

+        created: trackedEntity.createdAt ? new Date(trackedEntity.createdAt) : new Date(),
+        lastUpdated: trackedEntity.updatedAt ? new Date(trackedEntity.updatedAt) : new Date(),
+        createdByName: undefined,
+        hazardType: getValueFromMap("hazardType", trackedEntity) as HazardType,


isValueInUnionType also here.

Note that this kind of valud/type-validation are ok for simple structures like this. When we have more complex structures, as you know, we can always use purify-ts->Codec.

tokland · 2024-08-08T12:04:12Z

src/data/repositories/utils/DiseaseOutbreakMapper.ts

+function getValueFromDiseaseOutbreak(
+    key: (typeof DiseaseOutbreakCodes)[keyof typeof DiseaseOutbreakCodes],
+    diseaseOutbreak: DiseaseOutbreakEventBaseAttrs
+): string {


I can see only a minor drawback of the mapping approach (it creates an indirection), but it definitely left me thinking what would be the non-mapping approach :) I think it would be something like this:

type Code = GetValue<typeof DiseaseOutbreakCodes> const attributeValues: Record<Code, string> = { RTSL_ZEB_TEA_EVENT_id: diseaseOutbreak.eventId.toString(), RTSL_ZEB_TEA_EVENT_NAME: diseaseOutbreak.name, // ... }; const attributes = getAttributesFromAttributeValues(attributesMetadata, attributeValues)

(btw, this Code should not be here, instead move the definition to DiseaseOutbreakCodes. Modules must help their callers)

With both styles at hand, pick the one that looks more declarative to you.

9sneha-n · 2024-08-09T09:51:11Z

thanks @9sneha-n!! All good!! we'll wait to comment the username error in the tech meeting and just one more thing related to how is modeled the na fields in DHIS2 and how are in the app: https://github.com/EyeSeeTea/zebra-dev/pull/6/files#r1705539592

Thanks @anagperal Good point, its fixed now

9sneha-n · 2024-08-09T09:52:17Z

[code-only review] I've added some comments, but 99% very minor things, code looks good.

Thanks @tokland , addressed all the comments, except the ones requiring Jorge's input. Will ask him over slack or in technical decision meeting

tokland

Nice! Very close. Sorry for the nit-picking! :)

src/data/repositories/utils/AssertOrError.ts

src/webapp/pages/dashboard/DashboardPage.tsx

src/data/repositories/DiseaseOutbreakEventD2Repository.ts

src/data/repositories/consts/DiseaseOutbreakConstants.ts

anagperal · 2024-08-12T13:08:23Z

src/data/repositories/utils/DiseaseOutbreakMapper.ts

+        created: trackedEntity.createdAt ? new Date(trackedEntity.createdAt) : new Date(),
+        lastUpdated: trackedEntity.updatedAt ? new Date(trackedEntity.updatedAt) : new Date(),
+        createdByName: undefined,
+        hazardType: getHazardTypeValue(getValueFromMap("hazardType", trackedEntity)),


This does not seem to work because the hazard types in the trackedEntity are like BIOLOGICAL_ANIMAL, BIOLOGICAL_HUMAN ... (check https://metadata.eyeseetea.com/rsl-zebra/dhis-web-maintenance/index.html#/edit/otherSection/optionSet/p2N1GkG713S/options) so first we need to map from these codes to "Biological:Animal", "Biological:Human", "Chemical", "Environmental" or "Unknown". Also there is one new: Biological: Human and Animal

anagperal · 2024-08-12T13:09:31Z

src/data/repositories/consts/DiseaseOutbreakConstants.ts

+            diseaseOutbreak.earlyResponseActions.initiateInvestigation.toISOString(),
+        RTSL_ZEB_TEA_CONDUCT_EPIDEMIOLOGICAL_ANALYSIS:
+            diseaseOutbreak.earlyResponseActions.conductEpidemiologicalAnalysis.toISOString(),
+        RTSL_ZEB_TEA_LABORATORY_CONFIRMATION: diseaseOutbreak.earlyResponseActions


For NA values is the other way around:

RTSL_ZEB_TEA_LABORATORY_CONFIRMATION: diseaseOutbreak.earlyResponseActions .laboratoryConfirmation.na ? "" : "true"

Hi @anagperal In DHIS all of these are mandatory tracked entity attribute of Yes only type. They cannot be null or empty. The app implementation needs to take care of the true value, we can discuss further over call.

anagperal

thansk @9sneha-n ! I had approved the PR but in using it I noticed a couple of small bugs 🙏

tokland

All good as far as I can see, code-wise!

tokland · 2024-08-14T06:52:17Z

src/data/repositories/utils/DiseaseOutbreakMapper.ts

@@ -174,8 +165,8 @@ export function mapDiseaseOutbreakEventToTrackedEntityAttributes(
 }

 function getValueFromMap(
-    key: keyof typeof DiseaseOutbreakCodes,
+    key: keyof typeof diseaseOutbreakCodes,


[no need change] we may create type aliases to improve readability/re-usability

tokland · 2024-08-14T06:59:36Z

src/data/repositories/consts/DiseaseOutbreakConstants.ts

-    ].includes(hazardType);
+export function getHazardTypeValue(hazardType: string): HazardType {
+    const hazardTypeString = Object.keys(hazardTypeCodeMap).find(
+        key => hazardTypeCodeMap[key as HazardType] === hazardType


[no need to change] that's me being a TS zealot :) you are completely certain that hazardTypeCodeMap has the keys you want, it's a literal, but TS has structural typing, so that object could potentially have more props and key as HazardType (which is a "narrow down") would be type unsafe (and even then the function will not fail). Another way of implementing this is by inverting hazardTypeCodeMap so we have a Record<string, HazardType>.

anagperal

Thanks @9sneha-n !!

9sneha-n added 9 commits July 10, 2024 14:50

feat: get disease outbreak event by id repo impl

b3b6144

fix: fix warnings

2e0cd3c

fix: commenting failing test

3c17ed4

feat: populate options, org units, incident manager

ccf94da

refactor: some code improvements

9da61de

feat: Get all disease outbreak base repo impl

86ff4c6

feat: save disease outbreak event

c150e73

fix: modularize code in repo

d61e4dc

fix: use d2-api latest version

a16e103

9sneha-n changed the title ~~Feature/event tracker repo~~ Event tracker repository implementation Jul 31, 2024

9sneha-n marked this pull request as ready for review July 31, 2024 05:42

9sneha-n requested review from delemaf and anagperal July 31, 2024 05:42

anagperal reviewed Aug 1, 2024

View reviewed changes

src/data/repositories/OptionsD2Repository.ts Outdated Show resolved Hide resolved

anagperal reviewed Aug 1, 2024

View reviewed changes

src/data/repositories/utils/DiseaseOutbreakMapper.ts Outdated Show resolved Hide resolved

anagperal reviewed Aug 1, 2024

View reviewed changes

src/data/repositories/utils/DiseaseOutbreakMapper.ts Outdated Show resolved Hide resolved

anagperal reviewed Aug 1, 2024

View reviewed changes

src/data/repositories/utils/DiseaseOutbreakMapper.ts Outdated Show resolved Hide resolved

anagperal requested changes Aug 1, 2024

View reviewed changes

delemaf approved these changes Aug 2, 2024

View reviewed changes

9sneha-n added 2 commits August 6, 2024 16:26

reafctor: review comments

8e00ad0

reafactor: fix warning

9fe938d

9sneha-n requested review from anagperal and tokland August 6, 2024 10:58

anagperal requested changes Aug 6, 2024

View reviewed changes

tokland reviewed Aug 8, 2024

View reviewed changes

9sneha-n added 2 commits August 9, 2024 14:28

fix: refactor, abstract patterns, avoid as usage, coding stds

be5ede7

fix: use declarative map

0033e88

chore: merge with development

a8d7520

9sneha-n requested review from tokland and anagperal August 9, 2024 10:05

tokland reviewed Aug 9, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

anagperal reviewed Aug 12, 2024

View reviewed changes

anagperal requested changes Aug 12, 2024

View reviewed changes

fix: some more review comments

474f1b1

9sneha-n requested review from anagperal and tokland August 13, 2024 08:30

tokland approved these changes Aug 14, 2024

View reviewed changes

anagperal approved these changes Aug 14, 2024

View reviewed changes

bhavananarayanan merged commit 1e727e4 into development Aug 14, 2024
1 check passed

bhavananarayanan deleted the feature/event-tracker-repo branch August 14, 2024 07:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Event tracker repository implementation #6

Event tracker repository implementation #6

9sneha-n commented Jul 10, 2024 •

edited

Loading

anagperal Aug 1, 2024

9sneha-n Aug 6, 2024

anagperal Aug 6, 2024

anagperal Aug 6, 2024

9sneha-n Aug 6, 2024 •

edited

Loading

anagperal left a comment

delemaf left a comment

anagperal left a comment

tokland left a comment •

edited

Loading

tokland Aug 8, 2024 •

edited

Loading

tokland Aug 8, 2024

tokland Aug 8, 2024

9sneha-n Aug 9, 2024

tokland Aug 8, 2024

tokland Aug 8, 2024

tokland Aug 8, 2024

tokland Aug 8, 2024

tokland Aug 8, 2024 •

edited

Loading

tokland Aug 8, 2024 •

edited

Loading

9sneha-n commented Aug 9, 2024

9sneha-n commented Aug 9, 2024

tokland left a comment

This comment was marked as outdated.

anagperal Aug 12, 2024

anagperal Aug 12, 2024 •

edited

Loading

9sneha-n Aug 13, 2024

anagperal left a comment

tokland left a comment •

edited

Loading

tokland Aug 14, 2024 •

edited

Loading

tokland Aug 14, 2024

anagperal left a comment

		@@ -0,0 +1,21 @@
		import { D2Api } from "@eyeseetea/d2-api/2.36";

		export function getProgramTEAsMetadata(api: D2Api, programId: string) {

Event tracker repository implementation #6

Event tracker repository implementation #6

Conversation

9sneha-n commented Jul 10, 2024 • edited Loading

📌 References

📝 Implementation

📹 Screenshots/Screen capture

🔥 Notes to the tester

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9sneha-n Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

anagperal left a comment

Choose a reason for hiding this comment

delemaf left a comment

Choose a reason for hiding this comment

anagperal left a comment

Choose a reason for hiding this comment

tokland left a comment • edited Loading

Choose a reason for hiding this comment

tokland Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tokland Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

tokland Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

9sneha-n commented Aug 9, 2024

9sneha-n commented Aug 9, 2024

tokland left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

Choose a reason for hiding this comment

anagperal Aug 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anagperal left a comment

Choose a reason for hiding this comment

tokland left a comment • edited Loading

Choose a reason for hiding this comment

tokland Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anagperal left a comment

Choose a reason for hiding this comment

9sneha-n commented Jul 10, 2024 •

edited

Loading

9sneha-n Aug 6, 2024 •

edited

Loading

tokland left a comment •

edited

Loading

tokland Aug 8, 2024 •

edited

Loading

tokland Aug 8, 2024 •

edited

Loading

tokland Aug 8, 2024 •

edited

Loading

anagperal Aug 12, 2024 •

edited

Loading

tokland left a comment •

edited

Loading

tokland Aug 14, 2024 •

edited

Loading