Skip to content

chore: Add error toasts in remaining mutations to handle errors better.#2251

Merged
chronark merged 4 commits intounkeyed:mainfrom
harshsbhat:harshbhat/toast-error
Oct 8, 2024
Merged

chore: Add error toasts in remaining mutations to handle errors better.#2251
chronark merged 4 commits intounkeyed:mainfrom
harshsbhat:harshbhat/toast-error

Conversation

@harshsbhat
Copy link
Contributor

@harshsbhat harshsbhat commented Oct 7, 2024

What does this PR do?

Fixes #2225

Add toast.error for the remaining mutations. So that incase of failures users get a better error message through toast.

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?

  • Test A: Open any of the mutations from the changed list. Mess something up in the code so that it fails. Try using it on the dashboard. You should get a good toast.error .

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

    • Introduced error handling mechanisms across various components for permission and role management, providing toast notifications for failed operations.
  • Bug Fixes

    • Improved error handling in the deletion and update processes for roles and permissions to ensure better user experience and clarity on failures.

@changeset-bot
Copy link

changeset-bot bot commented Oct 7, 2024

⚠️ No Changeset found

Latest commit: 404084c

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

@vercel
Copy link

vercel bot commented Oct 7, 2024

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

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 7, 2024

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

Name Status Preview Comments Updated (UTC)
dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 8, 2024 1:56pm
engineering ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 8, 2024 1:56pm
play ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 8, 2024 1:56pm
workflows ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 8, 2024 1:56pm
www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 8, 2024 1:56pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 7, 2024

📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

The changes in this pull request implement an error handling mechanism across various components related to permissions and roles. Each mutation now includes an onError callback that triggers a toast notification to inform users of any errors that occur during the operations. This update applies to components responsible for creating, updating, and deleting permissions and roles.

Changes

File Path Change Summary
apps/dashboard/app/(app)/authorization/permissions/[permissionId]/client.tsx Added onError(err) to updatePermission mutation.
apps/dashboard/app/(app)/authorization/permissions/[permissionId]/delete-permission.tsx Added onError(err) to deletePermission mutation.
apps/dashboard/app/(app)/authorization/permissions/create-new-permission.tsx Added onError(err) to createPermission mutation.
apps/dashboard/app/(app)/authorization/roles/[roleId]/delete-role.tsx Added onError(err) to deleteRole mutation.
apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx Added onError(err) to updateRole mutation.

Assessment against linked issues

