-
Notifications
You must be signed in to change notification settings - Fork 65
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
RAD-377: Search by report template license #498
Conversation
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.
Looks good. Just handle the one comment I left
@Autowired | ||
private MrrtReportTemplateService mrrtReportTemplateService; | ||
|
||
SearchQuery searchQuery = new SearchQuery.Builder("Allows you to search for MrrtReportTemplate's by title") | ||
.withOptionalParameters(new String[] { REQUEST_PARAM_TITLE, REQUEST_PARAM_PUBLISHER, REQUEST_PARAM_TOTAL_COUNT }) | ||
.withOptionalParameters(new String[] { REQUEST_PARAM_TITLE, REQUEST_PARAM_PUBLISHER, REQUEST_PARAM_TOTAL_COUNT, | ||
REQUEST_PARAM_LICENSE }) | ||
.build(); |
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.
can you please put REQUEST_PARAM_TOTAL_COUNT at the end of the list?
assertThat(templates.size(), is(1)); | ||
assertThat(templates.get(0) | ||
.getDcTermsLicense(), | ||
is("General Public License")); |
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 dont you use your constant EXISTING_TEMPLATE_LICENSE
?
|
||
assertNotNull(licenses); | ||
assertThat(licenses.size(), is(2)); | ||
assertTrue(licenses.contains("Mozilla Public License")); |
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.
use your constants instead of the string literals and you should be able to use matchers that test that these two string constants are in the list without asserTrue.
@achabill are you still working on this? |
RAD-377: Request changes RAD-377: Request changes PR openmrs#498 - using string constant - List matchers without assertTrue
RAD-377: Search by report template license
Description
Provided a Java API + REST API for search of Mrrt report templates via the MrrtReportTemplate.dctermsLicense.
Related Issue
see https://issues.openmrs.org/browse/RAD-377
Checklist:
git pull --rebase upstream master
.mvn clean package
right before creating this pull request andadded all formatting changes to my commit.