Skip to content

GH-64 feat: Admin GUI for parcel, locker & user management#218

Merged
Jakubk15 merged 21 commits into
masterfrom
feature/admin-gui
Jun 22, 2026
Merged

GH-64 feat: Admin GUI for parcel, locker & user management#218
Jakubk15 merged 21 commits into
masterfrom
feature/admin-gui

Conversation

@Jakubk15

Copy link
Copy Markdown
Member

Closes #64.

Summary

Adds an Admin GUI (/parcellockers admin, gated by the existing parcellockers.admin permission) for full in-game management of parcels, lockers, and users, plus bulk actions.

  • Parcels — paginated list → per-field editor for name, description, priority, size, status, receiver, destination locker, and item contents. Receiver/destination use paginated picker GUIs; contents use an item-grid editor sized to the parcel.
  • Lockers — list → edit (rename via Dialog, teleport to position, delete).
  • Users — list → inspect a user's parcels.
  • Bulk — delete-all parcels / delete-all lockers, behind confirmation dialogs.

Text input and confirmations use the Paper Dialog API.

Safety invariants (per the feature requirements)

  • No data loss on resizeAdminParcelService#changeSize rejects a size change if the parcel's existing contents wouldn't fit the smaller capacity (SMALL=9 / MEDIUM=18 / LARGE=27 slots). The GUI routes every size change through the service; there is no direct-write path.
  • Graceful priority re-timing — toggling priority shifts the delivery timestamp by the duration delta (shorten when enabling, lengthen when disabling), clamped to never land before now. Persisted via a new delivery upsert path (never the throwing create).
  • Destination-full guard — changing destination rejects if the target locker is full.

In-transit parcel safety

ParcelSendTask was refactored to re-fetch parcel + delivery state at fire time (pure decide(...) → DELIVER / RESCHEDULE / ABORT) instead of acting on the snapshot captured at schedule time, so admin edits to an in-transit parcel are honored rather than clobbered. The ABORT path cleans up and logs stray-delivery deletion failures.

Supporting changes

  • New upsert/query paths: LockerRepository#update + LockerManager#rename, ParcelContentRepository#update + ParcelContentManager#update, DeliveryRepository#update + DeliveryManager#update, ParcelRepository#findPage + ParcelService#getAll.
  • Config: ~29 new GuiSettings fields and 11 new AdminMessages notices (all user-facing text via MessageConfig).
  • No DDL/schema changes — all new writes are ORMLite upserts on existing tables.

