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

Fix undefined access on not-existing segmentation layer #5583

Merged
merged 4 commits into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
### Fixed
- Fixed that a disabled "Center new Nodes" option didn't work correctly in merger mode. [#5538](https://github.com/scalableminds/webknossos/pull/5538)
- Fixed a bug where dataset uploads of zips with just one file inside failed. [#5534](https://github.com/scalableminds/webknossos/pull/5534)
- Fixed a benign error message when a dataset without a segmentation layer was opened in view mode or with a skeleton-only annotation. [#5583](https://github.com/scalableminds/webknossos/pull/5583)
- Fixed crashing tree tab which could happen when dragging a node and then switching directly to another tab (e.g., comments) and then back again. [#5573](https://github.com/scalableminds/webknossos/pull/5573)
- Fixed that the UI allowed mutating trees in the tree tab (dragging/creating/deleting trees and groups) in read-only tracings. [#5573](https://github.com/scalableminds/webknossos/pull/5573)
- Fixed "Create a new tree group for this file" setting in front-end import when a group id of 0 was used in the NML. [#5573](https://github.com/scalableminds/webknossos/pull/5573)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// @flow
import _ from "lodash";

import type { ExtractReturn } from "libs/type_helpers";
import { getIsInIframe } from "libs/utils";
import { navbarHeight } from "navbar";
import Constants, {
Expand Down Expand Up @@ -298,7 +299,6 @@ export const resetDefaultLayouts = () => {
getDefaultLayouts.cache.clear();
};

type ExtractReturn<Fn> = $Call<<T>(() => T) => T, Fn>;
type Layout = $Keys<ExtractReturn<typeof _getDefaultLayouts>>;

export const getCurrentDefaultLayoutConfig = () => {
Expand Down
4 changes: 1 addition & 3 deletions frontend/javascripts/oxalis/view/node_context_menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,7 @@ function NoNodeContextMenuOptions({
}: NoNodeContextMenuProps) {
useEffect(() => {
(async () => {
if (segmentationLayer) {
await maybeFetchMeshFiles(segmentationLayer, dataset, false);
}
await maybeFetchMeshFiles(segmentationLayer, dataset, false);
})();
}, []);

Expand Down
16 changes: 10 additions & 6 deletions frontend/javascripts/oxalis/view/right-menu/meshes_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const stlIsosurfaceConstants = {

// This file defines the component MeshesView.

const mapStateToProps = (state: OxalisState) => ({
const mapStateToProps = (state: OxalisState): * => ({
Copy link
Member Author

@philippotto philippotto Jun 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took me a while to figure out why the errors regarding the Props were not caught by flow. It turns out, flow simply assumed the props to be any. The reason for this is that the extraction of the return type for these functions here didn't work. After a bit of digging I found out that the * as a return type helps 🤦

Here's the comment: facebook/flow#7071 (comment)
The issue was opened 2.5 years ago. Yet another reason to ditch flow?

I also looked for ExtractReturn and there is only one other usage which doesn't seem to suffer from the same bug. So, hopefully no other bad suprises.

meshes: state.tracing != null ? state.tracing.meshes : [],
isImporting: state.uiInformation.isImportingMesh,
isosurfaces: state.isosurfaces,
Expand All @@ -75,7 +75,7 @@ const mapStateToProps = (state: OxalisState) => ({
currentMeshFile: state.currentMeshFile,
});

const mapDispatchToProps = (dispatch: Dispatch<*>) => ({
const mapDispatchToProps = (dispatch: Dispatch<*>): * => ({
updateRemoteMeshMetadata(id: string, meshMetaData: $Shape<RemoteMeshMetaData>) {
dispatch(updateRemoteMeshMetaDataAction(id, meshMetaData));
},
Expand Down Expand Up @@ -121,8 +121,9 @@ const mapDispatchToProps = (dispatch: Dispatch<*>) => ({
});

type DispatchProps = ExtractReturn<typeof mapDispatchToProps>;
type StateProps = ExtractReturn<typeof mapStateToProps>;

type Props = {| ...DispatchProps |};
type Props = {| ...DispatchProps, ...StateProps |};

type State = { hoveredListItem: ?number };

Expand Down Expand Up @@ -257,6 +258,9 @@ class MeshesView extends React.Component<Props, State> {
Toast.info("No cell found at centered position");
return;
}
if (!this.props.currentMeshFile || !this.props.segmentationLayer) {
return;
}
await loadMeshFromFile(
id,
pos,
Expand Down Expand Up @@ -295,12 +299,12 @@ class MeshesView extends React.Component<Props, State> {
);

const getLoadPrecomputedMeshButton = () => {
const hasCurrenMeshFile = this.props.currentMeshFile;
const hasCurrentMeshFile = this.props.currentMeshFile;
return (
<Tooltip
key="tooltip"
title={
hasCurrenMeshFile
this.props.currentMeshFile != null
? `Load mesh for centered cell from file ${this.props.currentMeshFile}`
: "There is no mesh file."
}
Expand All @@ -312,7 +316,7 @@ class MeshesView extends React.Component<Props, State> {
overlay={getMeshFilesDropdown()}
buttonsRender={([leftButton, rightButton]) => [
React.cloneElement(leftButton, {
disabled: !hasCurrenMeshFile,
disabled: !hasCurrentMeshFile,
style: { borderRightStyle: "dashed" },
}),
React.cloneElement(rightButton, { style: { borderLeftStyle: "dashed" } }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ import processTaskWithPool from "libs/task_pool";
const PARALLEL_MESH_LOADING_COUNT = 6;

export async function maybeFetchMeshFiles(
segmentationLayer: APIDataLayer,
segmentationLayer: ?APIDataLayer,
dataset: APIDataset,
mustRequest: boolean,
): Promise<void> {
if (!segmentationLayer) {
return;
}
const files = Store.getState().availableMeshFiles;

// only send new get request, if it hasn't happened before (files in store are null)
Expand Down