Objective Addressed Explanation
All mutations need to have toast.error (#2225)

Possibly related PRs

Suggested reviewers

  • mcstepp
  • chronark
  • perkinsjr

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 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 🕹️ 150 points Bug Something isn't working labels Oct 7, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2024

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

@vercel vercel bot temporarily deployed to Preview – engineering October 7, 2024 15:27 Inactive
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: 0

🧹 Outside diff range and nitpick comments (21)
apps/dashboard/components/opt-in.tsx (1)

38-40: LGTM! Error handling improvement implemented.

The addition of the onError callback with toast.error successfully addresses the PR objective of improving error handling for mutations. This change enhances user experience by providing feedback when the opt-in action fails.

Consider the following potential improvements:

  1. Error message sanitization: It might be beneficial to sanitize or format the error message before displaying it to the user. This can help ensure that sensitive information is not exposed and that the message is user-friendly.

  2. Error logging: Consider adding error logging for debugging purposes. This can help with troubleshooting issues in production.

Example implementation:

onError(err) {
  console.error('Opt-in error:', err);
  const userFriendlyMessage = sanitizeErrorMessage(err.message);
  toast.error(userFriendlyMessage);
},

Where sanitizeErrorMessage is a function that formats the error message appropriately for user display.

apps/dashboard/lib/trpc/routers/rbac/deleteRole.ts (1)

71-77: Approve changes with a minor suggestion

The added error handling for insertAuditLogs improves the robustness of the deleteRole mutation. The error message is user-friendly and consistent with other error messages in the file.

However, to improve debugging, consider including the error details in the console log:

- console.error(err);
+ console.error("Error inserting audit logs:", err);

This change will make it easier to identify the source of the error in the logs.

apps/dashboard/app/(app)/settings/general/update-workspace-name.tsx (1)

48-50: Approve changes with a minor suggestion for consistency.

The addition of error handling for the updateName mutation is a good improvement. It aligns well with the PR objectives and enhances the user experience by providing feedback for failed operations.

For consistency with the success message, consider using a more specific error message:

 onError(err) {
-  toast.error(err.message);
+  toast.error(`Failed to update workspace name: ${err.message}`);
 },

This change would provide more context to the user about which operation failed.

apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/update-namespace-name.tsx (1)

50-52: Approve changes with a minor suggestion for consistency.

The added error handling is well-implemented and addresses the PR objective of adding error toasts to mutations. It improves the user experience by providing feedback when the namespace update fails.

For consistency with the success message, consider using a more specific error message:

 onError(err) {
-  toast.error(err.message);
+  toast.error(`Failed to rename namespace: ${err.message}`);
 },

This change would provide more context to the user about which operation failed.

apps/dashboard/lib/trpc/routers/ratelimit/updateNamespaceName.ts (1)

84-89: Improved error handling, but consider transaction consistency

The addition of error handling for insertAuditLogs is a good improvement. It prevents unhandled promise rejections and provides a consistent error response to the client. However, there are two points to consider:

  1. The error message is generic and doesn't indicate that the audit log insertion failed. Consider making it more specific to help with debugging.

  2. If the audit log insertion fails, the transaction might be left in an inconsistent state because the namespace name has already been updated. Consider rolling back the transaction in this case.

Here's a suggested improvement:

 await tx
   .update(schema.ratelimitNamespaces)
   .set({
     name: input.name,
   })
   .where(eq(schema.ratelimitNamespaces.id, input.namespaceId))
   .catch((_err) => {
     throw new TRPCError({
       message:
         "We are unable to update the namespace name. Please contact support using support@unkey.dev",
       code: "INTERNAL_SERVER_ERROR",
     });
   });
 await insertAuditLogs(tx, {
   workspaceId: ws.id,
   actor: {
     type: "user",
     id: ctx.user.id,
   },
   event: "ratelimitNamespace.update",
   description: `Changed ${namespace.id} name from ${namespace.name} to ${input.name}`,
   resources: [
     {
       type: "ratelimitNamespace",
       id: namespace.id,
     },
   ],
   context: {
     location: ctx.audit.location,
     userAgent: ctx.audit.userAgent,
   },
 }).catch((_err) => {
+  // Rollback the transaction
+  await tx.rollback();
   throw new TRPCError({
     message:
-      "We are unable to update the namespace name. Please contact support using support@unkey.dev",
+      "We updated the namespace name but failed to log the action. Please contact support using support@unkey.dev",
     code: "INTERNAL_SERVER_ERROR",
   });
 });

This change ensures that the transaction is rolled back if the audit log insertion fails, maintaining data consistency. It also provides a more specific error message to aid in troubleshooting.

apps/dashboard/lib/trpc/routers/ratelimit/updateOverride.ts (1)

92-97: Approve with suggestions: Enhance error handling for audit logging

The addition of error handling for insertAuditLogs is a good improvement to the function's robustness. However, there are a couple of suggestions to enhance it further:

  1. Make the error message consistent with other error messages in the function by adding a period at the end:

    message:
      "We are unable to update the override. Please contact support using support@unkey.dev."
  2. Consider implementing more specific error handling for audit logging failures. This could involve:

    • Logging the specific error for debugging purposes.
    • Potentially using a different error code or message to distinguish audit logging failures from other types of errors.
apps/dashboard/app/(app)/ratelimits/create-namespace-button.tsx (1)

41-43: Approve changes with a minor suggestion for consistency.

The addition of error handling for the create mutation is a good improvement. It aligns well with the PR objectives to enhance user feedback.

For consistency with the success message, consider updating the error message to be more specific:

 onError(err) {
-  toast.error(err.message);
+  toast.error(`Failed to create namespace: ${err.message}`);
 },

This change would provide more context to the user about which operation failed.

apps/dashboard/app/(app)/authorization/permissions/[permissionId]/client.tsx (1)

52-54: Approve the addition of error handling with a suggestion for improvement.

The addition of the onError callback effectively addresses the PR objective by providing user feedback for failed permission updates. This improvement aligns well with the goals outlined in issue #2225.

Consider enhancing the error handling by adding more specific error messages or handling different error scenarios. For example:

onError(err) {
  const errorMessage = err instanceof Error ? err.message : 'An unexpected error occurred';
  toast.error(`Failed to update permission: ${errorMessage}`);
},

This approach provides a more informative error message and handles cases where err might not be an instance of Error.

apps/dashboard/lib/trpc/routers/ratelimit/deleteNamespace.ts (2)

112-118: Improved error handling, but consider refining the error message.

The addition of error handling for the insertAuditLogs call is a good improvement, aligning with the PR objectives of enhancing error feedback. However, there's a small inconsistency in the error message.

Consider updating the error message to refer to a single namespace:

- "We are unable to delete the namespaces. Please contact support using support@unkey.dev",
+ "We are unable to delete the namespace. Please contact support using support@unkey.dev",

This change would make the message more consistent with the function's purpose of deleting a single namespace.


112-118: Overall improvement in error handling and user feedback.

The changes successfully address the PR objectives by adding error handling to the deleteNamespace mutation. This enhancement provides better feedback to users when errors occur during the namespace deletion process. The use of a transaction ensures data consistency across related operations.

For consistency across the codebase, consider extracting the error message and support email into constants or a configuration file. This would make it easier to maintain and update these messages across multiple files if needed in the future.

Example:

import { ERROR_MESSAGES, SUPPORT_EMAIL } from '@/constants';

// ...

throw new TRPCError({
  code: "INTERNAL_SERVER_ERROR",
  message: `${ERROR_MESSAGES.NAMESPACE_DELETION_FAILED} ${SUPPORT_EMAIL}`,
});

This approach would improve maintainability and ensure consistency in error messages across the application.

apps/dashboard/app/(app)/authorization/roles/[roleId]/delete-role.tsx (1)

63-65: Approve changes with a minor suggestion for consistency.

The addition of the onError callback improves the user experience by providing feedback when the role deletion fails. This change aligns well with the PR objectives and enhances the overall error handling of the component.

For consistency with the existing code, consider using a more specific error message:

 onError(err) {
-  toast.error(err.message);
+  toast.error(`Failed to delete role: ${err.message}`);
 },

This change would make the error toast consistent with the success toast, which uses a specific message "Role deleted successfully".

apps/dashboard/app/(app)/authorization/permissions/[permissionId]/delete-permission.tsx (1)

62-64: Approve changes with a minor suggestion for consistency.

The addition of error handling for the deletePermission mutation is a good improvement. It aligns well with the PR objectives and addresses the issue of providing user feedback for failed operations.

For consistency with the success message, consider using a more specific error message:

 onError(err) {
-  toast.error(err.message);
+  toast.error(`Failed to delete permission: ${err.message}`);
 }

This change would provide more context to the user about which operation failed.

apps/dashboard/app/(app)/authorization/permissions/create-new-permission.tsx (1)

70-72: Approve changes with minor suggestions for improvement

The addition of the onError callback effectively addresses the PR objective of improving error handling for the createPermission mutation. This change enhances the user experience by providing feedback when permission creation fails.

Consider the following minor improvements:

  1. Add a type annotation for the err parameter to enhance type safety.
  2. Consider implementing a more user-friendly error message handling, possibly by wrapping err.message in a function that can sanitize or translate error messages for end-users.

Here's a suggested improvement:

onError(err: Error) {
  const userFriendlyMessage = getUserFriendlyErrorMessage(err.message);
  toast.error(userFriendlyMessage);
},

Where getUserFriendlyErrorMessage is a function you would implement to handle error message translation or sanitization.

apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx (2)

65-67: LGTM! Error handling added as per PR objectives.

The addition of the onError callback with toast notification successfully implements the required error handling for the updateRole mutation. This change aligns well with the PR objectives and enhances user feedback.

Consider implementing more granular error handling for specific error types. This could provide more user-friendly messages or handle certain errors differently. For example:

onError(err) {
  if (err instanceof TRPCClientError) {
    toast.error(`Failed to update role: ${err.message}`);
  } else {
    toast.error("An unexpected error occurred while updating the role. Please try again.");
  }
},

This approach allows for more specific error messages and potentially different handling based on the error type.


Line range hint 1-150: Enhance user feedback during form submission

The component implementation is solid, with good use of form validation and error handling. To further improve user experience, consider adding visual feedback when the form is submitting.

Add a disabled state to the form fields when submitting. This provides clear visual feedback that an action is in progress. Here's an example of how you could implement this:

  1. Add a new state variable:
const [isSubmitting, setIsSubmitting] = useState(false);
  1. Update the onSubmit function:
async function onSubmit(values: z.infer<typeof formSchema>) {
  setIsSubmitting(true);
  try {
    await updateRole.mutateAsync({
      id: role.id,
      name: values.name,
      description: values.description ?? null,
    });
  } finally {
    setIsSubmitting(false);
  }
}
  1. Add the disabled prop to form fields:
<Input
  {...field}
  disabled={isSubmitting}
  className="dark:focus:border-gray-700"
/>
  1. Update the submit button:
<Button type="submit" disabled={isSubmitting}>
  {isSubmitting ? <Loading className="w-4 h-4" /> : "Save"}
</Button>

These changes will provide clearer visual feedback to users during the form submission process.

apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/delete-namespace.tsx (2)

68-70: LGTM! Consider adding a more descriptive error message.

The addition of the onError callback with toast.error aligns well with the PR objectives and improves the user experience by providing feedback on failed operations. Good job!

Consider prefixing the error message with a descriptive text to provide more context to the user. For example:

onError(err) {
  toast.error(`Failed to delete namespace: ${err.message}`);
},

This change would make the error message more informative and user-friendly.


Line range hint 79-81: Consider enhancing error handling in the onSubmit function.

While the added onError callback in the mutation handles error display, it might be beneficial to add more robust error handling directly in the onSubmit function. This could include resetting the form state or providing additional user feedback for critical operations like deletion.

Consider updating the onSubmit function to handle errors more explicitly:

async function onSubmit(_values: z.infer<typeof formSchema>) {
  try {
    await deleteNamespace.mutateAsync({ namespaceId: namespace.id });
    // Success handling (if needed)
  } catch (error) {
    // Error handling (if needed beyond the toast)
    console.error('Failed to delete namespace:', error);
    // Optionally reset form or update UI state
  }
}

This approach allows for more fine-grained control over error handling and provides an opportunity to perform additional actions if needed.

apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/[overrideId]/settings.tsx (3)

82-84: LGTM! Consider enhancing error handling.

The addition of the onError callback with a toast notification aligns well with the PR objectives and improves user feedback. Good job!

Consider enhancing the error handling by categorizing different types of errors and providing more specific messages. For example:

onError(err) {
  if (err instanceof TRPCClientError) {
    toast.error("Failed to update rate limit", {
      description: err.message,
    });
  } else {
    toast.error("An unexpected error occurred", {
      description: "Please try again later",
    });
  }
},

This approach would provide more context to the user and handle unexpected errors gracefully.


Line range hint 86-92: Add error handling to deleteOverride mutation

For consistency with the update mutation and to improve user feedback, consider adding error handling to the deleteOverride mutation.

Here's a suggested implementation:

const deleteOverride = trpc.ratelimit.override.delete.useMutation({
  onSuccess() {
    toast.success("Override has been deleted", {
      description: "Changes may take up to 60s to propagate globally",
    });
    router.push("/ratelimits/");
  },
  onError(err) {
    toast.error("Failed to delete override", {
      description: err.message,
    });
  },
});

Line range hint 79-92: Consider consistent navigation approach

The component uses router.refresh() after a successful update and router.push() after a successful delete. Consider using a consistent approach for better user experience and easier maintenance.

You might want to use router.push() for both cases if you want to navigate away from the current page, or router.refresh() if you want to stay on the same page and just refresh the data. For example:

onSuccess() {
  toast.success("Limits have been updated", {
    description: "Changes may take up to 60s to propagate globally",
  });
  router.push("/ratelimits/"); // or router.refresh() if you want to stay on the same page
},

Apply the same approach to both update and deleteOverride mutations.

apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/create-new-override.tsx (1)

66-68: Approved: Error handling added as per PR objectives.

The addition of the onError callback with toast notification improves user feedback, which aligns well with the PR objectives. This change enhances the user experience by providing clear error notifications.

Consider wrapping the error message in a user-friendly format or using predefined error messages for common errors. This could improve readability for end-users. For example:

onError(err) {
  const userFriendlyMessage = getUserFriendlyErrorMessage(err.message);
  toast.error("Failed to create override", {
    description: userFriendlyMessage,
  });
},

Where getUserFriendlyErrorMessage is a function that maps known error messages to more user-friendly versions.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between fde83b4 and 157b33a.

📒 Files selected for processing (16)
  • apps/dashboard/app/(app)/authorization/permissions/[permissionId]/client.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/permissions/[permissionId]/delete-permission.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/permissions/create-new-permission.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/[roleId]/delete-role.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx (1 hunks)
  • apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/[overrideId]/settings.tsx (1 hunks)
  • apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/create-new-override.tsx (1 hunks)
  • apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/delete-namespace.tsx (1 hunks)
  • apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/update-namespace-name.tsx (1 hunks)
  • apps/dashboard/app/(app)/ratelimits/create-namespace-button.tsx (1 hunks)
  • apps/dashboard/app/(app)/settings/general/update-workspace-name.tsx (1 hunks)
  • apps/dashboard/components/opt-in.tsx (1 hunks)
  • apps/dashboard/lib/trpc/routers/ratelimit/deleteNamespace.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/ratelimit/updateNamespaceName.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/ratelimit/updateOverride.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/rbac/deleteRole.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (9)
apps/dashboard/lib/trpc/routers/rbac/deleteRole.ts (1)

71-77: Verify toast implementation for this error

While the error handling has been improved in this file, it's important to ensure that this error is properly caught and displayed as a toast to the user, as per the PR objectives.

To verify the toast implementation, you can run the following script:

This script will search for any frontend code that handles errors from the deleteRole mutation and displays them as toasts. If no results are found, you may need to implement the toast display for this error.

apps/dashboard/app/(app)/settings/general/update-workspace-name.tsx (1)

Line range hint 1-93: Verify the overall impact of the changes.

The addition of error handling to the updateName mutation is a focused and minimal change that enhances the component's functionality without introducing risks to the existing code. It completes the feedback loop for the user by handling both success and error cases.

To ensure that this change doesn't conflict with any other recent modifications and that error handling is consistently applied across similar components, please run the following verification script:

This script will help us ensure that:

  1. There are no conflicting recent changes to this file.
  2. Error handling is consistently implemented across similar mutation components.
✅ Verification successful

Error Handling Consistently Applied Across Mutation Components

The recent addition of error handling to the updateName mutation in update-workspace-name.tsx is consistent with similar implementations across other mutation components. There are no conflicting recent changes to this file.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify error handling consistency across mutation components

# Test 1: Check for other recent changes to this file
git log -n 5 --pretty=format:"%h - %s" -- apps/dashboard/app/\(app\)/settings/general/update-workspace-name.tsx

# Test 2: Search for similar mutation components and check their error handling
rg --type typescript -g '**/*.tsx' 'trpc\..+\.useMutation' -A 10 -B 5

Length of output: 618


Script:

#!/bin/bash
# Description: Verify error handling consistency across mutation components

# Test 1: Check for other recent changes to this file
git log -n 5 --pretty=format:"%h - %s" -- apps/dashboard/app/\(app\)/settings/general/update-workspace-name.tsx

# Test 2: Search for similar mutation components and check their error handling
rg --type tsx -g '**/*.tsx' 'trpc\..+\.useMutation' -A 10 -B 5

Length of output: 611


Script:

#!/bin/bash
# Description: Verify error handling consistency across mutation components

# Test 1: Check for other recent changes to this file
git log -n 5 --pretty=format:"%h - %s" -- apps/dashboard/app/(app)/settings/general/update-workspace-name.tsx

# Test 2: Search for similar mutation components and check their error handling
rg -g '**/*.tsx' 'trpc\..+\.useMutation' -A 10 -B 5

Length of output: 246


Script:

#!/bin/bash
# Description: Verify error handling consistency across mutation components

# Test 1: Check for other recent changes to this file
git log -n 5 --pretty=format:"%h - %s" -- 'apps/dashboard/app/(app)/settings/general/update-workspace-name.tsx'

# Test 2: Search for similar mutation components and check their error handling
rg -g '**/*.tsx' 'trpc\..+\.useMutation' -A 10 -B 5

Length of output: 86093


Script:

#!/bin/bash
# Description: Verify error handling consistency across mutation components

# Test 1: Check for other recent changes to this file
git log -n 5 --pretty=format:"%h - %s" -- 'apps/dashboard/app/(app)/settings/general/update-workspace-name.tsx'

# Test 2: Search for similar mutation components and check their error handling
rg -g '**/*.tsx' 'trpc\..+\.useMutation' -A 10 -B 5

Length of output: 86093

apps/dashboard/lib/trpc/routers/ratelimit/updateOverride.ts (1)

Line range hint 1-97: Overall assessment: Improved error handling aligns with PR objectives

The changes made to this file successfully implement error handling for the audit logging process, which aligns with the PR's objective of enhancing error handling across various mutations. This improvement ensures that users will be notified of any issues that occur during the rate limit override update process, including potential failures in audit logging.

The implementation is consistent with the existing error handling patterns in the file, maintaining code consistency. These changes contribute to a better user experience by providing more comprehensive error feedback.

apps/dashboard/app/(app)/ratelimits/create-namespace-button.tsx (1)

41-43: Verify the impact of error handling implementation.

The addition of error handling to the create mutation successfully addresses the objectives outlined in the PR and issue #2225. It enhances the user experience by providing feedback when namespace creation fails.

To ensure comprehensive error handling across the application, we should verify that similar changes have been made to other mutations mentioned in the PR objectives.

Let's verify the implementation of error handling in other mutations:

apps/dashboard/app/(app)/authorization/permissions/[permissionId]/delete-permission.tsx (1)

Line range hint 1-138: Overall impact assessment: Positive improvement with minimal risk.

The addition of error handling to the deletePermission mutation enhances the component's robustness without introducing significant changes to its structure or dependencies. This improvement aligns well with the PR objectives and contributes to a better user experience by providing clear feedback on operation failures.

The localized nature of the changes minimizes the risk of unintended side effects on the component's overall functionality.

apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx (1)

Line range hint 1-150: Summary of review

The changes successfully implement error handling for the updateRole mutation, meeting the PR objectives. The component's overall structure and implementation are solid, with good use of form validation and UI components. Suggestions for further improvements include more granular error handling and enhanced user feedback during form submission. These changes contribute positively to the user experience by providing better error feedback.

apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/delete-namespace.tsx (1)

Line range hint 1-156: Overall, the changes effectively implement error handling as per PR objectives.

The addition of the onError callback to the deleteNamespace mutation successfully implements error toast notifications, aligning with the PR objectives. The changes improve user feedback for failed operations, enhancing the overall user experience.

While the implementation is solid, consider the suggested improvements:

  1. Adding more context to the error message in the toast notification.
  2. Enhancing error handling in the onSubmit function for more robust error management.

These suggestions are not critical but could further improve the code quality and user experience.

apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/[overrideId]/settings.tsx (1)

Line range hint 1-184: Summary of review

The changes successfully implement error handling for the update mutation, which aligns with the PR objectives. The implementation improves user feedback and enhances the overall user experience.

Some suggestions for further improvement include:

  1. Enhancing the error handling to provide more specific error messages.
  2. Adding similar error handling to the deleteOverride mutation for consistency.
  3. Adopting a consistent navigation approach after successful mutations.

Overall, this is a valuable improvement to the codebase. Great work on enhancing the error handling!

apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/create-new-override.tsx (1)

Line range hint 1-168: Overall impact: Improved error handling without affecting existing functionality.

The changes in this file successfully implement error handling for the CreateNewOverride component as per the PR objectives. The addition of the onError callback enhances user experience by providing feedback when the creation of a new rate limit override fails.

Key points:

  1. The changes are minimal and focused, reducing the risk of introducing new issues.
  2. The existing functionality of the component remains intact.
  3. The implementation aligns well with the requirements outlined in the linked issue #2225.

To ensure that these changes don't conflict with any other parts of the codebase, let's verify the usage of the trpc.ratelimit.override.create mutation:

This will help us confirm that the error handling has been consistently applied across the codebase.

✅ Verification successful

Usage Verification Complete: No other instances of trpc.ratelimit.override.create found.

The error handling implemented in create-new-override.tsx is isolated to this component and does not impact other parts of the codebase. This ensures that the changes are safe and confined to their intended scope.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other usages of the ratelimit.override.create mutation
rg "trpc\.ratelimit\.override\.create" --type typescript

Length of output: 93


Script:

#!/bin/bash
# Search for other usages of the ratelimit.override.create mutation with corrected file type
rg "trpc\.ratelimit\.override\.create" --type typescriptreact

Length of output: 103


Script:

#!/bin/bash
# Search for other usages of the ratelimit.override.create mutation in .ts and .tsx files
rg "trpc\.ratelimit\.override\.create" --glob "*.ts" --glob "*.tsx"

Length of output: 215

@vercel vercel bot temporarily deployed to Preview – engineering October 8, 2024 05:55 Inactive
@chronark chronark enabled auto-merge October 8, 2024 13:39
@chronark chronark self-assigned this Oct 8, 2024
@vercel vercel bot temporarily deployed to Preview – engineering October 8, 2024 13:46 Inactive
@vercel vercel bot temporarily deployed to Preview – www October 8, 2024 13:52 Inactive
@vercel vercel bot temporarily deployed to Preview – play October 8, 2024 13:53 Inactive
@vercel vercel bot temporarily deployed to Preview – workflows October 8, 2024 13:54 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard October 8, 2024 13:56 Inactive
@chronark chronark added this pull request to the merge queue Oct 8, 2024
Merged via the queue into unkeyed:main with commit 0572c25 Oct 8, 2024
@oss-gg
Copy link

oss-gg bot commented Oct 8, 2024

Awarding harshsbhat: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/harshsbhat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All mutations need to have toast.error

2 participants