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

MARP-1059 market website rating does not work after login #140

Merged
merged 7 commits into from
Sep 16, 2024
Merged

MARP-1059 market website rating does not work after login #140

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a6a4c65
Fix NPE when response a request
ndkhanh-axonivy Sep 11, 2024
5185921
Merge branch 'develop' into bugfix/MARP-1059-Market-website-Rating-do…
ndkhanh-axonivy Sep 13, 2024
9dadac6
Merge branch 'develop' into bugfix/MARP-1059-Market-website-Rating-do…
ndkhanh-axonivy Sep 13, 2024
c0f4a1b
Merge branch 'develop' into bugfix/MARP-1059-Market-website-Rating-do…
ndkhanh-axonivy Sep 16, 2024
5022bf3
Handle feedbacks
ndkhanh-axonivy Sep 16, 2024
a7ff24f
Update GitHubServiceImpl.java
ndkhanh-axonivy Sep 16, 2024
f8f6dab
Add Unit Tests
ndkhanh-axonivy Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions marketplace-service/src/main/java/com/axonivy/market/constants/CommonConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ public class CommonConstants {
public static final String BEARER = "Bearer";
public static final String DIGIT_REGEX = "([0-9]+.*)";
public static final String ID_WITH_NUMBER_PATTERN = "%s-%s";
public static final String ERROR = "error";
public static final String MESSAGE = "message";
}
1 change: 1 addition & 0 deletions marketplace-service/src/main/java/com/axonivy/market/constants/ErrorMessageConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class ErrorMessageConstants {
public static final String INVALID_MISSING_HEADER_ERROR_MESSAGE = "Invalid or missing header";
public static final String WRONG_AUTHORIZED_CODE_ERROR_MESSAGE = "%s (Current clientId: %s)";
}
10 changes: 9 additions & 1 deletion marketplace-service/src/main/java/com/axonivy/market/controller/OAuth2Controller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
import static com.axonivy.market.constants.RequestMappingConstants.GIT_HUB_LOGIN;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import com.axonivy.market.constants.CommonConstants;
import com.axonivy.market.exceptions.model.Oauth2ExchangeCodeException;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.media.Content;
import io.swagger.v3.oas.annotations.media.Schema;
Expand Down Expand Up @@ -54,8 +57,13 @@ public ResponseEntity<Map<String, String>> gitHubLogin(@RequestBody Oauth2Author
try {
GitHubAccessTokenResponse tokenResponse = gitHubService.getAccessToken(oauth2AuthorizationCode.getCode(), gitHubProperty);
accessToken = tokenResponse.getAccessToken();
} catch (Oauth2ExchangeCodeException e) {
Map<String, String> errorResponse = new HashMap<>();
errorResponse.put(CommonConstants.ERROR, e.getError());
errorResponse.put(CommonConstants.MESSAGE, e.getErrorDescription());
return new ResponseEntity<>(errorResponse, HttpStatus.BAD_REQUEST);
} catch (Exception e) {
return new ResponseEntity<>(Map.of(e.getClass().getName(), e.getMessage()), HttpStatus.BAD_REQUEST);
return new ResponseEntity<>(Map.of(CommonConstants.MESSAGE, e.getMessage()), HttpStatus.BAD_REQUEST);
}

User user = gitHubService.getAndUpdateUser(accessToken);
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 block code to try {}

Expand Down
8 changes: 7 additions & 1 deletion ...place-service/src/main/java/com/axonivy/market/github/service/impl/GitHubServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.Map;
import java.util.Optional;

import com.axonivy.market.constants.ErrorMessageConstants;
import com.axonivy.market.constants.CommonConstants;
import org.kohsuke.github.GHContent;
import org.kohsuke.github.GHOrganization;
Expand Down Expand Up @@ -110,7 +111,8 @@ public GitHubAccessTokenResponse getAccessToken(String code, GitHubProperty gitH
GitHubAccessTokenResponse response = responseEntity.getBody();

if (response != null && response.getError() != null && !response.getError().isBlank()) {
throw new Oauth2ExchangeCodeException(response.getError(), response.getErrorDescription());
String errorDescription = buildErrorDescription(response.getErrorDescription(), gitHubProperty.getOauth2ClientId());
Copy link
Contributor

Choose a reason for hiding this comment

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

We should write a log and not throw exception with this info.

throw new Oauth2ExchangeCodeException(response.getError(), errorDescription);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should write a test for this Exception with ExceptionHandlers.

}

return response;
Expand Down Expand Up @@ -180,4 +182,8 @@ public List<Map<String, Object>> getUserOrganizations(String accessToken) throws
exception.getMessage()));
}
}

private String buildErrorDescription(String errorDescription, String clientId) {
return String.format(ErrorMessageConstants.WRONG_AUTHORIZED_CODE_ERROR_MESSAGE, errorDescription, clientId);
}
}
Loading