EPMRPP-114305 || update GitHub integration: upgrade dependencies and refactor user synchronization logic#5
Conversation
…refactor user synchronization logic
WalkthroughThe PR refactors ChangesExtension Command and Replicator Refactoring
OAuth2 Organization Restriction Parsing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/epam/reportportal/extension/github/GitHubExtension.java (1)
106-107:⚠️ Potential issue | 🟡 MinorRemove unused
personalProjectServicefield injection.The
personalProjectServicefield is@Autowiredbut never used in this class (only the field declaration at line 107 appears in the file). Remove this unused dependency injection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/epam/reportportal/extension/github/GitHubExtension.java` around lines 106 - 107, Remove the unused `personalProjectService` field from the GitHubExtension class. Delete both the `@Autowired` annotation and the private PersonalProjectService personalProjectService field declaration, as this dependency is never referenced anywhere in the class.
🧹 Nitpick comments (3)
src/main/java/com/epam/reportportal/extension/github/command/SynchronizeGithubUserCommand.java (1)
31-31: 💤 Low valueUnused
@Slf4jannotation.The
@Slf4jannotation generates alogfield, but this class doesn't use any logging. Consider removing the unused annotation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/epam/reportportal/extension/github/command/SynchronizeGithubUserCommand.java` at line 31, The SynchronizeGithubUserCommand class has an unused `@Slf4j` annotation that generates a log field which is never used in the class. Remove the `@Slf4j` annotation from the class declaration since no logging operations are performed within this class.src/main/java/com/epam/reportportal/extension/github/GitHubUserReplicator.java (1)
67-68: 💤 Low valueConsider adding context to log statements.
The log statements at lines 68 and 162 (
log.info("synchronizeUser")andlog.info("retrievePrimaryEmail")) lack context. Adding the user's login or email would improve traceability during debugging.Also applies to: 161-162
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/epam/reportportal/extension/github/GitHubUserReplicator.java` around lines 67 - 68, The log statements in the synchronizeUser method (line 68) and retrievePrimaryEmail method (line 162) lack contextual information about which user is being processed. Add the user's login or email identifier to both log.info calls to improve traceability during debugging. In synchronizeUser, include the user information derived from the GitHub API response or token context in the log message. Similarly, in retrievePrimaryEmail, add the user identifier to the log statement to track which user's email is being retrieved.src/main/java/com/epam/reportportal/extension/github/oauth/GitHubOAuth2UserService.java (1)
74-78: 💤 Low valueConsider simplifying the double cast.
The intermediate
(Object)cast is unnecessary sincerestrictions.get("organizations")already returnsObject. The upstream code inGitHubIntegrationStrategy.buildRestrictionsuses the simpler pattern:.map(r -> (List<String>) r.get(ORGANIZATIONS_KEY))The contract alignment with upstream is verified —
buildRestrictionsstores organizations asList<String>, so the direct cast is correct.♻️ Proposed simplification
private List<String> parseAllowedOrganizations(OAuthRegistrationResource registration) { return Optional.ofNullable(registration.getRestrictions()) - .map(restrictions -> (List<String>) (Object) restrictions.get("organizations")) + .map(restrictions -> (List<String>) restrictions.get("organizations")) .orElse(Collections.emptyList()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/epam/reportportal/extension/github/oauth/GitHubOAuth2UserService.java` around lines 74 - 78, In the parseAllowedOrganizations method, remove the unnecessary intermediate (Object) cast from the map operation. The restrictions.get("organizations") call already returns Object, so the direct cast to List<String> is sufficient. Simplify the cast chain from (List<String>) (Object) restrictions.get("organizations") to just (List<String>) restrictions.get("organizations"), which aligns with the simpler pattern used upstream in GitHubIntegrationStrategy.buildRestrictions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/main/java/com/epam/reportportal/extension/github/GitHubExtension.java`:
- Around line 106-107: Remove the unused `personalProjectService` field from the
GitHubExtension class. Delete both the `@Autowired` annotation and the private
PersonalProjectService personalProjectService field declaration, as this
dependency is never referenced anywhere in the class.
---
Nitpick comments:
In
`@src/main/java/com/epam/reportportal/extension/github/command/SynchronizeGithubUserCommand.java`:
- Line 31: The SynchronizeGithubUserCommand class has an unused `@Slf4j`
annotation that generates a log field which is never used in the class. Remove
the `@Slf4j` annotation from the class declaration since no logging operations are
performed within this class.
In
`@src/main/java/com/epam/reportportal/extension/github/GitHubUserReplicator.java`:
- Around line 67-68: The log statements in the synchronizeUser method (line 68)
and retrievePrimaryEmail method (line 162) lack contextual information about
which user is being processed. Add the user's login or email identifier to both
log.info calls to improve traceability during debugging. In synchronizeUser,
include the user information derived from the GitHub API response or token
context in the log message. Similarly, in retrievePrimaryEmail, add the user
identifier to the log statement to track which user's email is being retrieved.
In
`@src/main/java/com/epam/reportportal/extension/github/oauth/GitHubOAuth2UserService.java`:
- Around line 74-78: In the parseAllowedOrganizations method, remove the
unnecessary intermediate (Object) cast from the map operation. The
restrictions.get("organizations") call already returns Object, so the direct
cast to List<String> is sufficient. Simplify the cast chain from (List<String>)
(Object) restrictions.get("organizations") to just (List<String>)
restrictions.get("organizations"), which aligns with the simpler pattern used
upstream in GitHubIntegrationStrategy.buildRestrictions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18888dd8-7b0e-4ce5-9c2b-a2c1046fd61e
📒 Files selected for processing (5)
build.gradlesrc/main/java/com/epam/reportportal/extension/github/GitHubExtension.javasrc/main/java/com/epam/reportportal/extension/github/GitHubUserReplicator.javasrc/main/java/com/epam/reportportal/extension/github/command/SynchronizeGithubUserCommand.javasrc/main/java/com/epam/reportportal/extension/github/oauth/GitHubOAuth2UserService.java
Summary by CodeRabbit
Release Notes
Chores
Bug Fixes