Skip to content

fix(asset-library): show ACTIVE placements; document blocked-players 404 - #33

Merged
hanrw merged 4 commits into
mainfrom
fix/live-check-findings
Oct 11, 2026
Merged

hanrw merged 4 commits into
mainfrom
fix/live-check-findings

Conversation

@hanrw

@hanrw hanrw commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Why

Read-only checks against a live App Store Connect account after #32 found two problems.

What changed

  • Asset placements with state ACTIVE were dropped. App Store Connect sends ACTIVE, but neither its OpenAPI spec nor its docs list it. asc asset-placements list came back empty for images that are placed. ACTIVE is now a known state, treated as neither editable, in review nor live, so asc doesn't offer delete or reorderGroup for it.
  • blocked-players list gets 404 PATH_ERROR. The endpoint is in Apple's 4.5 spec and in the Game Center detail's own blockedPlayers link, but Apple doesn't serve it, with or without query parameters. Our request is correct, so this is documented as a Gotcha.

How it was verified

  • Failing tests first: the domain test for .active and an infrastructure test with the live response shape (red: type 'PlacementState' has no member 'active'). The state-names test now includes ACTIVE, because the spec it was written from is incomplete.
  • swift test passes.
  • Live, read-only: asset-placements list --image-id now returns the placement (ACTIVE, version localization).
  • Also confirmed live: asset-library get, asset-placement-groups list (50 groups including IPHONE_DUO_PROFILE) and asset-images list (73 images) decode correctly.

Checklist

  • A failing test came first; swift test passes
  • A command that returns data is also served by asc web-server
  • User-visible change: one line under ## [Unreleased] in CHANGELOG.md (n/a: fixes an unreleased feature; documented in the feature Gotchas)
  • make docs && make check-docs passes
  • No API key, .p8, issuer ID or Apple ID session in the diff, output or screenshots

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Asset placements reported as ACTIVE are now recognized and displayed with their status preserved.
    • Active placements do not offer delete or reorder-group actions in the app; those commands can still be run manually.
  • Documentation

    • Clarified that App Store Connect’s blocked-players list endpoint currently returns an error, and that player blocking and score moderation use separate endpoints.

hanrw and others added 2 commits October 10, 2026 23:34
Live App Store Connect returns placement state ACTIVE, which neither its
OpenAPI spec nor its docs list. asc dropped those placements as unknown,
so `asset-placements list` came back empty for images that are placed.
Treat ACTIVE as neither editable, in review nor live.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…point

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 31 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3d5c7b46-6079-4aee-b169-6e92b916f119

📥 Commits

Reviewing files that changed from the base of the PR and between 7b4293c and 8f62d0e.


📒 Files selected for processing (4)
  • Sources/Infrastructure/Apps/AssetLibrary/Images/SDKLibraryImageRepository.swift
  • Sources/Infrastructure/Apps/AssetLibrary/Placements/SDKAssetPlacementRepository.swift
  • Sources/Infrastructure/Apps/AssetLibrary/Videos/SDKLibraryVideoRepository.swift
  • Sources/Infrastructure/Shared/SDKFilter.swift

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9638847d-02c0-468a-8ba8-3b6d9fa275c9


📥 Commits

Reviewing files that changed from the base of the PR and between 6fb5ffb and 7b4293c.



📒 Files selected for processing (5)
  • Sources/Domain/Apps/AssetLibrary/Placements/AssetPlacement.swift
  • Tests/DomainTests/Apps/AssetLibrary/Placements/AssetPlacementTests.swift
  • Tests/InfrastructureTests/Apps/AssetLibrary/Placements/SDKAssetPlacementRepositoryTests.swift
  • docs/features/asset-library/README.md
  • docs/features/game-center/README.md


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




📝 Walkthrough
📝 Walkthrough

Walkthrough

The placement state model now includes ACTIVE, with tests for its properties and repository decoding. The documentation records this placement state and a separate Game Center endpoint response.

Changes

Asset Placement State

Layer / File(s) Summary
Add and verify ACTIVE placement state
Sources/Domain/Apps/AssetLibrary/Placements/AssetPlacement.swift, Tests/DomainTests/Apps/AssetLibrary/Placements/AssetPlacementTests.swift, Tests/InfrastructureTests/Apps/AssetLibrary/Placements/SDKAssetPlacementRepositoryTests.swift, docs/features/asset-library/README.md
PlacementState adds the ACTIVE raw value. Tests cover its state properties and repository decoding. The documentation notes that the CLI displays these placements without delete or reorderGroup affordances.

Game Center API Note

Layer / File(s) Summary
Document blocked-players list response
docs/features/game-center/README.md
The documentation notes that blocked-players list receives 404 PATH_ERROR and that player blocking and score moderation use separate endpoints.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix



Merge Risk: ⚪ Minimal · up to 7b429

The change adds support for ACTIVE placements and documents the reported Game Center response. Apple’s acceptance of manually invoked ACTIVE operations remains unknown, but no concrete merge-blocking impact is established.

Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check Passed The title clearly summarizes both primary changes: handling ACTIVE asset placements and documenting the blocked-players 404 response.
Description check Passed The description includes the required Why, What changed, How it was verified, and Checklist sections. It documents the two changes and verification results clearly. The issue link remains unspecified,…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
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.



✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR



🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR



  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

CI and release builds use Swift 6.2.4, which can't infer `.init(rawValue:)`
inside `value.flatMap { .init(…) }.map { [$0] }` (Swift 6.4 can), so main
has failed to build on CI since #32. Route the eight filter parameters
through a generic `sdkFilter(_:)` whose SDK type comes from the parameter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.54%. Comparing base (79d213a) to head (8f62d0e).
⚠️ Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
...brary/Placements/SDKAssetPlacementRepository.swift 50.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
+ Coverage   82.04%   82.54%   +0.49%     
==========================================
  Files         492      518      +26     
  Lines       14362    15649    +1287     
==========================================
+ Hits        11784    12918    +1134     
- Misses       2578     2731     +153     
Files with missing lines Coverage Δ
.../Apps/AssetLibrary/Placements/AssetPlacement.swift 82.85% <ø> (ø)
...ssetLibrary/Images/SDKLibraryImageRepository.swift 97.50% <100.00%> (ø)
...ssetLibrary/Videos/SDKLibraryVideoRepository.swift 98.66% <100.00%> (ø)
Sources/Infrastructure/Shared/SDKFilter.swift 100.00% <100.00%> (ø)
...brary/Placements/SDKAssetPlacementRepository.swift 91.53% <50.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hanrw
hanrw merged commit f65710b into main Oct 11, 2026
5 checks passed
@hanrw
hanrw deleted the fix/live-check-findings branch October 11, 2026 04:03
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