Skip to content

Thêm lệnh xoá dữ liệu của người chơi (closes gitlab/kitchen/plan/-/issues/4)#69

Merged
amadeusmz merged 15 commits into
masterfrom
minh/clear-player-milestone-data-cmd
Jan 11, 2026
Merged

Thêm lệnh xoá dữ liệu của người chơi (closes gitlab/kitchen/plan/-/issues/4)#69
amadeusmz merged 15 commits into
masterfrom
minh/clear-player-milestone-data-cmd

Conversation

@minhh2792
Copy link
Copy Markdown
Member

@minhh2792 minhh2792 commented Aug 27, 2025

Summary by CodeRabbit

  • New Features
    • Adds an admin command to permanently clear a player’s stored data with usage help, tab-completion, confirmation flow, and clear/no-data/error feedback.
  • Backend
    • Adds server-side support to delete all stored data for a specific player (UUID-scoped deletion) across supported storage engines.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 27, 2025

📝 Walkthrough

Walkthrough

Adds an admin cleardata subcommand that resolves a player's UUID and deletes all rows for that UUID via a new PlayerDataDAO.deleteAllDataByUuid API; provides H2/MySQL SQL script implementations and a server-side chat confirmation flow requiring explicit token input.

Changes

Cohort / File(s) Summary
Admin command extension
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
Adds cleardata subcommand and private handler clearPlayerData() with arg validation, tab-completion (online players filtered by prefix), UUID resolution via PlayerInfoDAO, console immediate execution, server-side confirmation via ChatListener expecting token XACNHAN (HUY/other cancels), transactional runNotSync execution, calls PlayerDataDAO.deleteAllDataByUuid(uuid), and reports success, no-data, or error messages.
DAO interface update
dotman-plugin/src/main/java/net/minevn/dotman/database/PlayerDataDAO.kt
Adds abstract fun deleteAllDataByUuidScript(): String and fun deleteAllDataByUuid(uuid: String): Int which executes the script with uuid and returns affected row count.
DAO impl scripts (H2 / MySQL)
dotman-plugin/src/main/java/net/minevn/dotman/database/h2/PlayerDataDAOImpl.kt, dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt
Implements override fun deleteAllDataByUuidScript() returning a trimmed SQL DELETE FROM dotman_player_data WHERE uuid = ? statement for each backend.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Admin as Admin User
  participant Cmd as AdminCmd
  participant PInfo as PlayerInfoDAO
  participant Chat as ChatListener
  participant PData as PlayerDataDAO
  participant DB as Database

  Admin->>Cmd: /dotman cleardata <playerName>
  Cmd->>PInfo: resolve UUID for playerName
  alt UUID not found
    Cmd-->>Admin: "Player not found"
  else UUID found
    alt sender is console
      Cmd->>PData: deleteAllDataByUuid(uuid)
    else sender is server
      Cmd->>Chat: prompt for confirmation ("XACNHAN" / "HUY")
      Chat->>Cmd: confirmation response
      alt confirmed ("XACNHAN")
        Cmd->>PData: deleteAllDataByUuid(uuid)
      else canceled
        Cmd-->>Admin: "Canceled"
      end
    end
    PData->>DB: execute DELETE FROM dotman_player_data WHERE uuid=?
    DB-->>PData: rows affected
    PData-->>Cmd: deleted count
    Cmd-->>Admin: success / no-data message
  end
  opt Error
    Cmd-->>Admin: error message (exception rethrown)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I twitch my whiskers, tap the key,
A confirm hop—XACNHAN for me.
UUID found, rows vanish in a blink,
H2 and MySQL tidy in a wink.
Hooray—cleaned fields! 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding a command to delete player data, which matches the PR objectives and all code changes (AdminCmd, PlayerDataDAO, and implementations).

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

@minhh2792 minhh2792 requested a review from Copilot August 27, 2025 16:14
@minhh2792 minhh2792 marked this pull request as ready for review August 27, 2025 16:14

This comment was marked as outdated.

