content: group a fleet into an asset model from the asset index (shelf.nu#2782) - #230
Conversation
…f.nu#2782) Triggered by: Shelf-nu/shelf.nu#2782 Two new asset index Actions items, Update asset model and Remove from asset model, replace the per-asset edit form and the CSV re-import as the way to group a fleet you already own. Completes the batch-actions list, which was missing seven of the sixteen menu entries, and updates every page that presented CSV as the only bulk route.
Preview deployment |
|
Warning Review limit reached
Next review available in: 53 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe changes document bulk asset-model grouping and removal across product content. They add an article and a media-pipeline script that captures and uploads related UI screenshots. ChangesBulk asset-model documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
content/updates/group-assets-into-an-asset-model-in-bulk.mdx (1)
5-6: 🩺 Stability & Availability | 🔵 TrivialVerify the published page after deployment.
Confirm that the image loads and that
readMoreUrlresolves to the intended section on the live site. As per coding guidelines, “After deployment, verify the live site and relevant analytics or UI behavior; never claim functionality works without evidence.”🤖 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 `@content/updates/group-assets-into-an-asset-model-in-bulk.mdx` around lines 5 - 6, After deployment, verify the published page for the asset model update: confirm the configured image loads successfully and that readMoreUrl resolves to the intended “grouping a fleet into an asset model” section on the live site. Do not claim these behaviors work without confirming them.Source: Coding guidelines
🤖 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 `@scripts/media-pipeline/articles/bulk-asset-model.mjs`:
- Around line 14-27: Update openActionsMenu to select a mixed asset set rather
than relying on the header checkbox’s visible-only selection: include at least
one quantity-tracked asset, then assert the dialog warning and skipped-count
content before capturing bulk-asset-model-dialog.png.
- Around line 7-10: Restore the missing env.mjs credential helper used by
loginToShelf and upload, ensuring it exports the workflow-secret lookup those
helpers require and allows bulk-asset-model.mjs to resolve its imports.
Alternatively, remove those dependencies only if the script no longer needs
loginToShelf or upload, while preserving credential validation for the workflow.
---
Nitpick comments:
In `@content/updates/group-assets-into-an-asset-model-in-bulk.mdx`:
- Around line 5-6: After deployment, verify the published page for the asset
model update: confirm the configured image loads successfully and that
readMoreUrl resolves to the intended “grouping a fleet into an asset model”
section on the live site. Do not claim these behaviors work without confirming
them.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cd37bac5-86c9-4397-967e-daf11b9d7e74
📒 Files selected for processing (10)
content/features/consumables-tracking.mdxcontent/glossary/asset-models.mdxcontent/knowledge-base/bulk-updating-assets-via-csv.mdxcontent/knowledge-base/how-to-choose-a-tracking-method.mdxcontent/knowledge-base/quantity-tracked-assets-and-consumables.mdxcontent/knowledge-base/using-batch-actions-in-shelf.mdxcontent/solutions/fixed-asset-tracking.mdxcontent/updates/group-assets-into-an-asset-model-in-bulk.mdxscripts/media-pipeline/articles/bulk-asset-model.mjsscripts/media-pipeline/run.mjs
| import { launchBrowser, createContext, loginToShelf, navigateTo } from "../lib/browser.mjs"; | ||
| import { screenshot } from "../lib/capture.mjs"; | ||
| import { toWebP } from "../lib/convert.mjs"; | ||
| import { upload } from "../lib/upload.mjs"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline scripts/media-pipeline/lib/browser.mjs --items all
ast-grep outline scripts/media-pipeline/lib/upload.mjs --items all
rg -n -C 4 'getShelfCredentials|getSupabaseConfig|process\.env' scripts/media-pipeline/lib .github/workflows
gh secret listRepository: Shelf-nu/website-v2
Length of output: 5683
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rm -f scripts/media-pipeline/lib/env.mjs
echo "removed env.mjs from repo if present"
fd -a 'browser\.mjs|upload\.mjs|env\.mjs' scripts/media-pipeline/lib | sed -n '1,40p'
fgrep -n "env\.mjs" scripts/media-pipeline/lib/browser.mjs scripts/media-pipeline/lib/upload.mjs || true
sed -n '1,80p' scripts/media-pipeline/lib/browser.mjs
sed -n '1,80p' scripts/media-pipeline/lib/upload.mjsRepository: Shelf-nu/website-v2
Length of output: 3713
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "GitHub refs:"
git ls-remote --heads --tags https://github.com/Shelf-nu/website-v2.git | sed -n '1,80p'
echo
echo "Files at last commit:"
git ls-tree -r --name-only HEAD | rg '^(scripts/media-pipeline/lib/(browser|upload|env)\.mjs|scripts/media-pipeline/articles/bulk-asset-model\.mjs|.github/workflows/.*\.ya?ml)$' || true
echo
echo "Import and env references from tracked files:"
rg -n -C 3 'import \{ getShelfCredentials|import \{ getSupabaseConfig|fromKeychain|process\.env\.(SHELF_MAIN_PATH|SUPABASE_URL|SUPABASE_SERVICE_KEY)|\$ \{\{ secrets\.' scripts/scripts/media-pipeline scripts/media-pipeline .github/workflows || true
echo
echo "Tracked env.mjs:"
git ls-files scripts/media-pipeline/lib/env.mjs || trueRepository: Shelf-nu/website-v2
Length of output: 9181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Current branch and diff stats:"
git branch --show-current
git diff --stat
echo
echo "Bulk model imports and usage excerpt:"
sed -n '1,120p' scripts/media-pipeline/articles/bulk-asset-model.mjs
echo
echo "env helper source:"
cat -n scripts/media-pipeline/lib/env.mjs
echo
echo "Workflow secret/env mapping searches:"
rg -n -C 3 '\$\{\{\s*secrets\.\w+\s*\}\}|env:\s*$|scripts/media-pipeline' .github/workflows || true
echo
echo "Workflow secret/env mapping searches in all workflows:"
find .github/workflows -maxdepth 1 -type f -name '*.y*ml' -print0 | while IFS= read -r -d '' f; do
echo "--- $f"
rg -n -C 3 '\$\{\{\s*secrets\.\w+\s*\}\}|SHELF_MAIN_PATH|SUPABASE_SERVICE_KEY|SUPABASE_URL|shelf-demo|supabase' "$f" || true
doneRepository: Shelf-nu/website-v2
Length of output: 3899
Restore the missing credential helper before relying on these imports.
bulk-asset-model.mjs still imports loginToShelf() from browser.mjs and upload() from upload.mjs; both helpers import ./env.mjs, which is absent in the current diff. Add the helper back or remove these dependencies so the script can resolve and the workflow secrets can be checked.
🤖 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 `@scripts/media-pipeline/articles/bulk-asset-model.mjs` around lines 7 - 10,
Restore the missing env.mjs credential helper used by loginToShelf and upload,
ensuring it exports the workflow-secret lookup those helpers require and allows
bulk-asset-model.mjs to resolve its imports. Alternatively, remove those
dependencies only if the script no longer needs loginToShelf or upload, while
preserving credential validation for the workflow.
Source: Coding guidelines
| async function openActionsMenu(page) { | ||
| await navigateTo(page, "/assets"); | ||
| const hdr = await page.$("thead th:first-child"); | ||
| if (!hdr) throw new Error("Header checkbox not found"); | ||
| await hdr.click(); | ||
| // Assert the selection actually registered before opening the menu. | ||
| await page.locator('button:has-text("Actions")').first().waitFor({ state: "visible" }); | ||
| await page.waitForTimeout(1000); | ||
| const actionsBtn = page.locator('button:has-text("Actions")').first(); | ||
| await actionsBtn.click(); | ||
| // The new items must be present, or the feature is not deployed yet. | ||
| await page.locator('text="Update asset model"').first().waitFor({ state: "visible" }); | ||
| await page.locator('text="Remove from asset model"').first().waitFor({ state: "visible" }); | ||
| await page.waitForTimeout(800); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)bulk-asset-model\.mjs$|content/knowledge-base/using-batch-actions-in-shelf\.mdx$' || true
echo
echo "== bulk-asset-model outline/size =="
file="$(git ls-files | rg '^scripts/media-pipeline/articles/bulk-asset-model\.mjs$' | head -n1 || true)"
if [ -n "$file" ]; then
wc -l "$file"
ast-grep outline "$file" || true
sed -n '1,140p' "$file" | cat -n
fi
echo
echo "== docs references =="
doc="$(git ls-files | rg '^content/knowledge-base/using-batch-actions-in-shelf\.mdx$' | head -n1 || true)"
if [ -n "$doc" ]; then
wc -l "$doc"
rg -n "Update asset model|Select asset model|quantity-tracked|skipped|warning|Actions|asset model|bulk-asset-model-dialog" "$doc" -C 3 || true
fi
echo
echo "== repo references =="
rg -n "bulk-asset-model-dialog\.png|Select asset model|quantity-tracked|Update asset model|Remove from asset model" . --glob '!node_modules' --glob '!dist/**' --glob '!build/**' || trueRepository: Shelf-nu/website-v2
Length of output: 46162
Make the dialog screenshot use a mixed asset selection.
select all XX entries selects only the assets currently visible, so this capture can miss the dialog warning. Ensure the selection includes at least one quantity-tracked asset, assert the warning/skipped count, then take bulk-asset-model-dialog.png.
🤖 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 `@scripts/media-pipeline/articles/bulk-asset-model.mjs` around lines 14 - 27,
Update openActionsMenu to select a mixed asset set rather than relying on the
header checkbox’s visible-only selection: include at least one quantity-tracked
asset, then assert the dialog warning and skipped-count content before capturing
bulk-asset-model-dialog.png.
CodeRabbit review on #230. The dialog shot exists to show the skip warning, which renders only when the selection holds pooled stock. The script now waits on that string instead of relying on the demo workspace's first page happening to contain a quantity-tracked asset. Re-ran the pipeline with the assertion in place; both shots re-captured and inspected.
|
Both CodeRabbit findings addressed. 1. Assert the quantity-tracked warning before capturing the dialog — valid, fixed in the commit above. The shot's whole point is the skip warning, and it was rendering only because the demo workspace's first page happens to hold four quantity-tracked assets. The script now waits on 2. "Restore the missing env.mjs credential helper" — not applicable. 3. Nitpick on verifying the published page — partially possible from here. Both image URLs return HTTP 200. The |
Triggered by
Product change summary
Two new items sit in the asset index Actions menu, directly under Update category: Update asset model links every eligible selected asset to one model, and Remove from asset model unlinks them. Until now the only ways to put an existing asset on a model were the asset edit form, one asset at a time, or a CSV re-import. Quantity-tracked assets in a mixed selection are skipped with a count rather than failing the batch, and the picker can create a model inline.
Verified against
bulk-actions-dropdown.tsx,bulk-asset-model-update-dialog.tsx,bulk-asset-model-remove-dialog.tsx, andapi+/assets.bulk-update-asset-model.tsonmain, not from the PR body. Every UI string quoted below is the one in the shipped source.Content changes
Factual corrections
content/knowledge-base/using-batch-actions-in-shelf.mdx— the "Available operations" list named 9 of the menu's 16 entries. It was missing Download QR Codes, Create audit, Add to existing audit, Add to kit, Remove from kit, and both new asset-model entries. The list is now complete and in menu order, with a note on which entries grey out and why. The video alt text also claimed "14 bulk operations"; it no longer states a count.Content enhancements
content/knowledge-base/using-batch-actions-in-shelf.mdx— new Grouping a Fleet into an Asset Model section with the four-step flow, the skip/move/no-op outcomes, and both screenshots.content/glossary/asset-models.mdx— the "Group a fleet you already own" bullet presented the CSV column as the only route. It now leads with the bulk action and keeps CSV as the multi-field alternative.content/knowledge-base/how-to-choose-a-tracking-method.mdx— the page twice promised that units "can join, leave, or switch models at any time" without saying how. Both spots now name the actual path.content/knowledge-base/bulk-updating-assets-via-csv.mdx— the FAQ "How do I assign an asset model to a whole fleet at once?" answered with CSV only. It now points at the bulk action for the model-only case and keeps CSV for edits that carry other fields.content/knowledge-base/quantity-tracked-assets-and-consumables.mdx— the asset models section implied Settings was the only management surface; adds the index route and the skip behaviour for pooled stock.content/features/consumables-tracking.mdx— same correction on the prospect-facing feature page.content/solutions/fixed-asset-tracking.mdx— this page pitched Asset Models purely as a create-time convenience for a new batch. A register that already exists is the more common fixed-asset situation, so it now covers grouping after the fact, and that only the model link changes.New content
content/updates/group-assets-into-an-asset-model-in-bulk.mdx— changelog entry, dated to the merge (2026-08-06).Tooling
scripts/media-pipeline/articles/bulk-asset-model.mjs+run.mjsregistration.Screenshots
Both were captured by the media pipeline against
app.shelf.nuthis morning and inspected before committing. The feature is live.bulk-asset-model-menu.webp— Actions menu with 20 assets selected, the two new items highlighted under Update category.bulk-asset-model-dialog.webp— the Group (20) assets into an asset model dialog, showing the real quantity-tracked warning ("4 quantity-tracked asset(s) in your selection will be skipped").No TODO markers remain.
Deliberately skipped
src/data/pricing.features.ts. The route guard isrequirePermission({ entity: asset, action: update }), the same gate as the existing bulk tag, location, and category actions, none of which have a matrix row. No new plan gate exists, so a row would invent one. Worth noting separately: the matrix has no Asset Models row at all today, which is a pre-existing gap rather than something this PR created.Review notes
pricing.features.ts), while this bulk action is not plan-gated. Free-tier users could previously only group a fleet by editing assets one at a time. I did not write that as a "now on Free" claim anywhere, because none of the pages frame the CSV route by plan, but it is the real reach of the change.#grouping-a-fleet-into-an-asset-modelanchor is linked from four pages; it matchesmdx-content.tsxslugification of## Grouping a Fleet into an Asset Model.node scripts/verify-search-coverage.mjspasses. No rendered-preview check was possible: the Cloudflare preview is behind Cloudflare Access, and this was a scheduled run with no browser session.Impact scope
Generated by Shelf Content Intelligence — PR→Website Sync
Summary by CodeRabbit