-
Notifications
You must be signed in to change notification settings - Fork 104
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
feat(obligation): list license obligation table data #2302
feat(obligation): list license obligation table data #2302
Conversation
d4fee59
to
a710935
Compare
3ba0737
to
1e2d97d
Compare
@RequestMapping(value = PROJECTS_URL + "/{id}/licenseObligations", method = RequestMethod.GET) | ||
public ResponseEntity<HalResource> getLicenseObligations(Pageable pageable, | ||
@Parameter(description = "Project ID.") @PathVariable("id") String id, HttpServletRequest request) | ||
throws TException, URISyntaxException, PaginationParameterException, ResourceClassNotFoundException { |
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.
URISyntaxException, PaginationParameterException, ResourceClassNotFoundException are never thrown
) | ||
@RequestMapping(value = PROJECTS_URL + "/{id}/licenseObligations", method = RequestMethod.GET) | ||
public ResponseEntity<HalResource> getLicenseObligations(Pageable pageable, | ||
@Parameter(description = "Project ID.") @PathVariable("id") String id, HttpServletRequest request) |
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.
unused param HttpServletRequest request
licenseInfoWithObligations, releaseIdToAcceptedCLI); | ||
Map<String, String> releaseIdToAcceptedCli = new HashMap<String, String>(); | ||
obligationStatusMap = licenseObligation.getObligationStatusMap(); | ||
Map<String, ObligationStatusInfo> newObligationMap = new HashMap<>(); |
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.
Unused variable
public Map<String, ObligationStatusInfo> setLicenseInfoWithObligations(Map<String, ObligationStatusInfo> obligationStatusMap, Map<String, String> releaseIdToAcceptedCLI, | ||
List<Release> releases, User user) { | ||
|
||
final Set<Release> excludedReleases = Sets.newHashSet(); |
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.
Contents of collection excludedReleases are updated at lines 186 and 195, but never queried. Can we remove it ?
@Test | ||
public void should_document_get_license_obligations() throws Exception { | ||
String accessToken = TestHelper.getAccessToken(mockMvc, testUserId, testUserPassword); | ||
mockMvc.perform(get("/api/projects/" + "123456733" + "/licenseObligations") |
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.
Id 123456733 should be replaced by project8.getId()
mockMvc.perform(get("/api/projects/" + "123456733" + "/licenseObligations") | |
mockMvc.perform(get("/api/projects/" + project8.getId() + "/licenseObligations") |
7cd9cd1
to
8811f76
Compare
Review comments addressed. |
Code look good to me |
Just a tip, you can add Depends-On in github pull request header: |
8811f76
to
c8050fa
Compare
Signed-off-by: Rudra Chopra <prabhuchopra@gmail.com>
c8050fa
to
4f65386
Compare
Issue: Closes #2301
Description:
This new endpoint will list license obligation data for the given project.