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.

Sign up for GitHub

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

Jump to bottom

feat(rest): Create new endpoint to download component template, attachment template, attachment information and release link sample in csv format. #2278

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(rest): Create new endpoint to download component template, attachment template, attachment information and release link sample in csv format. #2278

wants to merge 1 commit into from

Conversation

nikkuma7
Copy link

@nikkuma7 nikkuma7 commented Jan 19, 2024 •
edited
Loading

Please provide a summary of your changes here.

  • Which issue is this pull request belonging to and how is it solving it? (Refer to issue here)
  • Did you add or update any new dependencies that are required for your change?

Issue: #2277

Suggest Reviewer

You can suggest reviewers here with an @mention.

How To Test?

http://localhost:8080/resource/api/importExport/downloadComponentTemplate
http://localhost:8080/resource/api/importExport/downloadAttachmentSample
http://localhost:8080/resource/api/importExport/downloadAttachmentInfo
http://localhost:8080/resource/api/importExport/downloadReleaseSample

Note : user should have admin access.

How should these changes be tested by the reviewer?
Have you implemented any additional tests?

Checklist

Must:

  • All related issues are referenced in commit messages and in PR

@nikkuma7 nikkuma7 force-pushed the feat/ImportExport-downloadComponentTemplate branch 2 times, most recently from 043523c to b363166 Compare January 23, 2024 04:14
@nikkuma7 nikkuma7 changed the title feat(rest): Create new endpoint to download component template in csv format. feat(rest): Create new endpoint to download component template, attachment template and attachment information in csv format. Jan 23, 2024
@nikkuma7 nikkuma7 force-pushed the feat/ImportExport-downloadComponentTemplate branch from b363166 to 2bbf922 Compare January 23, 2024 07:20
@nikkuma7 nikkuma7 changed the title feat(rest): Create new endpoint to download component template, attachment template and attachment information in csv format. feat(rest): Create new endpoint to download component template, attachment template, attachment information and release link sample in csv format. Jan 23, 2024
@nikkuma7 nikkuma7 force-pushed the feat/ImportExport-downloadComponentTemplate branch from 2bbf922 to 2ef2fe8 Compare January 25, 2024 07:40
@ag4ums ag4ums added needs code review needs general test This is general testing, meaning that there is no org specific issue to check for labels Jan 30, 2024
@nikkuma7 nikkuma7 force-pushed the feat/ImportExport-downloadComponentTemplate branch from 2ef2fe8 to 8fa91d9 Compare March 4, 2024 05:42
@heliocastro heliocastro added the New-UI Level for the API and UI level changes for the new-ui label Mar 27, 2024
@rudra-superrr
Copy link
Contributor

Docs are broken.

image

@rudra-superrr
Copy link
Contributor

Endpoints working fine
image

@rudra-superrr
Copy link
Contributor

Endpoint for download component csv and download license archive are missing under export section.
image

@nikkuma7 nikkuma7 force-pushed the feat/ImportExport-downloadComponentTemplate branch from 8fa91d9 to 5ba7349 Compare May 16, 2024 08:09
@nikkuma7
Copy link
Author

Docs are broken.

image

@rudra-superrr , Rest Docs issue resolved.
image

@nikkuma7
Copy link
Author

Endpoint for download component csv and download license archive are missing under export section. image

@rudra-superrr , Already endpoint is there for Download license archive(under Admin tab -> license).

@rudra-superrr
Copy link
Contributor

Testing was successful and please look into Download Component CSV export button.

@nikkuma7 nikkuma7 force-pushed the feat/ImportExport-downloadComponentTemplate branch 2 times, most recently from 0a3c8fe to 30d42c9 Compare May 22, 2024 05:31
@keerthi-bl keerthi-bl self-requested a review August 29, 2024 06:24
keerthi-bl
keerthi-bl reviewed Aug 30, 2024
View reviewed changes
Copy link
Contributor

@keerthi-bl keerthi-bl left a comment

Choose a reason for hiding this comment

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

@nikkuma7 Please do update the review changes

...src/main/java/org/eclipse/sw360/rest/resourceserver/importexport/ImportExportController.java Outdated
}
)
@PreAuthorize("hasAuthority('WRITE')")
@RequestMapping(value = IMPORTEXPORT_URL + "/downloadComponentTemplate", method = RequestMethod.GET)
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to use specific variant like @GetMapping instead of generic representation of @RequestMapping in all the places.

