Skip to content

refactor: add ownerId to create key for backward compat #3312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chronark merged 3 commits into from
Jun 17, 2025
Merged

refactor: add ownerId to create key for backward compat #3312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
848134a
refactor: add ownerId to create key for backward compat
ogzhanolguncu Jun 10, 2025
becb210
fix: value mapping
ogzhanolguncu Jun 10, 2025
15952a8
fix: add externalId to update
ogzhanolguncu Jun 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .../dashboard/app/(app)/apis/[apiId]/_components/create-key/components/external-id-field.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { useState } from "react";

type ExternalIdFieldProps = {
value: string | null;
onChange: (id: string | null) => void;
onChange: (identityId: string | null, externalId: string | null) => void;
error?: string;
disabled?: boolean;
};
Expand All @@ -24,7 +24,7 @@ export const ExternalIdField = ({
const { identities, isFetchingNextPage, hasNextPage, loadMore } = useFetchIdentities();

const createIdentity = useCreateIdentity((data) => {
onChange(data.identityId);
onChange(data.identityId, data.externalId);
});

const handleCreateIdentity = () => {
Expand Down Expand Up @@ -97,7 +97,7 @@ export const ExternalIdField = ({
return;
}
const identity = identities.find((id) => id.id === val);
onChange(identity?.id || null);
onChange(identity?.id || null, identity?.externalId || null);
}}
placeholder={
<div className="flex w-full text-grayA-8 text-xs gap-1.5 items-center py-2">
Expand Down
10 changes: 8 additions & 2 deletions apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/general-setup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const GeneralSetup = () => {
const {
register,
formState: { errors },
setValue,
control,
} = useFormContext<FormValues>();

Expand All @@ -34,18 +35,23 @@ export const GeneralSetup = () => {
optional
{...register("prefix")}
/>

<Controller
name="externalId"
name="identityId"
control={control}
defaultValue=""
render={({ field }) => (
<ExternalIdField
value={field.value ?? null}
onChange={field.onChange}
onChange={(identityId: string | null, externalId: string | null) => {
field.onChange(identityId);
setValue("externalId", externalId);
}}
Comment on lines +40 to +49
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix error handling inconsistency.

The Controller now manages identityId but the error handling on line 50 still references errors.externalId?.message. This could display the wrong error message.

           <ExternalIdField
             value={field.value ?? null}
             onChange={(identityId: string | null, externalId: string | null) => {
               field.onChange(identityId);
               setValue("externalId", externalId);
             }}
-            error={errors.externalId?.message}
+            error={errors.identityId?.message}
           />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
name="identityId"
control={control}
defaultValue=""
render={({ field }) => (
<ExternalIdField
value={field.value ?? null}
onChange={field.onChange}
onChange={(identityId: string | null, externalId: string | null) => {
field.onChange(identityId);
setValue("externalId", externalId);
}}
name="identityId"
control={control}
defaultValue=""
render={({ field }) => (
<ExternalIdField
value={field.value ?? null}
onChange={(identityId: string | null, externalId: string | null) => {
field.onChange(identityId);
setValue("externalId", externalId);
}}
error={errors.identityId?.message}
/>
)}
🤖 Prompt for AI Agents 
In
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/general-setup.tsx
around lines 40 to 49, the error handling for the Controller managing identityId
incorrectly references errors.externalId?.message. Update the error reference to
errors.identityId?.message to ensure the correct error message is displayed for
the identityId field.

error={errors.externalId?.message}
/>
)}
/>

<FormInput
className="[&_input:first-of-type]:h-[36px]"
label="Bytes"
Expand Down
10 changes: 10 additions & 0 deletions apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ export const generalSchema = z.object({
.max(256, { message: "External ID cannot exceed 256 characters" })
.optional()
.nullish(),
identityId: z
.string()
.trim()
.max(256, { message: "Identity ID cannot exceed 256 characters" })
.optional()
.nullish(),
name: nameSchema,
environment: z
.string()
Expand Down Expand Up @@ -314,6 +320,10 @@ export const createKeyInputSchema = z.object({
.string()
.max(256, { message: "External ID cannot exceed 256 characters" })
.nullish(),
identityId: z
.string()
.max(256, { message: "Identity ID cannot exceed 256 characters" })
.nullish(),
meta: z.record(z.unknown()).optional(),
remaining: z.number().int().positive().optional(),
refill: z
Expand Down
6 changes: 5 additions & 1 deletion apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const formValuesToApiInput = (formValues: FormValues, keyAuthId: string):
prefix: formValues.prefix === "" ? undefined : formValues.prefix,
bytes: formValues.bytes,
externalId: formValues.externalId || null,
identityId: formValues.identityId || null,
name: formValues.name === "" ? undefined : formValues.name,
enabled: true,
environment: formValues.environment === "" ? undefined : formValues.name,
Expand Down Expand Up @@ -76,7 +77,10 @@ export const getFieldsFromSchema = (schema: unknown, prefix = ""): string[] => {
return [];
}

const schemaObj = schema as { shape: Record<string, unknown>; _def?: { typeName: string } };
const schemaObj = schema as {
shape: Record<string, unknown>;
_def?: { typeName: string };
};

return Object.keys(schemaObj.shape).flatMap((key) => {
const fullPath = prefix ? `${prefix}.${key}` : key;
Expand Down
7 changes: 6 additions & 1 deletion ...Id]/_components/components/table/components/actions/components/edit-external-id/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const EditExternalId = ({
const [selectedIdentityId, setSelectedIdentityId] = useState<string | null>(
keyDetails.identity_id || null,
);
const [selectedExternalId, setSelectedExternalId] = useState<string | null>(null);
const [isConfirmPopoverOpen, setIsConfirmPopoverOpen] = useState(false);
const clearButtonRef = useRef<HTMLButtonElement>(null);

Expand All @@ -36,6 +37,7 @@ export const EditExternalId = ({
ownerType: "v2",
identity: {
id: selectedIdentityId,
externalId: selectedExternalId,
},
});
};
Expand Down Expand Up @@ -114,7 +116,10 @@ export const EditExternalId = ({
</div>
<ExternalIdField
value={selectedIdentityId}
onChange={setSelectedIdentityId}
onChange={(identityId: string | null, externalId: string | null) => {
setSelectedIdentityId(identityId);
setSelectedExternalId(externalId);
}}
disabled={updateKeyOwner.isLoading || Boolean(originalIdentityId)}
/>
</DialogContainer>
Expand Down
8 changes: 7 additions & 1 deletion ...ents/components/table/components/selection-controls/components/batch-edit-external-id.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const BatchEditExternalId = ({
onClose,
}: BatchEditExternalIdProps): JSX.Element => {
const [selectedIdentityId, setSelectedIdentityId] = useState<string | null>(null);
const [selectedExternalId, setSelectedExternalId] = useState<string | null>(null);
const [isConfirmPopoverOpen, setIsConfirmPopoverOpen] = useState(false);
const clearButtonRef = useRef<HTMLButtonElement>(null);

Expand All @@ -32,6 +33,7 @@ export const BatchEditExternalId = ({
ownerType: "v2",
identity: {
id: selectedIdentityId,
externalId: selectedExternalId,
},
});
};
Expand All @@ -57,6 +59,7 @@ export const BatchEditExternalId = ({
ownerType: "v2",
identity: {
id: null,
externalId: null,
},
});
};
Expand Down Expand Up @@ -141,7 +144,10 @@ export const BatchEditExternalId = ({
<div className="my-2">
<ExternalIdField
value={selectedIdentityId}
onChange={setSelectedIdentityId}
onChange={(identityId: string | null, externalId: string | null) => {
setSelectedIdentityId(identityId);
setSelectedExternalId(externalId);
}}
disabled={updateKeyOwner.isLoading}
/>
</div>
Expand Down
3 changes: 2 additions & 1 deletion apps/dashboard/lib/trpc/routers/key/create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ export const createKey = t.procedure
name: input.name,
hash,
start,
identityId: input.externalId,
identityId: input.identityId,
ownerId: input.externalId,
meta: JSON.stringify(input.meta ?? {}),
workspaceId: ctx.workspace.id,
forWorkspaceId: null,
Expand Down
4 changes: 2 additions & 2 deletions apps/dashboard/lib/trpc/routers/key/updateOwnerId.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const ownerValidationV1 = z.object({
const ownerValidationV2 = z.object({
identity: z.object({
id: z.string().nullish(),
externalId: z.string().nullish(),
}),
ownerType: z.literal("v2"),
});
Expand Down Expand Up @@ -194,8 +195,7 @@ const updateOwnerV2 = async (
.update(schema.keys)
.set({
identityId: input.identity.id ?? null,
// Set ownerId to null to maintain consistency
ownerId: null,
ownerId: input.identity.externalId,
})
.where(
and(
Expand Down
Loading