Skip to content

Conversation

@CitralFlo
Copy link
Member

@CitralFlo CitralFlo commented Aug 30, 2025

Methods do not change so #1136 can be merged :>

@CitralFlo CitralFlo requested a review from a team as a code owner August 30, 2025 20:27
@CitralFlo CitralFlo changed the title User Manager uses repository now! GH-1147 User Manager uses repository now! Aug 30, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the UserManager to use a repository pattern for data persistence, which is a great improvement. I've identified a few issues, including a critical bug that causes double database writes and a race condition during initial data loading. Additionally, there are opportunities to improve code clarity and maintainability by simplifying some logic and improving API design. The new integration tests are a good addition, but could be improved by removing console output.

@CitralFlo CitralFlo requested a review from sadcenter September 1, 2025 19:59
@CitralFlo CitralFlo marked this pull request as draft September 1, 2025 21:29
Copy link

@sadcenter sadcenter left a comment

Choose a reason for hiding this comment

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

LGTM

@Jakubk15 Jakubk15 changed the title GH-1147 User Manager uses repository now! GH-1147 Make UserManager use UserRepository Sep 28, 2025
CitralFlo and others added 3 commits October 5, 2025 17:22
# Conflicts:
#	buildSrc/src/main/kotlin/Versions.kt
#	eternalcore-core/src/main/java/com/eternalcode/core/feature/fullserverbypass/FullServerBypassController.java
#	eternalcore-core/src/main/java/com/eternalcode/core/user/UserClientBukkitSettings.java
Copy link
Member

@Rollczi Rollczi left a comment

Choose a reason for hiding this comment

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

chyba tam testy wybuchają i takie tam

@vLuckyyy vLuckyyy marked this pull request as ready for review November 30, 2025 17:40
Copy link
Member

@Rollczi Rollczi left a comment

Choose a reason for hiding this comment

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

Część CR masz w commit - poprawki estetyczne - duplikacje i takie tam pierdoły.
Mam pewne pytania gdzie trochę nie jestem pewien intencji to zostawiam do twojej weryfikacji:

  • jakieś rzeczy w user manager
  • testy

Comment on lines +60 to +61
Optional<User> optionalUser = this.userManager.getUser(playerUUID);
User user = optionalUser.get();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Optional<User> optionalUser = this.userManager.getUser(playerUUID);
User user = optionalUser.get();
User user = this.userManager.getUser(playerUUID).orElseThrow();

UserManager userManager,
EventCaller eventCaller,
MsgToggleService msgToggleService
MsgToggleService msgToggleService, Server server
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MsgToggleService msgToggleService, Server server
MsgToggleService msgToggleService,
Server server

Comment on lines +58 to +66
UUID uuid = this.replies.getIfPresent(sender.getUniqueId());

if (uuid == null) {
this.noticeService.player(sender.getUniqueId(), translation -> translation.msg().noReply());

return;
}

Player target = this.server.getPlayer(uuid);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
UUID uuid = this.replies.getIfPresent(sender.getUniqueId());
if (uuid == null) {
this.noticeService.player(sender.getUniqueId(), translation -> translation.msg().noReply());
return;
}
Player target = this.server.getPlayer(uuid);
UUID lastReplyUuid = this.replies.getIfPresent(sender.getUniqueId());
if (lastReplyUuid == null) {
this.noticeService.player(sender.getUniqueId(), translation -> translation.msg().noReply());
return;
}
Player lastReply= this.server.getPlayer(lastReplyUuid );

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.

7 participants