Skip to content
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.

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

feat: add refill option 'never' in case you dont ever want to refil the number of uses #2780

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ShashankGupta10
Copy link

@ShashankGupta10 ShashankGupta10 commented Dec 23, 2024

What does this PR do?

Fixes #2742

This PR adds a "never" option for the refill interval when creating a key in the dashboard. Currently, when you enable "limited use" in the key creation process, there is no way to opt-out of refilling. This change solves that issue by introducing a "never" option, which disables refilling, allowing users to set a key with no refill.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Create a new key and enable the remaining option.
  • Select the refill interval and choose the never option.
  • Verify that the amount and refillDay fields are hidden or set to their default values (0 for amount, 1 for refillDay)

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

Summary by CodeRabbit

  • New Features

    • Added a new refill interval option "never" for API keys, allowing users to specify no refills.
    • Updated minimum refill amount to allow a value of 0.
  • Bug Fixes

    • Enhanced validation for refill settings to accommodate new options.
  • Documentation

    • Updated API documentation to reflect changes in refill configurations.
  • Style

    • Improved UI components to support new refill interval selection.
  • Tests

    • Added tests to validate new refill configurations and ensure correct functionality.

Copy link

changeset-bot bot commented Dec 23, 2024

⚠️ No Changeset found

Latest commit: e9bee56

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Dec 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
engineering ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 23, 2024 8:48pm

@CLAassistant
Copy link

CLAassistant commented Dec 23, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

vercel bot commented Dec 23, 2024

@ShashankGupta10 is attempting to deploy a commit to the Unkey Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

coderabbitai bot commented Dec 23, 2024

📝 Walkthrough

Walkthrough

This pull request introduces a new "never" option for the refill interval across multiple components of the Unkey application. The changes span the API, dashboard, and database schema, allowing users to explicitly choose not to automatically refill key verifications. The modification enables more flexible key management by providing an opt-out mechanism for automatic refills when creating or updating API keys.

Changes

