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: NextJS user api #246

Merged
merged 11 commits into from
Sep 6, 2023
Merged

Feat: NextJS user api #246

merged 11 commits into from
Sep 6, 2023

Conversation

SEBRATHEZEBRA
Copy link
Contributor

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Risk Level 2 - /home/runner/work/code-review-gpt/code-review-gpt/services/web-app/src/app/profile/page.tsx

  • There is an unused import statement: "use client".
  • The code is missing a return type annotation for the function component Profile().
  • The code is missing a return statement for the useEffect() hook.
  • The fetchData() function is defined inside the useEffect() hook, which is not recommended.
  • The fetchData() function is defined as an async function but is not awaited.
  • The fetchData() function is called conditionally based on the user state, but the user state is not included in the dependency array of the useEffect() hook.
  • The fetchData() function is called inside the useEffect() hook, but the useEffect() hook does not have a cleanup function.
  • The handleUpdateApiKey() function is missing a return type annotation.
  • The handleUpdateApiKey() function is missing an error handling mechanism.
  • The handleUpdateApiKey() function is missing a loading state to indicate when the API key update is in progress.
  • The handleUpdateApiKey() function logs a success message to the console, which is not necessary.
  • The code is missing a key prop for the ReturnToHome component.
  • The code is missing a key prop for the RepoTable component.
  • The code is missing a key prop for the Image component.
  • The code is missing a key prop for the UpdateAPIKey component.
  • The code is missing a key prop for the h1 element.
  • The code is missing a key prop for the div element.
  • The code is missing a key prop for the Image component inside the div element.
  • The code is missing a key prop for the h1 element inside the div element.
  • The code is missing a key prop for the UpdateAPIKey component inside the div element.
  • The code is missing a key prop for the RepoTable component inside the div element.

Suggested Changes:

  • Remove the unused import statement: "use client".
  • Add a return type annotation for the function component Profile():
export default function Profile(): JSX.Element {
  // code
}
  • Move the fetchData() function outside of the useEffect() hook and await it:
useEffect(() => {
  const fetchData = async () => {
    // code
  };

  if (!user) {
    await fetchData();
  }
}, [session?.user?.userId, user]);
  • Add a cleanup function to the useEffect() hook:
useEffect(() => {
  // code

  return () => {
    // cleanup code
  };
}, [session?.user?.userId, user]);
  • Add a return statement for the useEffect() hook:
useEffect(() => {
  // code

  return () => {
    // cleanup code
  };
}, [session?.user?.userId, user]);

// return statement

Risk Level 2 - /home/runner/work/code-review-gpt/code-review-gpt/services/web-app/src/pages/api/user/getUser.ts

  • The code is missing return type annotations for the getUser() function.
  • The code is missing a try-catch block to handle errors.
  • The code is missing a loading state to indicate when the API request is in progress.
  • The code is using the deprecated "data" property of the Axios response object.

Suggested Changes:

  • Add return type annotations for the getUser() function:
export const getUser = async (
  getUserProps: GetUserProps,
  axiosInstance: AxiosInstance
): Promise<UserBody> => {
  // code
};
  • Add a try-catch block to handle errors:
export const getUser = async (
  getUserProps: GetUserProps,
  axiosInstance: AxiosInstance
): Promise<UserBody> => {
  try {
    // code
  } catch (error) {
    console.error(\"Failed to getUser:\", error);
    throw error;
  }
};
  • Add a loading state to indicate when the API request is in progress:
export const getUser = async (
  getUserProps: GetUserProps,
  axiosInstance: AxiosInstance
): Promise<UserBody> => {
  const [loading, setLoading] = useState(false);

  try {
    setLoading(true);
    // code
  } catch (error) {
    console.error(\"Failed to getUser:\", error);
    throw error;
  } finally {
    setLoading(false);
  }
};
  • Use the "data" property of the Axios response object instead of the deprecated "data" property:
export const getUser = async (
  getUserProps: GetUserProps,
  axiosInstance: AxiosInstance
): Promise<UserBody> => {
  const response = await axiosInstance.get<string>(`/getUser?userId=${getUserProps.userId}`);
  const data = response.data;

  return JSON.parse(data) as UserBody;
};

🔍🔧🔑


Powered by Code Review GPT

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Test results summary:

✅ [PASS] - Test case: Bad variable name
❌ [FAIL] - Test case: Exposed secret
❌ [FAIL] - Test case: Too many nested loops
⚠️ [WARN] - Test case: Unawaited Promise

SUMMARY: ✅ PASS: 1 - ⚠️ WARN: 1 - ❌ FAIL: 2


Tests Powered by Code Review GPT

Comment on lines 22 to 23
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
config.headers["Authorization"] = `Bearer ${session.token}`;
Copy link
Owner

Choose a reason for hiding this comment

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

We are removing this?

import { useAxios } from "../../../lib/hooks/useAxios";
import { GetUserProps, UpdateUserProps } from "../../../lib/types";

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is complaining because we don't have types. I'll add a type now.

@@ -55,7 +55,7 @@ const authOptions: NextAuthOptions = {
async session({ session, token }) {
if (session.user) {
session.user.userId = token.sub;
session.token = token.token;
session.token = token;
Copy link
Owner

Choose a reason for hiding this comment

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

do we need this token now?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure on this to be honest. But in my mind we should just remove it because we aren't using tokens at all for the API anymore

@lizacullis lizacullis merged commit 8000d7f into main Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants