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: add public brain creation #1218

Merged
merged 3 commits into from
Sep 20, 2023
Merged

feat: add public brain creation #1218

merged 3 commits into from
Sep 20, 2023

Conversation

mamadoudicko
Copy link
Contributor

@mamadoudicko mamadoudicko commented Sep 20, 2023

  • add brain status input
  • update translations
  • add confirmation modal for brain status changing to public
Screen.Recording.2023-09-20.at.09.30.24.mp4

#1206

@mamadoudicko mamadoudicko temporarily deployed to preview September 20, 2023 07:27 — with GitHub Actions Inactive
@vercel
Copy link

vercel bot commented Sep 20, 2023

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

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 20, 2023 7:31am
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 20, 2023 7:31am

@github-actions
Copy link
Contributor

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/lib/components/AddBrainModal/hooks/useAddBrainModal.ts

The code is generally well written and follows the SOLID principles. However, there are a few areas that could be improved for better readability and maintainability:

  1. The useAddBrainModal hook has a lot of state and logic. Consider breaking it down into smaller, more manageable hooks.
  2. The handleSubmit function is quite large and does a lot of things. Consider breaking it down into smaller functions.
  3. The getCreatingBrainPromptId function is async but does not have error handling. Consider adding a try/catch block to handle potential errors.

Here is an example of how you could refactor the handleSubmit function:

const handleSubmit = async () => {
  const { name, description, setDefault } = getValues();

  if (name.trim() === \"\" || isPending) {
    publishError();
    return;
  }

  try {
    setIsPending(true);
    const prompt_id = await getCreatingBrainPromptId();
    const createdBrainId = await createBrain({ name, description, max_tokens: maxTokens, model, openai_api_key: openAiKey, temperature, prompt_id, status });
    handleBrainCreation(createdBrainId, setDefault);
  } catch (err) {
    handleBrainCreationError(err);
  } finally {
    setIsPending(false);
  }
};

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/lib/config/defaultBrainConfig.ts

The added line of code seems to be setting a status property to private. This is not inherently risky, but it could potentially cause issues if other parts of the codebase are not expecting this property or its value. Ensure that this new property is properly handled wherever this defaultBrainConfig object is used. Also, it's a good practice to avoid magic strings. Consider using an enum for status.

export enum Status {
  Private = 'private',
  // other statuses...
}

// then in your object...
status: Status.Private,

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/lib/components/AddBrainModal/AddBrainModal.tsx

The code seems to be well written and follows the SOLID principles. However, there are a few areas that could be improved for better readability and maintainability:

  1. The AddBrainModal component has a lot of props. Consider using a context or a reducer to manage the state.
  2. The useAddBrainModal hook is used inside the AddBrainModal component. It would be better to pass the necessary values as props to the AddBrainModal component.
  3. The Modal component is being used with a lot of props. It would be better to create a custom Modal component that wraps the Modal component and provides the necessary props.

Here is an example of how you could refactor the AddBrainModal component:

const AddBrainModal = ({ isOpen, onClose, ...props }) => {
  const { t } = useTranslation([\"translation\", \"brain\", \"config\"]);
  const { handleSubmit, register, ...rest } = useAddBrainModal(props);

  return (
    <Modal isOpen={isOpen} onClose={onClose}>
      <form onSubmit={handleSubmit} {...rest}>
        {/* form fields */}
      </form>
    </Modal>
  );
};

And the useAddBrainModal hook:

export const useAddBrainModal = ({ onConfirm, onCancel, ...props }) => {
  // hook logic

  const handleSubmit = (e) => {
    e.preventDefault();
    // form submission logic
    onConfirm();
  };

  return { handleSubmit, register, ...rest };
};

🔨🔧📚


Powered by Code Review GPT

@gozineb gozineb merged commit 37935c5 into main Sep 20, 2023
10 checks passed
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.

2 participants