fix: allow users to delete brains

[mamadoudicko]

Issue: #1362

Demo:

Screen.Recording.2023-11-06.at.16.53.02.mp4

[vercel]

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	Nov 6, 2023 3:53pm
quivr-strapi	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 6, 2023 3:53pm
quivrapp	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 6, 2023 3:53pm

[github-actions]

Risk Level 2 - /home/runner/work/quivr/quivr/backend/models/databases/supabase/knowledge.py

The changes in this file are mostly additions of new methods and classes. However, there is a potential issue in the remove_brain_all_knowledge method. It seems to be deleting all knowledge associated with a brain, but there is no error handling or checks to ensure that the brain_id provided is valid and exists in the database. This could potentially lead to unexpected behavior or errors. Consider adding error handling or checks to ensure the brain_id is valid. For example:

if not self.db.from_(\"knowledge\").filter(\"brain_id\", \"eq\", str(brain_id)).execute().data:
    raise ValueError(\"Invalid brain_id\")

This will raise an error if the brain_id does not exist in the database, preventing further execution of the method.

---

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/components/SettingsTab/hooks/usePrompt.ts

The code changes in this pull request are generally well written and follow good practices. However, there are a few areas that could be improved for better readability and maintainability:

1. Use of console.log: There is a console.log statement in the submitPrompt function. This should be removed as it can lead to potential security risks and performance issues if the logged data is large. It's also not a good practice to have console logs in production code.

- console.log(\"OTHER CONFIGS\", otherConfigs);

2. Error handling: In the catch block of the submitPrompt function, the error is being stringified and published. It would be better to provide a more user-friendly error message instead of stringifying the error object. Also, it's a good practice to log the error for debugging purposes.

3. Use of void operator: The void operator is used before calling updateFormValues and fetchAllBrains functions. This operator is used to disregard a promise that is returned and not handled. If these functions return a promise, it would be better to handle these promises properly to catch any potential errors.

4. Use of any type: The any type is used in several places. Using any defeats the purpose of using TypeScript, which is to provide static types. It would be better to replace any with more specific types if possible.

---

Risk Level 2 - /home/runner/work/quivr/quivr/backend/routes/knowledge_routes.py

The changes in this file are minimal and do not introduce any potential bugs, readability issues, or performance problems. However, there is a potential issue with the delete_endpoint function. It seems to be deleting a knowledge from a brain, but there is no error handling or checks to ensure that the knowledge_id provided is valid and exists in the database. This could potentially lead to unexpected behavior or errors. Consider adding error handling or checks to ensure the knowledge_id is valid. For example:

if not get_knowledge(knowledge_id):
    raise ValueError(\"Invalid knowledge_id\")

This will raise an error if the knowledge_id does not exist in the database, preventing further execution of the function.

---

🔍🐛🔧

---

Powered by Code Review GPT

[gozineb]

Well spotted ! Thanks mamadz

[sentry-io]

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

• ‼️ StorageException: {'statusCode': 400, 'error': 'Error', 'message': 'body/prefixes must NOT have fewer than 1 items'} /brains/{brain_id}/subscription View Issue

_{Did you find this useful? React with a 👍 or 👎}