Skip to content

feat: add /deco give to credit a bank or pouch - #29

Merged
ryanbarlow97 merged 1 commit into
mainfrom
feat/deco-give
Sep 27, 2026
Merged

ryanbarlow97 merged 1 commit into
mainfrom
feat/deco-give

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds a console/operator command to credit denars to a player:

/deco give <player> <amount> [bank|pouch]
  • The bank is the default account.
  • It works whether the player is online or offline. Payments go through OfflineModifier, the same path BarterShops and vehicle upkeep already use. An online player is credited in their live session. An offline player is credited in their saved PlayerData/<uuid>.json, and a failed save leaves the balance unchanged.
  • Player names are resolved by OfflineModifier.playerId (online players, Bukkit's cache, usercache.json and remembered names). An unknown name is refused rather than creating a new account.
  • The amount must be positive and in whole cents, the same rule as deposits and withdrawals.
  • Every payment is logged, for example CONSOLE gave 12.50 to Alex (<uuid>) bank. An online recipient is told they received it.
  • New permission denareconomy.give, op by default. Console always passes, as with reload.
  • Tab completion suggests give to anyone allowed to use it, then <player>, <amount> and bank|pouch.
  • New messages are added to the bundled messages.yml. Servers with an older messages.yml still get them through the existing fallback to bundled defaults.

Why

The live drops.yml used carrot, potato and beetroot instead of the block names carrots, potatoes and beetroots, so those crops never dropped coins. About 144 players are owed a refund. Until now there was no way to credit an account without hand-editing the JSON files, and that edit is lost if the player is online.

Tests

  • mvn clean verify: 213 tests pass, and JaCoCo's 100% line, branch and instruction check still passes.
  • CommandManagerTest covers permissions, usage, amount and account validation, unknown players, both accounts, online notification, refused and failed payments, and tab completion.
  • GiveCommandOfflineTest runs the command against a real saved account with no server or session and checks that the file on disk changes.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added /deco give <player> <amount> [bank|pouch] to credit a player’s bank or pouch, including when they are offline. The bank is used by default.
    • Console and authorized operators can use the command. Senders and online recipients are notified when a credit succeeds.
  • Bug Fixes
    • Invalid inputs, unknown players, and failed payments are reported without changing the recipient’s balance.
    • Reload access now defaults to operators.

Console and operators can now pay a player with
/deco give <player> <amount> [bank|pouch]. The bank is the default.
Payments go through OfflineModifier, so online players are credited in
their live session and offline players in their saved account file.
Amounts must be positive whole cents. Each payment is logged, and an
online recipient is told about it. The new denareconomy.give permission
is op by default.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 0dd26df4-84eb-4f46-b8b8-442da460cede

📥 Commits

Reviewing files that changed from the base of the PR and between 32a5bf5 and fc10ff6.

📒 Files selected for processing (6)
  • README.md
  • src/main/java/net/tfminecraft/denareconomy/managers/CommandManager.java
  • src/main/resources/messages.yml
  • src/main/resources/plugin.yml
  • src/test/java/net/tfminecraft/denareconomy/managers/CommandManagerTest.java
  • src/test/java/net/tfminecraft/denareconomy/managers/GiveCommandOfflineTest.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Adds /deco give for console and authorized players to credit online or offline players’ bank or pouch accounts. The command validates input, reports payment results, and provides tab completion.

Changes

Admin credit command

Layer / File(s) Summary
Command access and exposure
src/main/java/net/tfminecraft/denareconomy/managers/CommandManager.java, src/main/resources/plugin.yml, README.md
Adds the give permission, shared admin authorization, console-capable command dispatch, and permission-gated tab completion. The README describes the command and its permission defaults.
Credit processing and results
src/main/java/net/tfminecraft/denareconomy/managers/CommandManager.java, src/main/resources/messages.yml, src/test/java/net/tfminecraft/denareconomy/managers/CommandManagerTest.java, src/test/java/net/tfminecraft/denareconomy/managers/GiveCommandOfflineTest.java
Validates arguments and account selection, applies credits through OfflineModifier, and reports success or failure. Tests cover authorization, validation, notifications, failures, completions, and offline balance changes.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Sender
  participant CommandManager
  participant OfflineModifier
  participant OnlineRecipient
  Sender->>CommandManager: Run /deco give with player, amount, and optional account
  CommandManager->>OfflineModifier: Apply credit to target UUID and account
  OfflineModifier-->>CommandManager: Return payment result
  CommandManager-->>Sender: Send success or failure message
  CommandManager-->>OnlineRecipient: Send received message when target is online
Loading

Merge Risk: ⚪ Minimal · up to fc10f

The credit command’s inspected online and failure paths do not show a balance or reporting defect. It is ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to fc10f

The command validates players and amounts, but its console-style authorization also accepts other non-player senders. An online credit is reported as successful before it is saved, which matters for refunds and recovery after a crash.

Retained concerns

  • Medium · security · inferred: The new credit operation treats every non-player command sender as authorized, rather than limiting the permission bypass to console. Its effective reach includes any other sender type the server permits to invoke the command.
  • Medium · reliability · inferred: An online grant is logged and reported as successful after an in-memory balance change, before persistence. A crash before the later player save can leave an acknowledged refund absent from the saved account.
Security review details

Security Blast Radius

  • inferred — A sender that passes the administrative check can credit any name that resolves to a UUID, across live sessions and offline player files. No tenant or environment boundary beyond this server’s account store is established by the inspected source.

Security Findings and Attack Paths

  • inferred — If a server permits a non-console, non-player sender to invoke give, that sender reaches the credit sink without holding denareconomy.give. Whether such a sender is controllable by an unprivileged player is not established by the repository evidence.

Trust Boundaries and Controls

  • observed — The execution handler, not just tab completion, enforces player permission. It rejects invalid amounts, account names, and unresolved player names before calling the account mutation path.

Resilience and Maintainability Implications

  • inferred — Failed offline saves have rollback protection, but neither an online success message nor a grant retry is coupled to a durable payment record. These are distinct recovery states for an operator reconciling credits.

Hardening Proposals

  • proposed — Specify which non-player sender types may authorize grants, and enforce that choice at execution. For bulk refunds, consider a durable grant identifier and reconciliation procedure so an acknowledged credit and an operator retry have unambiguous outcomes.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 3 files. (3 skipped: 3… 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 concisely describes the main change: adding /deco give to credit a player's bank or pouch.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 3 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit checks the bank and pouch,
Then sends the credit on its route.
For offline friends, the balances grow,
Success or failure lets them know.
The rabbit hops beneath the moon.

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

@ryanbarlow97
ryanbarlow97 merged commit 32ba3a3 into main Sep 27, 2026
2 checks passed
@ryanbarlow97
ryanbarlow97 deleted the feat/deco-give branch September 27, 2026 23:19
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.

1 participant