Feature/615 optimize listing studies - Bugs strikes back

[smailio]

No description provided.

[hdinia]

I suggest simplifying the i18n keys to match their UI labels more directly:

studies.filters.showAllDescendants → studies.filters.showChildrens
studies.requestDeepScan → studies.recursiveScan
(in the code you dont see the UI label so it can be a bit confusing when you search a specific key that correspond to the UI text)

[hdinia]

For toast notification text, we should focus on user-friendly messages rather than technical details ("Details logged in the console"), I suggest you remove this one as you already raise an appropriate message for the error

[hdinia]

Tip: TypeScript can infer the type from useState's default value, so you can simplify:
useState<boolean>(false) → useState(false). Exception to this rule when initializing with a different type than what the state will eventually hold:

// Starting empty but hold numbers
const [items, setItems] = useState<number[]>([]);

[hdinia]

Should be something meaningful like:
id: "test-study-id",
name: "Test Study"

[hdinia]

please avoid using these kind of non-descriptive names, instead prefer names that have business meaning like:
"studies/projects/project1"
"studies/archived/study1"
"workspace1/active/study2"

[hdinia]

return type not necessary here

[hdinia]

you can omit the return type here

[hdinia]

you can omit the return type here

[hdinia]

buildTree is recreated on each render of StudyTree, which is potentially expensive for large tree structures with many children, also to keep the components cleaner it's better to refactor it into a dedicated module, here's what i suggest:

1. Extract the tree node rendering logic into a separate memoized component
2. Memoize the click handlers to prevent unnecessary re-renders (you can check if its really necessary)
3. Leverage component composition instead of using a recursive function

Suggested implementation:

const StudyTreeNode = memo(function StudyTreeNode({ node, parentId, onNodeClick }: StudyTreeNodeProps) {
  const id = parentId ? `${parentId}/${node.name}` : node.name;
  
  const handleClick = useCallback(() => {
    onNodeClick(id, node);
  }, [id, node, onNodeClick]);

  const isLoading = node.hasChildren && node.children.length === 0;

  return (
    <TreeItemEnhanced 
      itemId={id} 
      label={node.name} 
      onClick={handleClick}
    >
      {isLoading ? (
        <TreeItemEnhanced
          itemId={id + "loading"}
          label={t("studies.tree.fetchFolderLoading")}
        />
      ) : (
        node.children.map((child) => (
          <StudyTreeNode
            key={`${id}/${child.name}`}
            node={child}
            parentId={id}
            onNodeClick={onNodeClick}
          />
        ))
      )}
    </TreeItemEnhanced>
  );
});

// In StudyTree component:
function StudyTree() {
  const handleNodeClick = useCallback((itemId: string, node: StudyTreeNode) => {
    dispatch(updateStudyFilters({ folder: itemId }));
    updateTree(itemId, studiesTree, node);
  }, [dispatch, studiesTree]);

  return (
    <SimpleTreeView
      defaultExpandedItems={[...getParentPaths(folder), folder]}
      defaultSelectedItems={folder}
      sx={{ ... }}
    >
      <StudyTreeNode node={studiesTree} onNodeClick={handleNodeClick} />
    </SimpleTreeView>
  );
}

[hdinia]

nothing wrong about this but generally prefer using prefix is, like "isDoingSomething" for flags (the common React boolean flag naming convention), here isLoadingFolder or isLoading seems better

[MartinBelthle]

Not really clear. You could simplify by writing # true when has at least one non-study-folder children

[MartinBelthle]

Minor:
using the camel case to instantiate the class isn't perfect. You can try to use a dict and do NonStudyFolderDTO.model_validate(...) with the dict keys being has_children, it could work