fix(assets): name store-app assets from their chosen setting, not the manifest name#3115
Merged
Conversation
… manifest name
Store apps like the RSS Reader are one generic app configured per
install via a labelled select (the feed). The install form seeded the
asset name once from the static manifest name ("RSS Reader") and never
updated it, so adding several RSS Reader feeds produced multiple assets
all named "RSS Reader".
Derive the default name from the chosen option's label (e.g. the feed's
title "NPR — Top Stories") — the first select with x-enumLabels, in
manifest order, so it is generic rather than RSS-specific. The name
tracks config changes until the operator types their own, at which point
an @input flag stops the derivation so a custom name is never clobbered.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a regression in the “Add Asset → Apps” flow where multiple installs of a generic store app (e.g., RSS Reader) would all be named after the static manifest name, by deriving the default asset name from the chosen labeled select option and preventing subsequent config edits from clobbering an operator-edited name.
Changes:
- Add
suggestedName()to derive a default asset name from the firstselectsetting that providesx-enumLabels, falling back to the manifest name. - Track an operator edit via
nameEditedand stop auto-deriving the asset name once the name field is manually changed. - Wire the name input to set
nameEditedon user input.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/anthias_server/app/templates/_asset_modal.html | Marks the app name field as operator-edited on input to prevent later auto-derivation clobbering it. |
| src/anthias_server/app/static/src/apps.ts | Implements derived default naming from config, adds nameEdited state, and updates naming behavior on selection/config changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address PR review feedback on the store-app asset-name fix: - Extract selectOptionLabel() so the manifest-form select renderer and the asset-name derivation resolve an option's text the same way. A chosen option with a missing x-enumLabels entry now names the asset after the raw value the operator sees, instead of silently reverting to the manifest name. - Move the derivation into apps/suggested-name.ts (DOM-free) and add unit tests: default/updated select value, manifest-name fallback, first-labelled-select ordering, missing-label consistency, and a stray non-option value. - Wire the package.json test script to `bun test` now that real .test.ts files exist, and correct the stale assetName comment (it binds the visible Name input, not a hidden one). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues Fixed
Adding multiple RSS Reader apps produced several assets all named "RSS Reader" instead of being named after the chosen feed. (Regression surface from the signage app store added in #3114.)
Description
Store apps like the RSS Reader are one generic app configured per install via a labelled
select(the feed). The install form seeded the asset name once from the static manifestname("RSS Reader") and never updated it, so every install got the identical name.NPR — Top Stories) — the firstselectwithx-enumLabels, in manifest order, so it's generic rather than RSS-specific. Apps without such a setting keep the manifest name.@inputflag then stops the derivation so a custom name is never clobbered. Reset on re-select / back.Two source files change; the built JS bundle is gitignored and rebuilt at image-build time.
Verification:
tsc --noEmitclean, production bundle builds, and the realappsTab()component was driven through the add-app flow against the live RSS manifest (default = feed title, tracks feed changes, preserves a custom name after edit, resets on re-select).Checklist
🤖 Generated with Claude Code