[OCRVS-6124] Fix locations seeding when there are many locations #6399
Conversation
| } | ||
| }) | ||
| const response: fhir3.Bundle<fhir3.BundleEntryResponse> = await res.json() |
There was a problem hiding this comment.
@tahmidrahman-dsi The issue might be that we need to resolve the partOf values of the district locations with that of the actual values we get from hearth after creating the states.
There was a problem hiding this comment.
and these level by level segmentation might need to be moved to the actual handler in gateway due to this reason
| } | ||
| return fetchFromHearth('', 'POST', JSON.stringify(locationsBundle)) | ||
| return h.response().code(201) |
There was a problem hiding this comment.
Would it be possible to do this in createLocationHandler so Hapi related code wouldn't leak to this function?
There was a problem hiding this comment.
Thanks, @rikukissa, I am on it
| function createLocationSegments(locations: Location[]): Location[][] { | ||
| const segments = [] | ||
| for (const jurisdictionType of Object.keys(JurisdictionType)) { | ||
| const jurisdictionLocations = locations.filter( | ||
| (loc) => loc.jurisdictionType === jurisdictionType | ||
| ) | ||
| if (jurisdictionLocations.length) { | ||
| segments.push(jurisdictionLocations) | ||
| } | ||
| } | ||
| const facilitiesOrOffices = locations.filter((loc) => !loc.jurisdictionType) | ||
| if (facilitiesOrOffices.length) { | ||
| segments.push(facilitiesOrOffices) | ||
| } | ||
| return segments | ||
| } |
There was a problem hiding this comment.
I can't remember exactly but my recollection is the majority of locations in Cameroon are offices. Would this approach be sufficient if a country has lets say 90% offices? Would a numeric limit segmentation be more reliable?
There was a problem hiding this comment.
Agreed, I am also seeing not much significant difference in the case of Madagascar, I am going to add a numeric limit to the batch items
| locations: Location[], | ||
| h: Hapi.ResponseToolkit | ||
| ) { | ||
| let parentLocationsMap: Map<string, string> = new Map() |
There was a problem hiding this comment.
Could you clarify this parentLocationMap a bit. I don't understand what it does 😞
There was a problem hiding this comment.
parentLocationsMap has statistical id to fhirId mapping of parent locations, I agree, I should give it more clearer name. I am thinking to change it statisticalToFhirIDMapOfParentLocations
| function createLocationSegments(locations: Location[]): Location[][] { | ||
| const segments = [] | ||
| for (const jurisdictionType of Object.keys(JurisdictionType)) { | ||
| const jurisdictionLocations = locations.filter( | ||
| (loc) => loc.jurisdictionType === jurisdictionType | ||
| ) | ||
| if (jurisdictionLocations.length) { | ||
| segments.push(...createChunks(jurisdictionLocations, LOCATION_CHUNK_SIZE)) | ||
| } | ||
| } | ||
| const facilitiesOrOffices = locations.filter((loc) => !loc.jurisdictionType) | ||
| if (facilitiesOrOffices.length) { | ||
| segments.push(...createChunks(facilitiesOrOffices, LOCATION_CHUNK_SIZE)) | ||
| } | ||
| return segments | ||
| } |
There was a problem hiding this comment.
This logic is probably not needed anymore now that you implemented the length based approach, right?
There was a problem hiding this comment.
agreed, just putting the admin locations first would be sufficient
There was a problem hiding this comment.
Sorry, @rikukissa I think it should stay the same, as I am updating the statisticalToFhirIDMap after processing each segment. If we remove this jurisdiction-type filtering logic, there will be a chance of staying parent and child locations in the same segment. Then the fhirID of the parent location cannot be resolved as the map will not already be updated
There was a problem hiding this comment.
Is it possible to first, right in the beginning the whole process, create the full parent map from all locations? @tahmidrahman-dsi
There was a problem hiding this comment.
I cannot think of any way of doing it 🤔 ... because I cannot get the fhirIDs before sending them to hearth
There was a problem hiding this comment.
Annoying! Yea, you are right. It's not as easy as I thought. In that case this looks good to me 👍
…,location type" This reverts commit b5a493f.
tahmidrahman-dsi
force-pushed
the
ocrvs-6124
branch
from
45b9564
to
5ecada7
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## release-v1.3.3 #6399 +/- ##
==================================================
- Coverage 86.33% 86.31% -0.02%
==================================================
Files 728 728
Lines 126747 126613 -134
Branches 12045 11967 -78
==================================================
- Hits 109424 109292 -132
+ Misses 17318 17316 -2
Partials 5 5 ☔ View full report in Codecov by Sentry. |
Zangetsu101
added
💥 Hot Fixes
No description provided.