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

refactor: update the pattern for settings the flowSettings in state #2396

Closed
wants to merge 2 commits into from
Closed

refactor: update the pattern for settings the flowSettings in state #2396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc052a8
refactor: update flowSettings syntax to avoid custom setter
Mike-Heneghan Nov 7, 2023
92fa57f
--wip-- [skip ci]
Mike-Heneghan Nov 7, 2023
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
3 changes: 0 additions & 3 deletions editor.planx.uk/src/pages/FlowEditor/lib/store/settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { TeamStore } from "./team";

export interface SettingsStore {
flowSettings?: FlowSettings;
setFlowSettings: (flowSettings?: FlowSettings) => void;
globalSettings?: GlobalSettings;
setGlobalSettings: (globalSettings: GlobalSettings) => void;
updateFlowSettings: (newSettings: FlowSettings) => Promise<number>;
Expand All @@ -24,8 +23,6 @@ export const settingsStore: StateCreator<
> = (set, get) => ({
flowSettings: undefined,

setFlowSettings: (flowSettings) => set({ flowSettings }),

globalSettings: undefined,

setGlobalSettings: (globalSettings) => {
Expand Down
13 changes: 9 additions & 4 deletions editor.planx.uk/src/routes/flow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ const getFlowSettings = async (
where: { slug: { _eq: $slug }, team: { slug: { _eq: $team_slug } } }
) {
id
settings
flowSettings: settings
}
}
`,
Expand All @@ -194,7 +194,7 @@ const getFlowSettings = async (
team_slug: team,
},
});
return data.flows[0]?.settings;
return data.flows[0]?.flowSettings;
};

const routes = compose(
Expand All @@ -204,8 +204,13 @@ const routes = compose(

withView(async (req) => {
const [flow, ...breadcrumbs] = req.params.flow.split(",");
const settings: FlowSettings = await getFlowSettings(flow, req.params.team);
useStore.getState().setFlowSettings(settings);
const flowSettings: FlowSettings = await getFlowSettings(
flow,
req.params.team,
);
useStore.setState({ flowSettings });
const state = useStore.getState()
console.log('Flow state:', state)

return (
<>
Expand Down
6 changes: 3 additions & 3 deletions editor.planx.uk/src/routes/settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const flowSettingsRoutes = compose(
}
) {
id
settings
flowSettings: settings
}
}
`,
Expand All @@ -37,8 +37,8 @@ const flowSettingsRoutes = compose(
},
});

const settings: FlowSettings = data.flows[0].settings;
useStore.getState().setFlowSettings(settings);
const flowSettings: FlowSettings = data.flows[0].flowSettings;
useStore.setState({ flowSettings });

return {
title: makeTitle(
Expand Down
8 changes: 6 additions & 2 deletions editor.planx.uk/src/routes/views/published.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,24 @@ export const publishedView = async (req: NaviRequest) => {
const data = await fetchDataForPublishedView(flowSlug, teamSlug);

const flow = data.flows[0];
const flowSettings = data.flows[0]?.flowSettings;
if (!flow) throw new NotFoundError(req.originalUrl);

const publishedFlow = flow.publishedFlows[0]?.data;
const flowData = publishedFlow ? publishedFlow : await dataMerged(flow.id);
setPath(flowData, req);

useStore.setState({ flowSettings });

const state = useStore.getState();
// XXX: necessary as long as not every flow is published; aim to remove dataMergedHotfix.ts in future
// load pre-flattened published flow if exists, else load & flatten flow
state.setFlow({ id: flow.id, flow: flowData, flowSlug });
state.setGlobalSettings(data.globalSettings[0]);
state.setFlowSettings(flow.settings);
state.setTeam(flow.team);

console.log('Published state: ', state)

return (
<PublicLayout>
<SaveAndReturnLayout>
Expand Down Expand Up @@ -78,7 +82,7 @@ const fetchDataForPublishedView = async (
notifyPersonalisation: notify_personalisation
boundaryBBox: boundary_bbox
}
settings
flowSettings: settings
publishedFlows: published_flows(
limit: 1
order_by: { created_at: desc }
Expand Down
9 changes: 6 additions & 3 deletions editor.planx.uk/src/routes/views/standalone.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Flow, GlobalSettings } from "types";
import { getTeamFromDomain } from "../utils";

interface StandaloneViewData {
flows: Pick<Flow, "team" | "settings">[];
flows: Pick<Flow, "team" | "flowSettings">[];
globalSettings: GlobalSettings[];
}

Expand All @@ -25,16 +25,19 @@ const standaloneView = async (req: NaviRequest) => {
const data = await fetchDataForStandaloneView(flowSlug, teamSlug);

const {
flows: [{ team, settings: flowSettings }],
flows: [{ team, flowSettings }],
globalSettings,
} = data;

useStore.setState({ flowSettings });

const state = useStore.getState();
state.setFlowNameFromSlug(flowSlug);
state.setGlobalSettings(globalSettings[0]);
state.setFlowSettings(flowSettings);
state.setTeam(team);

console.log('Standalone state: ', state)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been trying to test the refactoring locally by logging out the state to ensure that the state is being set successfully in the relevant components.

Although I'm struggling to figure out how to setup a standalone view 🤔

I believe an example would be an invite to pay and I maybe just need to persevere to setup that up locally?

Copy link
Contributor

@DafyddLlyr DafyddLlyr Nov 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This might add some context here - https://trello.com/c/0Uw8ViZJ/2324-make-editor-sidebar-preview-behave-same-as-the-unpublished-preview-increase-granularity-of-previewenvironment

It's a bit of a pain point (at least for me!) and something I'd love to have a crack at refactoring / removing / rationalising.

This is referring to the PreviewEnvironment variable in shared.ts (the SharedStore slice) - standalone is essentially all pages that are not the Editor such as /preview 👌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the info on this @DafyddLlyr!

I'm still a bit confused because based on this I thought I'd see the output from console.log('Standalone state: ', state) when I was on a /preview route?

Although that seems to use the published view rather than the standalone view?


return (
<PublicLayout>
<View />
Expand Down
8 changes: 6 additions & 2 deletions editor.planx.uk/src/routes/views/unpublished.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,17 @@ export const unpublishedView = async (req: NaviRequest) => {

const flowData = await dataMerged(flow.id);

const flowSettings = flow.flowSettings;
useStore.setState({ flowSettings });

const state = useStore.getState();
state.setFlow({ id: flow.id, flow: flowData, flowSlug });
state.setFlowNameFromSlug(flowSlug);
state.setGlobalSettings(data.globalSettings[0]);
state.setFlowSettings(flow.settings);
state.setTeam(flow.team);

console.log('Unpublished settings: ', state)

return (
<PublicLayout>
<View />
Expand Down Expand Up @@ -69,7 +73,7 @@ const fetchDataForUnpublishedView = async (
notifyPersonalisation: notify_personalisation
boundaryBBox: boundary_bbox
}
settings
flowSettings: settings
slug
}

Expand Down
2 changes: 1 addition & 1 deletion editor.planx.uk/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface Flow {
id: string;
slug: string;
team: Team;
settings?: FlowSettings;
flowSettings?: FlowSettings;
}

export interface Team {
Expand Down
Loading