Skip to content

Commit

Permalink
Fix benign error toast for public annotations and other improvements (#…
Browse files Browse the repository at this point in the history
…6271)

* make modals lazy; don't show error toast when fetching token or teams failed; fix padding of share button

* update changelog

* rename makeModalLazy to makeComponentLazy; improve its typings and also use it for the ShareModalView, UserScriptsModal and MergeModalView

* improve typing

* don't try to fetch teams or token in sharing modal when not logged in
  • Loading branch information
philippotto authored Jun 13, 2022
1 parent 03e565b commit 193f39d
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 38 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ Added a warning for when the resolution in the XY viewport on z=1-downsampled da
- For the api routes that return annotation info objects, the user field was renamed to owner. User still exists as an alias, but will be removed in a future release. [#6250](https://github.com/scalableminds/webknossos/pull/6250)
- When creating a task from a base annotation, the starting position/rotation and bounding box as specified during task creation are now used and overwrite the ones from the original base annotation. [#6249](https://github.com/scalableminds/webknossos/pull/6249)


### Fixed
- Fixed that the context menu broke webKnossos when opening it in dataset-view-mode while no segmentation layer was visible. [#6259](https://github.com/scalableminds/webknossos/pull/6259)
- Fixed benign error toast when viewing a public annotation without being logged in. [#6271](https://github.com/scalableminds/webknossos/pull/6271)

### Removed

Expand Down
8 changes: 6 additions & 2 deletions frontend/javascripts/admin/admin_rest_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,13 +339,17 @@ export function updateTaskType(taskTypeId: string, taskType: APITaskType): Promi

// ### Teams
export async function getTeams(): Promise<Array<APITeam>> {
const teams = await Request.receiveJSON("/api/teams");
const teams = await Request.receiveJSON("/api/teams", {
doNotInvestigate: true,
});
assertResponseLimit(teams);
return teams;
}

export async function getEditableTeams(): Promise<Array<APITeam>> {
const teams = await Request.receiveJSON("/api/teams?isEditable=true");
const teams = await Request.receiveJSON("/api/teams?isEditable=true", {
doNotInvestigate: true,
});
assertResponseLimit(teams);
return teams;
}
Expand Down
1 change: 0 additions & 1 deletion frontend/javascripts/admin/datastore_health_check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const pingDataStoreIfAppropriate = memoizedThrottle(async (requestedUrl: string)
const healthEndpoint = `${url}/${path}/health`;
Request.triggerRequest(healthEndpoint, {
doNotInvestigate: true,
// @ts-expect-error ts-migrate(2345) FIXME: Argument of type '{ doNotInvestigate: true; mode: ... Remove this comment to see the full error message
mode: "cors",
timeout: 5000,
}).then(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,20 @@ class TeamSelectionComponent extends React.PureComponent<Props, State> {
this.setState({
isFetchingData: true,
});
const possibleTeams = this.props.allowNonEditableTeams
? await getTeams()
: await getEditableTeams();
this.setState({
possibleTeams,
isFetchingData: false,
});
try {
const possibleTeams = this.props.allowNonEditableTeams
? await getTeams()
: await getEditableTeams();
this.setState({
possibleTeams,
isFetchingData: false,
});

if (this.props.afterFetchedTeams != null) {
this.props.afterFetchedTeams(possibleTeams);
if (this.props.afterFetchedTeams != null) {
this.props.afterFetchedTeams(possibleTeams);
}
} catch (exception) {
console.error("Could not load teams.");
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useEffect, useRef } from "react";
import React, { useState, useEffect, useRef } from "react";
import { useStore } from "react-redux";
import type { OxalisState } from "oxalis/store";
// From https://overreacted.io/making-setinterval-declarative-with-react-hooks/
Expand Down Expand Up @@ -67,4 +67,22 @@ export function usePolledState(callback: (arg0: OxalisState) => void, interval:
callback(state);
}, interval);
}

export function makeComponentLazy<T extends { isVisible: boolean }>(
ComponentFn: React.ComponentType<T>,
): React.ComponentType<T> {
return function LazyModalWrapper(props: T) {
const [hasBeenInitialized, setHasBeenInitialized] = useState(false);
const isVisible = props.isVisible;
useEffect(() => {
setHasBeenInitialized(hasBeenInitialized || isVisible);
}, [isVisible]);

if (isVisible || hasBeenInitialized) {
return <ComponentFn {...props} />;
}
return null;
};
}

export default {};
1 change: 1 addition & 0 deletions frontend/javascripts/libs/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type RequestOptionsBase<T> = {
headers?: T;
host?: string;
method?: method;
mode?: RequestMode;
params?: string | Record<string, any>;
showErrorToast?: boolean;
timeout?: number;
Expand Down
1 change: 1 addition & 0 deletions frontend/javascripts/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ instead. Only enable this option if you understand its effect. All layers will n
"This webKnossos instance is not configured to run TIFF export jobs on a dedicated background worker. To learn more about this feature please contact us at ",
"annotation.python_do_not_share":
"These snippets are pre-configured and contain your personal access token and annotation meta data. Do not share this information with anyone you do not trust!",
"annotation.register_for_token": "Please log in to get an access token for the script below.",
"project.delete": "Do you really want to delete this project?",
"project.increase_instances":
"Do you really want to add one additional instance to all tasks of this project?",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ const warningColors = {
color: "rgb(255, 155, 85)",
borderColor: "rgb(241, 122, 39)",
};
const copyPositionDefaultStyle = {
padding: "0 10px",
};
const copyPositionErrorStyle = { ...copyPositionDefaultStyle, ...warningColors };
const iconErrorStyle = { ...warningColors };
const positionInputDefaultStyle = {
textAlign: "center",
};
Expand Down Expand Up @@ -94,8 +91,7 @@ class DatasetPositionView extends PureComponent<Props> {
render() {
const position = V3.floor(getPosition(this.props.flycam));
const { isOutOfDatasetBounds, isOutOfTaskBounds } = this.isPositionOutOfBounds(position);
const copyPositionStyle =
isOutOfDatasetBounds || isOutOfTaskBounds ? copyPositionErrorStyle : copyPositionDefaultStyle;
const iconColoringStyle = isOutOfDatasetBounds || isOutOfTaskBounds ? iconErrorStyle : {};
const positionInputStyle =
isOutOfDatasetBounds || isOutOfTaskBounds
? positionInputErrorStyle
Expand Down Expand Up @@ -125,7 +121,7 @@ class DatasetPositionView extends PureComponent<Props> {
<Tooltip title={message["tracing.copy_position"]} placement="bottomLeft">
<ButtonComponent
onClick={this.copyPositionToClipboard}
style={copyPositionStyle}
style={{ padding: "0 10px", ...iconColoringStyle }}
className="hide-on-small-screen"
>
<PushpinOutlined style={positionIconStyle} />
Expand All @@ -140,7 +136,7 @@ class DatasetPositionView extends PureComponent<Props> {
style={positionInputStyle}
allowDecimals
/>
<ShareButton dataset={this.props.dataset} style={copyPositionStyle} />
<ShareButton dataset={this.props.dataset} style={iconColoringStyle} />
</Input.Group>
{isArbitraryMode ? (
<Input.Group
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Divider, Modal, Checkbox, Row, Col, Tabs, Typography, Button } from "antd";
import { CopyOutlined } from "@ant-design/icons";
import React, { useState } from "react";
import { useFetch } from "libs/react_helpers";
import { makeComponentLazy, useFetch } from "libs/react_helpers";
import type { APIAnnotationType } from "types/api_flow_types";
import Toast from "libs/toast";
import messages from "messages";
Expand Down Expand Up @@ -117,7 +117,7 @@ function Footer({
) : null;
}

export default function DownloadModalView(props: Props): JSX.Element {
function _DownloadModalView(props: Props): JSX.Element {
const { isVisible, onClose, annotationType, annotationId, hasVolumeFallback } = props;

const [activeTabKey, setActiveTabKey] = useState("download");
Expand All @@ -127,6 +127,7 @@ export default function DownloadModalView(props: Props): JSX.Element {
const [selectedLayerName, setSelectedLayerName] = useState<string | null>(null);
const [selectedBoundingBoxID, setSelectedBoundingBoxId] = useState(-1);

const activeUser = useSelector((state: OxalisState) => state.activeUser);
const tracing = useSelector((state: OxalisState) => state.tracing);
const dataset = useSelector((state: OxalisState) => state.dataset);
const userBoundingBoxes = useSelector((state: OxalisState) =>
Expand Down Expand Up @@ -209,7 +210,9 @@ export default function DownloadModalView(props: Props): JSX.Element {
}}
type="warning"
>
{messages["annotation.python_do_not_share"]}
{activeUser != null
? messages["annotation.python_do_not_share"]
: messages["annotation.register_for_token"]}
</Text>
</Row>
);
Expand Down Expand Up @@ -253,11 +256,20 @@ export default function DownloadModalView(props: Props): JSX.Element {
lineHeight: "30px",
};

const authToken = useFetch(getAuthToken, "loading...", []);
const authToken = useFetch(
async () => {
if (activeUser != null) {
return getAuthToken();
}
return null;
},
"loading...",
[activeUser],
);
const wkInitSnippet = `import webknossos as wk
with wk.webknossos_context(
token="${authToken}",
token="${authToken || "<insert token here>"}",
url="${window.location.origin}"
):
annotation = wk.Annotation.download(
Expand Down Expand Up @@ -494,3 +506,6 @@ with wk.webknossos_context(
</Modal>
);
}

const DownloadModalView = makeComponentLazy(_DownloadModalView);
export default DownloadModalView;
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import Toast from "libs/toast";
import * as Utils from "libs/utils";
import api from "oxalis/api/internal_api";
import messages from "messages";
import { makeComponentLazy } from "libs/react_helpers";
type ProjectInfo = {
id: string;
label: string;
Expand Down Expand Up @@ -83,7 +84,7 @@ class ButtonWithCheckbox extends PureComponent<ButtonWithCheckboxProps, ButtonWi
}
}

class MergeModalView extends PureComponent<Props, MergeModalViewState> {
class _MergeModalView extends PureComponent<Props, MergeModalViewState> {
state: MergeModalViewState = {
projects: [],
selectedProject: null,
Expand Down Expand Up @@ -307,6 +308,8 @@ class MergeModalView extends PureComponent<Props, MergeModalViewState> {
}
}

const MergeModalView = makeComponentLazy(_MergeModalView);

function mapStateToProps(state: OxalisState): StateProps {
return {
annotationId: state.tracing.annotationId,
Expand Down
27 changes: 18 additions & 9 deletions frontend/javascripts/oxalis/view/action-bar/share_modal_view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import UrlManager from "oxalis/controller/url_manager";
import { setAnnotationVisibilityAction } from "oxalis/model/actions/annotation_actions";
import { setShareModalVisibilityAction } from "oxalis/model/actions/ui_actions";
import { ControlModeEnum } from "oxalis/constants";
import { makeComponentLazy } from "libs/react_helpers";
const RadioGroup = Radio.Group;
const sharingActiveNode = true;
type Props = {
Expand All @@ -44,22 +45,24 @@ function Hint({ children, style }: { children: React.ReactNode; style: React.CSS
}

export function useDatasetSharingToken(dataset: APIDataset) {
const activeUser = useSelector((state: OxalisState) => state.activeUser);
const [datasetToken, setDatasetToken] = useState("");

const fetchAndSetToken = async () => {
try {
const sharingToken = await getDatasetSharingToken(dataset, {
showErrorToast: false,
});
const sharingToken = await getDatasetSharingToken(dataset);
setDatasetToken(sharingToken);
} catch (error) {
console.error(error);
}
};

useEffect(() => {
if (!activeUser) {
return;
}
fetchAndSetToken();
}, [dataset]);
}, [dataset, activeUser]);
return datasetToken;
}
export function getUrl(sharingToken: string, includeToken: boolean) {
Expand Down Expand Up @@ -123,27 +126,30 @@ export function ShareButton(props: { dataset: APIDataset; style?: Record<string,
/>
);
}
export default function ShareModalView(props: Props) {

function _ShareModalView(props: Props) {
const { isVisible, onOk, annotationType, annotationId } = props;
const dataset = useSelector((state: OxalisState) => state.dataset);
const annotationVisibility = useSelector((state: OxalisState) => state.tracing.visibility);
const restrictions = useSelector((state: OxalisState) => state.tracing.restrictions);
const [visibility, setVisibility] = useState(annotationVisibility);
const [sharedTeams, setSharedTeams] = useState<APITeam[]>([]);
const sharingToken = useDatasetSharingToken(dataset);
const activeUser = useSelector((state: OxalisState) => state.activeUser);
const hasUpdatePermissions = restrictions.allowUpdate && restrictions.allowSave;
useEffect(() => setVisibility(annotationVisibility), [annotationVisibility]);

const fetchAndSetSharedTeams = async () => {
const fetchedSharedTeams = await getTeamsForSharedAnnotation(annotationType, annotationId, {
showErrorToast: false,
});
if (!activeUser) {
return;
}
const fetchedSharedTeams = await getTeamsForSharedAnnotation(annotationType, annotationId);
setSharedTeams(fetchedSharedTeams);
};

useEffect(() => {
fetchAndSetSharedTeams();
}, [annotationType, annotationId]);
}, [annotationType, annotationId, activeUser]);

const handleCheckboxChange = (event: RadioChangeEvent) => {
setVisibility(event.target.value as any as APIAnnotationVisibility);
Expand Down Expand Up @@ -348,3 +354,6 @@ export default function ShareModalView(props: Props) {
</Modal>
);
}

const ShareModalView = makeComponentLazy(_ShareModalView);
export default ShareModalView;
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import { fetchGistContent } from "libs/gist";
import { handleGenericError } from "libs/error_handling";
import Request from "libs/request";
import messages from "messages";
import { makeComponentLazy } from "libs/react_helpers";

const { TextArea } = Input;

type UserScriptsModalViewProps = {
onOK: (...args: Array<any>) => any;
isVisible: boolean;
Expand All @@ -20,7 +23,7 @@ type State = {
isLoading: boolean;
};

class UserScriptsModalView extends React.PureComponent<UserScriptsModalViewProps, State> {
class _UserScriptsModalView extends React.PureComponent<UserScriptsModalViewProps, State> {
state: State = {
code: "",
isCodeChanged: false,
Expand Down Expand Up @@ -147,4 +150,6 @@ class UserScriptsModalView extends React.PureComponent<UserScriptsModalViewProps
}
}

const UserScriptsModalView = makeComponentLazy(_UserScriptsModalView);

export default UserScriptsModalView;

0 comments on commit 193f39d

Please sign in to comment.