Skip to content

fix errors in export event program and tracker program #346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 17, 2025

Conversation

gqcorneby
Copy link

@gqcorneby gqcorneby commented Feb 3, 2025

📌 References

  • Issue: Closes #8697re161

📝 Implementation

  • updated response handling for /tracker/events and /tracker/trackedEntities to use a fallback (instances || (events/trackedEntities) || []
  • updated /tracker/events payload to not include programStage when it is null
  • use ouMode for calls to /tracker/trackedEntities

🔥 Notes for the reviewer

  • v38-40: compatible with ouMode only
  • v41: compatible with ouMode and orgUnitMode. Can't send both on v41 so I used ouMode for all
  • v41: error when programStage is empty

📹 Screenshots/Screen capture

image
image
image

📑 Others

@ifoche
Copy link
Member

ifoche commented Feb 3, 2025

@gqcorneby gqcorneby changed the title add v41 fallback for tracker/events and tracker/trackedEntities fix errors in export event program and tracker program Feb 4, 2025
Copy link

@MiquelAdell MiquelAdell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested against v38 and v41 and working (it downloads tracker programs with data)

@tokland tokland removed their request for review February 6, 2025 11:53
@tokland
Copy link

tokland commented Feb 10, 2025

@gqcorneby Sorry for the confusion, I removed the re-review because I saw no changes pushed since my previous review ("orgUnits is always truthy") and I thought you were in the processing of checking it. Let me know when that snippet is ready to be checked, all the other code looked good to me.

@gqcorneby
Copy link
Author

@tokland Thank you for the clarification! For some reason, I did not receive a notification for the comment (also I can't see it). But I will check this.

//can't use both orgUnitMode and ouMode in v41
return {
ouMode,
...(isOuReq && orgUnits && { orgUnit: buildOrgUnitsParameter(orgUnits) }),
Copy link

@tokland tokland Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

being an array (Ref[]) , orgUnits is always truthy, so the 2nd condition does nothing. Review if we need to check the length or what's the condition.

@tokland
Copy link

tokland commented Feb 10, 2025

I re-posted the review, I think that was it, sorry!

@gqcorneby gqcorneby requested a review from tokland February 10, 2025 12:06
Copy link

@tokland tokland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, we will only change the error message (it should not get to there anyway), but otherwise, all code looks good to me, approving.

@MiquelAdell MiquelAdell merged commit 26c81fe into development Feb 17, 2025
2 checks passed
@MiquelAdell MiquelAdell deleted the fix/v41-compatibility branch February 17, 2025 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants