Skip to content
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

Dp 781 #110

Merged
merged 7 commits into from
Sep 8, 2022
Merged

Dp 781 #110

merged 7 commits into from
Sep 8, 2022

Conversation

flopez-bao
Copy link
Contributor

  • Since getDataValueSets has been deprecated from datapackr, datimutils::getDataValueSets was altered to allow passing only a dataElementGroup as well as an orgUnitGroup
  • Now it will require missing both data or org elements to kick the stop error

- added exceptions for dataSet requirement to allow for dataElementGroup
- added exceptions for orgUnit requirement to allow for orgUnitGroup
-changed values to scalar operators
@flopez-bao flopez-bao requested a review from sam-bao September 6, 2022 12:18
Copy link
Collaborator

@sam-bao sam-bao left a comment

Choose a reason for hiding this comment

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

In addition to updating the two error messages, please look at the coverage report. It looks like we do not have unit tests which take us into all of the error handling paths.

R/getDataValueSets.R Outdated Show resolved Hide resolved
R/getDataValueSets.R Outdated Show resolved Hide resolved
- updated error message
- updated test coverage to cover both error changes
@flopez-bao
Copy link
Contributor Author

@sam-bao , updated the error messages to include the new condition and updated testing to cover both messages. Let me know if this suffices.

@sam-bao sam-bao merged commit 988a385 into master Sep 8, 2022
@sam-bao sam-bao deleted the DP-781 branch September 8, 2022 11:50
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.

2 participants