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

Austenem/CAT-898 R template updates #3540

Merged
merged 11 commits into from
Sep 17, 2024
Merged

Austenem/CAT-898 R template updates #3540

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
dae8a34
re-sort and add r tags
austenem Sep 13, 2024
f81e10f
filter templates correctly
austenem Sep 16, 2024
3abf78f
select r template automatically
austenem Sep 16, 2024
3685188
add tooltip to r templates
austenem Sep 16, 2024
5c647a7
add changelog
austenem Sep 16, 2024
736b2fd
naming updates
austenem Sep 16, 2024
7cc39d8
merge with main
austenem Sep 16, 2024
f1aa049
rely on r tag from api and review changes
austenem Sep 17, 2024
3959abd
merge main
austenem Sep 17, 2024
d35f8fe
review changes
austenem Sep 17, 2024
12107b1
resolve naming conflicts
austenem Sep 17, 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
1 change: 1 addition & 0 deletions CHANGELOG-r-template-updates.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Update Launch Workspace dialogs to account for new R template.
24 changes: 17 additions & 7 deletions context/app/static/js/components/workspaces/NewWorkspaceDialog/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,13 @@ import {
TemplatesResponse,
CreateTemplateNotebooksTypes,
TemplateTagsResponse,
TemplatesTypes,
TemplateExample,
} from 'js/components/workspaces/types';
import { useCreateAndLaunchWorkspace, useCreateTemplates } from 'js/components/workspaces/hooks';
import { buildDatasetSymlinks } from 'js/components/workspaces/utils';
import { useWorkspaceToasts } from 'js/components/workspaces/toastHooks';
import { useJobTypes } from 'js/components/workspaces/api';
import { DEFAULT_JOB_TYPE } from 'js/components/workspaces/constants';
import { DEFAULT_JOB_TYPE, R_JOB_TYPE, R_TEMPLATE_TAG } from 'js/components/workspaces/constants';

