feat(admin): signage AI providers page (PPT-2741) - #309
Open
camreeves wants to merge 7 commits into
Open
Conversation
A Data Stores shaped page for the vendor credentials behind signage image generation, plus a usage table so a domain's spend is visible. Credentials are only ever sent, never returned, so the boxes start empty on an edit and leaving them empty keeps what is stored. The form swaps its middle section per vendor rather than showing every field at once, and a test button asks the vendor for one small image so a wrong key is caught here rather than by a user mid poster. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
null means leave it alone, empty means unset it. Clearing the endpoint in the form sent null, so the old value survived the save and the provider kept talking to whatever it was pointed at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Testing a provider asks the vendor for a real image and is billed. The button had no disabled state and no confirmation, so a second click on a slow answer spent again. It now says what it costs and refuses a second run. A failed delete left the confirm dialog spinning for good. The domain picker is gone: the API answers for the domain this Backoffice is served from, and another customer's row is deliberately unreachable, so the picker was choosing between one option and a lie. A Google Vertex row needs a region and there was no field for one, so it could not be configured through the UI at all.
…icker A signal nothing set, a domain still forwarded into the provider modal, and three imports for a form that is no longer there.
Four things the page did its own way. The locale file's ADMIN block was re-sorted alphabetically when the new keys went in, which showed as 225 deleted lines of unrelated keys and buried the real change. Restored to insertion order with the new keys appended: the diff is now 46 added lines. `signage-ai.fn.ts` had its own query-string builder, when `common/api.ts` exports `toQueryString` with 26 tests against it and `build-list.component.ts` already uses it for exactly this. Five uses of `any`, in a tree that had none and uses `unknown` 216 times. No tests, where `storage.fn.spec.ts` is the direct model for this file. Adds 12 examples covering the model class, the query, the create-or-update split and the usage window. One of them caught a wrong assumption in the spec rather than the code, which is the point.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Review feedback: the comments explained why the implementation is the way it is, which is fluff for anyone reading the code and, in ts-client, for anyone consuming the library. user-interfaces already says as much in AGENTS.md — "comments should only explain hard to understand code not justify implementation". What each thing does, the @PARAM tags, and notes on mechanics a reader cannot infer — an external API quirk, a magic number's unit, an ordering requirement — all stay. Comments only, no behavioural change.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
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
3 participants
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.
Part of the AI signage feature (PPT-2741). Depends on PlaceOS/rest-api#447 being deployed.
What this adds
An admin page for configuring image generation providers, at Admin > Signage AI. It is entirely provider setup and credentials: nothing about generating images lives here, that is in the signage manager.
The page lists the configured providers and lets you add, edit, remove and test one. The form swaps its middle section per vendor, because OpenAI, Azure and Vertex do not take the same credentials. Credentials are only ever sent, never returned, so on an edit those boxes start empty and leaving them empty keeps what is stored.
There is also a usage table so you can see what has been spent per provider and model before the bill arrives.
Ordering, and one trap
This needs rest-api deployed first. The tab is pushed onto the admin list unconditionally with no capability check, so if this ships ahead of the API there is a Signage AI tab whose every call 404s. Worth either sequencing it or adding a gate, and I am happy to add the gate if you would prefer this not to be order-dependent.
Notes for review
The locale diff is deliberately small. An earlier version of this branch re-sorted the whole
ADMINblock alphabetically, which showed up as 225 deleted lines of unrelated keys and buried the actual change. It is back to insertion order with the new keys appended, so the diff is 46 added lines.signage-ai.fn.tsis a local functions file rather than an import from ts-client. That matches what the repo already does:storage.fn.tsis local for the same reason, the surface is small and only this page uses it. If the ts-client provider functions land first this could move over, but it does not need to.Testing
12 examples in
src/tests/admin/signage-ai.fn.spec.ts, modelled onstorage.fn.spec.ts. 935 tests pass, build is clean.