search

OpenFn / openfn-lime-pilot

MSF LIME Project - OpenFn Workflows for Iraq Pilot
1 stars 0 forks source link

WF2 improvements #36

Closed mtuchi closed 1 month ago

mtuchi commented 2 months ago

Description

These are improvements based on Joe's feedback #35

mtuchi commented 2 months ago

I am done with the implementation, @josephjclark please have a look

josephjclark commented 2 months ago

I'm really sorry but I don't have time for a deep review today. This also looks like a lot more changes than are in my feedback? So I'm a little lost.

I'm happy to approve but 3-get-encounters still jumps out at me. You shouldn't need to use .flat() if you process the array correctly. Spreading the map into state.encounters will help.

state.encounters.push(
    ...filteredEncounters.map(encounters => encounters[0]).filter(e => e)
  );

But it also looks like there's way too much filtering and mapping here. Can you simplify?

Also, what's up with encounters[0]? If we find a matching encounter we seem to only return the first one?

Maybe we should have a quick chat about this today

mtuchi commented 2 months ago

I think the biggest diff is coming from workflows/wf2/4-get-options-map.js, which you can ignore I just moved the globals into their own const at the top of the job instead of adding them to state and use them later on in my workflow.

mtuchi commented 2 months ago

The encounter[0] we always want the first encounter in that result array

aleksa-krolls commented 1 month ago

@mtuchi can you pls merge this to staging and resolve conflicts?

josephjclark commented 1 month ago

@mtuchi Ok, so long as you're happy with it please feel free to merge!