Skip to content

[WIP] Fernandorocha/ent 335 create chat popup store conversation#1

Open
fernando-plank wants to merge 3 commits intomainfrom
fernandorocha/ent-335-create-chat-popup-store-conversation
Open

[WIP] Fernandorocha/ent 335 create chat popup store conversation#1
fernando-plank wants to merge 3 commits intomainfrom
fernandorocha/ent-335-create-chat-popup-store-conversation

Conversation

@fernando-plank
Copy link
Collaborator

@fernando-plank fernando-plank commented Oct 1, 2024

Summary by Entelligence.AI

  • New Feature: Introduced new API functions to enhance interaction with the backend, including creating users and conversations, fetching conversation lists, repository agents, and user data.
  • New Feature: Implemented Zustand stores for managing user state and conversations, improving application state management and performance.
  • Refactor: Updated and refined interfaces and types for better type safety and code readability. This includes additional fields in headerObject, ConversationReqRepository, User, and Conversation interfaces.

@linear
Copy link

linear bot commented Oct 1, 2024

@github-actions
Copy link

github-actions bot commented Oct 1, 2024

Image description Entelligence.AI

Walkthrough

This update introduces new API functions for handling user and conversation data, along with zustand stores for managing state. It also refines the structure of several key interfaces to accommodate these changes, enhancing the overall functionality and data flow within the application.

Changes

