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

Add support for multiple environmental templates #208

Merged
merged 6 commits into from
Dec 11, 2024
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
5 changes: 4 additions & 1 deletion src/Router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ const Router: React.FC = () => {
It is unclear why the /create routes need to be listed after the /:id routes. It
seems backwards from what the react-router docs suggest, but it's what works.
*/}
<AuthRoute exact path={paths.sample(":submissionId", ":sampleIndex")}>
<AuthRoute
exact
path={paths.sample(":submissionId", ":template", ":sampleIndex")}
>
<SamplePage />
</AuthRoute>
<AuthRoute exact path={paths.studyEdit(":submissionId")}>
Expand Down
6 changes: 4 additions & 2 deletions src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ export interface SampleData {
}

export interface IndexedSampleData extends SampleData {
_index: number;
_flatIndex: number;
_templateIndex: number;
_template: TemplateName;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is among the first two files I've read in the diff so far—I haven't seen most of the diff yet. At this point, I'd prefer that this be named _templateName, since I think it contains a string representing the name of a template, as opposed to containing the template, itself. I don't know whether I'll still have that preference after I'm done reading the remainder of the diff.

Copy link
Collaborator

Choose a reason for hiding this comment

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

After having read the rest of the diff, I would prefer the various template variables that contain template names, be named templateName; to distinguish them from the object stored in TEMPLATES[templateName], which I assume is a "template".

Copy link
Collaborator Author

@pkalita-lbl pkalita-lbl Dec 11, 2024

Choose a reason for hiding this comment

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

Yeah I like the idea. I started doing that and then realized that it would actually be a lot of changes; enough that I'd be worried about breaking things. The changes would start branching out into files not originally touched here. So I think I might save that as a follow-on task.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

}

// This should eventually come from the schema itself
Expand Down Expand Up @@ -154,7 +156,7 @@ export type TemplateName = keyof typeof TEMPLATES;
export type SlotName = string;

export interface MetadataSubmission {
packageName: TemplateName | "";
packageName: TemplateName | TemplateName[] | "";
contextForm: ContextForm;
addressForm: AddressForm;
templates: string[];
Expand Down
15 changes: 15 additions & 0 deletions src/components/SampleList/SampleList.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
.searchAndFilterContainer {
display: flex;
flex-direction: row;
justify-content: space-between;
padding: 10px;
}

.filterContainer {
white-space: nowrap;
overflow-x: auto;
}

.searchButton {
margin-left: 10px;
}
130 changes: 92 additions & 38 deletions src/components/SampleList/SampleList.tsx
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
import React, { useEffect, useMemo } from "react";
import {
AlertInput,
IonButton,
IonChip,
IonCol,
IonGrid,
IonIcon,
IonItem,
IonLabel,
IonList,
IonListHeader,
IonNote,
IonRow,
IonSearchbar,
SearchbarInputEventDetail,
useIonAlert,
} from "@ionic/react";
import paths from "../../paths";
import { search as searchIcon } from "ionicons/icons";
import NoneOr from "../NoneOr/NoneOr";
import { IndexedSampleData, SubmissionMetadata } from "../../api";
import { IndexedSampleData, SubmissionMetadata, TemplateName } from "../../api";
import { IonSearchbarCustomEvent } from "@ionic/core/dist/types/components";
import { useMiniSearch } from "react-minisearch";
import { getSubmissionSamples } from "../../utils";
import { produce } from "immer";
import { getSubmissionTemplates, getSubmissionSamples } from "../../utils";
import Banner from "../Banner/Banner";
import { StepType } from "@reactour/tour";
import { useAppTour } from "../AppTourProvider/hooks";
import { TourId } from "../AppTourProvider/AppTourProvider";

import styles from "./SampleList.module.css";

// Make steps for the tour.
const steps: Array<StepType> = [
{
Expand All @@ -38,7 +38,7 @@ const steps: Array<StepType> = [
interface SampleListProps {
submission: SubmissionMetadata;
collapsedSize?: number;
onSampleCreate: () => void;
onSampleCreate: (template: TemplateName) => void;
sampleCreateFailureMessage?: string;
}

Expand All @@ -56,6 +56,11 @@ const SampleList: React.FC<SampleListProps> = ({

const [isSearchVisible, setIsSearchVisible] = React.useState(false);
const [searchTerm, setSearchTerm] = React.useState<string | null>();
const [templateFilter, setTemplateFilter] = React.useState<string | null>(
null,
);

const [presentAlert] = useIonAlert();

// Initialize the search index with no documents. The search index will be updated whenever the
// submission samples change.
Expand All @@ -66,29 +71,35 @@ const SampleList: React.FC<SampleListProps> = ({
addAll: searchIndexAddAll,
} = useMiniSearch([] as IndexedSampleData[], {
fields: ["samp_name"],
idField: "_index",
idField: "_flatIndex",
searchOptions: {
prefix: true,
combineWith: "AND",
},
});

const samples = useMemo(() => {
return produce(
getSubmissionSamples(submission) as IndexedSampleData[],
(draft) => {
// Because the submission portal backend needs to be tolerant of storing "invalid" data, some
// samples could essentially be empty. Therefore, there are no existing fields we can treat as a
// key (or persistent identifier). The samples are essentially identified by their position in the
// array. However, because the SampleList component allows the user to filter samples, we need to
// keep track of the original index of each sample.
draft.forEach((sample, index) => {
sample["_index"] = index;
const samplesMap = getSubmissionSamples(submission);
const flattenedSamples: IndexedSampleData[] = [];
// Here is where we flatten out the {template: [samples]} structure into a single array for
// display. While flattening, we need to keep track of which template the sample was associated
// with (`_template`) and the sample's index within that template's sample array
// (`templateIndex`). These are used to construct the router link to the sample page. We also
// need a unique identifier for each sample (`_flatIndex`) so that we can use it as the key in
// the list of samples and in the search index.
let flatIndex = 0;
Object.entries(samplesMap).forEach(([template, samples]) => {
const templateName = template as TemplateName;
samples.forEach((sample, index) => {
flattenedSamples.push({
...sample,
_flatIndex: flatIndex++,
_templateIndex: index,
_template: templateName,
});
// Reverse so that the most recent samples are shown first
draft.reverse();
},
);
});
});
return flattenedSamples.reverse();
}, [submission]);

// Whenever the samples change, update the search index.
Expand Down Expand Up @@ -122,14 +133,45 @@ const SampleList: React.FC<SampleListProps> = ({
setIsSearchVisible(false);
};

const displaySamples = searchTerm ? searchResults || [] : samples;
const handleNewClick = () => {
const templates = getSubmissionTemplates(submission);
if (templates.length > 1) {
void presentAlert({
header: "Select Template",
message: "Select the template for the new sample.",
inputs: templates.map(
(template) =>
({
label: template,
value: template,
type: "radio",
}) as AlertInput,
),
buttons: [
"Cancel",
{
text: "OK",
handler: (template) => {
onSampleCreate(template);
},
},
],
});
} else {
onSampleCreate(templates[0]);
}
};

const displaySamples = (searchTerm ? searchResults || [] : samples).filter(
(sample) => templateFilter == null || sample._template == templateFilter,
);

return (
<>
<IonListHeader>
<IonLabel>Samples {samples && <>({samples.length})</>}</IonLabel>
<IonButton
onClick={onSampleCreate}
onClick={handleNewClick}
data-tour={`${TourId.SampleList}-1`}
>
New
Expand All @@ -142,16 +184,24 @@ const SampleList: React.FC<SampleListProps> = ({
</Banner>
)}

<IonGrid className={isSearchVisible ? "ion-hide" : ""}>
<IonRow class="ion-justify-content-between">
<IonCol size="auto">
{submission.metadata_submission.templates.length > 0 && (
<IonChip outline>
Template: {submission.metadata_submission.templates[0]}
<div className={isSearchVisible ? "ion-hide" : ""}>
<div className={styles.searchAndFilterContainer}>
<div className={styles.filterContainer}>
{getSubmissionTemplates(submission).map((template) => (
<IonChip
key={template}
outline={templateFilter == null || template != templateFilter}
onClick={() =>
setTemplateFilter(
template == templateFilter ? null : template,
)
}
>
Template: {template}
</IonChip>
)}
</IonCol>
<IonCol size="auto">
))}
</div>
<div className={styles.searchButton}>
<IonButton
title="show sample search"
size="small"
Expand All @@ -160,9 +210,9 @@ const SampleList: React.FC<SampleListProps> = ({
>
<IonIcon icon={searchIcon} slot="icon-only"></IonIcon>
</IonButton>
</IonCol>
</IonRow>
</IonGrid>
</div>
</div>
</div>

<IonSearchbar
title="sample search"
Expand All @@ -181,8 +231,12 @@ const SampleList: React.FC<SampleListProps> = ({
.slice(0, isCollapsed ? collapsedSize : undefined)
.map((result) => (
<IonItem
key={result._index}
routerLink={paths.sample(submission.id, result._index)}
key={result._flatIndex}
routerLink={paths.sample(
submission.id,
result._template,
result._templateIndex,
)}
>
<IonLabel>
<h3>
Expand Down
6 changes: 3 additions & 3 deletions src/components/SampleView/SampleView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,21 @@ function formatSlotValue(value: SampleDataValue) {

interface SampleViewProps {
onSlotClick: (slot: SlotDefinition) => void;
packageName: TemplateName;
template: TemplateName;
sample?: SampleData;
schema: SchemaDefinition;
validationResults?: Record<string, string>;
visibleSlots?: SlotName[];
}
const SampleView: React.FC<SampleViewProps> = ({
onSlotClick,
packageName,
template,
sample,
schema,
validationResults,
visibleSlots,
}) => {
const schemaClass = TEMPLATES[packageName].schemaClass;
const schemaClass = TEMPLATES[template].schemaClass;
const slots: SlotDefinition[] = useMemo(() => {
const allSlots = Object.values(
schema.classes?.[schemaClass]?.attributes || {},
Expand Down
40 changes: 35 additions & 5 deletions src/components/StudyForm/StudyForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from "@ionic/react";
import RequiredMark from "../RequiredMark/RequiredMark";
import SectionHeader from "../SectionHeader/SectionHeader";
import { SubmissionMetadataCreate, TEMPLATES } from "../../api";
import { SubmissionMetadataCreate, TemplateName, TEMPLATES } from "../../api";
import { Controller, useForm } from "react-hook-form";
import { useStore } from "../../Store";
import { StepType } from "@reactour/tour";
Expand Down Expand Up @@ -79,7 +79,7 @@ const StudyForm: React.FC<StudyFormProps> = ({

const { loggedInUser } = useStore();

const { handleSubmit, control, formState, setValue } =
const { handleSubmit, control, formState, setValue, getValues } =
useForm<SubmissionMetadataCreate>({
defaultValues: submission,
mode: "onTouched",
Expand Down Expand Up @@ -257,11 +257,41 @@ const StudyForm: React.FC<StudyFormProps> = ({
data-tour={`${TourId.StudyForm}-2`}
className={`${(fieldState.isTouched || formState.isSubmitted) && "ion-touched"} ${fieldState.invalid && "ion-invalid"}`}
labelPlacement="floating"
multiple={Array.isArray(field.value)}
onIonDismiss={field.onBlur}
onIonChange={(e) => {
// these two fields need to stay in sync
setValue("metadata_submission.templates.0", e.detail.value);
field.onChange(e.detail.value);
// The `packageName` and `templates` fields need to stay in sync.
eecavanna marked this conversation as resolved.
Show resolved Hide resolved
const previousPackageName = getValues(
"metadata_submission.packageName",
);
const newPackageName = e.detail.value;
// The value of the `packageName` field is typed to be a string or an array of
// strings to ease the transition between the formats that the backend accepts.
// Once the transition is complete, this logic can be simplified to only use
// the array format.
if (typeof previousPackageName === "string") {
// If the previous value was a string, just replace the first element of
// the templates array with the new package name.
setValue("metadata_submission.templates.0", newPackageName);
} else if (Array.isArray(previousPackageName)) {
// If the previous value was a string, remove all the previous package names
// from the templates array and prepend the new package names.
const templates = getValues(
"metadata_submission.templates",
);
setValue(
"metadata_submission.templates",
newPackageName.concat(
templates.filter(
(template) =>
!previousPackageName.includes(
template as TemplateName,
),
),
),
);
}
field.onChange(newPackageName);
}}
{...field}
>
Expand Down
6 changes: 3 additions & 3 deletions src/components/StudyList/StudyList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
} from "@ionic/react";
import { SubmissionMetadata } from "../../api";
import Pluralize from "../Pluralize/Pluralize";
import { getSubmissionSamples } from "../../utils";
import { getSubmissionSamplesCount, getSubmissionTemplates } from "../../utils";
import paths from "../../paths";
import NoneOr from "../NoneOr/NoneOr";
import QueryErrorBanner from "../QueryErrorBanner/QueryErrorBanner";
Expand Down Expand Up @@ -98,11 +98,11 @@ const StudyList: React.FC = () => {
</h3>
<p>
<NoneOr placeholder="No template selected">
{submission.metadata_submission.templates[0]}
{getSubmissionTemplates(submission).join(", ")}
</NoneOr>
{" • "}
<Pluralize
count={getSubmissionSamples(submission).length}
count={getSubmissionSamplesCount(submission)}
singular="Sample"
showCount
/>
Expand Down
Loading
Loading