Skip to content

Commit

Permalink
Fix breadcrumbs (II) (#6753)
Browse files Browse the repository at this point in the history
* Revert "Gracefully handle failing breadcrumb generation (#6750)"

This reverts commit 527d258.

* revert conversion to Map as this breaks with react-query serialization; instead add minimal fix for broken breadcrumbs

* update changelog
  • Loading branch information
philippotto authored Jan 17, 2023
1 parent b6239d8 commit 7d2e9a3
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 31 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Fixed the validation of some neuroglancer URLs during import. [#6722](https://github.com/scalableminds/webknossos/pull/6722)
- Fixed a bug where deleting a dataset would fail if its representation on disk was already missing. [#6720](https://github.com/scalableminds/webknossos/pull/6720)
- Fixed a bug where a user with multiple organizations could not log in anymore after one of their organization accounts got deactivated. [#6719](https://github.com/scalableminds/webknossos/pull/6719)
- Fixed rare crash in new Datasets tab in dashboard. [#6750](https://github.com/scalableminds/webknossos/pull/6750)
- Fixed rare crash in new Datasets tab in dashboard. [#6750](https://github.com/scalableminds/webknossos/pull/6750) and [#6753](https://github.com/scalableminds/webknossos/pull/6753)

### Removed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,14 @@ export default function DatasetCollectionContextProvider({
}
const { itemById } = folderHierarchyQuery.data;

let currentFolder = itemById.get(dataset.folderId);
let currentFolder = itemById[dataset.folderId];
if (currentFolder == null) {
console.warn("Breadcrumbs could not be computed.");
return [];
}
const breadcrumbs = [currentFolder.title];
while (currentFolder?.parent != null) {
currentFolder = itemById.get(currentFolder.parent);
currentFolder = itemById[currentFolder.parent];
if (currentFolder == null) {
console.warn("Breadcrumbs could not be computed.");
return [];
Expand Down
17 changes: 5 additions & 12 deletions frontend/javascripts/dashboard/dataset/queries.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -537,13 +537,13 @@ function getUnobtrusivelyUpdatedDatasets(

type FolderHierarchy = {
tree: FolderItem[];
itemById: Map<string, FolderItem>;
itemById: Record<string, FolderItem>;
flatItems: FlatFolderTreeItem[];
};

export function getFolderHierarchy(folderTree: FlatFolderTreeItem[]): FolderHierarchy {
const roots: FolderItem[] = [];
const itemById: Map<string, FolderItem> = new Map();
const itemById: Record<string, FolderItem> = {};
for (const folderTreeItem of folderTree) {
const treeItem = {
key: folderTreeItem.id,
Expand All @@ -555,25 +555,18 @@ export function getFolderHierarchy(folderTree: FlatFolderTreeItem[]): FolderHier
if (folderTreeItem.parent == null) {
roots.push(treeItem);
}
itemById.set(folderTreeItem.id, treeItem);
itemById[folderTreeItem.id] = treeItem;
}

// Satisfy TypeScript
const get = (id: string): FolderItem =>
Utils.enforceValue(
itemById.get(id),
"Unexpected error during initialization of folder structure",
);

for (const folderTreeItem of folderTree) {
if (folderTreeItem.parent != null) {
get(folderTreeItem.parent).children.push(get(folderTreeItem.id));
itemById[folderTreeItem.parent].children.push(itemById[folderTreeItem.id]);
}
}

for (const folderTreeItem of folderTree) {
if (folderTreeItem.parent != null) {
get(folderTreeItem.parent).children.sort((a, b) => a.title.localeCompare(b.title));
itemById[folderTreeItem.parent].children.sort((a, b) => a.title.localeCompare(b.title));
}
}

Expand Down
18 changes: 9 additions & 9 deletions frontend/javascripts/dashboard/folders/folder_tree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ export function FolderTreeSidebar({
const [treeData, setTreeData] = useState<FolderItem[]>([]);
const context = useDatasetCollectionContext();
const [expandedKeys, setExpandedKeys] = useState<string[]>([]);
const itemByIdRef = useRef<Map<string, FolderItem>>(new Map());
const itemByIdRef = useRef<Record<string, FolderItem>>({});

const { data: folderHierarchy, isLoading } = context.queries.folderHierarchyQuery;

useEffect(() => {
const newTreeData = folderHierarchy?.tree || [];
const itemById = folderHierarchy?.itemById || new Map();
const itemById = folderHierarchy?.itemById || {};
const newExpandedKeys = deriveExpandedTrees(
newTreeData,
itemById,
Expand All @@ -45,7 +45,7 @@ export function FolderTreeSidebar({
itemByIdRef.current = itemById;
if (
newTreeData.length > 0 &&
(context.activeFolderId == null || itemById.get(context.activeFolderId) == null)
(context.activeFolderId == null || itemById[context.activeFolderId] == null)
) {
// Select the root if there's no active folder id or if the active folder id doesn't
// exist in the tree data (e.g., happens when deleting the active folder).
Expand Down Expand Up @@ -117,8 +117,8 @@ export function FolderTreeSidebar({
}

function moveIfAllowed(sourceId: string, targetId: string) {
const sourceAllowed = itemByIdRef.current.get(sourceId)?.isEditable ?? false;
const targetAllowed = itemByIdRef.current.get(targetId)?.isEditable ?? false;
const sourceAllowed = itemByIdRef.current[sourceId]?.isEditable ?? false;
const targetAllowed = itemByIdRef.current[targetId]?.isEditable ?? false;
if (sourceAllowed && targetAllowed) {
context.queries.moveFolderMutation.mutateAsync([sourceId, targetId]);
} else {
Expand Down Expand Up @@ -361,7 +361,7 @@ const nullableIdToArray = memoizeOne(_nullableIdToArray);

function deriveExpandedTrees(
roots: FolderItem[],
itemById: Map<string, FolderItem>,
itemById: Record<string, FolderItem>,
prevExpandedKeys: string[],
activeFolderId: string | null,
) {
Expand All @@ -371,18 +371,18 @@ function deriveExpandedTrees(
}

for (const oldExpandedKey of prevExpandedKeys) {
const maybeItem = itemById.get(oldExpandedKey);
const maybeItem = itemById[oldExpandedKey];
if (maybeItem != null) {
newExpandedKeySet.add(oldExpandedKey);
}
}

// Expand the parent chain of the active folder.
if (activeFolderId != null) {
let currentFolder = itemById.get(activeFolderId);
let currentFolder = itemById[activeFolderId];
while (currentFolder?.parent != null) {
newExpandedKeySet.add(currentFolder.parent as string);
currentFolder = itemById.get(currentFolder.parent);
currentFolder = itemById[currentFolder.parent];
}
}

Expand Down
7 changes: 0 additions & 7 deletions frontend/javascripts/libs/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,6 @@ export function enforce<A, B>(fn: (arg0: A) => B): (arg0: A | null | undefined)
};
}

export function enforceValue<T>(val: T | null, msg?: string): NonNullable<T> {
if (val == null) {
throw new Error(msg || "Unexpected null value");
}
return val;
}

export function maybe<A, B>(fn: (arg0: A) => B): (arg0: A | null | undefined) => Maybe<B> {
return (nullableA: A | null | undefined) => Maybe.fromNullable(nullableA).map(fn);
}
Expand Down

0 comments on commit 7d2e9a3

Please sign in to comment.