Skip to content

Parcel return follow-up cleanups (from #232 review) #233

Description

@Jakubk15

Follow-up cleanups from the #232 (parcel return, GH-69) review. None are blockers; all were triaged as follow-up material by the final whole-branch review.

Correctness hardening

  • AdminParcelService.changeStatus validates the in-memory GUI snapshot: an admin edit GUI opened while a parcel was DELIVERED can race a live collection and still resurrect a COLLECTED parcel (unconditional update + armDelivery). Make the status change a conditional UPDATE (WHERE status = <expected>), mirroring markCollected/markReturned, or re-fetch inside changeStatus.
  • Re-collect after a failed best-effort collected_parcels row delete inherits the old timestamp (save is insert-if-absent), shortening the second return window. Consider upsert semantics.

Cleanup / UX

  • ReturnWindowPurgeTask deletes parcel + content + collected row but leaves a stale deliveries row when the original cleanup failed; inert but leaks. Add a deliveryManager.delete.
  • returnFeeWithdrawn notice is sent before the commit point; if markReturned loses the race the refund is silent. Move the notice after the flip or add a refund notice.
  • insufficientFunds message reads "…to send this parcel" when triggered by a return fee. Add a dedicated notice or neutral wording.
  • ReturnGui.show: dead result == null check copied from CollectionGui.

Tests / docs / plumbing

  • ReturnItemEquivalenceTest: add name/lore relaxed-flag coverage.
  • ParcelReturnValidator: javadoc the transitive-equivalence assumption the multiset matcher relies on.
  • New-code logging uses getMessage() only; switch to LOGGER.log(Level.SEVERE, msg, throwable) for stack traces.
  • returnWindow is constructor-threaded through LockerGui/SendingGui/ItemStorageGui; consider passing PluginConfig or a settings view instead.

🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions