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.

Sign up for GitHub

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

Jump to bottom

Remove wkw datasource suggestions #7697

Merged
merged 22 commits into from
Jun 18, 2024
Merged

Remove wkw datasource suggestions #7697

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
2849111
Remove suggestion route
frcroth Mar 18, 2024
c9f752a
Remove WKWDataFormat
frcroth Mar 18, 2024
de8017a
Lint
frcroth Mar 18, 2024
c1b52f4
Merge branch 'master' into remove-wkw-suggestions
fm3 Mar 21, 2024
b264843
wip remove in frontend
fm3 Mar 21, 2024
1673c15
Merge branch 'master' into remove-wkw-suggestions
fm3 Apr 2, 2024
c1c4630
unify readInboxDataSource
fm3 Apr 2, 2024
97a9d6d
remove datasource suggestion
MichaelBuessemeyer May 24, 2024
1d5437b
Merge branch 'master' of github.com:scalableminds/webknossos into rem…
MichaelBuessemeyer May 24, 2024
b0be7cd
remove left over code line linking to dataset import and clean up code
MichaelBuessemeyer May 24, 2024
5dd5e4d
remove unused imports in backend
MichaelBuessemeyer May 24, 2024
cb4d67f
always show edit settings for datasets the user can edit
MichaelBuessemeyer May 29, 2024
ef4188b
Merge branch 'master' of github.com:scalableminds/webknossos into rem…
MichaelBuessemeyer May 31, 2024
db9ba6d
remove unused type
MichaelBuessemeyer May 31, 2024
f755c22
er-add dataset actions (show error & delete dataset) for broken datas…
MichaelBuessemeyer May 31, 2024
724445d
Merge branch 'master' into remove-wkw-suggestions
fm3 Jun 5, 2024
5f7c546
Merge branch 'master' into remove-wkw-suggestions
fm3 Jun 18, 2024
b5fb853
adapt wording in messages
fm3 Jun 18, 2024
67e3295
add changelog entry
MichaelBuessemeyer Jun 18, 2024
dd1f4e1
Merge branch 'remove-wkw-suggestions' of github.com:scalableminds/web…
MichaelBuessemeyer Jun 18, 2024
c1cee08
update migrationguide,changelog,docs
fm3 Jun 18, 2024
dab6a0c
Merge branch 'remove-wkw-suggestions' of github.com:scalableminds/web…
fm3 Jun 18, 2024
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
11 changes: 0 additions & 11 deletions frontend/javascripts/admin/admin_rest_api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type {
APIBuildInfo,
APIConnectomeFile,
APIDataSource,
APIDataSourceWithMessages,
APIDataStore,
APIDataset,
APIDatasetId,
Expand Down Expand Up @@ -1416,16 +1415,6 @@ export function startMaterializingVolumeAnnotationJob(
);
}

export function getDatasetDatasource(
dataset: APIMaybeUnimportedDataset,
): Promise<APIDataSourceWithMessages> {
return doWithToken((token) =>
Request.receiveJSON(
`${dataset.dataStore.url}/data/datasets/${dataset.owningOrganization}/${dataset.name}?token=${token}`,
),
);
}

