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
[RA-1257]- New Patient Registrations report completed #43
Conversation
|
||
<require_module version="${metadatadeployVersion}"> | ||
org.openmrs.module.metadatadeploy | ||
</require_module> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this module required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, without this particular, there is a runtime error when I deploy the referenceApplication module. I had a chat about this with Daniel on IRC. However I will try to run the module by removing this dependency. If it doesn't, I'll keep this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Rafal, we need this entry in order to load the metadatadeploy module at the time of referenceApplication module is initializing. It is explained in this wiki page. https://wiki.openmrs.org/display/docs/Requiring+another+module+in+your+module
context.addParameterValue("endDate", DateUtil.getDateTime(2017,6,16)); | ||
context.setBaseCohort(new Cohort("50, 51")); | ||
return context; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we check in this test for an actual result, e.g. that there were 5 patient registered in that period...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's a good point. I checked the unit tests in rwanda reports they haven't done it. Whether the report is actually displaying the correct records or not wasn't attested. I'll check on it more.
Nice progress! It seems the changes should actually go into https://github.com/openmrs/openmrs-module-referencemetadata module instead of https://github.com/openmrs/openmrs-module-referenceapplication. Could you please ask on talk, which is preferred going forward? |
Is this PR created instead of #41 ? Please do not create new PRs rather push to the same branch and your PR will be automatically updated. |
Yes Rafal. As Darius mentioned in our talk thread for GSoC project; I'll make a comment on the thread about this. Thank you for the review. BTW; sorry that I wasn't aware about the automatic PR modifications. |
Closing the PR as these changes decided to be moved to ReferenceMetaData module. |
Built-in reports for reference application GSoC project. This PR includes the work has been up to 3rd week.
Following work has been completed so far:
Implement suggested reports through SQL definitions. (Week RA-18: Determine compass/sass configuration for letting one module extend the css of another one #1)
Implement New Patient Registrations report through cohort definitions. (Week Updated login page controller to set the session location if none exists... #3)
Unit Test coverage for New Patient Registrations report. (Week Updated login page controller to set the session location if none exists... #3)
mvn clean install
reports were available once the module has deployed.