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: count public brains number of subscribers #1236

Merged
merged 5 commits into from
Sep 21, 2023
Merged

Conversation

mamadoudicko
Copy link
Contributor

@mamadoudicko mamadoudicko commented Sep 20, 2023

  • count brain number of subscribers

#1209

Screenshot 2023-09-20 at 18 25 35

@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 21, 2023 7:29am
quivr-strapi ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 21, 2023 7:29am
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 21, 2023 7:29am

@github-actions
Copy link
Contributor

github-actions bot commented Sep 20, 2023

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/lib/api/brain/brain.ts

The changes in this file are adding new functions and modifying existing ones. The risk is slightly higher due to the modifications of existing functions. However, the changes seem to be well implemented and are following good practices. The new function getPublicBrains is correctly using the axios instance to make a GET request to the /brains/public endpoint and returning the data.


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

The changes in this file are mostly related to the addition of new features and do not seem to introduce any major risks. However, there are a few points that could be improved for better code quality:

  1. Use of void 0: The use of void 0 is not very common and can be confusing. It would be better to use undefined directly for better readability.

  2. Hardcoded values in the range input: The min, max, and step values for the range input are hardcoded. It would be better to define these as constants at the top of the file or even better, in a separate constants file.

  3. Lack of error handling: There doesn't seem to be any error handling in the form submission. It would be good to add some error handling to improve the user experience.

  4. Use of any type: The use of any type in TypeScript can lead to potential type errors. It would be better to define a type or interface for the form values.


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

The changes in this file involve adding new methods to the Brain class. These methods are well-structured and follow the existing pattern of the class. However, there is a potential risk in the get_brain_subscribers_count method. If the brain with the given id does not exist, it raises a ValueError. This could potentially break the application if not handled properly. Consider returning a default value (like 0) or handling this exception in the calling code.

if len(response) == 0:
    return 0

👍🔧⚠️


Powered by Code Review GPT

@gozineb gozineb merged commit 0cf252e into main Sep 21, 2023
7 of 8 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