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 share sample duplicate names #1343

Merged
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.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* [Developer/UI]: Switched to pnpm from yarn for frontend dependency management. See [PR 1332](https://github.com/phac-nml/irida/pull/1334)
* [Developer]: Switched custom exception handling to use built in Spring Boot exception handling. See [PR 1319](https://github.com/phac-nml/irida/pull/1319)
* [UI]: Fixed issue with sorting OAuth clients table in admin panel. See [PR 1342](https://github.com/phac-nml/irida/pull/1342)
* [UI]: Updated share samples review page to list the actual samples which will not be shared with the target project either due to the same sample identifiers or the same samples names already in the target project. See [PR 1343](https://github.com/phac-nml/irida/pull/1343)

## [22.05.5] - 2022/06/28
* [UI]: Fixed bug preventing export of project samples table due to invalid url. [PR 1331](https://github.com/phac-nml/irida/pull/1331)
Expand Down
4 changes: 4 additions & 0 deletions doc/_includes/tutorials/common/samples/copy-samples.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ not be modifiable in the destination project.

![Share samples locked sample icon.]({{ site.baseurl }}/images/tutorials/common/samples/share-locked-sample.png)

If the destination project already has the same samples (sample ids and/or sample names) in it that are being shared from the source project, then you will see expandable warnings with these samples listed

![Share samples duplicates.]({{ site.baseurl }}/images/tutorials/common/samples/share-sample-duplicates.png)

### Moving Samples

If you want to move samples, which means they will be in the destination project, but removed from the current project,
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,17 @@ public List<Long> getSampleIdsForProject(@RequestParam Long id) {
return uiSampleService.getSampleIdsForProject(id);
}

/**
* Get a list of all {@link Sample} names within a specific project
*
* @param id Identifier for a Project
* @return {@link List} of {@link Sample} names
*/
@GetMapping("/names")
public List<String> getSampleNamesForProject(@RequestParam Long id) {
return uiSampleService.getSampleNamesForProject(id);
}

/**
* Share / Move samples between projects
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,18 @@ public List<Long> getSampleIdsForProject(Long projectId) {
return samples.stream().map(Sample::getId).collect(Collectors.toUnmodifiableList());
}

/**
* Get a list of all {@link Sample} names within a specific project
*
* @param projectId Identifier for a {@link Project}
* @return {@link List} of {@link Sample} names
*/
public List<String> getSampleNamesForProject(Long projectId) {
Project project = projectService.read(projectId);
List<Sample> samples = sampleService.getSamplesForProjectShallow(project);
return samples.stream().map(Sample::getLabel).collect(Collectors.toUnmodifiableList());
}

/**
* Share / Move samples with another project
*
Expand Down
3 changes: 2 additions & 1 deletion src/main/resources/i18n/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,8 @@ ShareSamples.remove=Remove
ShareSamplesList.title=Review samples to share
ShareSamples.no-samples.message=All samples exist in the target project
ShareSamples.no-samples.description=Since these samples exists, there is no reason to re-share them.
ShareSamples.some-samples.message={0} samples exist in the target project and will not be re-shared.
ShareSamples.some-samples-same-ids.message=The following samples cannot be shared/moved, as the target project already contains them.
ShareSamples.some-samples-same-names.message=The following samples cannot be shared/moved, as the target project already contains samples with the same names.
ShareSamples.checkbox.remove=Remove samples from current project (move samples)
ShareSamples.checkbox.lock=Prevent modification of samples in target project (only when copying samples)
ShareButton.button=Share Samples
Expand Down
11 changes: 11 additions & 0 deletions src/main/webapp/resources/css/app.css
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@
--blue-8: #0050b3;
--blue-9: #003a8c;
--blue-10: #002766;

--gold-1: #fffbe6;
--gold-2: #fff1b8;
--gold-3: #ffe58f;
--gold-4: #ffd666;
--gold-5: #ffc53d;
--gold-6: #faad14;
--gold-7: #d48806;
--gold-8: #ad6800;
--gold-9: #874d00;
--gold-10: #613400;
}

html,
Expand Down
6 changes: 6 additions & 0 deletions src/main/webapp/resources/js/apis/projects/samples.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ export const samplesApi = createApi({
url: `/identifiers?id=${projectId}`,
}),
}),
getSampleNamesForProject: builder.query({
query: (projectId) => ({
url: `/names?id=${projectId}`,
}),
}),
}),
});

Expand All @@ -52,6 +57,7 @@ export const {
useMergeMutation,
useRemoveMutation,
useGetSampleIdsForProjectQuery,
useGetSampleNamesForProjectQuery,
useShareSamplesWithProjectMutation,
} = samplesApi;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ import { Avatar, Button, List, Tooltip } from "antd";
import React from "react";
import { useDispatch } from "react-redux";
import { IconLocked, IconUnlocked } from "../../../components/icons/Icons";
import {
SampleDetailViewer
} from "../../../components/samples/SampleDetailViewer";
import { SampleDetailViewer } from "../../../components/samples/SampleDetailViewer";
import { green6 } from "../../../styles/colors";
import { removeSample } from "./shareSlice";

Expand All @@ -14,22 +12,30 @@ import { removeSample } from "./shareSlice";
* @param {object} style - style to apply to the list item
* @returns
*/
export default function ShareSamplesListItem({ sample, style }) {
export default function ShareSamplesListItem({
sample,
style,
actionsRequired,
}) {
const dispatch = useDispatch();

return (
<List.Item
style={{ ...style }}
className="t-share-sample"
actions={[
<Button
key="remove"
type="link"
onClick={() => dispatch(removeSample(sample.id))}
>
{i18n("ShareSamples.remove")}
</Button>,
]}
actions={
actionsRequired
? [
<Button
key="remove"
type="link"
onClick={() => dispatch(removeSample(sample.id))}
>
{i18n("ShareSamples.remove")}
</Button>,
]
: []
}
>
<List.Item.Meta
avatar={
Expand Down
77 changes: 61 additions & 16 deletions src/main/webapp/resources/js/pages/projects/share/ShareSamples.jsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,31 @@
import { Alert, Checkbox, Space, Typography } from "antd";
import { Alert, Checkbox, Collapse, Space, Typography } from "antd";
import React from "react";
import { useDispatch, useSelector } from "react-redux";
import ShareAssociated from "./ShareAssociated";
import { SharedSamplesList } from "./SharedSamplesList";
import { updatedLocked, updateMoveSamples } from "./shareSlice";
import { ExclamationCircleOutlined } from "@ant-design/icons";
import { SPACE_XS } from "../../../styles/spacing";

const { Panel } = Collapse;
/**
* React component to review the samples to be shared with another project.
*
* @returns {JSX.Element}
* @constructor
*/
export function ShareSamples({ samples = [] }) {
export function ShareSamples({
samples = [],
targetProjectSampleIdsDuplicate = [],
targetProjectSampleNamesDuplicate = [],
}) {
const dispatch = useDispatch();
const { associated, samples: originalSamples, locked, remove } = useSelector(
(state) => state.shareReducer
);
const {
associated,
samples: originalSamples,
locked,
remove,
} = useSelector((state) => state.shareReducer);

return (
<Space direction="vertical" style={{ width: `100%` }}>
Expand Down Expand Up @@ -56,17 +66,52 @@ export function ShareSamples({ samples = [] }) {
description={i18n("ShareSamples.no-samples.description")}
/>
)}
{originalSamples.length - samples.length > 0 && (
<Alert
className="t-same-samples-warning"
type="info"
showIcon
message={i18n(
"ShareSamples.some-samples.message",
originalSamples.length - samples.length
)}
/>
)}
{originalSamples.length - samples.length > 0 &&
targetProjectSampleIdsDuplicate.length > 0 && (
<Collapse>
<Panel
className="t-same-sample-ids-warning"
header={
<div>
<ExclamationCircleOutlined
style={{ color: `var(--gold-6)`, marginRight: SPACE_XS }}
/>
{i18n("ShareSamples.some-samples-same-ids.message")}
</div>
}
key="1"
style={{ backgroundColor: `var(--grey-1)` }}
>
<SharedSamplesList
list={targetProjectSampleIdsDuplicate}
itemActionsRequired={false}
/>
</Panel>
</Collapse>
)}
{originalSamples.length - samples.length > 0 &&
targetProjectSampleNamesDuplicate.length > 0 && (
<Collapse>
<Panel
className="t-same-sample-names-warning"
header={
<div>
<ExclamationCircleOutlined
style={{ color: `var(--gold-6)`, marginRight: SPACE_XS }}
/>
{i18n("ShareSamples.some-samples-same-names.message")}
</div>
}
key="1"
style={{ backgroundColor: `var(--grey-1)` }}
>
<SharedSamplesList
list={targetProjectSampleNamesDuplicate}
itemActionsRequired={false}
/>
</Panel>
</Collapse>
)}
</Space>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,17 @@ import ShareSamplesListItem from "./ShareSampleListItem";
* @returns {JSX.Element}
* @constructor
*/
export function SharedSamplesList({ list = [] }) {
export function SharedSamplesList({ list = [], itemActionsRequired = true }) {
const Row = ({ index, style }) => {
const sample = list[index];

return <ShareSamplesListItem sample={sample} style={style} />;
return (
<ShareSamplesListItem
sample={sample}
style={style}
actionsRequired={itemActionsRequired}
/>
);
};

const ROW_HEIGHT = 55;
Expand Down
40 changes: 36 additions & 4 deletions src/main/webapp/resources/js/pages/projects/share/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Provider, useSelector } from "react-redux";
import { useGetPotentialProjectsToShareToQuery } from "../../../apis/projects/projects";
import {
useGetSampleIdsForProjectQuery,
useGetSampleNamesForProjectQuery,
useShareSamplesWithProjectMutation,
} from "../../../apis/projects/samples";
import { setBaseUrl } from "../../../utilities/url-utilities";
Expand Down Expand Up @@ -65,12 +66,37 @@ function ShareApp() {
}
);

const { data: existingNames = [] } = useGetSampleNamesForProjectQuery(
targetProject?.identifier,
{
skip: !targetProject?.identifier,
}
);

const { data: projects, isLoading: projectsLoading } =
useGetPotentialProjectsToShareToQuery(currentProject, {
skip: !currentProject,
});

const filtered = samples.filter((sample) => !existingIds.includes(sample.id));
const filtered = samples.filter(
(sample) =>
!existingIds.includes(sample.id) && !existingNames.includes(sample.name)
);

/*
Samples in target project which have the same ids as the ones being shared from the source project
*/
const targetProjectSampleIdsDuplicate = samples.filter((sample) =>
existingIds.includes(sample.id)
);

/*
Samples in target project which have the same names as the ones being shared from the source project
*/
const targetProjectSampleNamesDuplicate = samples.filter(
(sample) =>
existingNames.includes(sample.name) && !existingIds.includes(sample.id)
);

const steps = [
{
Expand All @@ -79,7 +105,13 @@ function ShareApp() {
},
{
title: i18n("ShareLayout.samples"),
component: <ShareSamples samples={filtered} />,
component: (
<ShareSamples
samples={filtered}
targetProjectSampleIdsDuplicate={targetProjectSampleIdsDuplicate}
targetProjectSampleNamesDuplicate={targetProjectSampleNamesDuplicate}
/>
),
},
{ title: i18n("ShareLayout.restrictions"), component: <ShareMetadata /> },
];
Expand All @@ -93,12 +125,12 @@ function ShareApp() {
return;
} else if (step === 1) {
setPrevDisabled(false);
setNextDisabled(filtered.length === 0);
setNextDisabled(filtered.length === 0 || samples.length === 0);
return;
}
setPrevDisabled(false);
setNextDisabled(step === steps.length - 1);
}, [error, targetProject, step, steps.length]);
}, [error, targetProject, step, steps.length, samples.length]);

/*
1. No Samples - this would be if the user came to this page from anything
Expand Down
Loading