...src/main/java/org/eclipse/sw360/rest/resourceserver/importexport/ImportExportController.java
Copy link
Contributor

Choose a reason for hiding this comment

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

Exception need to be handled in all the places!

...c/main/java/org/eclipse/sw360/rest/resourceserver/importexport/Sw360ImportExportService.java
Copy link
Contributor

Choose a reason for hiding this comment

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

Exception need to be handled in all the places!

...c/main/java/org/eclipse/sw360/rest/resourceserver/importexport/Sw360ImportExportService.java Outdated
public void getDownloadCsvComponentTemplate(User sw360User, HttpServletResponse response) throws TException, IOException {
String fileConstant="ComponentsReleasesVendorsSample.csv";
if (!PermissionUtils.isUserAtLeast(UserGroup.ADMIN, sw360User)) {
throw new RuntimeException("Unable to download CSV component template. User is not admin");
Copy link
Contributor

Choose a reason for hiding this comment

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

throw a dedicated exception instead of using a generic one.

...c/main/java/org/eclipse/sw360/rest/resourceserver/importexport/Sw360ImportExportService.java Outdated
public void getDownloadAttachmentTemplate(User sw360User, HttpServletResponse response) throws TException, IOException {
String fileConstant="AttachmentInfo_Sample.csv";
if (!PermissionUtils.isUserAtLeast(UserGroup.ADMIN, sw360User)) {
throw new RuntimeException("Unable to download CSV attachment template. User is not admin");
Copy link
Contributor

Choose a reason for hiding this comment

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

throw a dedicated exception instead of using a generic one

...c/main/java/org/eclipse/sw360/rest/resourceserver/importexport/Sw360ImportExportService.java Outdated
}

public void getComponentDetailedExport(User sw360User, HttpServletResponse response)
throws TException, IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the declaration of thrown exception 'TException', as it cannot be thrown from method's body.

...c/main/java/org/eclipse/sw360/rest/resourceserver/importexport/Sw360ImportExportService.java Outdated

}

public void getDownloadAttachmentInfo(User sw360User, HttpServletResponse response) throws TException, IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove unused throw exceptions.

...c/main/java/org/eclipse/sw360/rest/resourceserver/importexport/Sw360ImportExportService.java Outdated
}

private Iface getThriftComponentClient() {
ComponentService.Iface componentClient = new ThriftClients().makeComponentClient();
Copy link
Contributor

Choose a reason for hiding this comment

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

Immediately return this expression instead of assigning it to the temporary variable "componentClient".

...c/main/java/org/eclipse/sw360/rest/resourceserver/importexport/Sw360ImportExportService.java
componentDetailedSummaryForExport = componentClient.getComponentDetailedSummaryForExport();
} catch (TException e) {
log.error("Problem fetching components", e);
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of null return, maybe its better to return emptyCollections.

...erver/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ImportExportSpecTest.java Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check the documentation once with latest main branch.

Copy link
Author

Choose a reason for hiding this comment

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

@keerthi-bl , comment Addressed.

@nikkuma7 nikkuma7 force-pushed the feat/ImportExport-downloadComponentTemplate branch from 30d42c9 to 975e35e Compare September 2, 2024 07:25
@nikkuma7 nikkuma7 force-pushed the feat/ImportExport-downloadComponentTemplate branch 2 times, most recently from c6e98f2 to cb10ce4 Compare September 3, 2024 04:26
feat(rest): Create new endpoint to download component template in csv…
d2de22f 
… format.

Signed-off-by: Nikesh kumar <kumar.nikesh@simens.com>
@nikkuma7 nikkuma7 force-pushed the feat/ImportExport-downloadComponentTemplate branch from cb10ce4 to d2de22f Compare September 3, 2024 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keerthi-bl keerthi-bl keerthi-bl left review comments

Assignees

@rudra-superrr rudra-superrr

@keerthi-bl keerthi-bl

Labels
needs code review needs general test This is general testing, meaning that there is no org specific issue to check for New-UI Level for the API and UI level changes for the new-ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nikkuma7 @rudra-superrr @keerthi-bl @heliocastro @ag4ums @smrutis1