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/kleros dapp #1755

Closed
wants to merge 3 commits into from
Closed

Feat/kleros dapp #1755

wants to merge 3 commits into from

Conversation

Harman-singh-waraich
Copy link
Contributor

@Harman-singh-waraich Harman-singh-waraich commented Nov 20, 2024

PR-Codex overview

This PR focuses on restructuring the kleros-app codebase by removing several utility files, updating dependencies, and enhancing the AtlasProvider. It also introduces new functionalities for user management and email handling while improving the overall application structure.

Detailed summary

  • Deleted several utility files in web/src/utils/atlas.
  • Updated prettier version in prettier-config/package.json.
  • Added new isUndefined utility function in kleros-app/src/utils/index.ts.
  • Introduced a new HTML structure in kleros-app/src/index.html.
  • Created a new App component in kleros-app/src/App.tsx.
  • Implemented useSessionStorage hook in kleros-app/src/lib/atlas/hooks/useSessionStorage.ts.
  • Added GraphQL functions for user management: getNonce, fetchUser, addUser, updateEmail, and loginUser.
  • Enhanced createMessage to accept an optional statement parameter.
  • Updated uploadToIpfs to include Products and Roles enums.
  • Improved AtlasProvider to manage user sessions and interactions with GraphQL.
  • Refactored AtlasProvider to use a new configuration structure.
  • Enhanced error handling and user feedback with toast notifications across various components.

The following files were skipped due to too many changes: yarn.lock

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

  • New Features

    • Introduced a new React component App as the entry point for the application.
    • Added a custom hook useSessionStorage for managing state synchronized with session storage.
    • Implemented a new context provider AtlasProvider for user authentication and interactions with the Atlas service.
    • Added various GraphQL mutations for user management, including addUser, loginUser, and updateEmail.
  • Enhancements

    • Improved user feedback with toast notifications for actions like email updates and user sign-ins.
    • Streamlined the EnsureAuth and EmailVerificationInfo components to provide better user interaction.
  • Bug Fixes

    • Enhanced error handling across multiple components to improve reliability during user interactions.
  • Chores

    • Updated various configuration files, including .gitignore and tsconfig.json, for better development experience.

Copy link
Contributor

coderabbitai bot commented Nov 20, 2024

Walkthrough

The pull request introduces significant changes across multiple files in the Kleros application. Key modifications include updates to the .gitignore file to exclude additional build artifacts and cache files, the creation of a new ESLint configuration, and the addition of a package.json file for the kleros-app package. Several React components and utility functions are introduced, including user authentication features and hooks for session storage. Additionally, existing files are updated for improved user feedback mechanisms during operations such as email updates and file uploads.

Changes

