feat(apikeys): add API key profile templates#1590
Conversation
a581442 to
2ae5827
Compare
Greptile SummaryThis PR adds a full-stack API key profile template feature: a new Confidence Score: 4/5Safe to merge after addressing the open P1 issues from previous review threads; the new findings in this pass are all P2. No new P0/P1 issues found in this review pass. The score ceiling of 4/5 reflects previously flagged P1 issues (unreachable SaveAsTemplate, misleading Default(1) for project_id, GraphQL variable type mismatch) that remain unresolved per the previous threads section. frontend/src/features/apikeys/data/schema.ts (nullable/required profile mismatch), frontend/src/features/apikeys/components/apikeys-profiles-dialog.tsx (onLoadComplete empty-profiles guard) Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as ProfilesDialog
participant Popover as LoadTemplatePopover
participant GQL as GraphQL API
participant Biz as APIKeyProfileTemplateSvc
participant DB as Database
Note over UI: User opens profiles dialog
UI->>Popover: Renders load template button
Note over Popover: User clicks Load Template
Popover->>GQL: loadApiKeyProfileTemplate(templateID, apiKeyID)
GQL->>Biz: LoadTemplate(ctx, templateID, apiKeyID)
Biz->>DB: BEGIN TX
Biz->>DB: GET template (privacy policy check)
Biz->>DB: GET apiKey (privacy policy check)
Biz->>Biz: Assert same projectID
Biz->>Biz: Clone template profile
Biz->>Biz: resolveProfileNameConflict()
Biz->>DB: UPDATE apiKey.profiles (append)
DB-->>Biz: Updated APIKey
Biz-->>GQL: Updated APIKey
GQL-->>Popover: loadApiKeyProfileTemplate result
Popover->>UI: onLoadComplete(profiles)
UI->>UI: form.reset(resetData)
UI->>UI: setTemplateLoadPending(true)
UI->>UI: Invalidate queries
Note over UI: User saves form
UI->>GQL: updateApiKeyProfiles(profiles)
GQL->>DB: UPDATE apiKey.profiles
Reviews (3): Last reviewed commit: "fix(apikeys): remove setTimeout race in ..." | Re-trigger Greptile |
There was a problem hiding this comment.
Code Review
This pull request introduces a template system for API key profiles, allowing users to save, load, and manage reusable configurations for model mappings, channel restrictions, and quotas. The implementation spans the full stack, adding a new database entity, a backend service layer, and several frontend dialogs. Feedback focuses on improving the robustness of numeric input handling in the UI and optimizing backend performance by replacing JSON-based deep copying with a manual cloning method.
| type='number' | ||
| min={1} | ||
| value={(field.value as unknown as number | null | undefined) ?? ''} | ||
| onChange={(e) => field.onChange(Number(e.target.value))} |
There was a problem hiding this comment.
The onChange handler for the pastDuration.value input should handle empty strings by setting the value to null or a default, similar to how other numeric inputs in this form are handled (e.g., line 424). Currently, Number('') evaluates to 0, which might bypass the intended min(1) validation in a confusing way or cause issues if the field is cleared.
| onChange={(e) => field.onChange(Number(e.target.value))} | |
| onChange={(e) => { const v = e.target.value; field.onChange(v === '' ? null : Number(v)); }} |
| func deepCopyProfile(profile *objects.APIKeyProfile) (*objects.APIKeyProfile, error) { | ||
| if profile == nil { | ||
| return nil, nil | ||
| } | ||
|
|
||
| data, err := json.Marshal(profile) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to marshal profile for deep copy: %w", err) | ||
| } | ||
|
|
||
| var copy objects.APIKeyProfile | ||
| if err := json.Unmarshal(data, ©); err != nil { | ||
| return nil, fmt.Errorf("failed to unmarshal profile for deep copy: %w", err) | ||
| } | ||
|
|
||
| return ©, nil | ||
| } No newline at end of file |
There was a problem hiding this comment.
Using json.Marshal and json.Unmarshal for deep copying the APIKeyProfile struct is functional but inefficient due to the overhead of serialization and reflection. Since this struct is well-defined and contains several slices, a manual deep copy implementation or a dedicated Clone() method on the APIKeyProfile object would be more performant and idiomatic.
|
there are some conflict, need generate again. |
- Add APIKeyProfileTemplate schema with CRUD operations - Implement service layer for template management - Add frontend dialog components for save/load templates - Add GraphQL resolvers and generated code - Include localization for EN and ZH-CN - Add unit tests for template business logic
- Implement ID/ProjectID resolvers for APIKeyProfileTemplate (was panicking) - Add ent Policy() with project-scoped read/write rules and OwnerRule - Return errors from deepCopyProfile instead of silently returning nil - Validate profile presence in CreateTemplate resolver - Remove dead commented stub code from axonhub.resolvers.go
ProfileCard referenced setSaveTemplateProfileIndex and setSaveTemplateOpen without receiving them as props, causing a ReferenceError at runtime. Add onSaveTemplate callback prop to ProfileCard and wire it from the parent ApiKeyProfilesDialog.
- Convert projectID to GUID string (gid://axonhub/Project/<id>) for GraphQL ID inputs in create mutation and list query where filter - Add projectID to template list query response fields - Relax createdAt/updatedAt Zod schema from .datetime() to .string() - Fix toast i18n keys from apikeys.templates.messages.* to apikeys.templates.successMessage / loadSuccessMessage
The ent Policy and X-Project-ID header already scope template queries to the current project. The where.projectID filter was causing strconv.Atoi errors because the GUID string format (gid://axonhub/Project/1) is not handled correctly by all where input code paths in the generated GraphQL layer.
Each template item in the load template popover now has a trash icon that calls the existing deleteApiKeyProfileTemplate mutation. Includes i18n keys for en and zh-CN.
P1: useDeleteApiKeyProfileTemplate hook was using wrong i18n key (successMessage = 'saved') and both hook + component fired toasts. Fixed hook to use delete-specific keys with template name from mutation response, removed duplicate component-level toasts. P2: Added AlertDialog confirmation before deleting a template, consistent with the codebase pattern for destructive actions (AlertDialogAction with destructive styling).
Add a Profile Templates button to the API keys page header alongside Create API Key, following the channels page toolbar pattern. Opens a dialog listing all saved profile templates with delete support. Add a Create Template button inside the dialog (and in the empty state) that opens a full profile editor for creating new templates from scratch, with model mappings, channel restrictions, tags, load balancer strategy, and quota configuration.
…bugs - Pass profile as direct mutation argument instead of broken context propagation via gqlgen input resolvers (P0: template creation always failed because input resolvers discard returned context) - Remove UserHasScope checks from template resolvers; rely on ent privacy layer for project-scoped access control, consistent with existing API key resolvers - Wrap LoadTemplate, SaveAsTemplate, DeleteTemplate in RunInTransaction to prevent TOCTOU race conditions - Remove double GUID wrapping of projectID in create template mutation - Remove duplicate toast notifications from mutation hooks (toasts now only in calling components) - Fix form.reset() after template load (rely on query invalidation) - Fix date locale ternary for Chinese users in load template popover - Remove NoScope tests that tested removed UserHasScope behavior
…le name The template name and profile name were separate fields creating user confusion. The template IS the profile stored at project level — one name suffices. - Service layer always overrides profile.Name = template.Name on create, update, save-as-template, and load - Removed profile name field from create template dialog UI - Frontend sets profile.name = template.name before mutations - Updated test assertions to match new behavior
The P0 fix moved profile from extended input fields to top-level mutation arguments in the GraphQL schema, but the frontend was still nesting profile inside the input object. GraphQL rejected the unknown field with 422. - Create mutation: destructure profile from input, send as separate $profile variable - Update mutation: same — profile is a top-level arg, not nested in input
Add edit button next to delete in each template item. Opens a full edit dialog pre-populated with the template's current data. - Extract formSchemaFactory and FormValues into shared template-form-schema.ts - Create edit template dialog mirroring create dialog structure - Add i18n keys for edit (en + zh-CN) - Remove hook-level toasts from useUpdateApiKeyProfileTemplate
P0: Add useEffect to sync profile.name from template name in create
dialog — form was silently failing validation with no feedback.
P1: Add type='button' to load template popover trigger inside form to
prevent unintended form submission.
P2: Wrap UpdateTemplate Get+Update in RunInTransaction (TOCTOU race).
Mark profile: APIKeyProfileInput! in create mutation schema (was
optional but resolver rejected nil). Import shared formSchemaFactory
in edit dialog instead of duplicating inline. Fix update hook
profile.name fallback to use existing name when input.name absent.
Remove .min(1) from output schema name (would fail entire list on
empty name). Fix projectID type from nullable to required. Reorder
unique index to lead with project_id for query performance. Fix
locale check to use startsWith('zh') for zh-CN support.
P3: Remove dead WithAPIKeyProfile/GetAPIKeyProfile context functions.
Remove stale commented-out input resolver code. Fix indentation in
graphql.go. Remove dead ProfileInputResolver and ScopeConstants tests.
Fix 'Delete template' → 'Delete Template' casing in i18n.
0e8f526 to
b4a37a3
Compare
Should be good to go now 👍 |
b4a37a3 to
e06bc2f
Compare
- Fix LoadTemplate using template name instead of profile name - Fix SaveAsTemplate overwriting profile name with template name - Re-enable gomodguard linter in .golangci.yml - Fix gci import formatting (blank line after import block) - Modernize type declarations in load_balancer.go - Apply to 82 files across internal/, cmd/, examples/, and integration_test/
…den profile cloning - Fix P1: declare $profile as APIKeyProfileInput! in createApiKeyProfileTemplate mutation to match server schema; remove null-coalescing code path in mutationFn - Remove unreachable SaveAsTemplate method and its tests (no GraphQL resolver ever exposed it) - Remove Default(1) from project_id in api_key_profile_template schema; Required() edge already enforces the constraint - Cap template list pagination from first: 1000 to first: 100 - Fix pastDuration.value onChange to map empty string to null instead of 0 (matching pattern used by other numeric inputs) - Replace json.Marshal/Unmarshal deep copy with manual Clone() method on APIKeyProfile for better performance and type safety - Deep-copy *int64 pointer fields (Requests, TotalTokens) in Clone() to prevent shared mutation
e06bc2f to
a0bf0c1
Compare
…n feedback - Remove arbitrary 5s setTimeout for clearing templateLoadPending; the flag is now reliably cleared by the React render cycle via the useEffect reset guard, eliminating the race where a late refetch could overwrite template-loaded form data - Restructure the useEffect to capture wasTemplatePending before clearing, so the flag is evaluated synchronously while the batched setState takes effect on the next render - Show 'Saving' label on submit button during templateLoadPending so users understand why the button is disabled
Summary
Add reusable API key profile templates, enabling users to save, load, and manage profile configurations for API keys. Templates are project-scoped with access control, and integrate into the existing API key creation/editing dialogs.
Spirit/Intent
Enable reusable API key profile configurations to reduce repetitive setup and ensure consistency across API keys.
Key Changes
APIKeyProfileTemplateentity with project-scoped edges, privacy policy, and CRUD resolversRisks