export function readDatasetDatasource(dataset: APIDataset): Promise<APIDataSource> {
return doWithToken((token) =>
Request.receiveJSON(
Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/admin/dataset/dataset_add_view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function DatasetAddView({ history }: RouteComponentProps) {
View the Dataset
</Button>
<Button
onClick={() => history.push(`/datasets/${organization}/${datasetName}/import`)}
onClick={() => history.push(`/datasets/${organization}/${datasetName}/edit`)}
Comment on lines 98 to +99
Copy link
Contributor

Choose a reason for hiding this comment

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

As according to the pr description, the "import" should always be named edit, I got rid of the import route (see router.tsx). I hope I didn't misunderstand this 🤞

>
Go to Dataset Settings
</Button>
Expand Down
109 changes: 3 additions & 106 deletions frontend/javascripts/dashboard/advanced_dataset/dataset_action_view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
import {
DeleteOutlined,
EllipsisOutlined,
EyeOutlined,
LoadingOutlined,
PlusCircleOutlined,
PlusOutlined,
ReloadOutlined,
SettingOutlined,
WarningOutlined,
} from "@ant-design/icons";
import window from "libs/window";
import { Link, LinkProps } from "react-router-dom";
import * as React from "react";
import type { APIDatasetId, APIDataset, APIDatasetCompact } from "types/api_flow_types";
import { clearCache, deleteDatasetOnDisk, getDataset } from "admin/admin_rest_api";
import { clearCache, getDataset } from "admin/admin_rest_api";
import Toast from "libs/toast";
import messages from "messages";
import CreateExplorativeModal from "dashboard/advanced_dataset/create_explorative_modal";
import { MenuProps, Modal, Typography } from "antd";
import { confirmAsync } from "dashboard/dataset/helper_components";
import { useQueryClient } from "@tanstack/react-query";
import { MenuProps } from "antd";
import { useState } from "react";

const disabledStyle: React.CSSProperties = {
Expand Down Expand Up @@ -118,7 +113,6 @@ function LinkWithDisabled({
}

function DatasetActionView(props: Props) {
const queryClient = useQueryClient();
const { dataset } = props;

const [isReloading, setIsReloading] = useState(false);
Expand All @@ -137,54 +131,6 @@ function DatasetActionView(props: Props) {
setIsReloading(false);
};

const onDeleteDataset = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function was only used in the importLink. And as I removed this, I also removed this function here

const dataset = await getDataset(props.dataset);

const deleteDataset = await confirmAsync({
title: "Danger Zone",
content: (
<>
<Typography.Title level={4} type="danger">
Deleting a dataset from disk cannot be undone. Are you certain to delete dataset{" "}
{dataset.name}?
</Typography.Title>
<Typography.Paragraph>
Note, WEBKNOSSOS cannot delete datasets that have annotations associated with them.
</Typography.Paragraph>
</>
),
okText: "Yes, delete dataset from disk",
okType: "danger",
});

if (!deleteDataset) {
return;
}

await deleteDatasetOnDisk(dataset.dataStore.url, dataset);

Toast.success(
messages["dataset.delete_success"]({
datasetName: dataset.name,
}),
);

// Invalidate the dataset list cache to exclude the deleted dataset
queryClient.setQueryData(
["datasetsByFolder", dataset.folderId],
(oldItems: APIDatasetCompact[] | undefined) => {
if (oldItems == null) {
return oldItems;
}
return oldItems.filter(
(item) =>
item.name !== dataset.name || item.owningOrganization !== dataset.owningOrganization,
);
},
);
queryClient.invalidateQueries({ queryKey: ["dataset", "search"] });
};

const disabledWhenReloadingStyle = getDisabledWhenReloadingStyle(isReloading);
const reloadLink = (
<a
Expand All @@ -197,48 +143,8 @@ function DatasetActionView(props: Props) {
Reload
</a>
);
const importLink = (
<div className="dataset-table-actions">
<Link
to={`/datasets/${dataset.owningOrganization}/${dataset.name}/import`}
className="import-dataset"
>
<PlusCircleOutlined className="icon-margin-right" />
Import
</Link>
{reloadLink}
<a
onClick={() =>
Modal.error({
title: "Cannot load this dataset",
content: (
<div>
<p>{dataset.status}</p>
{dataset.status === "Deleted by user." ? (
<p>
Even though this dataset was deleted by a user, it is still shown here, because
it was referenced by at least one annotation.
</p>
) : null}
</div>
),
})
}
>
<WarningOutlined className="icon-margin-right" />
Show Error
</a>
{dataset.status !== "Deleted by user." ? (
<a onClick={() => onDeleteDataset()}>
<DeleteOutlined className="icon-margin-right" />
Delete Dataset
</a>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure whether it's a good idea to delete this. The use case is:

  • a dataset exists on disk, but wk isn't able to read it properly (e.g., because of an invalid json)
  • in that case, as a user I want to be able to see the issue, be able to fix it or to delete the dataset directly from the table.

I don't think that removing the wkw datasource suggestions means that the above use cases goes away.

cc @fm3

Copy link
Member

Choose a reason for hiding this comment

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

I agree, it should still be possible to fix the json, and parsing errors should be reported. Just not the automated suggestions.

) : null}
</div>
);
return (
<div>
{dataset.isEditable && !dataset.isActive ? importLink : null}
{dataset.isActive ? (
<div className="dataset-table-actions nowrap">
<NewAnnotationLink
Expand Down Expand Up @@ -330,7 +236,7 @@ export function getDatasetActionContextMenu({
},
}
: null,
dataset.isEditable && dataset.isActive
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I correct, that we no longer have a "not imported" state for a dataset? In that case, is it possible for a dataset to not be active? The only option would be to delete a dataset 🤔

My thought is that we may not need this field isActive anymore / do not need to consider it any longer in the frontend 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Am I correct, that we no longer have a "not imported" state for a dataset? In that case, is it possible for a dataset to not be active?

I don't know what happens now if a datasource properties json does not exist (or is incomplete). I assume the dataset won't be active then. However, the front-end should still show the edit button. Otherwise, the dataset would not be usable/fixable as you say.

dataset.isEditable
? {
key: "edit",
label: "Open Settings",
Expand All @@ -340,15 +246,6 @@ export function getDatasetActionContextMenu({
}
: null,

dataset.isEditable && !dataset.isActive
? {
key: "import",
label: "Import",
onClick: () => {
window.location.href = `/datasets/${dataset.owningOrganization}/${dataset.name}/import`;
},
}
: null,
{
key: "reload",
label: "Reload",
Expand Down
4 changes: 0 additions & 4 deletions frontend/javascripts/dashboard/dataset/dataset_settings_data_tab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,12 @@ export default function DatasetSettingsDataTab({
form,
activeDataSourceEditMode,
onChange,
additionalAlert,
dataset,
}: {
allowRenamingDataset: boolean;
form: FormInstance;
activeDataSourceEditMode: "simple" | "advanced";
onChange: (arg0: "simple" | "advanced") => void;
additionalAlert?: React.ReactNode | null | undefined;
dataset?: APIDataset | null | undefined;
}) {
// Using the return value of useWatch for the `dataSource` var
Expand Down Expand Up @@ -113,8 +111,6 @@ export default function DatasetSettingsDataTab({
</Tooltip>
</div>

{additionalAlert}

<Hideable hidden={activeDataSourceEditMode !== "simple"}>
<RetryingErrorBoundary>
<SimpleDatasetForm
Expand Down
Loading