File Path Change Summary
.gitignore Added entries for .parcel-cache, Yarn v2 cache, Serverless Webpack, .stylelintcache, .svelte-kit, and .history.
kleros-app/eslint.config.mjs Created a new ESLint configuration with various plugins and rules for JavaScript/TypeScript projects.
kleros-app/package.json Introduced a new package.json file specifying metadata, scripts, and dependencies for the @kleros/kleros-app package.
kleros-app/src/App.tsx Added a new App component as the entry point for the React application.
kleros-app/src/index.html Created a new HTML file for the Kleros application with basic structure and a script tag for the main module.
kleros-app/src/lib/atlas/hooks/useSessionStorage.ts Introduced a custom hook for managing session storage state.
kleros-app/src/lib/atlas/index.ts Exported all entities from the providers and utils modules.
kleros-app/src/lib/atlas/providers/AtlasProvider.tsx Added a context provider for user authentication and interactions with an Atlas service.
kleros-app/src/lib/atlas/utils/addUser.ts Created a GraphQL mutation for adding a user with error handling.
kleros-app/src/lib/atlas/utils/createMessage.ts Updated the createMessage function to include an optional statement parameter.
kleros-app/src/lib/atlas/utils/fetchUser.ts Introduced functionality to fetch user data from a GraphQL API.
kleros-app/src/lib/atlas/utils/getNonce.ts Added a function to fetch a nonce value associated with an Ethereum address.
kleros-app/src/lib/atlas/utils/index.ts Added new enums for Products and Roles, and re-exported utility functions.
kleros-app/src/lib/atlas/utils/loginUser.ts Created a GraphQL mutation for user login functionality.
kleros-app/src/lib/atlas/utils/updateEmail.ts Introduced a GraphQL mutation for updating a user's email address.
kleros-app/src/lib/atlas/utils/uploadToIpfs.ts Added functionality for uploading files to IPFS with error handling.
web/src/components/EnsureAuth.tsx Enhanced the EnsureAuth component with toast notifications for user sign-in actions.
web/src/context/AtlasProvider.tsx Simplified the AtlasProvider component by utilizing the _AtlasProvider from @kleros/kleros-app.
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/EmailVerificationInfo.tsx Updated to include toast notifications for email update processes.
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx Enhanced with toast notifications for email updates and user additions.
web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx Improved error handling and user feedback during file uploads.
web/src/pages/Resolver/Policy/index.tsx Updated file upload handling with enhanced error notifications.
web/src/pages/Settings/EmailConfirmation/index.tsx Updated import paths and improved styled component formatting.
web/src/utils/index.ts Added a utility function isUndefined for type safety.
kleros-app/tsconfig.json Introduced a TypeScript configuration file for the Kleros application.
package.json Updated workspace structure and added new development dependencies.
prettier-config/package.json Upgraded the prettier dependency version.
web/package.json Added a dependency on the @kleros/kleros-app package.
web/src/utils/atlas/*.ts Removed various utility files related to user management and GraphQL operations.

Possibly related PRs

  • Atlas integration for file uploads to IPFS #1687: The main PR's changes to the .gitignore file may relate to the integration of IPFS file uploads, as the new entries could help manage files generated during the upload process.
  • fix(web): submit-evidence-button-disable-condition #1722: The modifications in the SubmitEvidenceModal component, which involve file uploads, are relevant as they may interact with the changes made in the main PR regarding file management in the .gitignore.
  • feat(web): email-updateable-at-info #1729: The updates to the user data model and email update process may indirectly relate to the main PR's changes, as both involve managing user interactions with the application, although they focus on different aspects.

Suggested labels

Type: Feature🗿, Package: Web

Suggested reviewers

  • alcercu

Poem

🐇 In the land of code where rabbits play,
New features hop in, brightening the day.
With hooks and toasts, our app's a delight,
Ignoring the clutter, our future is bright!
So let's raise a cheer for this code we adore,
With every new change, we leap and explore! 🌟


🪧 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 or @coderabbitai title 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.

Copy link

netlify bot commented Nov 20, 2024

Deploy Preview for kleros-v2-testnet-devtools failed. Why did it fail? →

Name Link
🔨 Latest commit 34f25c7
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-testnet-devtools/deploys/673db29045b129000822d710

Copy link

netlify bot commented Nov 20, 2024

Deploy Preview for kleros-v2-testnet failed. Why did it fail? →

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-testnet/deploys/673ed047a111e907ea9a22f4

Copy link

netlify bot commented Nov 20, 2024

Deploy Preview for kleros-v2-university failed. Why did it fail? →

Name Link
🔨 Latest commit 34f25c7
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-university/deploys/673db29076843e0008103d01

Copy link

codeclimate bot commented Nov 20, 2024

Code Climate has analyzed commit 34f25c7 and detected 30 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 6
Style 24

View more on Code Climate.

Copy link

netlify bot commented Nov 20, 2024

Deploy Preview for kleros-v2-neo failed. Why did it fail? →

Name Link
🔨 Latest commit 34f25c7
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-neo/deploys/673db29018bc32000890ab09

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
11.1% Duplication on New Code (required ≤ 10%)

See analysis details on SonarQube Cloud

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: 21

🧹 Outside diff range and nitpick comments (32)
kleros-app/src/App.tsx (2)

1-2: Consider using TypeScript features more effectively

Since this is a .tsx file, consider adding type declarations for better type safety and developer experience.

-import { createRoot } from "react-dom/client";
+import { Root, createRoot } from "react-dom/client";

1-18: Consider implementing error boundaries and adding documentation

The application could benefit from:

  1. Implementation of React Error Boundaries for graceful error handling
  2. JSDoc documentation for the App component
  3. A more structured layout with proper semantic HTML

Would you like me to provide an example implementation of these improvements?

prettier-config/package.json (1)

6-9: Consider adding peer dependencies

Since this is a shared configuration package, consider adding peer dependencies to explicitly state the compatible versions:

{
  "dependencies": {
    "eslint": "^8.57.1",
    "prettier": "^3.3.3",
    "prettier-plugin-solidity": "^1.3.1"
  },
+ "peerDependencies": {
+   "prettier": "^3.3.3"
+ },
+ "peerDependenciesMeta": {
+   "prettier": {
+     "optional": false
+   }
+ }
}
kleros-app/src/lib/atlas/utils/createMessage.ts (1)

3-3: Consider adding JSDoc documentation.

Adding JSDoc documentation would improve maintainability by documenting:

  • Purpose of the function
  • Parameter descriptions
  • Return value
  • Example usage
+/**
+ * Creates a SIWE (Sign-In with Ethereum) message for authentication.
+ * @param address - The Ethereum address of the signer
+ * @param nonce - A unique nonce for the message
+ * @param chainId - The chain ID of the network
+ * @param statement - Optional custom statement (defaults to "Sign In to Kleros with Ethereum.")
+ * @returns A formatted SIWE message string
+ */
 export const createMessage = (address: `0x${string}`, nonce: string, chainId: number, statement?: string) => {
kleros-app/src/lib/atlas/utils/getNonce.ts (4)

3-5: Consider a more descriptive type name

The type GetNonce could be more descriptive, such as GetNonceResponse, to better indicate its role as a response type.

-type GetNonce = {
+type GetNonceResponse = {
   nonce: string;
 };

7-11: Consider using query instead of mutation

The operation fetches a nonce without modifying server state, which aligns more with GraphQL query semantics rather than mutation. Unless there's a specific reason for using mutation (e.g., nonce generation triggers state changes), consider changing to a query operation.

 const query = gql`
-  mutation GetNonce($address: Address!) {
+  query GetNonce($address: Address!) {
     nonce(address: $address)
   }
 `;

22-24: Remove console.log from production code

Consider using a proper logging service or removing the console.log statement. If needed for debugging, consider using a logging utility that can be configured based on the environment.


13-29: Add input validation and retry mechanism

Consider these improvements for better reliability:

  1. Validate the Ethereum address format before making the request
  2. Implement a retry mechanism for transient failures
  3. Add timeout handling

Here's a suggested implementation:

import { isAddress } from 'ethers/lib/utils';
import { retry } from './retry'; // implement or use a retry utility

export async function getNonce(client: GraphQLClient, address: string): Promise<string> {
  if (!isAddress(address)) {
    throw new Error('Invalid Ethereum address');
  }

  const variables = { address };

  return retry(
    async () => {
      const response = await client.request<GetNonceResponse>(query, variables);
      return response.nonce;
    },
    {
      retries: 3,
      timeout: 5000,
      onError: (error) => {
        const errorMessage = error?.response?.errors?.[0]?.message 
          ?? `Error fetching nonce: ${error?.message ?? 'Unknown error'}`;
        throw new Error(errorMessage);
      }
    }
  );
}
kleros-app/src/lib/atlas/utils/index.ts (2)

1-10: Add JSDoc documentation for the Products enum

Consider adding JSDoc documentation to describe the purpose and use case of each product in the enum. This would help other developers understand when to use each product type.

+/**
+ * Enum representing different Kleros products and modules
+ */
 export enum Products {
+  /** Court v1 implementation */
   CourtV1 = "CourtV1",
+  /** Court v2 implementation */
   CourtV2 = "CourtV2",
   // ... add documentation for other products

25-32: Consider organizing utilities into sub-modules

The current structure mixes different types of utilities (authentication, user management, IPFS). Consider organizing these into logical sub-modules for better maintainability:

// auth/index.ts
export * from './loginUser';
export * from './getNonce';
export * from './createMessage';

// user/index.ts
export * from './addUser';
export * from './fetchUser';
export * from './updateEmail';
export * from './confirmEmail';

// storage/index.ts
export * from './uploadToIpfs';

Then in this file:

export * from './auth';
export * from './user';
export * from './storage';
kleros-app/src/lib/atlas/utils/fetchUser.ts (2)

12-20: Add documentation for authentication requirements

Consider adding JSDoc comments to document whether this query requires authentication and any potential error scenarios.

+/**
+ * GraphQL query to fetch authenticated user's data.
+ * Requires: Authentication token in the GraphQL client.
+ * @throws {Error} When user is not authenticated or server errors occur
+ */
const query = gql`
  query GetUser {
    user {

22-34: Consider adding retry logic and caching

To improve reliability and performance:

  1. Implement retry logic for transient failures
  2. Consider adding response caching to reduce API calls
  3. Add rate limiting protection

This would make the function more resilient in production environments.

Would you like me to provide an example implementation with these improvements?

kleros-app/src/lib/atlas/utils/addUser.ts (3)

4-8: Consider enhancing the GraphQL mutation definition

The mutation could benefit from:

  1. Input validation using GraphQL's built-in validation directives
  2. A more specific name like AddAtlasUser to prevent naming conflicts
  3. Explicit definition of the AddUserSettingsDto type
 const query = gql`
-  mutation AddUser($settings: AddUserSettingsDto!) {
+  mutation AddAtlasUser($settings: AddUserSettingsDto! @validateEmail) {
     addUser(addUserSettings: $settings)
   }
 `;

10-16: Enhance type definitions for better type safety

Consider strengthening the type definitions:

  1. Add email format validation
  2. Include additional user metadata in the response
export type AddUserData = {
  email: string & { __brand: 'Email' }; // Email branded type
};

type AddUserResponse = {
  addUser: {
    success: boolean;
    userId?: string;
    timestamp: string;
  };
};

18-37: Consider implementing retry logic for network failures

Given this is a critical user operation, consider implementing retry logic for transient failures.

Example implementation:

import { retry } from '@your-utils/retry';

export function addUser(client: GraphQLClient, userData: AddUserData): Promise<boolean> {
  return retry(
    () => executeAddUser(client, userData),
    {
      retries: 3,
      backoff: 'exponential',
      onRetry: (error) => {
        // Add monitoring/logging here
      }
    }
  );
}
kleros-app/src/lib/atlas/utils/updateEmail.ts (2)

1-8: LGTM! Consider adding email format validation.

The GraphQL mutation is well-structured. However, consider adding email format validation at the GraphQL schema level using a custom scalar or directive to ensure valid email formats before they reach the resolver.


1-35: Consider adding rate limiting and security measures.

Since this is an authentication-related endpoint:

  1. Implement rate limiting to prevent abuse
  2. Add request throttling for the same user
  3. Consider adding a confirmation step for email changes to prevent unauthorized modifications
  4. Add audit logging for security-sensitive operations
web/src/components/EnsureAuth.tsx (1)

Line range hint 29-39: Consider enhancing loading state feedback.

While the button shows a loading state, consider adding a more descriptive loading message or progress indicator to improve user experience during the signing process.

Example implementation:

 return isVerified ? (
   children
 ) : (
+  <div className="auth-container">
     <Button
       text="Sign In"
       onClick={handleClick}
       disabled={isSigningIn || !address}
       isLoading={isSigningIn}
       {...{ className }}
     />
+    {isSigningIn && (
+      <span className="loading-text">
+        Waiting for wallet signature...
+      </span>
+    )}
+  </div>
 );
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/EmailVerificationInfo.tsx (1)

Line range hint 89-103: Consider separating concerns and improving state management

The component currently mixes UI rendering with business logic. Consider:

  1. Moving the email verification logic to a custom hook
  2. Extracting the verification status check into a separate function
  3. Using a loading state while verification is in progress

This would improve maintainability and make the component more testable.

kleros-app/eslint.config.mjs (4)

21-24: Consider extending ignore patterns for build outputs and caches

Add common build and cache directories to the ignore list to improve linting performance.

  {
-   ignores: ["src/assets"],
+   ignores: [
+     "src/assets",
+     "dist",
+     "build",
+     "coverage",
+     "node_modules",
+     ".cache"
+   ],
  },

25-35: Consider adding security-focused ESLint configurations

For enhanced security coverage, consider extending security-focused configs.

    compat.extends(
      "plugin:react/recommended",
      "plugin:react-hooks/recommended",
      "plugin:import/recommended",
      "plugin:import/react",
      "plugin:@typescript-eslint/recommended",
+     "plugin:security/recommended",
+     "plugin:xss/recommended",
      "plugin:prettier/recommended",
      "prettier"
    )

44-61: Consider updating ECMAScript version

The current ECMAScript version (2020) could be updated to a more recent version for access to newer language features.

-     ecmaVersion: 2020,
+     ecmaVersion: 2023,

63-73: Consider using 'detect' for React version

Instead of hardcoding the React version, consider using 'detect' to automatically determine the version from package.json.

    settings: {
      react: {
-       version: "^18.3.1",
+       version: "detect",
      },
.gitignore (1)

60-61: LGTM! Consider moving the parcel cache entry to a more appropriate section.

The addition of .parcel-cache is appropriate as these are build artifacts that shouldn't be version controlled. However, for better organization, consider moving this entry to the build output section where similar entries like .next, dist, and .nuxt are located.

web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx (2)

118-124: Enhance error handling robustness

While the added error handling is good, there are a few improvements we can make:

  1. Add a fallback for error messages
  2. Use console.error for error logging
  3. Make the error handling flow more consistent
-    fileURI = await uploadFile(file, Roles.Evidence).catch((err) => {
-      console.log(err);
-      toast.error(`Upload failed: ${err?.message}`, toastOptions);
-      return null;
-    });
-    if (!fileURI) throw new Error("Error uploading evidence file");
-    toast.success("Uploaded successfully!", toastOptions);
+    fileURI = await uploadFile(file, Roles.Evidence).catch((err) => {
+      console.error("File upload failed:", err);
+      toast.error(`Upload failed: ${err?.message || "Unknown error occurred"}`, toastOptions);
+      return null;
+    });
+    if (fileURI) {
+      toast.success("Uploaded successfully!", toastOptions);
+    } else {
+      throw new Error("Error uploading evidence file");
+    }

Line range hint 1-132: Consider implementing an error boundary

Since this component handles critical operations like file uploads and contract interactions, consider wrapping it with an error boundary to gracefully handle unexpected errors and prevent the entire UI from crashing.

web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (1)

Line range hint 45-151: Consider splitting component responsibilities

The FormContactDetails component handles multiple concerns including form state management, user operations, and email validation. Consider breaking it down into smaller, more focused components:

  1. A pure form component for email input and validation
  2. A container component for user operations (create/update)
  3. A separate hook for form state management

This would improve maintainability and testability.

web/src/context/AtlasProvider.tsx (1)

5-5: Consider avoiding React.FC and typing props directly

Using React.FC is often discouraged because it can introduce unnecessary complexity and may interfere with certain TypeScript features. Instead, you can define your component without React.FC and type the props directly.

Apply this diff to refactor the component:

-const AtlasProvider: React.FC<{ children: React.ReactNode }> = ({ children }) => {
+interface AtlasProviderProps {
+  children: React.ReactNode;
+}
+
+const AtlasProvider = ({ children }: AtlasProviderProps) => {
kleros-app/src/lib/atlas/utils/uploadToIpfs.ts (2)

15-37: Refactor to use async/await syntax for better readability

To improve readability and maintain consistency, consider refactoring the uploadToIpfs function to use async/await syntax throughout instead of mixing it with .then(). This enhances the clarity of asynchronous operations and error handling.

Consider applying this diff:

 export async function uploadToIpfs(config: Config, payload: IpfsUploadPayload): Promise<string | null> {
   const formData = new FormData();
   formData.append("file", payload.file, payload.name);
   formData.append("name", payload.name);
   formData.append("product", payload.product);
   formData.append("role", payload.role);

-  return fetch(`${config.baseUrl}/ipfs/file`, {
+  const response = await fetch(`${config.baseUrl}/ipfs/file`, {
     method: "POST",
     headers: {
       authorization: `Bearer ${config.authToken}`,
     },
     body: formData,
-  }).then(async (response) => {
-    if (!response.ok) {
-      const error = await response.json().catch(() => ({ message: "Error uploading to IPFS" }));
-
-      if (response.status === 401) throw new AuthorizationError(error.message);
-      throw new Error(error.message);
-    }
-
-    return await response.text();
-  });
+  });

+  if (!response.ok) {
+    const error = await response.json().catch(() => ({ message: "Error uploading to IPFS" }));
+    if (response.status === 401) throw new AuthorizationError(error.message);
+    throw new Error(error.message);
+  }

+  return await response.text();
 }

40-49: Ensure proper subclassing of Error in AuthorizationError

To maintain correct prototype chaining and fully leverage the built-in Error functionalities, explicitly set the prototype in the AuthorizationError constructor. Additionally, setting the name property is redundant since it's automatically assigned.

Consider updating the AuthorizationError class:

 export class AuthorizationError extends Error {
-  readonly name = "AuthorizationError" as const;
   constructor(message: string) {
     super(message);
+    Object.setPrototypeOf(this, AuthorizationError.prototype);
     if (Error.captureStackTrace) {
       Error.captureStackTrace(this, this.constructor);
     }
   }
 }
kleros-app/src/lib/atlas/providers/AtlasProvider.tsx (2)

215-217: Preserve Original Errors When Rethrowing

When rethrowing caught errors, it's better to rethrow the original error to preserve the stack trace and error message.

Apply this diff to each catch block:

In the addUser function:

} catch (err: any) {
-   throw new Error(err);
+   throw err;
} finally {

In the updateEmail function:

} catch (err: any) {
-   throw new Error(err);
+   throw err;
} finally {

In the uploadFile function:

} catch (err: any) {
-   throw new Error(err);
+   throw err;
} finally {

Also applies to: 239-241, 265-267


287-291: Avoid Using console.log in Production Code

Using console.log for error handling in production code is discouraged. Consider removing the console log or using a proper logging mechanism.

Apply this diff to remove the console log:

} catch (err: any) {
-   // eslint-disable-next-line
-   console.log("Confirm Email Error : ", err?.message);
    return { isConfirmed: false, isTokenExpired: false, isTokenInvalid: false, isError: true };
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 884674e and 34f25c7.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (38)
  • .gitignore (1 hunks)
  • kleros-app/eslint.config.mjs (1 hunks)
  • kleros-app/package.json (1 hunks)
  • kleros-app/src/App.tsx (1 hunks)
  • kleros-app/src/index.html (1 hunks)
  • kleros-app/src/lib/atlas/hooks/useSessionStorage.ts (1 hunks)
  • kleros-app/src/lib/atlas/index.ts (1 hunks)
  • kleros-app/src/lib/atlas/providers/AtlasProvider.tsx (1 hunks)
  • kleros-app/src/lib/atlas/providers/index.ts (1 hunks)
  • kleros-app/src/lib/atlas/utils/addUser.ts (1 hunks)
  • kleros-app/src/lib/atlas/utils/createMessage.ts (2 hunks)
  • kleros-app/src/lib/atlas/utils/fetchUser.ts (1 hunks)
  • kleros-app/src/lib/atlas/utils/getNonce.ts (1 hunks)
  • kleros-app/src/lib/atlas/utils/index.ts (1 hunks)
  • kleros-app/src/lib/atlas/utils/loginUser.ts (1 hunks)
  • kleros-app/src/lib/atlas/utils/updateEmail.ts (1 hunks)
  • kleros-app/src/lib/atlas/utils/uploadToIpfs.ts (1 hunks)
  • kleros-app/src/lib/index.ts (1 hunks)
  • kleros-app/src/utils/index.ts (1 hunks)
  • kleros-app/tsconfig.json (1 hunks)
  • package.json (3 hunks)
  • prettier-config/package.json (1 hunks)
  • web/package.json (1 hunks)
  • web/src/components/EnsureAuth.tsx (2 hunks)
  • web/src/context/AtlasProvider.tsx (1 hunks)
  • web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/EmailVerificationInfo.tsx (2 hunks)
  • web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (3 hunks)
  • web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx (2 hunks)
  • web/src/pages/Resolver/Policy/index.tsx (2 hunks)
  • web/src/pages/Settings/EmailConfirmation/index.tsx (3 hunks)
  • web/src/utils/atlas/addUser.ts (0 hunks)
  • web/src/utils/atlas/fetchUser.ts (0 hunks)
  • web/src/utils/atlas/getNonce.ts (0 hunks)
  • web/src/utils/atlas/index.ts (0 hunks)
  • web/src/utils/atlas/loginUser.ts (0 hunks)
  • web/src/utils/atlas/updateEmail.ts (0 hunks)
  • web/src/utils/atlas/uploadToIpfs.ts (0 hunks)
  • web/src/utils/uploadFormDataToIPFS.ts (0 hunks)
💤 Files with no reviewable changes (8)
  • web/src/utils/atlas/addUser.ts
  • web/src/utils/atlas/fetchUser.ts
  • web/src/utils/atlas/getNonce.ts
  • web/src/utils/atlas/index.ts
  • web/src/utils/atlas/loginUser.ts
  • web/src/utils/atlas/updateEmail.ts
  • web/src/utils/atlas/uploadToIpfs.ts
  • web/src/utils/uploadFormDataToIPFS.ts
✅ Files skipped from review due to trivial changes (7)
  • kleros-app/package.json
  • kleros-app/src/index.html
  • kleros-app/src/lib/atlas/index.ts
  • kleros-app/src/lib/atlas/providers/index.ts
  • kleros-app/src/lib/index.ts
  • kleros-app/tsconfig.json
  • web/src/pages/Settings/EmailConfirmation/index.tsx
🧰 Additional context used
📓 Learnings (1)
web/src/context/AtlasProvider.tsx (1)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1687
File: web/src/context/AtlasProvider.tsx:225-244
Timestamp: 2024-11-12T04:49:48.060Z
Learning: In `web/src/context/AtlasProvider.tsx`, the `atlasUri` variable comes from environment variables and does not change, so it does not need to be included in dependency arrays.
🔇 Additional comments (22)
prettier-config/package.json (1)

8-8: Verify the impact of Prettier v3 upgrade

The upgrade from Prettier v2 to v3 is a major version bump that includes breaking changes. Please ensure:

  • CI/CD pipelines are updated to use Node.js ≥14
  • Team is aware of new formatting defaults
  • All repositories using this config are tested with the new version
kleros-app/src/lib/atlas/utils/createMessage.ts (1)

Line range hint 3-19: LGTM! Well-structured implementation of SIWE message creation.

The implementation is secure and follows best practices:

  • Proper type safety with TypeScript
  • Secure message creation using the SIWE standard
  • Protection against replay attacks with expiration
  • Safe domain/origin handling
kleros-app/src/lib/atlas/hooks/useSessionStorage.ts (2)

22-22: LGTM!

The return type is well-defined and follows React conventions for state hooks.


1-23: Consider security implications for sensitive data storage

Since this hook is used in authentication contexts, be cautious about storing sensitive information in session storage:

  1. Avoid storing sensitive tokens or PII
  2. Consider using more secure alternatives like HttpOnly cookies for auth tokens
  3. Implement storage cleanup on session end
kleros-app/src/lib/atlas/utils/index.ts (1)

12-23: Document roles and verify string values

  1. Consider adding JSDoc documentation to describe each role's purpose and usage context.
  2. Verify that the kebab-case string values match the expected values in the system, especially if they're used in URLs or file paths.
+/**
+ * Enum representing different content types and roles in the system
+ * The string values are used in URLs/file paths and should match the backend expectations
+ */
 export enum Roles {
+  /** Evidence files uploaded during disputes */
   Evidence = "evidence",
+  /** Generic file uploads */
   Generic = "generic",
   // ... add documentation for other roles
kleros-app/src/lib/atlas/utils/updateEmail.ts (1)

10-16: LGTM! Types are well-defined.

The type definitions are clear, properly exported, and accurately represent the data structures for both request and response.

kleros-app/src/lib/atlas/utils/loginUser.ts (2)

9-18: LGTM! Well-structured type definitions

The types are well-defined with proper TypeScript typing, including the use of template literal type for the signature format.


20-38: Consider additional security measures

For authentication endpoints, consider implementing:

  1. Rate limiting to prevent brute force attacks
  2. CSRF protection
  3. Secure headers
  4. Audit logging for security events
web/src/components/EnsureAuth.tsx (2)

1-8: LGTM! Imports are well-organized and necessary.

All imported modules are utilized effectively in the implementation.


34-34: LGTM! Button handler implementation is correct.

The onClick handler properly uses the memoized callback, and the disabled state is appropriately managed.

package.json (1)

28-29: LGTM! Workspace additions are well-structured

The addition of tsconfig and kleros-app workspaces aligns well with TypeScript adoption and modular development approach.

web/src/pages/Resolver/Policy/index.tsx (3)

8-8: LGTM! Import consolidation improves maintainability

The change to import from @kleros/kleros-app package aligns with the modular architecture approach.


60-60: Verify the role change and remove debug logging

  1. Please confirm if changing the role from Policy to Evidence is intentional, as this might affect how the file is categorized in the system.
  2. Remove the console.log statement in the error handler for production code.
- console.log(err);

64-68: LGTM! Improved user feedback with toast notifications

The addition of success and error toast notifications provides better user feedback during the upload process.

web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/EmailVerificationInfo.tsx (1)

8-10: LGTM: Import changes align with centralization strategy

The updated imports properly centralize the Atlas provider functionality and add necessary toast notification capabilities.

kleros-app/eslint.config.mjs (2)

1-19: LGTM! Well-structured setup using modern ESLint flat config

The implementation correctly handles ESM modules and provides backward compatibility through FlatCompat.


101-103: Verify the necessity of disabled TypeScript checks

Disabling no-non-null-assertion and no-explicit-any might lead to type-unsafe code. Consider enabling these rules and addressing the underlying issues.

web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx (1)

10-10: LGTM! Good architectural decision.

Moving Roles and useAtlasProvider to @kleros/kleros-app improves code organization and maintainability by centralizing these common utilities.

web/package.json (1)

77-77: LGTM! Please verify workspace setup.

The addition of @kleros/kleros-app as a workspace dependency follows proper conventions.

Please ensure:

  • The workspace package is properly initialized and built
  • There are no circular dependencies between packages
  • The dependency tree remains clean when running yarn install
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (1)

9-9: Verify @kleros/kleros-app package dependency

The AtlasProvider import has been moved to @kleros/kleros-app. Ensure this dependency is properly declared in the project's package.json and that the version is compatible with the current implementation.

Also applies to: 20-21

web/src/context/AtlasProvider.tsx (1)

8-8: Verify that REACT_APP_ATLAS_URI is correctly defined

Ensure that import.meta.env.REACT_APP_ATLAS_URI is correctly set in the environment variables and accessible at runtime to prevent potential issues with AtlasProvider initialization.

kleros-app/src/lib/atlas/utils/uploadToIpfs.ts (1)

1-13: Type definitions and imports look good

The import statement and type definitions for IpfsUploadPayload and Config are clear and well-structured, accurately defining the expected data shapes.

kleros-app/src/utils/index.ts Show resolved Hide resolved
kleros-app/src/App.tsx Show resolved Hide resolved
kleros-app/src/App.tsx Show resolved Hide resolved
@Harman-singh-waraich Harman-singh-waraich marked this pull request as draft November 20, 2024 10:16
@Harman-singh-waraich
Copy link
Contributor Author

Closing as too many conflicts in yarn.lock making it dirty. Migrated changes to #1756 on latest dev branch

@Harman-singh-waraich Harman-singh-waraich deleted the feat/kleros-dapp branch November 21, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant