fix(steam): persist ReleaseDate from SteamReleaseDate on import (#156) - #160
fix(steam): persist ReleaseDate from SteamReleaseDate on import (#156)#160mforce wants to merge 2 commits into
Conversation
SteamStoreImportService.NewImportedGame derived only the release Year from Steam's SteamReleaseDate; the full Game.ReleaseDate (added in #88) stayed null until manual edit or a provider backfill. Mirror the Year line with the existing ToDateOnly helper so the UTC calendar date lands on ReleaseDate. The issue's suggested line (FromUnixTimeSecondsOrNull(...)?.UtcDateTime.Date) was dropped: ReleaseDate is DateOnly?, and .UtcDateTime.Date is DateTime - no implicit conversion, would not compile. ToDateOnly(long) is the existing pattern, already used for LastPlayedOn. Regression test: Import_CapturesRichMetadataFromStoreBrowse now asserts ReleaseDate == 2021-01-01 from SteamReleaseDate=1609459200. Red before the fix (Actual: null), green after. Closes #156
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9138ced129
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // SteamReleaseDate is a unix timestamp with day granularity; land its UTC | ||
| // calendar date on the full ReleaseDate field (#156), not just the | ||
| // derived Year above. Null when Steam omits a release date (0 / invalid). |
There was a problem hiding this comment.
This block merely paraphrases the immediately following SteamReleaseDate-to-ReleaseDate assignment and its null behavior rather than explaining non-inferable intent, violating the repository convention against “what” comments; the analogous comment above the test assertion should be removed as well.
AGENTS.md reference: AGENTS.md:L194-L194
Useful? React with 👍 / 👎.
Review follow-up: Import_ProceedsWhenStoreBrowseMetadataUnavailable asserted Developer/Publisher/Year/Description were null but not ReleaseDate. Pin it so a future change that set ReleaseDate in the no-metadata path is caught. Mutation M2 (ReleaseDate defaults to unix-1 in the no-metadata path) went red exactly at this new Assert.Null (Actual: 1/1/1970), confirming the pin is live.
1 participant
Closes #156
What
SteamStoreImportService.NewImportedGamederived only the release Year from Steam'smeta.Release.SteamReleaseDate; the full nullableGame.ReleaseDatefield (added in #88) stayednulluntil manual edit or a provider backfill. This lands the UTC calendar date onReleaseDate.Fix
Reuses the existing
ToDateOnly(long)helper (already used forLastPlayedOn).Game.ReleaseDateisDateOnly?;ToDateOnlyyieldsDateOnly?and degrades tonullwhen Steam omits the date (unix 0 / invalid).Test
Import_CapturesRichMetadataFromStoreBrowsenow assertsReleaseDate == new DateOnly(2021, 1, 1)fromSteamReleaseDate = 1609459200.Assert.Equal() Failure — Expected 1/1/2021, Actual: null(exact Steam import: persist ReleaseDate from SteamReleaseDate #156 symptom).Import_ProceedsWhenStoreBrowseMetadataUnavailable) stays green → ReleaseDate remains null when no metadata (positive control).Verification
Collectify.Tests: 623 passed / 0 failed.ReleaseDateline, comment-only so it compiles): named test goes red at line 275 (Expected 1/1/2021, Actual: null), restored, rebuild clean.Collectify.PostgresTests(9 failures) = Docker Testcontainers unavailable here (/var/run/docker.sock) — environment-only, pre-existing, unrelated to this server change.Server-only change; no client/enum-parity impact.
Phase 13: merge is the caller's ⛔ gate.---
Independent review (feature-driver round)
Independent reviewer on the regression test + fix, head
9138ced: no merge-blocking findings. One FOLLOW-UP pinned and applied:Import_ProceedsWhenStoreBrowseMetadataUnavailablenow assertsReleaseDateis null (previously only Year/Developer/Publisher/Description). Mutation M2 (ReleaseDate defaulted to unix-1 in the no-metadata path) goes red at that exact assertion (Actual: 1/1/1970), proving the pin is live.New head:
349ddc0. Full suite 623/623, Release build 0 errors.