coderabbitai Bot added a commit that referenced this pull request Aug 27, 2025
Docstrings generation was requested by @minhh2792.

* #69 (comment)

The following files were modified:

* `dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt`
* `dotman-plugin/src/main/java/net/minevn/dotman/database/PlayerDataDAO.kt`
* `dotman-plugin/src/main/java/net/minevn/dotman/database/h2/PlayerDataDAOImpl.kt`
* `dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt`
@minevn minevn deleted a comment from coderabbitai Bot Aug 27, 2025
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (4)
dotman-plugin/src/main/java/net/minevn/dotman/database/PlayerDataDAO.kt (1)

73-79: Document LIKE semantics to avoid accidental over-deletes; consider escaping helpers.

The method is safe (prepared statement) and returns affected rows. However, callers must know that _ and % are wildcards. A tiny helper or KDoc note recommending escaping _/% (and pairing with an ESCAPE clause in backend SQL) would reduce misuse.

If you prefer a helper, I can add something like:

/** Builds a LIKE pattern for a literal prefix: escapes '_' and '%' and appends '%' */
fun likePrefix(prefix: String, escape: Char = '\\') =
    prefix.replace("_", "$escape" + "_").replace("%", "$escape" + "%") + "%"

And overload:

fun deleteDataByKeyWithPrefix(uuid: String, keyPrefix: String): Int =
    deleteDataByKeyLike(uuid, likePrefix(keyPrefix))
dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt (1)

44-47: Add ESCAPE clause so callers can pass escaped _/% for literal matching.

Without an explicit ESCAPE, patterns with backslash may be ambiguous across DBs. Using ESCAPE '' makes intent clear and consistent with H2.

Apply:

     override fun deleteDataByKeyLikeScript() = """
         DELETE FROM `dotman_player_data`
-        WHERE `uuid` = ? AND `key` LIKE ?;
+        WHERE `uuid` = ? AND `key` LIKE ? ESCAPE '\';
     """.trimIndent()
dotman-plugin/src/main/java/net/minevn/dotman/database/h2/PlayerDataDAOImpl.kt (1)

51-54: Mirror ESCAPE clause in H2 to keep cross-backend behavior identical.

This ensures \_ and \% are treated as literals when used by callers.

Apply:

     override fun deleteDataByKeyLikeScript() = """
         DELETE FROM "dotman_player_data"
-        WHERE "uuid" = ? AND "key" LIKE ?;
+        WHERE "uuid" = ? AND "key" LIKE ? ESCAPE '\';
     """.trimIndent()
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (1)

319-360: Use constant for key prefix, escape underscores, and surface deleted count

No other occurrences of the hard-coded pattern were found—only in AdminCmd.kt at lines 350–351.

• File: dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (around line 350)
– Replace the literal "DONATE_TOTAL_ALL%" with a pattern built from TOP_KEY_DONATE_TOTAL, escaping _ to avoid over-matching.
– Capture and include the deleted row count in the confirmation message.

Apply this diff:

