[8793] Do not allow a user to select a soft publishing dependency for publish that they are not allowed to publish - #8893
Conversation
WalkthroughThe publish workflow now checks item publish permissions. Soft dependency checkboxes show localized explanations when disabled. Hard dependency permission failures and dependency fetch failures disable submission and display separate alerts. ChangesPublish permission controls
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx`:
- Around line 141-145: Track dependency validation failure separately from
dependencyData in PublishDialogContainer, preserving that state when the
calculation error handler clears dependencyData and isFetchingItems. Update
hardDependenciesWithoutPublishPermission and the submission guard around the
existing dependency checks so submission remains disabled unless dependency
validation completed successfully. Update the alert logic to show an explicit
error that dependency validation could not complete instead of hiding it when
dependencyData is unavailable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c37877da-e452-48f5-8121-fc71baffa9e9
📒 Files selected for processing (4)
studio-ui/ui/app/src/components/PackageItems/utils.tsxstudio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsxstudio-ui/ui/app/src/components/PublishDialog/PublishPackageItemsView.tsxstudio-ui/ui/app/src/models/Item.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx (1)
519-532: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winShow validation errors in first-publish mode.
These alerts are rendered only when
publishedis truthy. Whenpublishedisfalseornull, the component renders only the first-publish message, whilesubmitDisabledremains true for dependency validation failures and permission failures.Move these alerts outside the
publishedconditional, or render them in the first-publish branch. Otherwise, users can be blocked without seeing the reason.🤖 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 `@studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx` around lines 519 - 532, Update the conditional rendering around packageDependenciesFetchFailed and hardDependenciesWithoutPublishPermission in PublishDialogContainer so both validation alerts render when published is false or null as well as when it is truthy. Move them outside the published-only branch or include them in the first-publish branch, preserving submitDisabled behavior and preventing users from being blocked without an explanation.
🤖 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.
Inline comments:
In `@studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx`:
- Around line 142-152: Update the dependency validation flow around
dependencyData and dependencyValidationIncomplete so results from a previous
publishing target or dependency input are not treated as current during
revalidation. Associate dependencyData with the inputs used to calculate it, or
clear it and account for the in-flight request, and keep submission disabled
until the current dependency calculation succeeds.
- Around line 235-239: Update handleSubmit in PublishDialogContainer so it
returns immediately after preventDefault() when submitDisabled is true,
preventing form submission via Enter when dependency validation is incomplete or
hard dependencies lack publish permission. Keep the existing submission flow
unchanged when submission is enabled.
---
Outside diff comments:
In `@studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx`:
- Around line 519-532: Update the conditional rendering around
packageDependenciesFetchFailed and hardDependenciesWithoutPublishPermission in
PublishDialogContainer so both validation alerts render when published is false
or null as well as when it is truthy. Move them outside the published-only
branch or include them in the first-publish branch, preserving submitDisabled
behavior and preventing users from being blocked without an explanation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3976e622-2b15-4159-9f3a-6387c586e656
📒 Files selected for processing (1)
studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx
|
@coderabbitai how does this work look to be merged? Any issues you can point out? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR diff ==='
git diff --find-renames origin/master...HEAD -- \
studio-ui/ui/app/src/components/PackageItems/utils.tsx \
studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx \
studio-ui/ui/app/src/components/PublishDialog/PublishPackageItemsView.tsx \
studio-ui/ui/app/src/models/Item.ts
echo '=== Changed file outlines ==='
for f in \
studio-ui/ui/app/src/components/PackageItems/utils.tsx \
studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx \
studio-ui/ui/app/src/components/PublishDialog/PublishPackageItemsView.tsx \
studio-ui/ui/app/src/models/Item.ts
do
echo "--- $f ---"
ast-grep outline "$f" --items all || true
done
echo '=== Publish-related tests ==='
fd -t f . studio-ui/ui/app | rg -i '(publish.*(test|spec)|package.*(test|spec)|(test|spec).*publish)'
echo '=== Permission properties: declarations, producers, and consumers ==='
rg -n -C 3 --glob '*.{ts,tsx}' \
'\b(canRequestPublish|canApprove)\b' \
studio-ui/ui/app/srcLength of output: 1412 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Current branch and refs ==='
git status --short --branch
git branch -a
git remote -v
git show-ref --heads --remotes | sed -n '1,160p'
base=""
for candidate in \
origin/main origin/develop origin/next origin/master \
main develop next master
do
if git rev-parse --verify -q "$candidate" >/dev/null; then
base="$candidate"
break
fi
done
if [ -z "$base" ]; then
echo 'ERROR: No conventional base ref is available.'
echo '=== Recent commits ==='
git log --oneline -n 20
exit 0
fi
merge_base="$(git merge-base "$base" HEAD)"
echo "=== Selected base: $base ==="
echo "=== Merge base: $merge_base ==="
echo '=== Changed paths ==='
git diff --name-status "$merge_base"...HEAD
echo '=== Targeted diff ==='
git diff --find-renames "$merge_base"...HEAD -- \
studio-ui/ui/app/src/components/PackageItems/utils.tsx \
studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx \
studio-ui/ui/app/src/components/PublishDialog/PublishPackageItemsView.tsx \
studio-ui/ui/app/src/models/Item.ts
echo '=== Changed file outlines ==='
for f in \
studio-ui/ui/app/src/components/PackageItems/utils.tsx \
studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx \
studio-ui/ui/app/src/components/PublishDialog/PublishPackageItemsView.tsx \
studio-ui/ui/app/src/models/Item.ts
do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" --items all || true
fi
done
echo '=== Publish and package test files ==='
fd -t f . studio-ui/ui/app | rg -i '(publish|package).*\.(test|spec)\.[cm]?[jt]sx?$|(\.test|\.spec)\.[cm]?[jt]sx?$'
echo '=== Permission properties: declarations, producers, and consumers ==='
rg -n -C 3 --glob '*.{ts,tsx}' \
'\b(canRequestPublish|canApprove)\b' \
studio-ui/ui/app/srcLength of output: 3661 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="origin/develop"
merge_base="$(git merge-base "$base" HEAD)"
echo "=== Merge base: $merge_base ==="
echo '=== Changed paths ==='
git diff --name-status "$merge_base"...HEAD
echo '=== Targeted diff ==='
git diff --find-renames "$merge_base"...HEAD -- \
studio-ui/ui/app/src/components/PackageItems/utils.tsx \
studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx \
studio-ui/ui/app/src/components/PublishDialog/PublishPackageItemsView.tsx \
studio-ui/ui/app/src/models/Item.ts
echo '=== Source: PackageItems utils ==='
sed -n '1,280p' studio-ui/ui/app/src/components/PackageItems/utils.tsx
echo '=== Source: PublishDialog container ==='
sed -n '1,420p' studio-ui/ui/app/src/components/PublishDialog/PublishDialogContainer.tsx
echo '=== Source: Publish package-items view ==='
sed -n '1,300p' studio-ui/ui/app/src/components/PublishDialog/PublishPackageItemsView.tsx
echo '=== LightItem interface ==='
rg -n -A70 -B10 --glob 'Item.ts' '\b(interface|type)\s+LightItem\b' studio-ui/ui/app/src/models
echo '=== Permission properties: declarations, producers, and consumers ==='
rg -n -C 4 --glob '*.{ts,tsx}' \
'\b(canRequestPublish|canApprove)\b' \
studio-ui/ui/app/src
echo '=== Candidate test files ==='
fd -t f . studio-ui/ui/app | rg -i '(publish|package).*\.(test|spec)\.[cm]?[jt]sx?$|(\.test|\.spec)\.[cm]?[jt]sx?$'Length of output: 49724 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== calculatePackage definition and response types ==='
rg -n -C 12 --glob '*.{ts,tsx}' '\bcalculatePackage\b' studio-ui/ui/app/src
echo '=== Publishing service source ==='
service_file="$(fd -t f '^publishing\.(ts|tsx)$' studio-ui/ui/app/src/services | head -n 1 || true)"
if [ -n "$service_file" ]; then
echo "--- $service_file ---"
sed -n '1,360p' "$service_file"
fi
echo '=== Server-side calculate-package endpoints and permission fields ==='
rg -n -i -C 10 \
'(calculate.?package|calculatePackage|includeSoftDeps|canRequestPublish|canApprove)' \
--glob '*.{java,kt,groovy,xml,json,yml,yaml}' \
.
echo '=== Test directories and files near the changed components ==='
fd -t d -i '(test|tests|__tests__)' studio-ui/ui/app | sed -n '1,160p'
fd -t f -e ts -e tsx -e js -e jsx studio-ui/ui/app | rg -i \
'(/|^)(publishdialog|publishpackageitemsview|packageitems).*(test|spec)?|((test|spec).*(publishdialog|publishpackageitemsview|packageitems))'
echo '=== Test framework configuration ==='
fd -t f -i 'package.json' studio-ui/ui/app -x sh -c 'echo "--- $1 ---"; sed -n "1,240p" "$1"' sh {}Length of output: 30200 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== CalculatedPublishPackageResult declaration ==='
result_file="$(fd -t f -e java 'CalculatedPublishPackageResult' studio | head -n 1 || true)"
if [ -n "$result_file" ]; then
echo "--- $result_file ---"
cat -n "$result_file"
fi
echo '=== Server LightItem declarations ==='
fd -t f -e java '^LightItem\.java$' studio | while IFS= read -r f; do
echo "--- $f ---"
cat -n "$f"
done
echo '=== Server package-calculation implementation ==='
rg -n -C 15 --glob '*.java' \
'\b(calculatePublishPackage|recalculatePublishPackage)\s*\(' \
studio/src
echo '=== Server permission checks used during publishing ==='
rg -n -i -C 8 --glob '*.java' \
'(request.?publish|publish.?permission|canRequest|Permission.*publish|publish.*Permission)' \
studio/src/main/java/org/craftercms/studio
echo '=== Frontend test files ==='
fd -t f . studio-ui/ui/app | rg '\.(test|spec)\.[cm]?[jt]sx?$' || true
echo '=== Test configuration ==='
if [ -f studio-ui/ui/app/package.json ]; then
cat -n studio-ui/ui/app/package.json | sed -n '1,260p'
fiLength of output: 140371
The server response uses As a result, This causes two regressions:
The fix needs an authoritative per-item publish-request permission field in the calculate-package response. The UI should consume that field only after the backend provides it. Alternatively, use the existing item available-actions data if it is already available for each calculated dependency. I also found no frontend test files or test script under
You are interacting with an AI system. |
|
@coderabbitai, the backend work has not been merged yet. It'll include the |
|
Tip For best results, initiate chat on the files or code changes.
I recommend merging this PR only with the backend change, or after the backend change is already available. The calculate-package response must include The current frontend intentionally treats a missing field as unauthorized. This is safe, but it means a frontend-only deployment will disable soft dependencies and block packages with hard dependencies. Before merge, verify these conditions:
Subject to that dependency and integration verification, I do not see another merge blocker from the changes described here. ✏️ Learnings added
You are interacting with an AI system. |
1 participant
#8793
This needs to be merged with #8888
Summary by CodeRabbit