Files Summary
src/api/*.ts Introduced new Axios POST request functions including createConversationForRepository, createUser, fetchConversationList, fetchRepositoryAgent, fetchUser, fetchUserByEmail, and storeConversation. Each function handles errors and returns relevant data.
src/store/conversationStore.ts, src/store/userStore.ts Implemented new zustand stores for managing user and conversation states. Functions for setting and retrieving user information, as well as adding, setting, and updating conversations are included.
src/types/index.ts Extended the headerObject, ConversationReqRepository, User, and Conversation interfaces with additional fields, impacting the structure of data passed around in the codebase.

🎉 With every line of code, we grow,
New functions here, a store there, oh what a show! 🚀
Interfaces refined, data flowing like a stream,
Celebrating changes, as we code the dream. 💻🌟


Uplevel your code reviews with Entelligence.AI Pro

Entelligence.AI Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, Entelligence.AI Pro is free for open source projects.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Image description Entelligence.AI

Commits Files that changed from the base of the PR and between fbdfb6a and 6d6ec1b commits.
Files selected (10)
  • src/api/createConversationForRepository.ts (1)
  • src/api/createUser.ts (1)
  • src/api/fetchConversationList.ts (1)
  • src/api/fetchRepositoryAgent.ts (1)
  • src/api/fetchUser.ts (1)
  • src/api/storeConversation.ts (1)
  • src/app/index.tsx (1)
  • src/store/conversationStore.ts (1)
  • src/store/userStore.ts (1)
  • src/types/index.ts (1)
Files ignored due to filter (2)
  • package.json
  • pnpm-lock.yaml
Files not summarized due to errors (1)
  • src/app/index.tsx (diff tokens exceeds limit)
Files skipped from review due to trivial changes (1)
  • src/api/fetchRepositoryAgent.ts
Review comments generated (22)
  • Review: 18
  • LGTM: 4

Tips

Chat with Image description Entelligence.AI Bot (@Entelligence.AI)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @Entelligence.AI in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @Entelligence.AI: ignore anywhere in the PR description to pause further reviews from the bot.

Comment on lines +12 to +16

const fetchConversationList = async (
UserUUID: string,
headerObj: headerObject
): Promise<Conversation[] | undefined> => {
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The function fetchConversationList takes a parameter UserUUID which does not follow the camelCase naming convention typically used in JavaScript. Consider renaming it to userUUID for consistency.

- UserUUID: string,
+ userUUID: string,

Comment on lines +21 to +27

const options = {
method: "POST",
url: FETCH_CONVERSATION_LIST_URL,
headers,
data: { UserUUID },
};
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

In this Axios request, you are using a POST method to fetch data, which is not conventional. Typically, GET is used for fetching data and POST is used for sending data. If your API supports it, consider changing this to a GET request.

- method: "POST",
+ method: "GET",
- data: { UserUUID },
+ params: { userUUID },

Comment on lines +28 to +39

try {
const response = await axios.request(options);
return response.data.Conversations;
} catch (error) {
if (error instanceof AxiosError) {
console.error("Error fetching conversation list:", error.message);
} else {
console.error("Unexpected error:", error);
}
return undefined;
}
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

Error handling here could be improved by throwing the error after logging it. This would allow the calling function to handle the error as well. Currently, if an error occurs, the function will return undefined, which might not be the expected behavior for the caller.

- console.error("Error fetching conversation list:", error.message);
+ throw new Error(`Error fetching conversation list: ${error.message}`);

Comment on lines +1 to +33
import axios, { AxiosError } from "axios";
import { apiUrl } from "@/utils/contants";
import { headerObject } from "@/types";

const CREATE_USER_URL = `${apiUrl}createUser/`;

const BASE_HEADERS: Record<string, string> = {
"Content-Type": "application/json",
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "POST,OPTIONS",
};

interface CreateUserProps {
headerObj: headerObject;
Email: string;
}

const createUser = async ({ headerObj, Email }: CreateUserProps): Promise<any> => {
const headers = {
...BASE_HEADERS,
...headerObj,
};

const data = {
Email,
};

const response = await axios.post(CREATE_USER_URL, data, { headers });

return response.data;
};

export default createUser;
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The createUser function does not handle any potential errors that might occur during the execution of the axios.post request. This could lead to unhandled promise rejections if the request fails for any reason, such as network issues or server errors. It's a good practice to wrap your async calls in a try-catch block to handle these scenarios gracefully.

Here's how you can modify the createUser function to include error handling:

const createUser = async ({ headerObj, Email }: CreateUserProps): Promise<any> => {
      const headers = {
            ...BASE_HEADERS,
            ...headerObj,
      };

      const data = {
            Email,
      };

      try {
          const response = await axios.post(CREATE_USER_URL, data, { headers });
          return response.data;
      } catch (error) {
          if (axios.isAxiosError(error)) {
              // Handle Axios error here
          } else {
              // Handle generic error here
          }
      }
};

export default createUser;

In the catch block, I've added a check to see if the error is an AxiosError, which would allow you to handle Axios-specific errors separately from other types of errors. You can replace the comments with your actual error handling logic.

Comment on lines +13 to +37
const createConversationForRepository = async (
req: ConversationReqRepository,
headerObj: headerObject
): Promise<string | undefined> => {
const headers = {
...BASE_HEADERS,
...headerObj,
};

const options = {
method: "POST",
url: CREATE_CONVERSATION_URL,
headers,
data: req,
};

try {
const res = await axios.request(options);
const data = JSON.parse(res.data);
return data.ConversationUUID;
} catch (err) {
console.error("Error creating conversation:", err instanceof AxiosError ? err.message : err);
return undefined;
}
};
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The function createConversationForRepository is well written and handles errors properly. However, it's not a good practice to return undefined in case of an error. It would be better to throw the error and let the caller handle it. This way, the caller can decide what to do with the error (e.g., retry, log, or show an error message to the user). Here's how you could modify the function:

      try {
            const res = await axios.request(options);
            const data = JSON.parse(res.data);
            return data.ConversationUUID;
      } catch (err) {
-           console.error("Error creating conversation:", err instanceof AxiosError ? err.message : err);
-           return undefined;
+           throw new Error(`Error creating conversation: ${err instanceof AxiosError ? err.message : err}`);
      }

Also, consider using axios.post instead of axios.request with method "POST". It simplifies the code and makes it more readable.

      const options = {
-           method: "POST",
            url: CREATE_CONVERSATION_URL,
            headers,
            data: req,
      };

      try {
-           const res = await axios.request(options);
+           const res = await axios.post(CREATE_CONVERSATION_URL, req, { headers });

Comment on lines +37 to +46
export interface Conversation {
UUID: string;
UserUUID: string;
Name: string;
StartDate: string;
RepositoryUUID: string;
TicketUUID: string;
IsActive: boolean;
VectorDBUrl: string;
}
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

In the Conversation interface, all properties are mandatory. If this is intentional and all properties are indeed always available, then this is fine. Otherwise, consider adding optional flags where necessary.

Comment on lines +17 to +40
const fetchUser = async (UserUUID: string, headerObj: headerObject) => {
headers = {
...headers,
...headerObj,
};

if (!UserUUID) return undefined;

const options = {
method: 'POST',
url: url,
headers: headers,
data: {
UserUUID: UserUUID,
},
};
try {
const res = await axios.request(options);
const data = JSON.parse(res.data);
return data.User;
} catch (err) {
console.error(err);
}
};
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The fetchUser function is not handling errors properly. It's currently just logging the error to the console and not providing any feedback to the caller about what went wrong. This could lead to silent failures in your application where an API call fails but the user or calling code isn't aware of it. Instead, consider throwing the error so that the calling code can handle it appropriately.

  try {
    const res = await axios.request(options);
    const data = JSON.parse(res.data);
    return data.User;
  } catch (err) {
-   console.error(err);
+   throw err;
  }

Comment on lines +42 to +68
export const fetchUserByEmail = async (
Email: string,
headerObj: headerObject = {} as any
): Promise<User | undefined> => {
headers = {
...headers,
...headerObj,
};

if (!Email) return undefined;

const options = {
method: 'POST',
url: url,
headers: headers,
data: {
Email: Email,
},
};
try {
const res = await axios.request(options);
const data = JSON.parse(res.data);

return data.User;
} catch (err) {
console.error(err);
}
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

Similar to the fetchUser function, the fetchUserByEmail function is also not handling errors properly. Consider throwing the error so that the calling code can handle it appropriately.

  try {
    const res = await axios.request(options);
    const data = JSON.parse(res.data);

    return data.User;
  } catch (err) {
-   console.error(err);
+   throw err;
  }

Comment on lines +11 to +15
let headers: any = {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Methods': 'POST,OPTIONS',
};
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The headers object is defined outside of the functions and then mutated within each function. This could potentially lead to race conditions if multiple requests are made at the same time. Consider moving the headers definition inside each function to avoid this issue.

- let headers: any = {
-   'Content-Type': 'application/json',
-   'Access-Control-Allow-Origin': '*',
-   'Access-Control-Allow-Methods': 'POST,OPTIONS',
- };
...
const fetchUser = async (UserUUID: string, headerObj: headerObject) => {
+ let headers: any = {
+   'Content-Type': 'application/json',
+   'Access-Control-Allow-Origin': '*',
+   'Access-Control-Allow-Methods': 'POST,OPTIONS',
+ };
  headers = {
    ...headers,
    ...headerObj,
  };
...
export const fetchUserByEmail = async (
  Email: string,
  headerObj: headerObject = {} as any
): Promise<User | undefined> => {
+ let headers: any = {
+   'Content-Type': 'application/json',
+   'Access-Control-Allow-Origin': '*',
+   'Access-Control-Allow-Methods': 'POST,OPTIONS',
+ };
  headers = {
    ...headers,
    ...headerObj,
  };

Comment on lines +1 to +71
import axios from 'axios';
import apiUrl from './api';
import headerObject from '@/types/headerObj';
import User from '@/types/userType';

const localHost = apiUrl;

const endPoint = 'fetchUser/';
const url = localHost + endPoint;

let headers: any = {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Methods': 'POST,OPTIONS',
};

const fetchUser = async (UserUUID: string, headerObj: headerObject) => {
headers = {
...headers,
...headerObj,
};

if (!UserUUID) return undefined;

const options = {
method: 'POST',
url: url,
headers: headers,
data: {
UserUUID: UserUUID,
},
};
try {
const res = await axios.request(options);
const data = JSON.parse(res.data);
return data.User;
} catch (err) {
console.error(err);
}
};

export const fetchUserByEmail = async (
Email: string,
headerObj: headerObject = {} as any
): Promise<User | undefined> => {
headers = {
...headers,
...headerObj,
};

if (!Email) return undefined;

const options = {
method: 'POST',
url: url,
headers: headers,
data: {
Email: Email,
},
};
try {
const res = await axios.request(options);
const data = JSON.parse(res.data);

return data.User;
} catch (err) {
console.error(err);
}
};

export default fetchUser;
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The fetchUser and fetchUserByEmail functions have a lot of duplicated code. Consider creating a helper function that both can use to reduce duplication and make the code easier to maintain.

+ const makePostRequest = async (url: string, headers: any, data: any) => {
+   const options = {
+     method: 'POST',
+     url: url,
+     headers: headers,
+     data: data,
+   };
+   try {
+     const res = await axios.request(options);
+     const data = JSON.parse(res.data);
+     return data.User;
+   } catch (err) {
+     throw err;
+   }
+ }

- const fetchUser = async (UserUUID: string, headerObj: headerObject) => {
+ const fetchUser = async (UserUUID: string, headerObj: headerObject): Promise<User | undefined> => {
    let headers: any = {
      'Content-Type': 'application/json',
      'Access-Control-Allow-Origin': '*',
      'Access-Control-Allow-Methods': 'POST,OPTIONS',
    };
    headers = {
      ...headers,
      ...headerObj,
    };
    if (!UserUUID) return undefined;
-   const options = {
-     method: 'POST',
-     url: url,
-     headers: headers,
-     data: {
-       UserUUID: UserUUID,
-     },
-   };
-   try {
-     const res = await axios.request(options);
-     const data = JSON.parse(res.data);
-     return data.User;
-   } catch (err) {
-     throw err;
-   }
+   return makePostRequest(url, headers, { UserUUID: UserUUID });
  };

- export const fetchUserByEmail = async (
+ export const fetchUserByEmail = async (
    Email: string,
    headerObj: headerObject = {} as any
  ): Promise<User | undefined> => {
    let headers: any = {
      'Content-Type': 'application/json',
      'Access-Control-Allow-Origin': '*',
      'Access-Control-Allow-Methods': 'POST,OPTIONS',
    };
    headers = {
      ...headers,
      ...headerObj,
    };
    if (!Email) return undefined;
-   const options = {
-     method: 'POST',
-     url: url,
-     headers: headers,
-     data: {
-       Email: Email,
-     },
-   };
-   try {
-     const res = await axios.request(options);
-     const data = JSON.parse(res.data);
-     return data.User;
-   } catch (err) {
-     throw err;
-   }
+   return makePostRequest(url, headers, { Email: Email });
  };

@@ -0,0 +1,33 @@
import axios, { AxiosError } from "axios";
import { apiUrl } from "@/utils/contants";
Copy link
Contributor

Choose a reason for hiding this comment

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

apiUrl import has incorrect path ('contants' vs 'constants'), breaking URL construction

Comment on lines +15 to +18
organization,
repoName,
abortSignal
}: FetchRepositoryAgentParams) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential undefined behavior with question[0]?.text without validation when questions array is empty

Comment on lines +10 to +13

let headers: any = {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*',
Copy link
Contributor

Choose a reason for hiding this comment

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

Global headers variable creates security risk by potentially leaking sensitive information across requests

Comment on lines +20 to +29
},
setInitialConversationList: async (UserUUID: string) => {
if (!UserUUID) {
return [];
}

const convList = await fetchConversationList(UserUUID, {});
set({
conversations: await convList,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

State corruption risk: conversations state being set to Promise instead of resolved value

Comment on lines +30 to +31
const res = await axios.request(options);
const data = JSON.parse(res.data);
Copy link
Contributor

Choose a reason for hiding this comment

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

axios already parses JSON responses automatically - double parsing with JSON.parse() will cause errors

Comment on lines +17 to +30

const createUser = async ({ headerObj, Email }: CreateUserProps): Promise<any> => {
const headers = {
...BASE_HEADERS,
...headerObj,
};

const data = {
Email,
};

const response = await axios.post(CREATE_USER_URL, data, { headers });

return response.data;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing error handling for API requests - needs try/catch for AxiosError and network failures

Comment on lines +7 to +11
const BASE_HEADERS: Record<string, string> = {
"Content-Type": "application/json",
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "POST,OPTIONS",
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Client-side CORS headers are ignored and pose security risks - should be removed and set server-side only

Comment on lines +19 to +23
const url = `${apiUrl}repositoryAgent/`;
const data = {
question: question[0]?.text,
history: [],
vectorDBUrl: `${organization}&${repoName}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

URL construction using & separator is unsafe - needs proper URL encoding

Comment on lines +12 to +17
export const fetchRepositoryAgent = async ({
question,
apiKey,
organization,
repoName,
abortSignal
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing input validation - question[0] could be undefined causing API failure

Comment on lines +10 to +14

let headers: any = {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Methods': 'POST,OPTIONS',
Copy link
Contributor

Choose a reason for hiding this comment

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

Global mutable headers object creates race conditions in concurrent requests

@@ -0,0 +1 @@
export const apiUrl = "https://entelligence.ddbrief.com/"; No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Hardcoded API URL prevents environment configuration - should use environment variables

Comment on lines +29 to +31
try {
const res = await axios.request(options);
const data = JSON.parse(res.data);
Copy link
Contributor

Choose a reason for hiding this comment

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

res.data parsing may cause runtime error if already JSON - use res.data directly

Comment on lines +6 to +12
"Content-Type": "application/json",
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "POST,OPTIONS",
};

const CREATE_CONVERSATION_URL = `${apiUrl}createConversation/`;

Copy link
Contributor

Choose a reason for hiding this comment

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

CORS headers incorrectly set client-side instead of server-side, creating security vulnerability

Comment on lines +17 to +27

const createUser = async ({ headerObj, Email }: CreateUserProps): Promise<any> => {
const headers = {
...BASE_HEADERS,
...headerObj,
};

const data = {
Email,
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Incorrect case in import headerObject vs HeaderObject causing potential module resolution failure

Comment on lines +35 to +44
return response.data;
} catch (error) {
if (error instanceof AxiosError) {
console.error('Error fetching repository agent:', error.message);
throw new Error(`Failed to fetch repository agent: ${error.message}`);
}
console.error('Unexpected error:', error);
throw error;
}
}; No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Incorrect import path contants (typo) will cause module resolution failure

Comment on lines +18 to +28
conversations: [...state.conversations, conversation as Conversation]
}));
},
setInitialConversationList: async (UserUUID: string) => {
if (!UserUUID) {
return [];
}

const convList = await fetchConversationList(UserUUID, {});
set({
conversations: await convList,
Copy link
Contributor

Choose a reason for hiding this comment

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

Invalid await usage inside Zustand set causing runtime error

Comment on lines +33 to +34
try {
const res = await axios.request(options);
Copy link
Contributor

Choose a reason for hiding this comment

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

JSON.parse() on res.data is redundant and dangerous as Axios auto-parses JSON responses

Comment on lines +17 to +30

const createUser = async ({ headerObj, Email }: CreateUserProps): Promise<any> => {
const headers = {
...BASE_HEADERS,
...headerObj,
};

const data = {
Email,
};

const response = await axios.post(CREATE_USER_URL, data, { headers });

return response.data;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing error handling for API requests - should handle AxiosError and network failures

Comment on lines +7 to +11
const BASE_HEADERS: Record<string, string> = {
"Content-Type": "application/json",
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "POST,OPTIONS",
};
Copy link
Contributor

Choose a reason for hiding this comment

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

CORS headers on client-side pose security risk and are ignored - should be removed

Comment on lines +19 to +23
const url = `${apiUrl}repositoryAgent/`;
const data = {
question: question[0]?.text,
history: [],
vectorDBUrl: `${organization}&${repoName}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Unsafe URL construction using & separator without proper encoding

Comment on lines +10 to +14

let headers: any = {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Methods': 'POST,OPTIONS',
Copy link
Contributor

Choose a reason for hiding this comment

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

Global mutable headers object creates race condition risk in concurrent requests

@@ -0,0 +1 @@
export const apiUrl = "https://entelligence.ddbrief.com/"; No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Hardcoded API URL in source code is a security risk - should use environment variables

Comment on lines 73 to 74
specifier: ^4.0.0
version: 4.0.0

Choose a reason for hiding this comment

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

The eslint-plugin-react-hooks version 5.1.0-rc-fb9a90fa48-20240614 is a release candidate and may contain unstable features. Consider using a stable version to ensure reliability.

Comment on lines +31 to +32
const data = JSON.parse(res.data);
return data.ConversationUUID;

Choose a reason for hiding this comment

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

The JSON.parse on res.data assumes res.data is a JSON string, which may not always be true. Use res.data directly if it's already parsed.

Comment on lines +7 to +10
const BASE_HEADERS: Record<string, string> = {
"Content-Type": "application/json",
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "POST,OPTIONS",

Choose a reason for hiding this comment

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

The 'Access-Control-Allow-Origin' header is set to '*', which can lead to security vulnerabilities by allowing any origin to access the API.

Comment on lines +16 to +20
repoName,
abortSignal
}: FetchRepositoryAgentParams) => {
const url = `${apiUrl}repositoryAgent/`;
const data = {

Choose a reason for hiding this comment

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

question[0]?.text may cause unexpected behavior if question is empty. Implement default values or check presence.

Comment on lines +9 to +14
const url = localHost + endPoint;

let headers: any = {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Methods': 'POST,OPTIONS',

Choose a reason for hiding this comment

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

Using any type for headers weakens type safety and may lead to unintended behavior. Replace with specific type in headerObj.

Comment on lines +31 to +34
},
};
try {
const res = await axios.request(options);

Choose a reason for hiding this comment

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

Potential JSON.parse error as res.data might already be an object when using Axios. Consider removing JSON.parse().

Comment on lines +11 to +39
let headers: any = {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Methods': 'POST,OPTIONS',
};

const fetchUser = async (UserUUID: string, headerObj: headerObject) => {
headers = {
...headers,
...headerObj,
};

if (!UserUUID) return undefined;

const options = {
method: 'POST',
url: url,
headers: headers,
data: {
UserUUID: UserUUID,
},
};
try {
const res = await axios.request(options);
const data = JSON.parse(res.data);
return data.User;
} catch (err) {
console.error(err);
}

Choose a reason for hiding this comment

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

Header mutation in each function call could lead to unexpected behavior in a concurrent environment. Use function-scoped headers.

Comment on lines +47 to +48
} catch (error) {
if (error instanceof AxiosError) {

Choose a reason for hiding this comment

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

Throwing a generic Error object without status codes or response data can obscure debugging and handling in the caller code.

Comment on lines +16 to +26
addConversation: (conversation: Partial<Conversation>) => {
set((state) => ({
conversations: [...state.conversations, conversation as Conversation]
}));
},
setInitialConversationList: async (UserUUID: string) => {
if (!UserUUID) {
return [];
}

const convList = await fetchConversationList(UserUUID, {});

Choose a reason for hiding this comment

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

The function setInitialConversationList returns [] prematurely when UserUUID is falsy, which doesn't apply any state update derived from this list. Consider moving the return statement after the set call.

Comment on lines +30 to +31
const res = await axios.request(options);
const data = JSON.parse(res.data);

Choose a reason for hiding this comment

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

The JSON "res.data" might already be a JavaScript object. Directly parsing it could result in an error if it's already parsed.

Comment on lines +34 to +36
console.error("Error creating conversation:", err instanceof AxiosError ? err.message : err);
return undefined;
}

Choose a reason for hiding this comment

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

When handling AxiosError, the error message does not capture non-Axios errors clearly. Enhance logging for all error types.

Comment on lines +1 to +2
import axios, { AxiosError } from "axios";
import { apiUrl } from "@/utils/contants";

Choose a reason for hiding this comment

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

The import path @/utils/contants is likely incorrect and should be corrected to ensure the module is correctly imported.

@@ -0,0 +1,33 @@
import axios, { AxiosError } from "axios";
import { apiUrl } from "@/utils/contants";
import { headerObject } from "@/types";

Choose a reason for hiding this comment

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

The import path @/types is likely incorrect and should be corrected to ensure the headerObject type is accurately imported.

Comment on lines +8 to +13
"Content-Type": "application/json",
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "POST,OPTIONS",
};

interface CreateUserProps {

Choose a reason for hiding this comment

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

Declared structured types like Record<string, string> should strictly adhere to expected field names and values; the headerObj type might not align with BASE_HEADERS. Fix any mismatches.

Comment on lines +7 to +9
const BASE_HEADERS: Record<string, string> = {
"Content-Type": "application/json",
"Access-Control-Allow-Origin": "*",

Choose a reason for hiding this comment

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

Allowing all origins with Access-Control-Allow-Origin: '*' may expose the API to unwanted CORS requests.

Comment on lines +14 to +23
apiKey,
organization,
repoName,
abortSignal
}: FetchRepositoryAgentParams) => {
const url = `${apiUrl}repositoryAgent/`;
const data = {
question: question[0]?.text,
history: [],
vectorDBUrl: `${organization}&${repoName}`,

Choose a reason for hiding this comment

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

Possibly accessing an undefined 'text' property in question[0] could lead to a TypeError. Add a check to ensure question array is not empty.

Comment on lines +36 to +37
return data.User;
} catch (err) {

Choose a reason for hiding this comment

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

Error Handling: Catch block only logs to the console, possibly hiding errors during silent failures. Consider more robust error-handling strategies, such as re-throwing errors or returning error objects.

@@ -0,0 +1 @@
export const apiUrl = "https://entelligence.ddbrief.com/"; No newline at end of file

Choose a reason for hiding this comment

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

The absence of a newline at the end of the file may cause some tools or programs to behave unexpectedly, leading to integration issues in some environments.

Comment on lines +24 to 25
"@types/react": "^18.3.10",
"@types/react-dom": "^18.3.0",

Choose a reason for hiding this comment

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

The version 5.1.0-rc-fb9a90fa48-20240614 for eslint-plugin-react-hooks is a specific commit hash, which may not be stable or secure. Consider using a stable release version.

@@ -0,0 +1 @@
export const apiUrl = "https://entelligence.ddbrief.com/"; No newline at end of file

Choose a reason for hiding this comment

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

The file is missing a newline at the end, which can cause issues with some tools or version control systems. Add a newline at the end of the file.

@@ -0,0 +1,39 @@
import axios, { AxiosError } from "axios";
import { apiUrl } from "@/utils/contants";

Choose a reason for hiding this comment

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

Imported and used constant apiUrl is misspelled as contants, which will cause runtime errors when resolving the URL.

try {
const res = await axios.request(options);
const data = JSON.parse(res.data);
return data.ConversationUUID;

Choose a reason for hiding this comment

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

JSON parsing is executed on res.data, which may already be a JSON object. Using res.data directly could suffice depending on the axios response config.

Comment on lines +6 to +8
"Content-Type": "application/json",
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "POST,OPTIONS",

Choose a reason for hiding this comment

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

Unnecessary use of CORS headers Access-Control-Allow-Origin and Access-Control-Allow-Methods, already handled by back-end server or endpoint configurations.

Comment on lines +17 to +19

const createUser = async ({ headerObj, Email }: CreateUserProps): Promise<any> => {
const headers = {

Choose a reason for hiding this comment

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

The function return type of Promise<any> can obscure type safety and should specify a precise type related to the expected API response to prevent runtime errors.

Comment on lines +22 to +25
history: [],
vectorDBUrl: `${organization}&${repoName}`,
};
const config = {

Choose a reason for hiding this comment

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

The vectorDBUrl in the data object is incorrectly formatted as it uses & instead of '/' or another valid separator for paths or URLs.

Comment on lines +11 to +17
let headers: any = {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Methods': 'POST,OPTIONS',
};

const fetchUser = async (UserUUID: string, headerObj: headerObject) => {

Choose a reason for hiding this comment

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

Headers are modified based on headerObj but do not account for subsequent function calls, leading to potential misuse in concurrent situations.

Comment on lines +49 to +55
};

if (!Email) return undefined;

const options = {
method: 'POST',
url: url,

Choose a reason for hiding this comment

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

Same axios options configuration is used for both user queries without differentiating endpoint actions which may lead to incorrect requests.

Comment on lines +44 to +53
try {
const response = await axios.post(STORE_CONVERSATION_URL, data, { headers });
return response.data;
} catch (error) {
if (error instanceof AxiosError) {
console.error("Error storing conversation:", error.message);
throw new Error(`Failed to store conversation: ${error.message}`);
}
console.error("Unexpected error:", error);
throw error;

Choose a reason for hiding this comment

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

Does not handle network connectivity issues explicitly, leading to ungraceful failures when the network is down. Use network status checks before making requests.

Comment on lines +21 to +24
setInitialConversationList: async (UserUUID: string) => {
if (!UserUUID) {
return [];
}

Choose a reason for hiding this comment

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

await is used unnecessarily on assignments from fetchConversationList, which might introduce unhandled promise rejection issues.

Comment on lines +18 to +29
conversations: [...state.conversations, conversation as Conversation]
}));
},
setInitialConversationList: async (UserUUID: string) => {
if (!UserUUID) {
return [];
}

const convList = await fetchConversationList(UserUUID, {});
set({
conversations: await convList,
});

Choose a reason for hiding this comment

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

setInitialConversationList returns inconsistent values as it can return either an empty array or a promise, which may lead to runtime errors for consumers expecting a consistent return type.

Comment on lines 26 to +27
"@types/react-syntax-highlighter": "^15.5.13",
"@vitejs/plugin-react": "^4.3.1",
"@vitejs/plugin-react": "^4.3.2",

Choose a reason for hiding this comment

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

The version 5.1.0-rc-fb9a90fa48-20240614 for eslint-plugin-react-hooks is a specific commit hash, which may not be stable or secure. Consider using a stable release version.

@@ -0,0 +1 @@
export const apiUrl = "https://entelligence.ddbrief.com/"; No newline at end of file

Choose a reason for hiding this comment

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

The file is missing a newline at the end, which can cause issues with some tools or version control systems. Add a newline at the end of the file.

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.

3 participants