feat: add knowledge tab on brains settings page

[mamadoudicko]

• Add knowledge tab with explore page content
• Improve UX by adding tanstack query for knowledge fetching

Screen.Recording.2023-09-13.at.15.52.34.mov

#1128

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
docs	✅ Ready (Inspect)	Visit Preview		Sep 13, 2023 1:55pm
quivrapp	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 13, 2023 1:55pm

[github-actions]

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/BrainManagementTabs.tsx

The code seems to be well written and follows good practices. However, there is a potential issue with the BrainManagementTabs component. If brainId is undefined, the component returns an empty div. This could potentially lead to a confusing user experience. Consider providing a fallback UI or a loading state instead of an empty div.

if (brainId === undefined) {
  return <div />;
}

Replace with:

if (brainId === undefined) {
  return <Loading />;
}

Where Loading is a component that indicates to the user that the application is in a loading state.

---

Risk Level 3 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/components/KnowledgeTab/DocumentItem/index.tsx

The deleteDocument function is defined inside the DocumentItem component and it updates the documents state. This could potentially lead to performance issues if there are many DocumentItem components rendered at the same time. Consider moving this function to a higher level component or using a context to manage the documents state.

const deleteDocument = async (name: string) => {
  ...
  setDocuments((docs) => docs.filter((doc) => doc.name !== name)); // Optimistic update
  ...
};

Replace with:

// In a higher level component or context
const deleteDocument = async (name: string) => {
  ...
  setDocuments((docs) => docs.filter((doc) => doc.name !== name)); // Optimistic update
  ...
};

// In the DocumentItem component
const deleteDocument = useContext(DeleteDocumentContext);

---

Risk Level 4 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/components/KnowledgeTab/DocumentItem/DocumentData.tsx

1. The DocumentDetails type is set to any. This is a bad practice as it defeats the purpose of TypeScript's static typing. Consider defining a proper interface for DocumentDetails.

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type DocumentDetails = any;

Replace with:

interface DocumentDetails {
  // Define the properties here
}

2. The useEffect hook is used to fetch documents but the error handling could be improved. Currently, if an error occurs during the fetch, it is logged to the console and the documents state is set to an empty array. This could lead to a confusing user experience. Consider providing a way to handle and display errors to the user.

try {
  const res = await axiosInstance.get<{ documents: DocumentDetails[] }>(
    `/explore/${documentName}/`
  );
  setDocuments(res.data.documents);
} catch (error) {
  setDocuments([]);
  console.error(error);
}

Replace with:

try {
  const res = await axiosInstance.get<{ documents: DocumentDetails[] }>(
    `/explore/${documentName}/`
  );
  setDocuments(res.data.documents);
} catch (error) {
  setError(error);
  console.error(error);
}

Where setError is a function that sets an error state.

---

🔍🔄🚀

---

Powered by Code Review GPT