interface UserTemplatesTypes {
templatesURL: string;
Expand All @@ -42,16 +41,27 @@ function useUserTemplatesAPI<T>({ templatesURL, config = { fallbackData: {} } }:
function useWorkspaceTemplates(tags: string[] = []) {
const { userTemplatesEndpoint } = useAppContext();

const queryParams = tags.map((tag, i) => `${i === 0 ? '' : '&'}tags=${encodeURIComponent(tag)}`).join('');

const url = `${userTemplatesEndpoint}/templates/jupyter_lab/?${queryParams}`;
const url = `${userTemplatesEndpoint}/templates/jupyter_lab`;
const result = useUserTemplatesAPI<TemplatesResponse>({ templatesURL: url });

const templates = result?.data?.data ?? {};

// Manually update tags and title for R templates
const updatedTemplates = Object.fromEntries(
Object.entries(templates).map(([key, template]) => {
const isRTemplate = template.job_types?.includes(R_JOB_TYPE);

const updatedTitle = isRTemplate ? `${template.title} (${R_TEMPLATE_TAG})` : template.title;
const updatedTags = [...template.tags, ...(isRTemplate ? [R_TEMPLATE_TAG] : [])];

return [key, { ...template, tags: updatedTags, title: updatedTitle }];
}),
);
NickAkhmetov marked this conversation as resolved.
Show resolved Hide resolved

// Filter templates by tags
const filteredTemplates = Object.fromEntries(
Object.entries(templates).filter(([, template]) => !template?.is_hidden),
) as TemplatesTypes;
Object.entries(updatedTemplates).filter(([_, template]) => tags.every((tag) => template.tags?.includes(tag))),
);

return {
templates: filteredTemplates,
Expand Down
4 changes: 2 additions & 2 deletions context/app/static/js/components/workspaces/NewWorkspaceDialog/useCreateWorkspaceForm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
DEFAULT_JOB_TYPE,
DEFAULT_MEMORY_MB,
DEFAULT_NUM_CPUS,
DEFAULT_TEMPLATE_KEY,
DEFAULT_PYTHON_TEMPLATE_KEY,
DEFAULT_TIME_LIMIT_MINUTES,
} from '../constants';
import { useDatasetsAutocomplete } from '../AddDatasetsTable/hooks';
Expand Down Expand Up @@ -59,7 +59,7 @@ const schema = z

function useCreateWorkspaceForm({
defaultName,
defaultTemplate = DEFAULT_TEMPLATE_KEY,
defaultTemplate = DEFAULT_PYTHON_TEMPLATE_KEY,
defaultJobType = DEFAULT_JOB_TYPE,
defaultResourceOptions = {
num_cpus: DEFAULT_NUM_CPUS,
Expand Down
23 changes: 21 additions & 2 deletions ...ext/app/static/js/components/workspaces/SelectableTemplateGrid/SelectableTemplateGrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, ChangeEvent } from 'react';
import React, { useCallback, ChangeEvent, useMemo, useEffect } from 'react';
import Typography from '@mui/material/Typography';
import Button from '@mui/material/Button';
import Box from '@mui/material/Box';
Expand All @@ -7,9 +7,11 @@ import { useController, Control, Path } from 'react-hook-form';
import { SpacedSectionButtonRow } from 'js/shared-styles/sections/SectionButtonRow';
import { useSelectItems } from 'js/hooks/useSelectItems';
import ErrorOrWarningMessages from 'js/shared-styles/alerts/ErrorOrWarningMessages';
import { DEFAULT_R_TEMPLATE_KEY, R_JOB_TYPE } from 'js/components/workspaces/constants';
import { TemplatesTypes } from '../types';
import TemplateGrid from '../TemplateGrid';
import { FormWithTemplates } from '../NewWorkspaceDialog/useCreateWorkspaceForm';
import { sortTemplates } from '../utils';

interface TemplateGridProps {
disabledTemplates?: TemplatesTypes;
Expand Down Expand Up @@ -43,6 +45,22 @@ function SelectableTemplateGrid<FormType extends FormWithTemplates>({
field.value satisfies FormType[typeof inputName],
);

const { field: jobType } = useController({
name: 'workspaceJobTypeId' as Path<FormType>,
control,
rules: { required: true },
});

// If the Python + R job type is selected, select the default R template
useEffect(() => {
if (jobType.value === R_JOB_TYPE) {
setSelectedTemplates([...selectedTemplates, DEFAULT_R_TEMPLATE_KEY]);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [jobType.value, setSelectedTemplates]);

austenem marked this conversation as resolved.
Show resolved Hide resolved
const sortedTemplates = useMemo(() => sortTemplates(templates, disabledTemplates), [templates, disabledTemplates]);

const updateTemplates = useCallback(
(templateKeys: string[]) => {
setSelectedTemplates(templateKeys);
Expand Down Expand Up @@ -87,10 +105,11 @@ function SelectableTemplateGrid<FormType extends FormWithTemplates>({
}
/>
<TemplateGrid
templates={templates}
templates={sortedTemplates}
selectItem={selectItem}
selectedTemplates={selectedTemplates}
disabledTemplates={disabledTemplates}
jobType={jobType.value as string}
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this assertion necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without this, jobType.value could potentially be a string array - however I believe the workspaceJobTypeIdField defines the field value as a string.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like workspaceJobTypeId isn't defined in FormType, which only contains templates: string[] since it extends FormWithTemplates:

image

Since there's no property name that matches workspaceJobTypeIdField in the form type, TS treats this as either a string or string-array by default.

I believe that extending FormType to include workspace ID should work, e.g.:

interface FormWithWorkspaceJobId extends FormWithTemplates {
  workspaceJobTypeId: string;
}

...

interface ControllerProps<FormType extends FormWithWorkspaceJobId> {

...

function SelectableTemplateGrid<FormType extends FormWithWorkspaceJobId>({

I tested this approach out to see if it would work, but it doesn't seem to do the trick, oddly enough.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we file a ticket to resolve this later?

</Box>
);
Expand Down
19 changes: 17 additions & 2 deletions context/app/static/js/components/workspaces/TemplateGrid/TemplateGrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,40 @@ import Grid from '@mui/material/Grid';
import SelectableCard from 'js/shared-styles/cards/SelectableCard/SelectableCard';
import { InternalLink } from 'js/shared-styles/Links';
import { sortTemplates } from 'js/components/workspaces/utils';
import { R_JOB_TYPE } from 'js/components/workspaces/constants';
import { TemplatesTypes } from 'js/components/workspaces/types';

interface TemplateGridProps {
templates: TemplatesTypes;
selectItem?: (e: ChangeEvent<HTMLInputElement>) => void;
selectedTemplates?: Set<string>;
disabledTemplates?: TemplatesTypes;
jobType?: string;
}

function TemplateGrid({
templates,
selectItem,
selectedTemplates = new Set([]),
disabledTemplates = {},
jobType,
}: TemplateGridProps) {
const getTooltip = (templateKey: string, job_types?: string[]) => {
if (templateKey in disabledTemplates) {
return 'This template is already in your workspace.';
// If the template is an R template and the job type is not R
}
austenem marked this conversation as resolved.
Show resolved Hide resolved
if (jobType !== R_JOB_TYPE && job_types?.includes(R_JOB_TYPE)) {
return 'This template is not compatible with your current environment. To avoid potential issues, please ensure that you have selected the correct environment for your workspace.';
}
return undefined;
};

const sortedTemplates = useMemo(() => sortTemplates(templates, disabledTemplates), [templates, disabledTemplates]);

return (
<Grid container alignItems="stretch" sx={{ maxHeight: '625px', overflowY: 'auto' }}>
{Object.entries(sortedTemplates).map(([templateKey, { title, description, tags }]) => (
{Object.entries(sortedTemplates).map(([templateKey, { title, description, tags, job_types }]) => (
<Grid item md={4} xs={12} key={templateKey} paddingBottom={2} paddingX={1}>
<SelectableCard
title={<InternalLink href={`/templates/${templateKey}`}>{title}</InternalLink>}
Expand All @@ -34,7 +48,8 @@ function TemplateGrid({
sx={{ height: '100%', minHeight: 225 }}
key={templateKey}
disabled={templateKey in disabledTemplates}
tooltip={templateKey in disabledTemplates ? 'This template is already in your workspace.' : undefined}
tooltip={getTooltip(templateKey, job_types)}
jobTypes={job_types}
/>
</Grid>
))}
Expand Down
13 changes: 7 additions & 6 deletions context/app/static/js/components/workspaces/TemplateSelectStep/TemplateSelectStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import React from 'react';
import { Control } from 'react-hook-form';
import Stack from '@mui/material/Stack';
import Typography from '@mui/material/Typography';

import { SelectedItems } from 'js/hooks/useSelectItems';
import Step, { StepDescription } from 'js/shared-styles/surfaces/Step';
import ContactUsLink from 'js/shared-styles/Links/ContactUsLink';
import Typography from '@mui/material/Typography';
import SelectableTemplateGrid from '../SelectableTemplateGrid';
import { TemplatesTypes } from '../types';
import { FormWithTemplates } from '../NewWorkspaceDialog/useCreateWorkspaceForm';
import TemplateTagsAutocomplete from '../TemplateTagsAutocomplete';
import SelectableTemplateGrid from 'js/components/workspaces/SelectableTemplateGrid';
import { TemplatesTypes } from 'js/components/workspaces/types';
import { FormWithTemplates } from 'js/components/workspaces/NewWorkspaceDialog/useCreateWorkspaceForm';
import TemplateTagsAutocomplete from 'js/components/workspaces/TemplateTagsAutocomplete';
import { R_TEMPLATE_TAG } from 'js/components/workspaces/constants';

function ContactPrompt() {
return (
Expand All @@ -24,7 +25,7 @@ const description = [
<ContactPrompt key="configure-workspace-contact" />,
];

const recommendedTags = ['visualization', 'api'];
const recommendedTags = ['visualization', 'api', R_TEMPLATE_TAG];

interface TemplateSelectProps<FormType extends FormWithTemplates> {
title: string;
Expand Down
7 changes: 6 additions & 1 deletion context/app/static/js/components/workspaces/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
/* Workspace defaults */
export const DEFAULT_JOB_TYPE = 'jupyter_lab';
export const DEFAULT_TEMPLATE_KEY = 'blank';
export const DEFAULT_PYTHON_TEMPLATE_KEY = 'blank';
export const DEFAULT_R_TEMPLATE_KEY = 'blank_r';

/* Workspace R templates */
export const R_TEMPLATE_TAG = 'Jupyter Lab: Python + R';
export const R_JOB_TYPE = 'jupyter_lab_r';

/* Workspace resource defaults */
export const DEFAULT_NUM_CPUS = 1;
Expand Down
1 change: 1 addition & 0 deletions context/app/static/js/components/workspaces/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ interface TemplateTypes {
is_multi_dataset_template: boolean;
template_format: string;
is_hidden: boolean;
job_types?: string[];
examples: TemplateExample[];
}

Expand Down
18 changes: 13 additions & 5 deletions context/app/static/js/components/workspaces/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import {
DEFAULT_JOB_TYPE,
DEFAULT_MEMORY_MB,
DEFAULT_NUM_CPUS,
DEFAULT_TEMPLATE_KEY,
DEFAULT_PYTHON_TEMPLATE_KEY,
DEFAULT_R_TEMPLATE_KEY,
DEFAULT_TIME_LIMIT_MINUTES,
} from './constants';
import {
Expand Down Expand Up @@ -316,8 +317,8 @@ function getDefaultJobType({ workspace }: { workspace: Workspace }) {
}

/**
* Sort templates alphabetically by title, with all disabled templates first, then the default template.
* Ex: [disabled1, disabled2, default, templateA, templateB, ...]
* Sort templates alphabetically by title, with all disabled templates first, then the default python template, then the default R template.
* Ex: [disabled1, disabled2, defaultPython, defaultR, templateA, templateB, ...]
* @param templates The templates to sort.
* @param disabledTemplates The templates that are disabled.
* @returns The sorted templates.
Expand All @@ -328,12 +329,19 @@ function sortTemplates(templates: TemplatesTypes, disabledTemplates?: TemplatesT
const isSelectedA = disabledTemplates && keyA in disabledTemplates;
const isSelectedB = disabledTemplates && keyB in disabledTemplates;

// Disabled templates
if (isSelectedA && !isSelectedB) return -1;
if (!isSelectedA && isSelectedB) return 1;

if (keyA === DEFAULT_TEMPLATE_KEY && keyB !== DEFAULT_TEMPLATE_KEY) return -1;
if (keyB === DEFAULT_TEMPLATE_KEY && keyA !== DEFAULT_TEMPLATE_KEY) return 1;
// Default Python template
if (keyA === DEFAULT_PYTHON_TEMPLATE_KEY && keyB !== DEFAULT_PYTHON_TEMPLATE_KEY) return -1;
if (keyB === DEFAULT_PYTHON_TEMPLATE_KEY && keyA !== DEFAULT_PYTHON_TEMPLATE_KEY) return 1;

// Default R template
if (keyA === DEFAULT_R_TEMPLATE_KEY && keyB !== DEFAULT_PYTHON_TEMPLATE_KEY) return -1;
if (keyB === DEFAULT_R_TEMPLATE_KEY && keyA !== DEFAULT_PYTHON_TEMPLATE_KEY) return 1;

// Alphabetical sorting by title for remaining templates
return templateA.title.localeCompare(templateB.title);
}),
) as TemplatesTypes;
Expand Down
Loading