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

feat(Templates): Support auto selecting template #6498

Closed
wants to merge 6 commits into from
Closed

feat(Templates): Support auto selecting template #6498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
77c1506
Support auto selecting template
tonytang-microsoft-com Dec 20, 2024
7b181ff
changed templateNameLocked to isTemplateNameLocked
Elaina-Lee Jan 28, 2025
2fa81c9
moved logic over to data provider
Elaina-Lee Jan 28, 2025
95c9d87
removed viewTemplate from templatedesigner props
Elaina-Lee Jan 28, 2025
243c19b
updated according to discussion
Elaina-Lee Jan 28, 2025
3ac3211
Merge branch 'main' into elaina/indexer-edit
Elaina-Lee Jan 28, 2025
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
9 changes: 7 additions & 2 deletions libs/designer/src/lib/core/state/templates/__test__/templatesStore.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { describe, beforeAll, it, expect } from 'vitest';
import { AppStore, setupStore } from '../store';
import { changeCurrentTemplateName, updateKind, updateWorkflowName } from '../templateSlice';
import { manifestSlice, setavailableTemplates, setavailableTemplatesNames } from '../manifestSlice';
import { lockTemplate, changeCurrentTemplateName, updateKind, updateWorkflowName } from '../templateSlice';
import { setavailableTemplates, setavailableTemplatesNames } from '../manifestSlice';
import { SkuType, WorkflowKindType } from '../../../../../../../logic-apps-shared/src/utils/src/lib/models/template';

describe('template store reducers', () => {
Expand Down Expand Up @@ -49,8 +49,13 @@ describe('template store reducers', () => {

it('update state call tests for template slice', async () => {
store.dispatch(changeCurrentTemplateName('templateName1'));
expect(store.getState().template.isTemplateNameLocked).toBe(undefined);
expect(store.getState().template.templateName).toBe('templateName1');

store.dispatch(lockTemplate('templateNameX'));
expect(store.getState().template.isTemplateNameLocked).toBe(true);
expect(store.getState().template.templateName).toBe('templateNameX');

store.dispatch(updateWorkflowName({ id: 'default', name: 'workflowName1' }));
expect(store.getState().template.workflows['default'].workflowName).toBe('workflowName1');

Expand Down
6 changes: 6 additions & 0 deletions libs/designer/src/lib/core/state/templates/templateSlice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { getCurrentWorkflowNames, validateConnectionsValue, validateParameterVal
import { initializeTemplateServices, loadTemplate, validateWorkflowName, type TemplatePayload } from '../../actions/bjsworkflow/templates';

export interface TemplateState extends TemplatePayload {
isTemplateNameLocked?: boolean;
templateName?: string;
servicesInitialized: boolean;
}
Expand All @@ -28,6 +29,10 @@ export const templateSlice = createSlice({
changeCurrentTemplateName: (state, action: PayloadAction<string>) => {
state.templateName = action.payload;
},
lockTemplate: (state, action: PayloadAction<string>) => {
state.templateName = action.payload;
state.isTemplateNameLocked = true;
},
updateWorkflowName: (state, action: PayloadAction<{ id: string; name: string | undefined }>) => {
const { id, name } = action.payload;
if (state.workflows[id]) {
Expand Down Expand Up @@ -148,6 +153,7 @@ export const templateSlice = createSlice({

export const {
changeCurrentTemplateName,
lockTemplate,
updateWorkflowName,
updateKind,
validateWorkflowsBasicInfo,
Expand Down
30 changes: 28 additions & 2 deletions libs/designer/src/lib/core/templates/TemplatesDataProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ import {
import { useAreServicesInitialized } from '../state/templates/templateselectors';
import type { ConnectionReferences } from '../../common/models/workflow';
import { getFilteredTemplates } from './utils/helper';
import { initializeTemplateServices } from '../actions/bjsworkflow/templates';
import { initializeTemplateServices, loadTemplate } from '../actions/bjsworkflow/templates';
import type { Template } from '@microsoft/logic-apps-shared';
import { lockTemplate } from '../state/templates/templateSlice';
import { openCreateWorkflowPanelView } from '../state/templates/panelSlice';

export interface TemplatesDataProviderProps {
isConsumption: boolean | undefined;
Expand All @@ -29,12 +31,22 @@ export interface TemplatesDataProviderProps {
services: TemplateServiceOptions;
connectionReferences: ConnectionReferences;
customTemplates?: Record<string, Template.Manifest>;
viewTemplate?: {
templateName: string;
};
children?: React.ReactNode;
}

const DataProviderInner = ({ customTemplates, isConsumption, existingWorkflowName, children }: TemplatesDataProviderProps) => {
const DataProviderInner = ({
customTemplates,
isConsumption,
existingWorkflowName,
viewTemplate,
children,
}: TemplatesDataProviderProps) => {
Comment on lines +40 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we be loading all templates data when the ask from portal blade is to only open one single template? This is unnecessarily going to increase the panel load time.. that is why I was saying why do you want to lock the template

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are looking for the least amount of code change for this purpose + the background of the template list view.....
I understand that we discussed about the concern of locking the template, but we had multiple meetings involving Divya where she agreed to the use case of this having to be the case..
@tonytang-microsoft-com would you have thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this would be the least we could do to align with the current template experience. If we load only one template, then the UX will only have 1 template shown in the background, which looks different. I don't think this is a blocker and we could optimize the performance later if needed IMHO.

const dispatch = useDispatch<AppDispatch>();
const { githubTemplateNames, availableTemplates, filters } = useSelector((state: RootState) => state?.manifest);
const { isTemplateNameLocked } = useSelector((state: RootState) => state.template);

useEffect(() => {
dispatch(loadGithubManifestNames());
Expand Down Expand Up @@ -71,6 +83,20 @@ const DataProviderInner = ({ customTemplates, isConsumption, existingWorkflowNam
}
}, [dispatch, existingWorkflowName]);

useEffect(() => {
if (viewTemplate?.templateName && githubTemplateNames?.includes(viewTemplate.templateName) && !isTemplateNameLocked) {
const templateManifest = availableTemplates?.[viewTemplate.templateName];
if (templateManifest) {
dispatch(lockTemplate(viewTemplate.templateName));
dispatch(loadTemplate({ preLoadedManifest: templateManifest, isCustomTemplate: false }));

if (Object.keys(templateManifest?.workflows ?? {}).length === 0) {
preetriti1 marked this conversation as resolved.
Show resolved Hide resolved
dispatch(openCreateWorkflowPanelView());
}
}
}
}, [dispatch, availableTemplates, viewTemplate, isTemplateNameLocked, githubTemplateNames]);

return <>{children}</>;
};

Expand Down
12 changes: 9 additions & 3 deletions libs/designer/src/lib/ui/panel/templatePanel/createWorkflowPanel/createWorkflowPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface CreateWorkflowTabProps {
nextTabId?: string;
hasError: boolean;
shouldClearDetails: boolean;
isTemplateNameLocked?: boolean;
}

export interface CreateWorkflowPanelProps {
Expand All @@ -37,11 +38,12 @@ export const CreateWorkflowPanel = ({ createWorkflow, onClose, clearDetailsOnClo
const dispatch = useDispatch<AppDispatch>();
const intl = useIntl();
const { refetch: refetchWorkflowNames } = useExistingWorkflowNames();
const { selectedTabId, manifest, isOpen, currentPanelView } = useSelector((state: RootState) => ({
const { selectedTabId, manifest, isOpen, currentPanelView, isTemplateNameLocked } = useSelector((state: RootState) => ({
selectedTabId: state.panel.selectedTabId,
manifest: state.template.manifest,
isOpen: state.panel.isOpen,
currentPanelView: state.panel.currentPanelView,
isTemplateNameLocked: state.template.isTemplateNameLocked,
}));
const isMultiWorkflow = useMemo(() => !!manifest && isMultiWorkflowTemplate(manifest), [manifest]);

Expand Down Expand Up @@ -69,14 +71,18 @@ export const CreateWorkflowPanel = ({ createWorkflow, onClose, clearDetailsOnClo
};

const dismissPanel = useCallback(() => {
if (isTemplateNameLocked) {
return;
}

dispatch(closePanel());

if (clearDetailsOnClose) {
dispatch(clearTemplateDetails());
}

onClose?.();
}, [clearDetailsOnClose, dispatch, onClose]);
}, [isTemplateNameLocked, clearDetailsOnClose, dispatch, onClose]);
Elaina-Lee marked this conversation as resolved.
Show resolved Hide resolved

const onRenderHeaderContent = useCallback(
() => (
Expand All @@ -103,7 +109,7 @@ export const CreateWorkflowPanel = ({ createWorkflow, onClose, clearDetailsOnClo
customWidth={'50%'}
isOpen={isOpen && currentPanelView === TemplatePanelView.CreateWorkflow}
onDismiss={dismissPanel}
hasCloseButton={true}
hasCloseButton={!isTemplateNameLocked}
onRenderHeader={onRenderHeaderContent}
onRenderFooterContent={onRenderFooterContent}
layerProps={layerProps}
Expand Down
4 changes: 2 additions & 2 deletions libs/designer/src/lib/ui/panel/templatePanel/createWorkflowPanel/tabs/basicsTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const WorkflowBasics = () => {
export const basicsTab = (
intl: IntlShape,
dispatch: AppDispatch,
{ shouldClearDetails, isCreating, nextTabId, hasError }: CreateWorkflowTabProps
{ shouldClearDetails, isCreating, nextTabId, hasError, isTemplateNameLocked }: CreateWorkflowTabProps
): TemplatePanelTab => ({
id: constants.TEMPLATE_PANEL_TAB_NAMES.BASIC,
title: intl.formatMessage({
Expand Down Expand Up @@ -48,6 +48,6 @@ export const basicsTab = (
dispatch(clearTemplateDetails());
}
},
secondaryButtonDisabled: isCreating,
secondaryButtonDisabled: isCreating || isTemplateNameLocked,
preetriti1 marked this conversation as resolved.
Show resolved Hide resolved
},
});
4 changes: 2 additions & 2 deletions libs/designer/src/lib/ui/panel/templatePanel/createWorkflowPanel/tabs/connectionsTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const ConnectionsPanel: React.FC = () => {
export const connectionsTab = (
intl: IntlShape,
dispatch: AppDispatch,
{ shouldClearDetails, previousTabId, isCreating, nextTabId, hasError }: CreateWorkflowTabProps
{ shouldClearDetails, previousTabId, isCreating, nextTabId, hasError, isTemplateNameLocked }: CreateWorkflowTabProps
): TemplatePanelTab => ({
id: constants.TEMPLATE_PANEL_TAB_NAMES.CONNECTIONS,
title: intl.formatMessage({
Expand Down Expand Up @@ -63,6 +63,6 @@ export const connectionsTab = (
}
}
},
secondaryButtonDisabled: isCreating,
secondaryButtonDisabled: isCreating || (!previousTabId && isTemplateNameLocked),
},
});
4 changes: 2 additions & 2 deletions libs/designer/src/lib/ui/panel/templatePanel/createWorkflowPanel/tabs/parametersTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const ParametersPanel: React.FC = () => {
export const parametersTab = (
intl: IntlShape,
dispatch: AppDispatch,
{ isCreating, shouldClearDetails, previousTabId, hasError }: CreateWorkflowTabProps
{ isCreating, shouldClearDetails, previousTabId, hasError, isTemplateNameLocked }: CreateWorkflowTabProps
): TemplatePanelTab => ({
id: constants.TEMPLATE_PANEL_TAB_NAMES.PARAMETERS,
title: intl.formatMessage({
Expand Down Expand Up @@ -60,6 +60,6 @@ export const parametersTab = (
}
}
},
secondaryButtonDisabled: isCreating,
secondaryButtonDisabled: isCreating || (!previousTabId && isTemplateNameLocked),
},
});
3 changes: 2 additions & 1 deletion libs/designer/src/lib/ui/panel/templatePanel/createWorkflowPanel/tabs/reviewCreateTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ export const reviewCreateTab = (
errorMessage,
isPrimaryButtonDisabled,
previousTabId,
isTemplateNameLocked,
}: {
errorMessage: string | undefined;
isPrimaryButtonDisabled: boolean;
Expand Down Expand Up @@ -214,6 +215,6 @@ export const reviewCreateTab = (
}
}
},
secondaryButtonDisabled: isCreating,
secondaryButtonDisabled: isCreating || (!previousTabId && isTemplateNameLocked),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have this logic in all secondary buttons related to template locked? in reviewCreateTab previousTabId is always true right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

previousTabId shouldn't be present in case of consumption template with no connections/parameters

},
});
22 changes: 19 additions & 3 deletions libs/designer/src/lib/ui/panel/templatePanel/createWorkflowPanel/usePanelTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const useCreateWorkflowPanelTabs = ({
existingWorkflowName,
selectedTabId,
templateName,
isTemplateNameLocked,
workflowAppName,
isConsumption,
parameterDefinitions,
Expand All @@ -45,6 +46,7 @@ export const useCreateWorkflowPanelTabs = ({
isConsumption: state.workflow.isConsumption,
selectedTabId: state.panel.selectedTabId,
templateName: state.template.templateName,
isTemplateNameLocked: state.template.isTemplateNameLocked,
parameterDefinitions: state.template.parameterDefinitions,
errors: state.template.errors,
templateConnections: state.template.connections,
Expand Down Expand Up @@ -164,9 +166,10 @@ export const useCreateWorkflowPanelTabs = ({
: Constants.TEMPLATE_PANEL_TAB_NAMES.REVIEW_AND_CREATE,
hasError: Object.values(workflows).some((workflowData) => workflowData.errors.kind || workflowData.errors.workflow),
isCreating,
isTemplateNameLocked,
}),
}),
[intl, dispatch, isMultiWorkflowTemplate, connectionsExist, parametersExist, workflows, isCreating]
[intl, dispatch, isMultiWorkflowTemplate, connectionsExist, parametersExist, workflows, isCreating, isTemplateNameLocked]
);

const connectionsTabItem = useMemo(
Expand All @@ -177,9 +180,10 @@ export const useCreateWorkflowPanelTabs = ({
nextTabId: parametersExist ? Constants.TEMPLATE_PANEL_TAB_NAMES.PARAMETERS : Constants.TEMPLATE_PANEL_TAB_NAMES.REVIEW_AND_CREATE,
hasError: !!connectionsError,
isCreating,
isTemplateNameLocked,
}),
}),
[intl, dispatch, isMultiWorkflowTemplate, isConsumption, isCreating, connectionsError, parametersExist]
[intl, dispatch, isMultiWorkflowTemplate, isConsumption, isCreating, connectionsError, parametersExist, isTemplateNameLocked]
);

const parametersTabItem = useMemo(
Expand All @@ -193,9 +197,19 @@ export const useCreateWorkflowPanelTabs = ({
: Constants.TEMPLATE_PANEL_TAB_NAMES.BASIC,
hasError: hasParametersValidationErrors,
isCreating,
isTemplateNameLocked,
}),
}),
[intl, dispatch, isMultiWorkflowTemplate, isConsumption, isCreating, hasParametersValidationErrors, connectionsExist]
[
intl,
dispatch,
isMultiWorkflowTemplate,
isConsumption,
isCreating,
hasParametersValidationErrors,
connectionsExist,
isTemplateNameLocked,
]
);

const reviewCreateTabItem = useMemo(
Expand All @@ -213,6 +227,7 @@ export const useCreateWorkflowPanelTabs = ({
: isConsumption
? undefined
: Constants.TEMPLATE_PANEL_TAB_NAMES.BASIC,
isTemplateNameLocked,
}),
}),
[
Expand All @@ -228,6 +243,7 @@ export const useCreateWorkflowPanelTabs = ({
hasParametersValidationErrors,
parametersExist,
connectionsExist,
isTemplateNameLocked,
]
);

Expand Down
1 change: 1 addition & 0 deletions libs/designer/src/lib/ui/templates/TemplatesDesigner.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export type CreateWorkflowHandler = (
connectionsMapping: ConnectionMapping,
parametersData: Record<string, Template.ParameterDefinition>
) => Promise<void>;

export interface TemplatesDesignerProps {
detailFilters: TemplateDetailFilterType;
createWorkflowCall: CreateWorkflowHandler;
Expand Down
1 change: 0 additions & 1 deletion libs/designer/src/lib/ui/templates/templateslist.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export const TemplatesList = ({ detailFilters, createWorkflowCall }: TemplatesDe
useEffect(() => setLayerHostSelector('#msla-layer-host'), []);
const intl = useIntl();
const dispatch = useDispatch<AppDispatch>();

const { templateName, workflows } = useSelector((state: RootState) => state.template);
const {
filteredTemplateNames,
Expand Down
Loading