Review fixes (from final whole-branch review)

  • Content cascade on deleteParcelService.delete/deleteAll now also delete the parcel's ParcelContent rows (mirroring the collect/rollback cleanup), closing an orphaned-row leak that the new one-click delete/delete-all buttons would otherwise have made reachable at scale.
  • MiniMessage injection — player-set parcel/locker names are now escapeTags-escaped before interpolation into delete-confirmation dialog titles, closing a markup-injection vector (Dialog click events execute with the clicking admin's permissions).

Testing

  • AdminParcelServiceTest — size-fit/capacity, priority delta-shift direction & now-clamp.
  • ParcelSendTaskTestdecide(...) totality across parcel/delivery/time combinations.
  • ParcelFindPageIntegrationTest, ParcelContentUpdateIntegrationTest (Testcontainers/MySQL, auto-skipped without Docker).
  • ./gradlew compileJava, ./gradlew test, and ./gradlew shadowJar all BUILD SUCCESSFUL.

Known follow-ups (non-blocking, tracked in review notes)

  • Delivery rows for admin-deleted in-transit parcels self-heal when their scheduled task fires (ABORT); a server restart before that fire could leak the bounded row — pre-existing, also affects the existing delete command.
  • No service-level automated test yet locks in the delete content-cascade (no Mockito / no service-level harness in the suite; repo-level content delete is covered).

🤖 Generated with Claude Code

Jakubk15 and others added 18 commits June 21, 2026 18:06
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16
- ParcelRepository#findPage(Page): CompletableFuture<PageResult<Parcel>>
- ParcelRepositoryOrmLite#findPage: delegates to queryPage with identity builder
- ParcelService#getAll(Page): CompletableFuture<PageResult<Parcel>>
- ParcelServiceImpl#getAll: calls findPage, warms parcelsByUuid cache
- GuiManager#getAllParcels(Page): delegates to parcelService.getAll
- ParcelFindPageIntegrationTest: TDD test (skipped without Docker)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16
…last-page termination

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16
…y/destination invariants

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16
…n delete dialogs

Final-review fixes:
- ParcelServiceImpl.delete(UUID)/deleteAll now also delete the parcel's
  ParcelContent rows, mirroring the collect/rollback cleanup paths. The admin
  delete and delete-all buttons previously orphaned content rows (an unbounded
  leak now that deletion is a one-click bulk operation).
- AdminParcelEditGui/AdminLockerEditGui escape player-set parcel/locker names
  with MiniMessage.escapeTags before interpolating them into delete-confirmation
  dialog titles, closing a markup-injection vector (Dialog click events execute
  with the clicking admin's permissions).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16

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

Copy link
Copy Markdown
Contributor

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 introduces a comprehensive in-game Admin GUI for managing parcels, lockers, and users, backed by a new AdminParcelService and a refactored ParcelSendTask to prevent stale-snapshot overwrites. Feedback focuses on critical thread-safety violations in Bukkit, specifically firing events asynchronously in ParcelSendTask, handling callbacks asynchronously in AdminParcelEditGui, and populating GUI items asynchronously in AdminDestinationPickerGui. Additionally, the reviewer noted that the ability to view sent parcels is currently inaccessible due to showSent being hardcoded to false in AdminUserListGui.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Addresses PR review (gemini-code-assist): AdminUserInspectGui already had a
showSent code path (getParcelsBySender) but it was unreachable — AdminUserListGui
hardcoded showSent=false and there was no in-GUI control. The design doc requires
inspecting a user's received AND sent parcels, so add a toggle button (slot 48)
that flips the mode and reopens, backed by a new configurable
adminToggleSentReceivedButton item.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmSMZXMtfG9F2HBtrPXU16
@Jakubk15

Copy link
Copy Markdown
Member Author

Review triage (gemini-code-assist)

Thanks for the review. Verified each finding against the actual runtime threading model and the codebase; decisions below.

❌ Rejected — ParcelSendTask callEvent "must be on the primary thread" (critical)

ParcelDeliverEvent is an asynchronous Bukkit event — ParcelDeliverEvent extends CancellableEvent with super(true), which propagates to Event(boolean isAsync). Paper's contract is the inverse of the suggestion: an async event fired from the primary thread throws IllegalStateException: Asynchronous event ... cannot be triggered synchronously from primary server thread. Wrapping the DELIVER case in scheduler.run(...) would therefore break delivery at runtime. The single listener (DiscordDeliverNotificationController) touches no Bukkit API — only Discord services and CompletableFutures — so firing it off-thread is correct and is also what master did pre-refactor. No change.

❌ Rejected — async GUI population in AdminParcelEditGui / AdminDestinationPickerGui (high)

The only Bukkit-thread-sensitive operation here — opening / mutating the live inventory — already runs on the main thread: every admin GUI wraps gui.open(player) in scheduler.run(...). The flagged setItem/addItem calls do not touch the live inventory: in triumph-gui 3.1.13 BaseGui.setItem is just guiItems.put(slot, item) on an internal Map (verified in the bytecode), and the map is rendered into the inventory later inside open()populateGui() on the main thread. The remaining off-thread work (ItemStack construction, NoticeService send) is thread-safe and is the established pattern across the entire plugin's GUIs. So there is no live-inventory mutation off the main thread. No change.

✅ Accepted — sent/received parcels unreachable in AdminUserInspectGui (medium)

Correct and a real spec gap: the design doc requires inspecting a user's received and sent parcels, but showSent was hardcoded false with no UI to flip it. Fixed in 23a7b64 by adding a configurable toggle button (adminToggleSentReceivedButton, slot 48) that switches mode and reopens — both getParcelsByReceiver and getParcelsBySender paths are now reachable.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Thanks for the detailed breakdown and verification, Jakubk15. Your analysis regarding the asynchronous nature of the Bukkit event and the thread-safety of the triumph-gui map operations is spot on. It's clear that the original concerns were based on a misunderstanding of the specific runtime threading model here. The adjustments made to the ParcelSendTask and the implementation of the AdminParcelService correctly address the safety invariants required for the admin GUI. The inclusion of the design spec and the comprehensive integration tests is excellent practice for maintaining this level of complexity. This looks solid.

@Jakubk15

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown

Claude finished @Jakubk15's task in 2s —— View job


I'll analyze this and get back to you.

@Jakubk15 Jakubk15 changed the title feat: Admin GUI for parcel, locker & user management (#64) GH-64 feat: Admin GUI for parcel, locker & user management Jun 22, 2026
@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown

Claude finished @Jakubk15's task in 2s —— View job


I'll analyze this and get back to you.

Comment thread src/main/java/com/eternalcode/parcellockers/gui/GuiManager.java Outdated
Comment thread src/main/java/com/eternalcode/parcellockers/gui/GuiManager.java Outdated
@Jakubk15 Jakubk15 merged commit afa2d08 into master Jun 22, 2026
1 check passed
@Jakubk15 Jakubk15 deleted the feature/admin-gui branch June 22, 2026 16:34
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.

Admin GUI (deleting, overwriting parcels, parcel lockers management and modifying them)

1 participant