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

feat(rest): listing license clearing info of a project. #2029

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

rudra-superrr
Copy link
Contributor

@rudra-superrr rudra-superrr commented Jul 10, 2023

Issue: #2030

Description: GET request will get all releases of license clearing.

@rudra-superrr rudra-superrr added REST New-UI Level for the API and UI level changes for the new-ui needs code review needs general test This is general testing, meaning that there is no org specific issue to check for labels Jul 10, 2023
@nikkuma7
Copy link

Test successful.
image

@ag4ums ag4ums removed the needs general test This is general testing, meaning that there is no org specific issue to check for label Jul 11, 2023
@ag4ums ag4ums requested a review from KoukiHama July 19, 2023 08:05
})).collect(Collectors.toList());

List<EntityModel<Release>> releaseList = releases.stream().map(sw360Release -> wrapTException(() -> {
final Release embeddedRelease = restControllerHelper.convertToEmbeddedLinkedRelease(sw360Release);
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this snippet to RestControllerHelper (same as addEmbeddedRelease)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return embeddedRelease;
}

public void addEmbeddedReleasess(HalResource halResource, List<EntityModel<Release>> releases) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo addEmbeddedReleasess -> addEmbeddedReleases


final User sw360User = restControllerHelper.getSw360UserFromAuthentication();
Project sw360Project = projectService.getProjectForUserById(id, sw360User);
String transitive = "true";
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, transitive should be a parameter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added as a parameter.

@hoangnt2
Copy link
Contributor

hoangnt2 commented Jul 19, 2023

@rudra-superrr Screenshot from 2023-07-19 16-53-32
There is a nested list in sw360:releases list. Could you remove it?

@rudra-superrr
Copy link
Contributor Author

Hi @hoangnt2 , made the changes as requested.
Kindly verify it once.

@@ -1122,6 +1147,24 @@ public RepositoryLinksResource process(RepositoryLinksResource resource) {
return resource;
}

private HalResource<Project> createHalLicenseClearing(Project sw360Project, User sw360User, List<EntityModel<Release>> releases)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused sw360User, TException is never thrown

}

public void addEmbeddedProjectReleases(HalResource halResource, List<EntityModel<Release>> releases) {
halResource.addEmbeddedResource("sw360:release", releases);
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep sw360:release if you can

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already sw360:release.

Copy link
Contributor

Choose a reason for hiding this comment

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

@rudra-superrr Oh, sorry I mean sw360:releases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I keep sw360:releases, then nested list will come in response structure.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

If I keep sw360:releases, then nested list will come in response structure.

image

@rudra-superrr, I got it, thanks

@rudra-superrr
Copy link
Contributor Author

Review comments resolved.

Signed-off-by: rudra-superrr <rudra.chopra@siemens.com>
@hoangnt2
Copy link
Contributor

hoangnt2 commented Aug 2, 2023

Code look good

@rudra-superrr rudra-superrr added ready ready to merge and removed needs code review labels Aug 2, 2023
@ag4ums
Copy link
Contributor

ag4ums commented Aug 2, 2023

closes #2030

@ag4ums ag4ums merged commit d0a8501 into eclipse-sw360:main Aug 2, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New-UI Level for the API and UI level changes for the new-ui ready ready to merge REST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants