feat: Add default bytes and prefix#2146
Conversation
harshsbhat
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
|
|
@harshsbhat is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request enhance the API key management system within the dashboard application. New components for managing default bytes and prefixes have been introduced, along with modifications to existing components to accept new props for default values. The schema for the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
🕹️ 300 points
app:dashboard
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Outside diff range and nitpick comments (11)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/page.tsx (1)
Line range hint
1-33: Consider enhancing error handling and type safety.While the changes look good, here are some suggestions to improve the overall robustness of the code:
- Consider adding explicit error handling for potential database errors.
- Verify the presence of
defaultBytesanddefaultPrefixproperties on thekeyAuthobject before passing them to theCreateKeycomponent.- Consider defining explicit types for
defaultBytesanddefaultPrefixto ensure type safety.Here's a suggested implementation incorporating these improvements:
import { getTenantId } from "@/lib/auth"; import { db } from "@/lib/db"; import { notFound } from "next/navigation"; import { CreateKey } from "./client"; interface KeyAuth { id: string; defaultBytes?: number; defaultPrefix?: string; workspace: { tenantId: string }; } export default async function CreateKeypage(props: { params: { apiId: string; keyAuthId: string; }; }) { const tenantId = getTenantId(); let keyAuth: KeyAuth | null; try { keyAuth = await db.query.keyAuth.findFirst({ where: (table, { eq, and, isNull }) => and(eq(table.id, props.params.keyAuthId), isNull(table.deletedAt)), with: { workspace: true, api: true, }, }); } catch (error) { console.error("Error fetching keyAuth:", error); return notFound(); } if (!keyAuth || keyAuth.workspace.tenantId !== tenantId) { return notFound(); } return ( <CreateKey keyAuthId={keyAuth.id} apiId={props.params.apiId} defaultBytes={keyAuth.defaultBytes} defaultPrefix={keyAuth.defaultPrefix} /> ); }This implementation adds explicit error handling for the database query, defines a
KeyAuthinterface for better type checking, and maintains the existing logic while improving overall type safety.internal/db/src/schema/keyAuth.ts (1)
19-20: LGTM: New fields added as per requirements.The new
defaultPrefixanddefaultBytesfields have been correctly added to thekeyAuthtable, aligning with the PR objectives. ThedefaultPrefixis a varchar(8), which seems reasonable for a prefix length. ThedefaultByteshas a default value of 16, matching the current default mentioned in the PR objectives.One minor suggestion:
Consider adding a comment explaining the purpose of these new fields, especially if there are any constraints or expectations for their usage. For example:
// Default prefix for key generation, max 8 characters defaultPrefix: varchar("default_prefix", { length: 8 }), // Default number of bytes for key generation, minimum recommended value is 16 defaultBytes: int("default_bytes").default(16),This would provide clarity for future developers working with this schema.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (2)
163-164: Default values updated to use new props.The form's default values for
prefixandbytesnow use the newdefaultPrefixanddefaultBytesprops. This change allows for customizable default values when creating a new key.Consider using nullish coalescing for
bytesto improve clarity:- bytes: defaultBytes || 16, + bytes: defaultBytes ?? 16,This change makes it explicit that
16is used whendefaultBytesisnullorundefined, not when it's0or any other falsy value.
Line range hint
323-341: Consider enhancing form fields to reflect new default values.The
prefixandbytesform fields could be updated to better reflect the new default values:
- For the
prefixfield, consider adding a placeholder that shows the default value when provided:<Input {...field} + placeholder={defaultPrefix || "Enter prefix"} onBlur={(e) => { if (e.target.value === "") { return; } }} />
- For the
bytesfield, consider adding a placeholder and adjusting the field description:<Input type="number" {...field} + placeholder={defaultBytes?.toString() || "16"} /> <FormDescription> - How long the key will be. Longer keys are harder to guess and more secure. + How long the key will be (default: {defaultBytes || 16}). Longer keys are harder to guess and more secure. </FormDescription>These changes would make the new default values more visible to the user and provide better context.
Also applies to: 376-394
apps/dashboard/lib/trpc/routers/api/setDefaultBytes.ts (1)
64-64: Ensure consistent and descriptive audit log messages.The description in the audit logs uses different identifiers on lines 64 and 84 (
keyAuth.workspaceIdandkeyAuth.id). For clarity and consistency, consider using the API name to make the logs more meaningful.Apply this diff to update the descriptions:
// Line 64 - description: `Changed ${keyAuth.workspaceId} default byte size for keys from ${keyAuth.defaultBytes} to ${input.defaultBytes}`, + description: `Changed default byte size for keys from ${keyAuth.defaultBytes} to ${input.defaultBytes} for API ${keyAuth.api.name}`, // Line 84 - description: `Changed ${keyAuth.id} default byte size for keys from ${keyAuth.defaultBytes} to ${input.defaultBytes}`, + description: `Changed default byte size for keys from ${keyAuth.defaultBytes} to ${input.defaultBytes} for API ${keyAuth.api.name}`,Also applies to: 84-84
apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx (3)
55-58: Move validation logic fordefaultPrefixlength into Zod schemaThe validation for
defaultPrefixlength is currently handled manually in theonSubmitfunction (lines 56-58). To ensure consistent validation and better form handling, consider moving this validation into the ZodformSchema(lines 20-24) using the.max()method.Apply this diff to update the schema and remove the manual check:
const formSchema = z.object({ keyAuthId: z.string(), workspaceId: z.string(), - defaultPrefix: z.string() + defaultPrefix: z.string().max(8, "Default prefix must be at most 8 characters long.") }); async function onSubmit(values: z.infer<typeof formSchema>) { - if (values.defaultPrefix.length > 8) { - return toast.error("Default prefix is too long, maximum length is 8 characters."); - } if (values.defaultPrefix === keyAuth.defaultPrefix) { return toast.error("Please provide a different prefix than the existing default."); } await setDefaultPrefix.mutateAsync(values); }Also applies to: 20-24
60-60: Improve error message for clarityThe error message on line 60 can be rephrased for better grammar and clarity.
Apply this diff to update the error message:
- return toast.error("Please provide a different prefix than already existing one as default"); + return toast.error("Please provide a different prefix than the existing default.");
71-71: Standardize capitalization in UI textIn the
CardDescriptioncomponent on line 71, the word "Prefix" is capitalized inconsistently. For consistency and readability, consider using sentence case.Apply this diff to update the text:
- Set default Prefix for the keys under this API. + Set default prefix for the keys under this API.apps/dashboard/app/(app)/apis/[apiId]/settings/default-bytes.tsx (3)
78-78: Provide a visible label for better accessibilityThe label for "Default Bytes" is currently hidden using
className="hidden sr-only". While this makes it available to screen readers, displaying a visible label can enhance usability for all users, especially those using assistive technologies.Consider updating the label to be visible:
-<label className="hidden sr-only">Default Bytes</label> +<label htmlFor="defaultBytes">Default Bytes</label>
95-99: Simplify button variant logicThe
variantprop of theButtoncomponent is conditionally set based on the form state. Since thedisabledprop already controls the button's interactivity, you can simplify thevariantprop to always be"primary".Apply this diff to simplify the button component:
<Button - variant={ - form.formState.isValid && !form.formState.isSubmitting ? "primary" : "disabled" - } disabled={!form.formState.isValid || form.formState.isSubmitting} + variant="primary" type="submit" >
32-34: Update 'defaultBytes' type to exclude 'null'In the
Propstype,defaultBytesis typed asnumber | undefined | null. Sincenullis not actively used, and to prevent potentialnullvalue issues, consider updating the type tonumber | undefined.Apply this diff to adjust the type:
keyAuth: { id: string; workspaceId: string; - defaultBytes: number | undefined | null; + defaultBytes: number | undefined; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (2 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/page.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/settings/default-bytes.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx (2 hunks)
- apps/dashboard/lib/trpc/routers/api/setDefaultBytes.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/api/setDefaultPrefix.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/index.ts (2 hunks)
- internal/db/src/schema/keyAuth.ts (2 hunks)
🔇 Additional comments (13)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/page.tsx (2)
Line range hint
1-33: Summary: Changes implement the desired feature with room for minor improvements.The modifications successfully address the PR objectives by adding
defaultBytesanddefaultPrefixprops to theCreateKeycomponent. This enhancement will allow users to set default values for bytes and prefix, streamlining the key creation process as intended.While the implementation is correct, consider the suggested improvements for error handling and type safety to further enhance the robustness of the code.
Overall, the changes are approved and align well with the project goals.
26-33: LGTM! The changes align with the PR objectives.The addition of
defaultBytesanddefaultPrefixprops to theCreateKeycomponent successfully implements the feature to allow setting default values for bytes and prefix. This change directly addresses the goal of streamlining the key creation process as outlined in the PR objectives.To ensure type safety, let's verify the types of the new props:
internal/db/src/schema/keyAuth.ts (2)
2-2: LGTM: Import statement updated correctly.The addition of the
inttype to the import statement is consistent with the newdefaultBytesfield being added to the schema.
Line range hint
1-33: Implementation looks good, consider verifying usage.The changes to the
keyAuthtable schema are well-implemented and align with the PR objectives. The new fields allow for setting default values for the prefix and bytes, which should improve the user experience when creating multiple keys.To ensure these changes are fully utilized, it would be beneficial to verify that the application code has been updated to use these new default values when creating keys. You can use the following script to check for usage of these new fields:
This will help confirm that the new fields are being properly utilized in the key creation process.
✅ Verification successful
Verification Complete:
defaultPrefixanddefaultBytesare in use.The changes to the
keyAuthtable schema have been successfully verified. ThedefaultPrefixanddefaultBytesfields are being utilized across the codebase in various modules, ensuring that the new default values are properly applied during key creation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of new defaultPrefix and defaultBytes fields # Search for usage of defaultPrefix echo "Searching for usage of defaultPrefix:" rg --type typescript 'defaultPrefix' # Search for usage of defaultBytes echo "Searching for usage of defaultBytes:" rg --type typescript 'defaultBytes'Length of output: 310
Script:
#!/bin/bash # Description: Check for usage of new defaultPrefix and defaultBytes fields in TypeScript files # Search for usage of defaultPrefix echo "Searching for usage of defaultPrefix in .ts and .tsx files:" rg 'defaultPrefix' --glob '*.ts' --glob '*.tsx' # Search for usage of defaultBytes echo "Searching for usage of defaultBytes in .ts and .tsx files:" rg 'defaultBytes' --glob '*.ts' --glob '*.tsx'Length of output: 3851
apps/dashboard/lib/trpc/routers/index.ts (3)
4-5: LGTM: New imports added correctlyThe new import statements for
setDefaultApiBytesandsetDefaultApiPrefixare correctly placed and follow the existing naming conventions. These imports correspond to the new router methods added later in the file, which aligns with the PR objectives of allowing users to set default values for bytes and prefix.
Line range hint
1-124: Overall assessment: Changes look good and align with PR objectivesThe additions to this file successfully implement the router methods for setting default API prefix and bytes. These changes directly address the PR objectives of enabling custom defaults for key creation, which should improve the user experience. The code is well-structured and consistent with the existing patterns in the file.
To fully validate the implementation:
- Ensure that the imported functions (
setDefaultApiPrefixandsetDefaultApiBytes) are properly implemented with necessary error handling and data validation.- Verify that these new router methods are properly integrated with the frontend to allow users to set and save their preferred defaults.
- Consider adding unit tests for these new router methods to ensure their correct functionality.
78-79: LGTM: New router methods added correctlyThe new router methods
setDefaultPrefixandsetDefaultBytesare correctly added to theapirouter. These additions directly address the PR objectives of enabling custom defaults for key prefix and bytes length, which should improve the user experience when creating multiple keys.To ensure the completeness of the implementation, please run the following script to check the contents of the imported functions:
This will help us verify that these functions are properly implemented with necessary error handling and data validation.
✅ Verification successful
Verification Successful: Implementations are Robust and Reliable
The implementations of
setDefaultApiPrefixandsetDefaultApiBytesinclude comprehensive error handling and data validation as required. Both functions utilizezodfor input validation, manage errors gracefully withTRPCError, and ensure data integrity through transactions and audit logging. These implementations effectively address the PR objectives and adhere to best practices, ensuring stability and reliability.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of setDefaultApiPrefix and setDefaultApiBytes # Check setDefaultApiPrefix echo "Checking setDefaultApiPrefix:" fd --type f --exec grep -Hn "export.*setDefaultApiPrefix" {} \; fd --type f --exec grep -A 10 -Hn "function setDefaultApiPrefix" {} \; # Check setDefaultApiBytes echo "Checking setDefaultApiBytes:" fd --type f --exec grep -Hn "export.*setDefaultApiBytes" {} \; fd --type f --exec grep -A 10 -Hn "function setDefaultApiBytes" {} \;Length of output: 689
Script:
#!/bin/bash # Description: Inspect the implementation of setDefaultApiPrefix and setDefaultApiBytes for error handling and data validation echo "Inspecting setDefaultApiPrefix implementation:" cat ./apps/dashboard/lib/trpc/routers/api/setDefaultPrefix.ts echo "Inspecting setDefaultApiBytes implementation:" cat ./apps/dashboard/lib/trpc/routers/api/setDefaultBytes.tsLength of output: 6001
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (3)
148-149: Props type updated to include new default values.The
Propstype has been extended to includedefaultBytesanddefaultPrefix, both of which can be null. This change allows for more flexible key creation by providing default values.
152-152: Component signature updated to use new props.The
CreateKeycomponent now acceptsdefaultBytesanddefaultPrefixas props, aligning with the updatedPropstype.
Line range hint
1-724: Summary: Successfully implemented default bytes and prefix feature.The changes in this PR successfully implement the ability to set default values for bytes and prefix when creating a new API key. This enhancement addresses the issue #148, allowing users to streamline their key creation process.
Key points:
- The
Propstype and component signature have been updated to includedefaultBytesanddefaultPrefix.- The form's default values now use these new props, falling back to previous defaults when not provided.
- The implementation is correct and doesn't introduce any obvious bugs.
Minor suggestions for improvement have been made to enhance clarity and user experience, but overall, the changes effectively solve the problem described in the linked issue.
apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx (3)
7-8: Imports added for new componentsThe imports for
DefaultBytesandDefaultPrefixare added correctly, and the paths are accurate. This allows for the new default settings components to be used in the page.
46-56: Fetching and validatingkeyAuthobjectThe code appropriately fetches the
keyAuthobject associated with the API, ensuring it is not deleted and belongs to the correct tenant. The null checks and tenant ID verification provide necessary security and correctness.
61-62: Rendering new default settings componentsThe
DefaultBytesandDefaultPrefixcomponents are correctly integrated into the settings page, receiving thekeyAuthobject as a prop. This ensures they have access to the necessary data to function properly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx (1)
1-98: Overall implementation is solid, with two issues to addressThe DefaultPrefix component is well-implemented and successfully achieves the PR objective of allowing users to set default values for the key prefix. The use of react-hook-form and zod for form handling and validation is appropriate and well-executed.
Key strengths:
- Clear component structure
- Proper use of UI components
- Good form state management
- Effective error handling and user feedback
However, there are two issues that need to be addressed:
- The async mutation in the onSubmit function should be awaited.
- The hidden inputs for workspaceId and keyAuthId should be properly registered with react-hook-form.
Once these issues are resolved, the component will be fully functional and align with best practices.
Consider adding a unit test for this component to ensure its functionality remains intact during future changes. The test should cover:
- Successful form submission
- Validation errors (e.g., prefix too long, same as existing)
- Proper handling of the trpc mutation
apps/dashboard/app/(app)/authorization/permissions/page.tsx (1)
6-6: LGTM! Consider grouping related imports.The import of
permissionsfrom@unkey/db/src/schemais now being utilized in the file, which is good. This change aligns with the PR objectives and may be part of the implementation for custom defaults.For better code organization, consider grouping this import with other database-related imports, such as moving it next to the
import { asc, db } from "@/lib/db";line.apps/api/src/pkg/testutil/harness.ts (2)
290-291: LGTM! Consider adding type annotations for clarity.The addition of
defaultPrefixanddefaultBytesproperties to theunkeyKeyAuthobject aligns well with the PR objectives. These properties will allow for setting default values for bytes and prefix, enhancing the key creation process.Consider adding type annotations to these new properties for improved code clarity:
defaultPrefix: string | null; defaultBytes: number | null;
302-303: LGTM! Consider adding type annotations for consistency.The addition of
defaultPrefixanddefaultBytesproperties to theuserKeyAuthobject is consistent with the changes made tounkeyKeyAuth. This implementation aligns well with the PR objectives and maintains consistency across the codebase.For consistency with the previous suggestion, consider adding type annotations to these new properties:
defaultPrefix: string | null; defaultBytes: number | null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- apps/api/src/pkg/testutil/harness.ts (2 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/settings/default-bytes.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx (1 hunks)
- apps/dashboard/app/(app)/authorization/permissions/page.tsx (2 hunks)
- packages/ratelimit/package.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ratelimit/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/app/(app)/apis/[apiId]/settings/default-bytes.tsx
🔇 Additional comments (10)
apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx (6)
1-24: LGTM: Imports and schema definition are well-structuredThe imports cover all necessary components and utilities for the implementation. The zod schema correctly defines the required fields (keyAuthId, workspaceId, defaultPrefix) as per the PR objectives.
26-32: LGTM: Props type definition is accurateThe Props type correctly defines the structure of the keyAuth object, including all necessary fields (id, workspaceId, defaultPrefix) for setting the default prefix. The typing of defaultPrefix as
string | undefined | nullappropriately handles cases where it might not be set.
34-43: LGTM: Component initialization and form setup are well-implementedThe DefaultPrefix component is correctly set up as a React functional component. The useForm hook is properly utilized with zod schema validation. Default form values are accurately set using the keyAuth prop, with appropriate handling of potentially null defaultPrefix.
84-95: LGTM: Form footer and submit button are well-implementedThe form footer and submit button are implemented effectively:
- The button's variant and disabled state are correctly set based on the form's validity and submission state.
- The use of a loading indicator during form submission enhances the user experience.
- The conditional rendering of "Save" text or loading indicator is a nice touch.
These implementations align well with best practices for form UX.
45-63:⚠️ Potential issueAwait the asynchronous mutation to handle errors properly
The mutation setup and form submission logic are generally well-implemented. However, there's an important issue that needs to be addressed:
In the
onSubmitfunction, the call tosetDefaultPrefix.mutateAsync(values);is asynchronous but not awaited. This means errors may not be caught properly, and the function may proceed before the mutation completes.Apply this diff to await the mutation:
async function onSubmit(values: z.infer<typeof formSchema>) { if (values.defaultPrefix.length > 8) { return toast.error("Default prefix is too long, maximum length is 8 characters."); } if (values.defaultPrefix === keyAuth.defaultPrefix) { return toast.error("Please provide a different prefix than already existing one as default"); } - setDefaultPrefix.mutateAsync(values); + await setDefaultPrefix.mutateAsync(values); }This change ensures that any errors from the mutation are properly caught and handled.
65-83:⚠️ Potential issueRegister hidden inputs with React Hook Form
The form rendering is generally well-structured and uses appropriate UI components. However, there's an important issue that needs to be addressed:
The hidden
<input>elements forworkspaceIdandkeyAuthIdare not registered with React Hook Form. As a result, these values may not be included in the form submission, leading to missing data when submitting the form.Apply this diff to register the hidden fields:
- <input type="hidden" name="workspaceId" value={keyAuth.workspaceId} /> - <input type="hidden" name="keyAuthId" value={keyAuth.id} /> + <FormField + control={form.control} + name="workspaceId" + render={({ field }) => ( + <input type="hidden" {...field} value={keyAuth.workspaceId} /> + )} + /> + <FormField + control={form.control} + name="keyAuthId" + render={({ field }) => ( + <input type="hidden" {...field} value={keyAuth.id} /> + )} + />This change ensures that the
workspaceIdandkeyAuthIdfields are properly registered with React Hook Form and included in the form submission.apps/dashboard/app/(app)/authorization/permissions/page.tsx (3)
22-22: LGTM! Improved query formatting.The reformatting of the
orderByclause improves code consistency and maintainability. While it doesn't change the functionality, the new array syntax is more aligned with Drizzle ORM's query builder and allows for easier addition of multiple ordering criteria in the future if needed.
Line range hint
1-114: Summary: Minor improvements, but main feature implementation unclear.The changes in this file are positive improvements to code organization and consistency. However, they don't directly implement the functionality described in the PR objectives (setting default values for bytes and prefix for each API).
To ensure a comprehensive review:
- Could you provide more context on how these changes contribute to the overall feature implementation?
- Are there other files where the main functionality for setting default bytes and prefix is implemented?
- If this is part of a larger change, consider updating the PR description to clarify the role of each modified file in achieving the PR objectives.
These clarifications will help ensure that the changes align with the intended feature and that no critical parts of the implementation are overlooked in the review process.
Line range hint
1-114: Please clarify how these changes relate to the PR objectives.While the changes in this file are improvements, they don't seem to directly implement the functionality described in the PR objectives (setting default values for bytes and prefix for each API). Could you please clarify how these changes contribute to the overall feature implementation? Are there other files where the main functionality is implemented?
To help understand the context better, let's check for related changes in other files:
✅ Verification successful
Please see the updated shell scripts below to search for relevant implementations:
[/run_scripts]
Clarification Provided
The changes in
apps/dashboard/app/(app)/authorization/permissions/page.tsxare part of the overall implementation for setting default bytes and prefixes for each API. The actual functionality for setting these defaults is handled in other parts of the codebase, as identified in multiple files related to API configurations. Therefore, the modifications in this file support the PR objectives by integrating with the existing default settings mechanisms.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for files that might contain the implementation of default bytes and prefix functionality echo "Searching for files with potential implementation of default bytes and prefix:" rg -i -t typescript -t javascript "(default.*bytes|default.*prefix|setDefault)" --glob '!**/node_modules/**' echo "\nSearching for changes to API-related files:" rg -i -t typescript -t javascript "api.*default" --glob '!**/node_modules/**'Length of output: 528
Script:
#!/bin/bash # Search for files that might contain the implementation of default bytes and prefix functionality using glob patterns echo "Searching for files with potential implementation of default bytes and prefix:" rg -i "(default.*bytes|default.*prefix|setDefault)" --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' --glob '!**/node_modules/**' echo "\nSearching for changes to API-related files:" rg -i "api.*default" --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' --glob '!**/node_modules/**'Length of output: 9061
apps/api/src/pkg/testutil/harness.ts (1)
290-291: Summary: Changes align well with PR objectivesThe additions of
defaultPrefixanddefaultBytesproperties to bothunkeyKeyAuthanduserKeyAuthobjects are well-implemented and align perfectly with the PR objectives. These changes will enable setting default values for bytes and prefix, enhancing the key creation process as requested in issue #148.Key points:
- The changes are minimal and focused, reducing the risk of introducing bugs.
- The implementation is consistent across both objects, which is good for maintainability.
- Initializing the new properties to
nullallows for optional setting of these values.- The existing functionality of the
Harnessclass remains unaffected.To ensure these changes are properly utilized, please verify that the logic for setting and using these default values is implemented in the relevant key creation functions. You may want to run the following command to check for usage of these new properties:
Also applies to: 302-303
✅ Verification successful
Verified: Changes are consistent and correctly implemented
The additions of
defaultPrefixanddefaultBytesset tonullinapps/api/src/pkg/testutil/harness.tsalign with their usage across the codebase. This approach allows for optional settings without impacting existing functionality.No issues were found regarding the integration of these properties.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
rg "defaultPrefix|defaultBytes" --type typescriptLength of output: 86
Script:
#!/bin/bash # Attempting to search for `defaultPrefix` and `defaultBytes` in TypeScript files using the correct type identifier rg "defaultPrefix|defaultBytes" --type tsLength of output: 3626
|
@perkinsjr this should be open for review now. We had to include the defaultBytes and defaultPrefix in the harness. That was the reason API build was failing |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
5 participants
What does this PR do?
Allows to set default bytes and prefix for each API.
https://www.loom.com/share/ba99adb6585940f2be8834442a46210a?sid=ad746bca-d592-4533-b110-dda6b94bf0e9
Fixes #148
If there is not an issue with this, please create one first. This is used for tracking purposes and also helps us understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
DefaultBytesandDefaultPrefixcomponents for managing default settings of API keys.CreateKeycomponent to accept new configuration options for bytes and prefix.Bug Fixes
Documentation