fix: ready to publish filter to apply only if all children are ready as well - #62
fix: ready to publish filter to apply only if all children are ready as well#62maikschneider wants to merge 1 commit into
Conversation
… publish as well (cherry picked from commit 3672925939c0717186697b27ffe6d0c8d3a6e594)
WalkthroughThe pull request modifies the Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Classes/Controller/AbstractBackendController.php (1)
837-846:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCached
_referencesToPublishis now a filtered subset, leaking into downstream consumers.Storing only the ready-stage children under
_referencesToPublishmeans the cache no longer represents "all workspace-modified children" — it now represents "ready children only" whenever theonlyReadyToPublishfilter is active. Two downstream callers reuse this cached value via$record['_referencesToPublish'] ?? $this->collectReferencesToPublish($record):
addWorkspaceMetadata()(Line 892): the workspace state badge /referencesToPublishexposed to the view will hide non-ready modified children for live parents whenever the filter is active. The status text (childrenModified) and thet3ver_stageaggregation at Line 897-900 will also be derived from the filtered subset, making the live parent always look like "all children ready" even when it has additional modified children that are not yet ready.onlyOfflineRecordsbranch (Line 858): if both filters are toggled on, this branch inherits the pre-filtered subset instead of the full set.Consider keeping the full result cached and only using the filtered subset for the gating decision, e.g.:
♻️ Proposed fix
if ($this::WORKSPACE_ID > 0 && !is_array($vRecord)) { $childRefs = $this->collectReferencesToPublish($record); $readyChildRefs = array_values(array_filter( $childRefs, fn (array $ref): bool => $ref['t3ver_stage'] === self::WORKSPACE_STAGE_READY_TO_PUBLISH )); - if ($readyChildRefs !== []) { - $record['_referencesToPublish'] = $readyChildRefs; - } else { + if ($readyChildRefs === []) { $record = null; continue; } + $record['_referencesToPublish'] = $childRefs; } else {If the intent really is to surface only the ready children to the view when this filter is active, please confirm — otherwise the metadata/state computed in
addWorkspaceMetadata()will silently change for live parents under this filter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Classes/Controller/AbstractBackendController.php` around lines 837 - 846, The cached $record['_referencesToPublish'] is being overwritten with only-ready children, leaking a filtered subset into downstream consumers; change the logic in the block that builds $readyChildRefs so that you still cache the full set returned by collectReferencesToPublish($record) (or a new variable like $allChildRefs) and use a separate filtered variable (e.g. $readyChildRefs) only for the gating decision that checks WORKSPACE_STAGE_READY_TO_PUBLISH; update references in addWorkspaceMetadata() and the onlyOfflineRecords branch to continue reading the unfiltered cached key ($record['_referencesToPublish']) while using the filtered $readyChildRefs only to decide whether to mark/skip the record when the onlyReadyToPublish filter is active.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Classes/Controller/AbstractBackendController.php`:
- Around line 837-846: The cached $record['_referencesToPublish'] is being
overwritten with only-ready children, leaking a filtered subset into downstream
consumers; change the logic in the block that builds $readyChildRefs so that you
still cache the full set returned by collectReferencesToPublish($record) (or a
new variable like $allChildRefs) and use a separate filtered variable (e.g.
$readyChildRefs) only for the gating decision that checks
WORKSPACE_STAGE_READY_TO_PUBLISH; update references in addWorkspaceMetadata()
and the onlyOfflineRecords branch to continue reading the unfiltered cached key
($record['_referencesToPublish']) while using the filtered $readyChildRefs only
to decide whether to mark/skip the record when the onlyReadyToPublish filter is
active.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2642448e-1744-44d4-ac40-3cb3329c0713
📒 Files selected for processing (1)
Classes/Controller/AbstractBackendController.php
1 participant
Summary by CodeRabbit