File Change Summary
apps/api/src/pkg/key_migration/message.ts Added "never" to refill.interval type
apps/api/src/routes/schema.ts Updated keySchema to include "never" in interval enum
apps/api/src/routes/v1_* files Modified request schemas to support "never" interval and allow 0 as minimum refill amount
apps/dashboard/* files Updated UI components, form schemas, and validation to accommodate "never" interval
internal/db/src/schema/* Extended database schema to support "never" in refillInterval

Assessment against linked issues

Objective Addressed Explanation
Provide option to opt out of refilling
Allow creating key with remaining without forced refill

Possibly related PRs

Suggested labels

Feature, Dashboard, 🕹️ oss.gg, :joystick: 300 points, hacktoberfest

Suggested reviewers

  • mcstepp
  • perkinsjr
  • chronark
  • MichaelUnkey

Tip

CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. We would love to hear your feedback on Discord.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the Bug Something isn't working label Dec 23, 2024
Copy link
Contributor

github-actions bot commented Dec 23, 2024

Thank you for following the naming conventions for pull request titles! 🙏

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (4)
apps/dashboard/lib/zod-helper.ts (1)

34-34: Additional 'never' option in interval enum.
This modification is consistent with the PR objective. However, consider clarifying any subtle differences between 'none' and 'never' if they both remain valid choices.

apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/validation.ts (1)

54-54: New default for interval remains 'monthly'.
Implementers might consider whether a default of 'monthly' is ideal or if 'never' is more consistent with the newly introduced behavior.

apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (2)

101-102: Ensure refill properties are removed if limitEnabled is false.

When the limit toggle is off, you remove some properties from values. But you separately define “refill” here. To avoid sending extraneous properties to the server, consider removing the refill object if limitEnabled is false.


538-538: Default to zero for 'limit.refill.amount'.

Here, you default to zero if "limitEnabled" is set but the field is undefined. That’s consistent with the logic for not refilling. Confirm that users clearly see that 0 is set, and it doesn’t cause confusion if the interval is daily/monthly.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ea2eb58 and ddc9f50.

📒 Files selected for processing (14)
  • apps/api/src/pkg/key_migration/message.ts (1 hunks)
  • apps/api/src/routes/schema.ts (1 hunks)
  • apps/api/src/routes/v1_keys_createKey.ts (1 hunks)
  • apps/api/src/routes/v1_keys_updateKey.ts (1 hunks)
  • apps/api/src/routes/v1_migrations_createKey.ts (1 hunks)
  • apps/api/src/routes/v1_migrations_enqueueKeys.ts (1 hunks)
  • apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-remaining.tsx (3 hunks)
  • apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (5 hunks)
  • apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/validation.ts (2 hunks)
  • apps/dashboard/lib/trpc/routers/key/create.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/key/updateRemaining.ts (1 hunks)
  • apps/dashboard/lib/zod-helper.ts (1 hunks)
  • internal/db/src/schema/key_migrations.ts (1 hunks)
  • internal/db/src/schema/keys.ts (1 hunks)
🔇 Additional comments (15)
apps/api/src/pkg/key_migration/message.ts (1)

17-17: Implementation aligns with the new 'never' refill option.
This type change correctly introduces "never" to accommodate the use case. No issues here; just ensure that any downstream consumers handle this new interval appropriately.

internal/db/src/schema/key_migrations.ts (1)

33-37: Database schema updated to include 'never' interval.
This extension matches the broader PR objective. Double-check that existing records don’t conflict with this new option (e.g., migrations referencing an old enum that doesn’t include "never"). Otherwise, looks good.

✅ Verification successful

Let me analyze the results and generate a final response.

Schema update for 'never' interval is safely integrated
The codebase shows consistent handling of the "never" interval across all relevant components:

  • Database schema in keys.ts includes "never" in the enum
  • API routes and validation schemas properly include "never" as a valid option
  • Frontend components (dashboard) correctly handle the "never" option in forms and UI
  • Migration-related code properly supports the new interval option

No conflicts or inconsistencies found with existing records or schema definitions. The change has been properly propagated throughout the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verifying references to old refill intervals ('daily','monthly') in codebase
rg -A 3 "refill(\s*)?\{.*interval:\s?\"daily\"|\"monthly\"" 

Length of output: 15048

apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/validation.ts (1)

65-66: Allowing a minimum of 0 and making it optional.
Relaxing the constraint from 1 to 0 is appropriate for the "never" scenario. Making this field optional may reduce strictness. Verify whether the UI should supply a default or prompt the user when the value is missing.

✅ Verification successful

Let me gather more information about this validation and its usage in the UI to verify the concerns.


Let me check the client component to understand how this validation is used in the UI.


Based on the analysis of both the validation schema and the client component, I can now provide the final response:

Making the refill amount optional with min(0) is appropriate for this use case.
The validation is correctly implemented as the refill amount is only required when the limit is enabled and the refill interval is not "never". The UI handles this by:

  1. Disabling the refill amount input when interval is "never"
  2. Setting a default value of 0 when the field is enabled
  3. Properly cleaning up the form when limit features are disabled
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Find the validation schema and its usage
ast-grep --pattern 'const $_ = z.object({
  $$$
})'  apps/dashboard/app/\(app\)/apis/[apiId]/keys/[keyAuthId]/new/validation.ts

# Find the UI components that use this validation
rg -l "validation" apps/dashboard/app/\(app\)/apis/[apiId]/keys/[keyAuthId]/new/

# Check for any default value assignments or form handling
rg "defaultValues" apps/dashboard/app/\(app\)/apis/[apiId]/keys/[keyAuthId]/new/

Length of output: 11951


Script:

#!/bin/bash
# Check the client component implementation
cat apps/dashboard/app/\(app\)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx

Length of output: 35564

apps/dashboard/lib/trpc/routers/key/updateRemaining.ts (1)

15-16: Expanded refill interval and lowered minimum refill amount
The addition of "never" to the interval enum and lowering the refill amount’s minimum from 1 to 0 elegantly aligns with the new feature allowing no refills. Be sure to test edge cases where the interval is "never" and amount is set to 0 or left out entirely.

internal/db/src/schema/keys.ts (1)

66-66: New “never” enum entry for refillInterval
Introducing “never” in the database enum ensures consistency with the rest of the application. Confirm you have no constraints forcing non-null in cases where “none” was previously used, as the system now stores “never” instead of ignoring a refill interval.

✅ Verification successful

Let me gather information about the usage of refillInterval and any potential constraints in the database schema.

This will help us:

  1. Find all files using refillInterval to check for constraints
  2. Look for schema definitions and validations
  3. Find any SQL migrations that might have NOT NULL constraints

Let's check the actual schema definition and any validation logic around refillInterval to understand the constraints.

This will help us:

  1. See the complete schema context around refillInterval
  2. Check how "never" is handled in create/update operations
  3. Look for any validation rules that might enforce non-null constraints

Let's check for any NOT NULL constraints in the database migrations and look at the actual usage of refillInterval in the codebase.

This will help us:

  1. Find any NOT NULL constraints in migrations
  2. Understand how refillInterval is handled during key creation/updates
  3. Check for any default values that might have been set

Based on the gathered information, I can now provide a final response:

Verified: "never" enum value is safely implemented
The codebase shows that refillInterval is properly handled as a nullable field, with explicit null handling in both create and update operations. The schema allows for the new "never" value, and there are no NOT NULL constraints that would conflict with this change. The update logic specifically handles null cases by setting related fields (refillAmount, refillDay, lastRefillAt) to null as well.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for refillInterval usage and constraints in schema files
rg -l "refillInterval|refill_interval" --type ts

# Look for NOT NULL constraints or related validations
ast-grep --pattern 'refillInterval: $$$'

# Search for any migration files that might contain refill_interval
fd -e sql . | xargs grep -l "refill_interval"

Length of output: 1121


Script:

#!/bin/bash
# Check the schema definition in keys.ts
rg -A 5 -B 5 "refillInterval" internal/db/src/schema/keys.ts

# Look for validation logic in create/update operations
rg -A 5 "refillInterval.*never" apps/api/src/routes/v1_keys_createKey.ts apps/api/src/routes/v1_keys_updateKey.ts

# Check for any zod validations
rg -A 5 "refillInterval" apps/dashboard/lib/zod-helper.ts

Length of output: 858


Script:

#!/bin/bash
# Search for migrations with refill_interval
fd migration .sql$ | xargs cat

# Check how refillInterval is used in key creation and updates
rg -B 5 -A 5 "refill_interval|refillInterval" apps/api/src/routes/v1_keys_createKey.ts
rg -B 5 -A 5 "refill_interval|refillInterval" apps/api/src/routes/v1_keys_updateKey.ts

# Look for any default values or constraints in schema
rg -B 5 -A 5 "refill_interval.*NOT NULL|refill_interval.*DEFAULT" 

Length of output: 3549

apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-remaining.tsx (3)

45-49: Refill interval expansion & zero-limit
The addition of "never" as an interval and acceptance of 0 for the refill amount align with the new “no refill” feature. The changes look good from an input validation standpoint.


74-74: Prop type update
Expanding the type to include "never" is consistent with schema changes. This helps avoid runtime type errors.


198-198: Dropdown addition for “never”
Adding the “Never” option to the UI matches the updated backend schema, ensuring users can visually select the new interval.

apps/api/src/routes/v1_migrations_enqueueKeys.ts (1)

126-130: Ensure logical consistency for “never” interval usage.

With the introduction of "never" as a valid interval, consider verifying that clients do not provide a refillDay or amount that could cause confusion. For clarity, you might enforce or document that if interval = "never", the refillDay or refill amount is ignored or must be null.

✅ Verification successful

Let me gather more information about how the interval, refillDay, and amount fields are used in the codebase.


Based on the search results, I can now provide a final response. The codebase already handles the logical consistency for "never" interval appropriately:

  1. In the UI (apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx), the amount and refillDay fields are automatically disabled when interval is set to "never".
  2. The schema definitions consistently allow amount to be 0 (z.number().int().min(0)) and refillDay to be optional across all routes.
  3. The OpenAPI descriptions clearly document the behavior of these fields.

No changes needed - the implementation already handles "never" interval consistently

The current implementation correctly handles the case when interval is "never":

  • The UI automatically disables related fields
  • The schema allows zero amount and optional refillDay
  • The API documentation clearly describes the behavior
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for the schema definition and usage of these fields
ast-grep --pattern 'z.object({
  $$$
  interval: $_,
  $$$
})'

# Also search for any direct references to these fields
rg -A 5 "interval.*never" 

Length of output: 28863

apps/api/src/routes/v1_keys_updateKey.ts (1)

146-150: Validate “never” interval edge cases.

When interval is "never", ensure that no refill logic (e.g., refillDay) applies. Consider restricting refillDay or amount if interval = "never" to prevent conflicting configurations.

✅ Verification successful

Let me search for related code to verify the handling of "never" interval and its interaction with refill logic.


Based on the search results, I can now provide a definitive response about the handling of the "never" interval:

Validation for "never" interval is properly implemented.

The codebase correctly handles the "never" interval case:

  • When interval is "never", the UI disables the amount input field (apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx)
  • The database schema (internal/db/src/schema/keys.ts) properly supports the "never" value in the refill_interval enum
  • When refill.interval is "never" or "none", the refill configuration is removed entirely (apps/dashboard/lib/trpc/routers/key/updateRemaining.ts)
  • The refill functionality is only activated when interval is "daily" or "monthly", effectively making "never" act as a no-refill state
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for refill-related logic and interval handling
rg -A 10 "interval.*never" 

Length of output: 16169


Script:

#!/bin/bash
# Search for refillDay or amount validation logic
ast-grep --pattern 'z.object({
  $$$
  interval: $_,
  $$$
  amount: $_,
  $$$
})'

Length of output: 17592


Script:

#!/bin/bash
# Look for refill implementation logic
rg -A 10 "refill.*interval"

Length of output: 66218

apps/api/src/routes/v1_migrations_createKey.ts (1)

126-130: Check for complete handling of “never” interval.

If the interval is set to "never", clarify how the amount and refillDay should be interpreted. Consider enforcing a constraint that refillDay must be null or ignored in this case for consistency.

✅ Verification successful

Let me gather more information about how the "never" interval is handled in the codebase.


Let me search for any validation or processing logic specifically related to refillDay.


Based on the search results, I can now provide a final response:

Handling of "never" interval is properly implemented

The codebase shows consistent handling of the "never" interval case:

  • For "daily" interval: refillDay must be null (explicitly validated)
  • For "monthly" interval: refillDay is used to specify the day of refill (1-31)
  • For "never" interval: refillDay is effectively ignored as no refills occur, and the schema makes refillDay optional

The implementation in v1_keys_createKey.ts and v1_apis_listKeys.ts shows that refillDay is only considered when interval is "monthly", which is the correct behavior. The schema's description accurately documents that refillDay is only meaningful for monthly intervals.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for the schema definition and handling of interval, amount, and refillDay
rg -A 10 'interval.*enum.*never' apps/api/src/routes/

Length of output: 5583


Script:

#!/bin/bash
# Look for any validation or processing logic related to the "never" interval
ast-grep --pattern 'if ($expr.includes("never")) { $$$ }'

Length of output: 59


Script:

#!/bin/bash
# Search for any database operations or API handlers dealing with "never" interval
rg -B 5 -A 5 'interval.*never' apps/api/src/

Length of output: 5896


Script:

#!/bin/bash
# Search for validation logic related to refillDay and interval combinations
ast-grep --pattern 'if ($expr.interval === "never") { $$$ }'

Length of output: 62


Script:

#!/bin/bash
# Look for any transformations or validations of refillDay
rg -B 5 -A 5 'refillDay.*null' apps/api/src/

Length of output: 9761


Script:

#!/bin/bash
# Search for any comments or documentation about refillDay behavior
rg -B 2 -A 2 'refillDay.*when.*interval' apps/api/src/

Length of output: 56

apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (3)

515-515: Refill interval "Never" option is consistent with the PR objective.

Adding “Never” addresses the need for a ‘do not refill’ option, aligning with the PR’s goal. This looks good.


554-555: Refill day is disabled for 'daily' and 'never' intervals.

This aligns with the idea that monthly is the only case needing a specific day. Looks good.


526-526: Accessibility of 'limit.refill.amount' when interval is "never".

Disabling the input if interval is "never" ensures no accidental fill of an irrelevant field, which is correct. Validate that the unintended submission doesn’t sneak in a leftover value due to form states.

✅ Verification successful

Let me gather information about the form implementation and validation logic to verify this concern.


Let me check the validation schema to verify how the form handles the limit.refill.amount when interval is "never".


Based on the code analysis, I can now provide the final response:

Form validation and submission are properly handling the "never" interval case

The implementation is correct and safe because:

  1. The form field is properly disabled when interval is "never" through the disabled prop
  2. The validation schema makes the amount field optional with optional()
  3. The form submission logic in onSubmit handles the refill data appropriately by:
    • Removing the entire limit object if limitEnabled is false
    • Only including refill data when it exists
    • Setting a default amount of 0 when needed

No unintended submissions of leftover values can occur as the validation and submission logic properly handle this case.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for the form implementation and validation logic in the file
cat "apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx"

Length of output: 35564


Script:

#!/bin/bash
# Check the validation schema
cat "apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/validation.ts"

Length of output: 3240

apps/api/src/routes/schema.ts (1)

61-61: Expansion of 'refill.interval' to include 'never'.

This matches the changes in the UI, maintaining consistent validation logic across the system. Good addition.

Comment on lines +21 to +22
interval: z.enum(["daily", "monthly", "never"]),
amount: z.coerce.number().int().min(0),
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the code analysis, I can now generate the final response:

Zero refill amounts are not allowed for daily/monthly intervals

The codebase enforces validation that prevents setting a refill amount to zero when using daily or monthly intervals:

  • In apps/dashboard/lib/trpc/routers/key/create.ts, the schema validation requires amount: z.coerce.number().int().min(0) but additional checks are present in the codebase:
  • In apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-remaining.tsx, there's explicit validation:
    if (values.refill?.interval !== "none" && !values.refill?.amount) {
      form.setError("refill.amount", {
        message: "Please enter the number of uses per interval",
      });
      return;
    }
  • The API routes in apps/api/src/routes/v1_keys_createKey.ts and related files also enforce that refill amounts must be provided when using intervals.
🔗 Analysis chain

Allowing zero refill amounts for “never”
Permitting a minimum value of 0 is consistent with “never” refills. Verify that logic remains sound for daily/monthly intervals if a user attempts to set the amount to zero.


Let me gather more information about how the refill amount is handled for different intervals.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for usage of interval and amount in the codebase
rg -A 10 "interval.*amount" --type ts

Length of output: 2298


Script:

#!/bin/bash
# Look for validation or processing logic related to refills
ast-grep --pattern 'if ($_ === "never") {
  $$$
}'

Length of output: 52


Script:

#!/bin/bash
# Search for any refill-related logic
rg "refill" --type ts -A 5

Length of output: 90681

Comment on lines +113 to +116
interval: z.enum(["daily", "monthly", "never"]).openapi({
description: "Unkey will automatically refill verifications at the set interval.",
}),
amount: z.number().int().min(1).positive().openapi({
amount: z.number().int().min(0).openapi({
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the evidence gathered, I can now provide a conclusive response:

Validation needed for "never" interval and refill settings

The codebase shows inconsistent handling of the "never" interval with refill settings:

  1. While the UI disables the refill amount input when interval is "never", there's no explicit validation in the API to prevent setting an amount.
  2. The codebase has validation for daily intervals (if (req.refill?.interval === "daily" && req.refill.refillDay)), but lacks similar validation for "never" intervals.
  3. The schema descriptions and validations focus only on "daily" and "monthly" cases, leaving the "never" case undefined.

To maintain consistency:

  • Add validation in v1_keys_createKey.ts to ensure refillDay and amount are null/undefined when interval is "never"
  • Update the OpenAPI schema description to clarify the behavior with "never" interval
🔗 Analysis chain

Refill logic for “never” interval.

Similar to the other routes, confirm that specifying interval = "never" is compatible with any provided amount or refillDay. It may be worthwhile to systematically disallow setting a refillDay if interval = "never" to avoid ambiguity.


Let me search for related code to verify the refill logic implementation.


Based on the search results, I found important evidence about how the "never" interval is handled:

  1. In the UI (dashboard), there are explicit checks to disable certain fields when interval is "never":
disabled={form.watch("limit.refill.interval") === "never"}
  1. The schema definitions consistently show that refillDay is described specifically for monthly intervals:
refillDay: z.number().min(1).max(31).optional().openapi({
  description: "The day verifications will refill each month, when interval is set to 'monthly'",
})

Let me check for any additional validation logic.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for refill logic implementation and validation
rg -A 10 'interval.*"never"' 

Length of output: 16171


Script:

#!/bin/bash
# Search for schema definitions related to refillDay
rg -A 5 'refillDay.*z\.' 

Length of output: 5353


Script:

#!/bin/bash
# Search for any validation or business logic related to refill intervals
ast-grep --pattern 'if ($_.interval === "never") {
  $$$
}'

Length of output: 61


Script:

#!/bin/bash
# Search for validation logic around refill intervals and refillDay
rg -A 5 'if.*interval.*refillDay' 

Length of output: 3286


Script:

#!/bin/bash
# Look for any validation functions related to refills
ast-grep --pattern 'function $name($_) {
  $$$
  refill
  $$$
}'

Length of output: 66

if (refill?.interval === "daily") {
refill?.refillDay === undefined;
refill.refillDay === undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Likely a no-op statement.

The line “refill.refillDay === undefined;” is a strict equality check and does not assign or return anything. This likely has no effect and may be a bug or stale code. Remove or replace it with the intended assignment or logic.

-    refill.refillDay === undefined;
+    // Possibly intended to reset the field:
+    refill.refillDay = undefined;
📝 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
refill.refillDay === undefined;
// Possibly intended to reset the field:
refill.refillDay = undefined;

@harshsbhat
Copy link
Contributor

harshsbhat commented Dec 24, 2024

Why would you create a PR for an issue assigned to me? 🤔. Not like I have a problem with giving up on any issue. You should have at least told me that you are working on this. Anyways good luck @ShashankGupta10

Also looking at the PR I am pretty sure this issue is only meant for the dashboard and not the API

curl --request POST \
  --url https://api.unkey.dev/v1/keys.createKey \
  --header 'Authorization: Bearer unkey_3Za' \
  --header 'Content-Type: application/json' \
  --data '{
  "apiId": "api_2gF",
  "remaining": 5
}'

I can easily create the remaining keys without any refill through API. The problem is on the dashboard where creating the key remaining without any refill is not possible.

@ShashankGupta10
Copy link
Author

Hey @harshsbhat, did not mean to take up your issue. If you want to work on it, go ahead. Forked this repo 2 days back trying to look in when i found this issue and started working on it. You can make another PR if you think the issue can be solved in a better way. That way I get to learn as well! Thanks and good luck!

@harshsbhat
Copy link
Contributor

harshsbhat commented Dec 24, 2024

Hey, @ShashankGupta10

I think this PR will be merged soon by the Unkey team. So I don't think creating multiple PRs is the right thing to do here. 😄

@ogzhanolguncu
Copy link
Contributor

We are currently on Christmas holiday, but I will try to review this PR this week. Thanks for your efforts, everyone. And please don't argue over PRs - we have plenty of them to work on 😀.

@chronark
Copy link
Collaborator

Quickly glancing at this, I saw it's making changes to the API, which we can't take as is.

This should not change the api at all, since we already have another PR to address that.

@ShashankGupta10
Copy link
Author

So just making changes to the frontend with the never option and handling that specific case onSubmit would do?
It would be great if you could give some guidance. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Dashboard Unkey dashboard related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating a key in the UI with remaining, forces you to choose an automatic refill interval
5 participants