feat(bdpImport): Use of proper error message when importing projects #680
Conversation
|
|
||
| // Project Import: user actions | ||
| public static final String PROJECT_IMPORT_USER_ACTION__IMPORT = "projectImport_import_action"; | ||
| public static final String PROJECT_IMPORT_USER_ACTION__IMPORT_BDP = "projectImport_import_bdp_data"; |
There was a problem hiding this comment.
how about calling this PROJECT_IMPORT_USER_ACTION__IMPORT_DATA?
| public static final String PROJECT_IMPORT_RESPONSE__SUCCESS = "projectImport_response_bdp_import_success"; | ||
| public static final String PROJECT_IMPORT_RESPONSE__FAILURE = "projectImport_response_bdp_import_failure"; | ||
| public static final String PROJECT_IMPORT_RESPONSE__GENERAL_FAILURE = "projectImport_response_bdp_import_general_failure"; | ||
|
|
There was a problem hiding this comment.
maybe removing bdp could be also OK
| @@ -128,12 +131,12 @@ private boolean isImportSuccessful(ImportStatus importStatus) { | |||
| return (importStatus.isSetRequestStatus() && importStatus.getRequestStatus().equals(RequestStatus.SUCCESS) && importStatus.getFailedIds().isEmpty()); | |||
| } | |||
|
|
|||
| private ImportStatus importDatasources(List<String> toImport, User user, RemoteCredentials remoteCredentials) { | |||
| private ImportStatus importDatasources(List<String> toImport, User user, RemoteCredentials remoteCredentials) { | |||
| ImportStatus importStatus = new ImportStatus(); | |||
| try { | |||
| importStatus = bdpImportClient.importDatasources(toImport, user, remoteCredentials); | |||
| if (!isImportSuccessful(importStatus)) { | |||
There was a problem hiding this comment.
if the thrift file could be renamed this could be also renamed at the same occasion.
|
Since you are adjusting labels / names, such as
How about eliminating also the references to BDP from the sw360portal code base? for example. it does not make sense to have the file "bdpImport.thrift" (https://github.com/sw360/sw360portal/blob/master/libraries/lib-datahandler/src/main/thrift/bdpimport.thrift), but it could be renamed to "projectImport.thrift" At the same time, a variables named with |
bodetc
force-pushed
the
dev/654-bdp-error-messages
branch
from
0f2fce2
to
c0684a7
Compare
… references for BDP in main repository
bodetc
force-pushed
the
dev/654-bdp-error-messages
branch
from
c0684a7
to
dc2d820
Compare
mcjaeger
added
depends on other PR
needs test
needs code review
and removed
has merge conflicts
labels
maxhbr
added
increases major version
increases minor version
and removed
increases major version
labels
|
hi, I am not sure about the error message improvement. Got the following from screenshot (see below), what I did it is:
I keep on testing because the log says that the importing of the license crashes and I guess this does not work with the plain user:
|
|
Update: with an admin user it seems to work. I guess the message is also improved. So, what do we do about the "License cannot be created" problem ... I think the import of projects is maybe not for every sw360 user but only for those who can create licenses anyways? As such the import menu item could be made visible only to those during deployment (requires updated lar though) Attached a screenshot of the out that you maybe anticipated. It works both with admins and clearing admins |
There was a problem hiding this comment.
So, having this as question from review: what do we do about the "License cannot be created" problem read from the logs ... I think the import of projects is maybe not for every sw360 user but only for those who can create licenses anyways (admin and clearing admin)? As such the import menu item could be made visible only to those during deployment (requires updated lar though). OK for that?
|
The "License cannot be created" is unfortunately hard to catch, as it comes as a exception in the |
Instead of a list of IDs, now a error message is shown for each failed ID with a reason for the failure.
Backend PR: sw360/sw360bdpImportService#5
Depends on PR: #661
(note that before the message was like 'there was some problem', now it shall display which projects etc)