--- a/dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
+++ b/dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
@@ action {
-    val donateKeyOrPattern = "DONATE_TOTAL_ALL%"
-    dataDAO.deleteDataByKeyLike(uuid, donateKeyOrPattern)
+    val donatePrefix = "${TOP_KEY_DONATE_TOTAL}_ALL"
+    val donatePattern = donatePrefix.replace("_", "\\_") + "%"
+    val deleted = dataDAO.deleteDataByKeyLike(uuid, donatePattern)
@@ action {
-    sender.send("§aĐã xoá dữ liệu top nạp thẻ/milestone của người chơi §b$playerName")
+    sender.send("§aĐã xoá dữ liệu top nạp thẻ/milestone của người chơi §b$playerName §7($deleted bản ghi)")

And add at the top of the file:

import net.minevn.dotman.DotMan.Companion.TOP_KEY_DONATE_TOTAL
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3852219 and 636d492.

📒 Files selected for processing (4)
  • dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (3 hunks)
  • dotman-plugin/src/main/java/net/minevn/dotman/database/PlayerDataDAO.kt (2 hunks)
  • dotman-plugin/src/main/java/net/minevn/dotman/database/h2/PlayerDataDAOImpl.kt (1 hunks)
  • dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
dotman-plugin/src/main/java/net/minevn/dotman/database/PlayerDataDAO.kt (2)
dotman-plugin/src/main/java/net/minevn/dotman/database/h2/PlayerDataDAOImpl.kt (1)
  • deleteDataByKeyLikeScript (51-54)
dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt (1)
  • deleteDataByKeyLikeScript (44-47)
dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt (2)
dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerInfoDAOImpl.kt (2)
  • updateScript (5-15)
  • updateScript (6-10)
dotman-plugin/src/main/java/net/minevn/dotman/database/h2/PlayerInfoDAOImpl.kt (2)
  • updateScript (5-29)
  • updateScript (6-19)
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (2)
dotman-plugin/src/main/java/net/minevn/dotman/utils/Utils.kt (1)
  • runNotSync (50-60)
dotman-plugin/src/main/java/net/minevn/dotman/DotMan.kt (2)
  • transactional (162-167)
  • updateLeaderBoard (122-157)
🔇 Additional comments (2)
dotman-plugin/src/main/java/net/minevn/dotman/database/PlayerDataDAO.kt (1)

17-17: API addition aligns with existing DAO script pattern.

Abstracting the DELETE script per backend is consistent with the current design of script providers. No issues.

dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (1)

8-8: Import addition is appropriate.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
runNotSync { transactional {
try {
val uuid = infoDAO.getUUID(playerName)
?: run {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

?: nên để inline, tương tự L259

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

em ddax đưa về inline

Comment on lines +350 to +351
val playerTotalDonateKey = "DONATE_TOTAL_ALL%"
dataDAO.deleteDataByKeyLike(uuid, playerTotalDonateKey)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Key hình như bị dư dấu % rồi.
Với lại, mình xóa all data theo tên player luôn, không phân biệt key

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

em đã sửa lại, lọc theo pattern % tức mọi key

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
@minhh2792 minhh2792 force-pushed the minh/clear-player-milestone-data-cmd branch from e071454 to c9a02ba Compare December 30, 2025 17:40
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (1)

308-309: Consider consistent DAO variable naming.

The codebase pattern uses Dao suffix (e.g., logDao at lines 106, 274), but the new code uses DAO suffix (logDAO, infoDAO, dataDAO). For consistency, consider using lowercase Dao.

🔎 Proposed naming consistency fix
-                    val logDAO = LogDAO.getInstance()
-                    val transactionDetails = logDAO.getTransactionDetailsById(transactionId).firstOrNull()
+                    val logDao = LogDAO.getInstance()
+                    val transactionDetails = logDao.getTransactionDetailsById(transactionId).firstOrNull()
-                val infoDAO = PlayerInfoDAO.getInstance()
-                val dataDAO = PlayerDataDAO.getInstance()
+                val infoDao = PlayerInfoDAO.getInstance()
+                val dataDao = PlayerDataDAO.getInstance()

Then update references at lines 344 and 349 accordingly.

Also applies to: 333-334

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c9a02ba and e071454.

📒 Files selected for processing (1)
  • dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-09-22T03:29:30.524Z
Learnt from: amadeusmz
Repo: minevn/dotman PR: 48
File: dotman-plugin/src/main/java/net/minevn/dotman/database/LogDAO.kt:216-216
Timestamp: 2024-09-22T03:29:30.524Z
Learning: DAO class chỉ có nghĩa vụ trả về giá trị; công việc xử lý nên được đưa ra ngoài.

Applied to files:

  • dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
🧬 Code graph analysis (1)
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (1)
dotman-plugin/src/main/java/net/minevn/dotman/utils/Utils.kt (1)
  • runNotSync (50-60)
🔇 Additional comments (2)
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (2)

323-330: Tab completion implementation looks good.

The tab completion logic correctly returns empty for no args, filtered online player names for single arg, and empty otherwise. Clean implementation.


342-355: Transaction handling follows best practices.

The implementation correctly uses runNotSync to avoid blocking the main thread and wraps the database operation in transactional for atomicity. Error handling with user-friendly messaging is appropriate, and the DAO pattern separates data access from business logic.

Based on learnings, DAOs should only return values with processing done outside—this implementation follows that principle correctly.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
@amadeusmz
Copy link
Copy Markdown
Member

@minhh2792
Cân nhắc sửa các comment của AI nha em

@minhh2792 minhh2792 changed the title Thêm lệnh xoá dữ liệu top nạp thẻ/milestone của người chơi (closes gitlab/kitchen/plan/-/issues/4) Thêm lệnh xoá dữ liệu của người chơi (closes gitlab/kitchen/plan/-/issues/4) Jan 10, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt:
- Around line 319-321: The function name clearMilestoneData() conflicts with its
description and implementation which delete all player data using a "%"
wildcard; either rename clearMilestoneData() to clearPlayerData() and update its
command/usage/description to indicate it removes all player data, or change the
implementation to only target milestone-related keys and update the description
to reflect milestone-only deletion (ensure you update the command declaration,
the usage string, and any key patterns/wildcards accordingly).
- Around line 342-357: The deletion command currently deletes all player data in
the runNotSync { transactional { ... }} block (using infoDao.getUUID and
dataDao.deleteDataByKeyLike) without confirmation or audit logging; modify the
command handler to require an explicit confirmation (either a second “confirm”
invocation or a required flag like -confirm) before calling
dataDao.deleteDataByKeyLike, and if confirmation is missing send a clear message
to the sender explaining how to confirm; after a successful deletion (inside the
try after dataDao.deleteDataByKeyLike) insert an audit record via
logDao.insertLog (include timestamp, admin/sender.name, target playerName/UUID,
operation "DATA_CLEARED" and affected record count if available) and then send
the success message; keep exception handling as-is but ensure the audit log is
only written on success.
🧹 Nitpick comments (1)
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (1)

349-350: Consider capturing the deletion count for more accurate user feedback.

The deleteDataByKeyLike method returns Int (number of affected rows), but the return value is not captured or used. The current message always indicates successful deletion regardless of whether any records were actually deleted. Consider capturing the row count:

-dataDao.deleteDataByKeyLike(uuid, "%")
-sender.send("§aĐã xoá toàn bộ dữ liệu của người chơi §b$playerName")
+val deletedCount = dataDao.deleteDataByKeyLike(uuid, "%")
+if (deletedCount > 0) {
+    sender.send("§aĐã xoá §b$deletedCount §adữ liệu của người chơi §b$playerName")
+} else {
+    sender.send("§eKhông có dữ liệu nào để xoá cho người chơi §b$playerName")
+}
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e071454 and 1053523.

📒 Files selected for processing (1)
  • dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-09-22T03:29:30.524Z
Learnt from: amadeusmz
Repo: minevn/dotman PR: 48
File: dotman-plugin/src/main/java/net/minevn/dotman/database/LogDAO.kt:216-216
Timestamp: 2024-09-22T03:29:30.524Z
Learning: DAO class chỉ có nghĩa vụ trả về giá trị; công việc xử lý nên được đưa ra ngoài.

Applied to files:

  • dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
🧬 Code graph analysis (1)
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (2)
dotman-plugin/src/main/java/net/minevn/dotman/utils/Utils.kt (1)
  • runNotSync (50-60)
dotman-plugin/src/main/java/net/minevn/dotman/DotMan.kt (1)
  • transactional (169-174)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Prepare
🔇 Additional comments (5)
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (5)

8-8: LGTM: Import addition is appropriate.

The PlayerDataDAO import is correctly added to support the new data deletion functionality.


32-32: LGTM: Subcommand registration follows established pattern.

The registration is correctly placed and follows the same pattern as other admin subcommands.


323-330: LGTM: Tab completion is well-implemented.

The tab completion correctly provides case-insensitive filtering of online player names and handles all argument positions appropriately.


332-340: LGTM: Argument validation is correct.

The validation properly checks for required arguments and provides appropriate usage feedback.


351-354: LGTM: Error handling follows established pattern.

The exception handling is consistent with other admin commands in this file and appropriately rethrows for server-side logging while providing user-friendly feedback.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/database/PlayerDataDAO.kt Outdated
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt Outdated
lệnh gõ trên console thì không cần xác nhận
Comment on lines +73 to +79
fun deleteDataByKeyLike(uuid: String, likePattern: String): Int {
return deleteDataByKeyLikeScript().statement {
setString(1, uuid)
setString(2, likePattern)
update()
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mục đích của funciton này là cho phép xóa key theo prefix (key LIKE 'prefix_%')
Tuy nhiên, khuôn khổ của PR này là xóa toàn bộ key, và function mới chỉ nên tập trung vào mục đích trên.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Đã xử lý ở commit 4f4bd27

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
@dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt:
- Line 38: The triple-quoted SQL returned by the method getAllDataScript in
PlayerDataDAOImpl contains an extraneous leading "K" (it starts with """K) which
will prepend a literal "K" to the SQL; remove that "K" so the string begins
directly with the SQL (e.g., start with """SELECT ..."""), or otherwise
trim/clean the string before returning to ensure valid SQL is produced.
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ca64f50 and 4f4bd27.

📒 Files selected for processing (4)
  • dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
  • dotman-plugin/src/main/java/net/minevn/dotman/database/PlayerDataDAO.kt
  • dotman-plugin/src/main/java/net/minevn/dotman/database/h2/PlayerDataDAOImpl.kt
  • dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
🔇 Additional comments (3)
dotman-plugin/src/main/java/net/minevn/dotman/database/h2/PlayerDataDAOImpl.kt (1)

51-54: LGTM! Clean H2 DELETE implementation.

The SQL syntax is correct for H2, follows the existing pattern in the class, and properly parameterizes the UUID filter.

dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt (1)

44-47: LGTM! Clean MySQL DELETE implementation.

The SQL syntax is correct for MySQL, uses proper backtick quoting, and follows the existing pattern in the class.

dotman-plugin/src/main/java/net/minevn/dotman/database/PlayerDataDAO.kt (1)

17-17: LGTM! All safeguards properly implemented.

The DAO implementation is correct and follows existing patterns. The calling code (AdminCmd.kt) has verified safeguards in place:

  • Authorization: dotman.admin permission required (plugin.yml)
  • User confirmation: Chat confirmation flow (XACNHAN to confirm, HUY to cancel) for players
  • Return value handling: Checks deletedCount > 0 with appropriate feedback messages
  • Console bypass: Direct execution for console (appropriate for trusted admin access)

Consider adding audit logging around deletion operations for compliance/security auditing purposes.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt Outdated
amadeusmz and others added 2 commits January 10, 2026 16:39
…layerDataDAOImpl.kt

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
@dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt:
- Around line 43-46: The SQL string in getAllDataScript() contains a duplicated
SELECT block (the same "SELECT `key`, `value` FROM `dotman_player_data` WHERE
`uuid` = ?;" appears twice), which concatenates two full statements and breaks
SQL; open PlayerDataDAOImpl.kt, locate the getAllDataScript() method and remove
the repeated SELECT/...WHERE `uuid` = ?; lines so only a single SELECT statement
remains in the returned script.
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f4bd27 and 50f8beb.

📒 Files selected for processing (1)
  • dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-21T06:30:03.444Z
Learnt from: amadeusmz
Repo: minevn/dotman PR: 0
File: :0-0
Timestamp: 2025-12-21T06:30:03.444Z
Learning: In the minevn/dotman repository (Kotlin project): Avoid using `data` as a variable name because it conflicts with Kotlin's `data` keyword.

Applied to files:

  • dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt
🔇 Additional comments (1)
dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt (1)

48-51: LGTM: Clean deletion script implementation.

The new deleteAllDataByUuidScript() method correctly implements the MySQL DELETE statement for clearing player data by UUID. The parameterized query approach prevents SQL injection, and the implementation follows the established pattern of other script methods in this class.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (1)

351-358: Small refactor: use startsWith(..., ignoreCase = true) to avoid extra lowercase allocations.

Proposed diff
-                    args.size == 1 -> Bukkit.getOnlinePlayers().map { it.name }
-                        .filter { it.lowercase().startsWith(args.last().lowercase()) }
+                    args.size == 1 -> Bukkit.getOnlinePlayers().map { it.name }
+                        .filter { it.startsWith(args.last(), ignoreCase = true) }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50f8beb and 29d9144.

📒 Files selected for processing (2)
  • dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
  • dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-21T06:30:03.444Z
Learnt from: amadeusmz
Repo: minevn/dotman PR: 0
File: :0-0
Timestamp: 2025-12-21T06:30:03.444Z
Learning: In the minevn/dotman repository (Kotlin project): Avoid using `data` as a variable name because it conflicts with Kotlin's `data` keyword.

Applied to files:

  • dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt
📚 Learning: 2024-09-22T03:29:30.524Z
Learnt from: amadeusmz
Repo: minevn/dotman PR: 48
File: dotman-plugin/src/main/java/net/minevn/dotman/database/LogDAO.kt:216-216
Timestamp: 2024-09-22T03:29:30.524Z
Learning: DAO class chỉ có nghĩa vụ trả về giá trị; công việc xử lý nên được đưa ra ngoài.

Applied to files:

  • dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
📚 Learning: 2025-12-21T06:30:03.444Z
Learnt from: amadeusmz
Repo: minevn/dotman PR: 0
File: :0-0
Timestamp: 2025-12-21T06:30:03.444Z
Learning: In the minevn/dotman repository (Kotlin project): Use clear and descriptive function names. When creating variations of a function, use method overloading with the same name (e.g., `applyReplacements`) instead of creating functions with different names for similar operations.

Applied to files:

  • dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
🧬 Code graph analysis (1)
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (2)
dotman-plugin/src/main/java/net/minevn/dotman/utils/Utils.kt (1)
  • runNotSync (50-60)
dotman-plugin/src/main/java/net/minevn/dotman/DotMan.kt (1)
  • transactional (169-174)
🔇 Additional comments (3)
dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt (2)

34-35: Subcommand wire-up looks correct.


8-19: Imports are correct and all in use. ChatListener is from the external library net.minevn:minevnlib-plugin:1.2.1-beta3 and usage patterns across the codebase (CardProvider with nested listeners, AdminCmd with early returns) confirm it operates as a one-shot auto-unregistering listener. The async/thread handling is sound: confirmation prompts on main thread, ChatListener receives events on main thread, and the destructive removePlayerData call properly delegates database work to async via runNotSync { transactional { ... } }.

dotman-plugin/src/main/java/net/minevn/dotman/database/mysql/PlayerDataDAOImpl.kt (1)

44-47: Looks good; uuid is already indexed via the UNIQUE constraint on (uuid, key) from the schema definition, so DELETE performance is covered.

The composite index on (uuid, key) (created in 0002.sql) will be used by the query optimizer for DELETE WHERE uuid = ? since uuid is the leftmost column.

Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
Comment thread dotman-plugin/src/main/java/net/minevn/dotman/commands/AdminCmd.kt
Copy link
Copy Markdown
Member

@amadeusmz amadeusmz left a comment

Choose a reason for hiding this comment

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

Tests

  • Hoạt động ok trên console và trong server
  • Dữ liệu trên DB sẽ bị xóa
  • Dữ liệu top nạp sẽ được xóa sau khi cache được cập nhật
Image Image

@amadeusmz amadeusmz merged commit 0432d84 into master Jan 11, 2026
3 checks passed
@amadeusmz amadeusmz deleted the minh/clear-player-milestone-data-cmd branch January 11, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants