Skip to content

Persist listedPrice in offer JSON export#76

Open
rl-enjoyer wants to merge 1 commit intoFlipping-Utilities:masterfrom
rl-enjoyer:persist-listed-price
Open

Persist listedPrice in offer JSON export#76
rl-enjoyer wants to merge 1 commit intoFlipping-Utilities:masterfrom
rl-enjoyer:persist-listed-price

Conversation

@rl-enjoyer
Copy link
Copy Markdown

@rl-enjoyer rl-enjoyer commented Mar 24, 2026

Summary

  • Remove transient from listedPrice in OfferEvent and add @SerializedName("lp") so it's included in the JSON export.
  • The existing price field ("p") stores the average fill price, which is 0 when nothing has filled. External tools reading lastOffers from the JSON file currently have no way to know what price an unfilled offer is listed at.
  • This is a one-field change with no behavioral impact on the plugin itself — listedPrice is already populated from GrandExchangeOffer.getPrice() in fromGrandExchangeEvent() and used in SlotPanel.

Test

  • Added OfferEventSerializationTest with two cases:
    • Round-trip: serialize an offer with listedPrice=136, deserialize, verify it survives
    • Backward compat: deserialize old JSON without "lp" field, verify it defaults to 0
  • Registered in TestRunner
  • Could not run tests locally (Gradle 6.6.1 + Java 21 incompatibility), but the change is minimal — just removing transient and adding @SerializedName

Backward compatibility

  • Old JSON files without "lp" deserialize to 0 (Java int default), same as today.
  • lastOffers entries are overwritten on login when GE slot events fire, so existing users pick up the field automatically.

Use case

Building a market scanner tool that reads the plugin's JSON export to give hold/cut advice on open positions. Without the listed price, we can't tell if an unfilled sell offer is priced above market — which is the most common reason a sell isn't filling.

Summary by CodeRabbit

  • Improvements

    • Offer listed prices are now persisted and included in data exports, allowing external tools to access this information for all offer states, including unfilled slots.
  • Tests

    • Added serialization tests to verify listed prices are correctly preserved during data export and import cycles, including backward compatibility with legacy data.

The listedPrice field (the user's GE offer price) was marked transient,
so it was lost on serialization. External tools reading lastOffers from
the JSON export had no way to know the offer price for unfilled slots
(the existing "p" field only records avg fill price, which is 0 when
nothing has filled).

Remove transient and add @SerializedName("lp") so the listed price is
written to disk. Backward compatible — old files without "lp" deserialize
to 0 (same as today), and lastOffers is overwritten on login anyway.

Also add OfferEventSerializationTest with round-trip and backward compat
test cases, and register in TestRunner.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 24, 2026

📝 Walkthrough

Walkthrough

The listedPrice field in OfferEvent is converted from a transient (non-persisted) field to a persisted field annotated with @SerializedName("lp"), allowing external tools to access offer listed price data via JSON serialization. Tests validate round-trip serialization and backward compatibility.

Changes

Cohort / File(s) Summary
Model Serialization
src/main/java/com/flippingutilities/model/OfferEvent.java
Removed transient modifier from listedPrice field and added @SerializedName("lp") annotation to enable JSON persistence of listed price values.
Serialization Tests
src/test/java/com/flippingutilities/OfferEventSerializationTest.java
New test class with two test methods: listedPriceSurvivesRoundTrip validates JSON round-trip serialization, and oldJsonWithoutListedPriceDefaultsToZero verifies backward compatibility by ensuring missing lp field defaults to zero.
Test Suite Configuration
src/test/java/com/flippingutilities/TestRunner.java
Added OfferEventSerializationTest.class to the @Suite.SuiteClasses annotation to include the new serialization tests in the test runner.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A field once hidden, now revealed in code,
Through JSON's gentle flow, the price is showed,
Old data gracefully defaults to zero's grace,
Tests ensure the path from bytes to place! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 'Persist listedPrice in offer JSON export' directly and clearly describes the main change: making listedPrice persist in JSON exports by removing the transient modifier and adding @SerializedName annotation.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
src/test/java/com/flippingutilities/OfferEventSerializationTest.java (1)

71-71: Use the static JsonParser.parseString() API on line 71 for consistency with current Gson best practices.

The instance method new JsonParser().parse(json) has been deprecated since Gson 2.8.6 in favor of the static JsonParser.parseString(json) method.

♻️ Proposed update
-		JsonObject obj = new JsonParser().parse(json).getAsJsonObject();
+		JsonObject obj = JsonParser.parseString(json).getAsJsonObject();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test/java/com/flippingutilities/OfferEventSerializationTest.java` at line
71, Replace the deprecated instance usage new
JsonParser().parse(json).getAsJsonObject() in OfferEventSerializationTest with
the static API JsonParser.parseString(json).getAsJsonObject(): locate the line
creating JsonObject obj and change the call to JsonParser.parseString(json) so
the test uses the current Gson best-practice static method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/test/java/com/flippingutilities/OfferEventSerializationTest.java`:
- Line 71: Replace the deprecated instance usage new
JsonParser().parse(json).getAsJsonObject() in OfferEventSerializationTest with
the static API JsonParser.parseString(json).getAsJsonObject(): locate the line
creating JsonObject obj and change the call to JsonParser.parseString(json) so
the test uses the current Gson best-practice static method.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b88bfeab-d34f-47dd-9171-d1296984ad13

📥 Commits

Reviewing files that changed from the base of the PR and between ea51a0d and b61974e.

📒 Files selected for processing (3)
  • src/main/java/com/flippingutilities/model/OfferEvent.java
  • src/test/java/com/flippingutilities/OfferEventSerializationTest.java
  • src/test/java/com/flippingutilities/TestRunner.java

@rl-enjoyer
Copy link
Copy Markdown
Author

(comment by Claude on behalf of @rl-enjoyer)

Good catch on the deprecated JsonParser API — however the test file was added here as supporting evidence for the listedPrice change, and it can't actually compile on the current build config (Gradle 6.6.1 / Java 8). That build modernization is addressed separately in #77. Happy to fix the deprecation once #77 lands and the tests are runnable. Keeping this PR focused on the one-field transient@SerializedName change.

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