Skip to content

EPMRPP-114305 || update GitHub integration: upgrade dependencies and refactor user synchronization logic#5

Merged
grabsefx merged 1 commit into
developfrom
EPMRPP-114305
Jun 15, 2026
Merged

EPMRPP-114305 || update GitHub integration: upgrade dependencies and refactor user synchronization logic#5
grabsefx merged 1 commit into
developfrom
EPMRPP-114305

Conversation

@grabsefx

@grabsefx grabsefx commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

Release Notes

  • Chores

    • Updated Lombok Gradle plugin to version 9.5.0
    • Updated service-api dependency for improved stability
  • Bug Fixes

    • Fixed organization restriction parsing in OAuth configuration to properly handle organization lists

@grabsefx grabsefx self-assigned this Jun 15, 2026
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The PR refactors SynchronizeGithubUserCommand to extend AbstractExtensionCommand with OrganizationRepositoryCustom and ProjectRepository dependencies, simplifies GitHubUserReplicator's constructor by removing project-related dependencies, migrates its logging to Lombok @Slf4j, updates GitHubExtension to inject OrganizationRepositoryCustom and override command exposure methods, and changes OAuth2 org restriction parsing from CSV-split to direct List<String> cast. Build dependencies are also bumped.

Changes

Extension Command and Replicator Refactoring

Layer / File(s) Summary
SynchronizeGithubUserCommand and GitHubUserReplicator constructor refactoring
src/main/java/com/epam/reportportal/extension/github/command/SynchronizeGithubUserCommand.java, src/main/java/com/epam/reportportal/extension/github/GitHubUserReplicator.java
SynchronizeGithubUserCommand switches base class from AuthenticatedUserContextCommand to AbstractExtensionCommand<OperationCompletionRS>, adds ProjectRepository and OrganizationRepositoryCustom constructor params, and sets minUserRole = UserRole.USER. GitHubUserReplicator removes ProjectRepository and PersonalProjectService from its constructor and gains the @Slf4j annotation.
GitHubExtension wiring and command exposure
src/main/java/com/epam/reportportal/extension/github/GitHubExtension.java
Adds an OrganizationRepositoryCustom injected field, rekeys commonCommands map to ExtensionCommand<?>, removes project-related args from GitHubUserReplicator construction, passes organizationRepository to SynchronizeGithubUserCommand, changes getCommonCommand() to return null, and overrides getCommonExtensionCommands() to return the commands map.
GitHubUserReplicator Lombok logging migration
src/main/java/com/epam/reportportal/extension/github/GitHubUserReplicator.java
All LOGGER.info/warn call sites in synchronizeUser, replicateUser, updateUser, createUser, uploadAvatar, retrievePrimaryEmail, and resolveEmail are replaced with the Lombok-generated log.info/warn.
Build dependency bumps
build.gradle
Lombok Gradle plugin updated from 9.2.0 to 9.5.0; com.github.reportportal:service-api and its annotationProcessor bumped to 336be2c.

OAuth2 Organization Restriction Parsing

Layer / File(s) Summary
parseAllowedOrganizations: direct List cast
src/main/java/com/epam/reportportal/extension/github/oauth/GitHubOAuth2UserService.java
parseAllowedOrganizations now directly casts the "organizations" restriction value to List<String> instead of splitting it as a comma-separated string; Guava Splitter import removed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hop hop, the replicator sheds its load,
No more project repos upon its road!
Commands now extend a grander base,
And org lists arrive pre-parsed with grace.
Lombok logs with log so neat —
The bunny's refactor is complete! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main changes: upgrading dependencies (Lombok, service-api) and refactoring user synchronization logic across multiple classes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch EPMRPP-114305

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 | 🟡 Minor

Remove unused personalProjectService field injection.

The personalProjectService field is @Autowired but 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 value

Unused @Slf4j annotation.

The @Slf4j annotation generates a log field, 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 value

Consider adding context to log statements.

The log statements at lines 68 and 162 (log.info("synchronizeUser") and log.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 value

Consider simplifying the double cast.

The intermediate (Object) cast is unnecessary since restrictions.get("organizations") already returns Object. The upstream code in GitHubIntegrationStrategy.buildRestrictions uses the simpler pattern:

.map(r -> (List<String>) r.get(ORGANIZATIONS_KEY))

The contract alignment with upstream is verified — buildRestrictions stores organizations as List<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

📥 Commits

Reviewing files that changed from the base of the PR and between 04642c8 and ff80fa0.

📒 Files selected for processing (5)
  • build.gradle
  • src/main/java/com/epam/reportportal/extension/github/GitHubExtension.java
  • src/main/java/com/epam/reportportal/extension/github/GitHubUserReplicator.java
  • src/main/java/com/epam/reportportal/extension/github/command/SynchronizeGithubUserCommand.java
  • src/main/java/com/epam/reportportal/extension/github/oauth/GitHubOAuth2UserService.java

@grabsefx grabsefx merged commit a0c27a5 into develop Jun 15, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants