fix: Deleting the override doesn't remove it from the overrides. Refreshing it will give you a client side error. #2246 #2341
Conversation
const namespace = await db.query.ratelimitNamespaces.findFirst({
where: (table, { eq, and, isNull }) =>
and(
eq(table.id, props.params.namespaceId),
isNull(table.deletedAt)
),
with: {
overrides: {
columns: {
id: true,
identifier: true,
limit: true,
duration: true,
async: true,
},
where: (table, { isNull }) => // excluding soft deletes columns
isNull(table.deletedAt)
},
workspace: {
columns: {
id: true,
tenantId: true,
features: true,
},
},
},
});
kohi9noor
requested review from
perkinsjr,
chronark and
mcstepp
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
📝 Walkthrough📝 WalkthroughWalkthroughThe changes made in the pull request focus on the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OverridePage
participant Database
User->>OverridePage: Request to view overrides
OverridePage->>Database: Fetch overrides with condition (isNull(table.deletedAt))
Database-->>OverridePage: Return non-deleted overrides
OverridePage-->>User: Display overrides or empty placeholder
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 (
|
|
@kohi9noor is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
kohi9noor
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx (2)
39-40: Effective filtering of deleted overridesThe addition of the
whereclause in theoverridesquery is an excellent improvement:
- It directly addresses the PR objective by filtering out soft-deleted overrides.
- This change will prevent client-side errors when refreshing, as mentioned in the PR objectives.
For consistency with the namespace query, consider using the
andfunction, even though there's currently only one condition. This will make future additions easier:-where: (table, { isNull }) => - isNull(table.deletedAt) +where: (table, { and, isNull }) => + and( + isNull(table.deletedAt) + )
Additional
ratelimitNamespaces.findFirstQueries MissingdeletedAtFiltersThe following files contain
ratelimitNamespaces.findFirstqueries withoutdeletedAtfilters and should be updated to maintain data integrity:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/layout.tsxapps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/page.tsxapps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/logs/page.tsxapps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/settings/page.tsxapps/api/src/routes/v1_ratelimit_limit.tsThese instances need to include
deletedAtnull checks to ensure consistency and prevent potential data issues.🔗 Analysis chain
Line range hint
1-89: Summary of changes and their impactThe modifications in this file effectively address the PR objectives:
- Both namespace and override queries now filter out soft-deleted records.
- These changes prevent client-side errors when refreshing and ensure that only valid data is displayed.
- The implementation is consistent across queries and improves overall data integrity.
The rest of the file, including the component's rendering logic, remains unchanged. This focused approach minimizes the risk of introducing new issues while solving the targeted problem.
To ensure that these changes don't affect other parts of the application, please run the following verification:
This script will help identify any other locations where similar changes might be necessary to maintain consistency across the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any other usage of ratelimitNamespaces or overrides queries that might need similar updates # Search for ratelimitNamespaces queries echo "Searching for ratelimitNamespaces queries:" rg --type typescript 'ratelimitNamespaces\.findFirst' --glob '!**/overrides/page.tsx' # Search for overrides queries echo "Searching for overrides queries:" rg --type typescript 'overrides:.*where:' --glob '!**/overrides/page.tsx'Length of output: 403
Script:
#!/bin/bash # Description: Check for any other usage of ratelimitNamespaces or overrides queries that might need similar updates # Search for ratelimitNamespaces queries in TypeScript files excluding overrides/page.tsx echo "Searching for ratelimitNamespaces queries:" rg 'ratelimitNamespaces\.findFirst' -g '*.ts' -g '*.tsx' --glob '!apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx' # Search for overrides queries in TypeScript files excluding overrides/page.tsx echo "Searching for overrides queries:" rg 'overrides\s*:\s*.*where\s*:' -g '*.ts' -g '*.tsx' --glob '!apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx'Length of output: 1696
Script:
#!/bin/bash # Description: Identify ratelimitNamespaces.findFirst queries without deletedAt null checks echo "Searching for ratelimitNamespaces.findFirst queries without deletedAt filters:" # Find all ratelimitNamespaces.findFirst usages in .ts and .tsx files, excluding the overrides/page.tsx rg 'ratelimitNamespaces\.findFirst' -g '*.ts' -g '*.tsx' --glob '!apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx' \ | grep -v 'deletedAt'Length of output: 1578
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (1)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx (1)
26-29: Improved query structure and data integrityThe restructuring of the
whereclause for the namespace query is a positive change:
- It improves code readability by separating conditions.
- The addition of
isNull(table.deletedAt)ensures that only non-deleted namespaces are retrieved, which aligns with the PR objective of excluding soft-deleted records.This change enhances data integrity and prevents potential issues with deleted namespaces.
|
Hi @kohi9noor this issue is assigned to another user. Please do not work on issues unless they are approved and assigned to you. It is unfair to other participants |
|
@kohi9noor please sign the CLA |
|
@chronark done I have signed the CLA |
What does this PR do?
This PR ensures that soft-deleted records are excluded when querying the ratelimitNamespaces, overrides, and workspace tables. Specifically, it filters out records where the
deletedAtcolumn is notNULL, thereby preventing outdated or deleted data from being returned. This resolves an issue related to the improper display of soft-deleted records.Type of Change
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin main4e50f2a6-55ac-45f2-a20e-b442e99fb026.webm
Summary by CodeRabbit