feat: add select for sorting flow list #4137

RODO94 · 2025-01-10T14:03:28Z

What does this PR do?

Working towards this EPIC: https://trello.com/c/7av8lsVr/3160-epic-as-an-editor-i-want-to-have-a-way-to-organise-my-flows-so-that-i-can-easily-find-what-im-looking-for

The main task was to add the ability to Sort flows in the Team page, the page which shows a Team's flows.

Approach

The Team page component is quite long, so I created a new component separate from this to handle the logic of sorting and also contain a Select component for users to interact with.

This new component takes in state variables flows and setFlows from the Team page component, which felt right for me so the flow list content always lives in one place but is given to other components to augment. I'd want to repeat this pattern for filtering the flows.

My hope for the sort component was to provide a maintainable, scalable component for sorting the list of flows on the Team page using keys we receive from getFlows() contained within the FlowSummary interface. It may need to be used across many different tables, so I have enabled a generic Type to be added when the component is used, so it will now received a record as an Array, a way of setting that record setRecord, and options for sorting the Array sortOptions. This should enable the Type safety I want for this specific use case while extending it for future ones

See this as just the building blocks of the sort component I can add into Ian's work on the new Team page and filter.

What's included?

Right now it covers: Name, Last Published, and Last Updated. I added an ascending or descending toggle as well.

In order to make this work, I have extended the getFlows() query to pull in published_flows data and status data from Hasura.

Future Improvement:

If more items have to be added to the search params we could create a hook for it (useSearchParamState())

Important

This component is hidden using a FeatureFlag - SORT_FLOWS

RODO94 · 2025-01-10T16:14:41Z

editor.planx.uk/src/pages/FlowEditor/lib/store/editor.ts

@@ -390,6 +400,13 @@ export const editorStore: StateCreator<
                lastName: last_name
              }
            }
+            publishedFlows: published_flows(
+              order_by: { created_at: desc }
+              limit: 1


Seen this was only used by the Team page, so though it felt better to expand this initial query

RODO94 · 2025-01-10T16:16:04Z

editor.planx.uk/src/ui/editor/SortFlowsSelect.tsx

+}) => {
+  const [sortBy, setSortBy] = useState<SortObject>(sortArray[0]);
+  const [sortDirection, setSortDirection] = useState<SortDirection>("asc");
+  useEffect(() => {


Went with a useEffect here to provide stability for adding more sorting types, would be interested to know if there's cleaner techniques

RODO94 · 2025-01-10T16:18:04Z

editor.planx.uk/src/ui/editor/SortFlowsSelect.tsx

+        if (!b[sortKey][0]) return -1;
+
+        const aValue = a[sortKey][0][nestedKey];
+        const bValue = b[sortKey][0][nestedKey];


Although this possibly looks overly complicated, it allowed me to use the type safety to build the sortArray while not having to map sorting names to data fields with ternaries or conditional statements.

I iterated this a bit and found that the extra code came a lot from the nested keys in the flows array, but the extra lines always provided better maintainability for adding or removing sorting criteria while remaining aligned with what data is provided to us from the getFlows query

Thanks for the the insightful comment on how you get here 👍 This makes things easier to review and understand - much appreciated!

First off - a useEffect() seems fine for this in order to maintain that reactivity - no real issues here.

The logic here is pretty complex and dense - I think removing the entire concept of nestedKeys (see previous comments) would help out a lot here as there's just a single level to deal with.

github-actions · 2025-01-10T16:41:41Z

Removed vultr server and associated DNS entries

DafyddLlyr

A few general comments on the code that might simplify things a lot here.

However, there's a more general design concept to consider here. We know that we're going to need this sort of sort component in a few places fairly soon. The logic of the component is very closely coupled with the incoming data - I'd recommend splitting these up.

Think about a pattern like this -

A "sort" component which takes in a generic T[] - the items to be sorted
It can call a callback function to update state when the user interacts with the object
Sort keys have to be keyof typeof T (I think type-fest allows deep keys also)
We pass in a list of flows, and the setFlows function, and a list of keys

We now would have a component which could easily be used for the feedback, submission and flows pages with no extra code or unit tests required.

Right now, it would have a static UI (this is actually fine) but a future enhancement could to make it a bit more headless or to have variants (e.g. "compact")

Super happy to talk this through tomorrow if you'd like 👍

DafyddLlyr · 2025-01-13T16:22:22Z

editor.planx.uk/src/ui/editor/SortFlowsSelect.tsx

+import TrendingDownIcon from "@mui/icons-material/TrendingDown";
+import TrendingUpIcon from "@mui/icons-material/TrendingUp";


I actually don't associate these icons with sorting, I'd expect something more like a caret or just an "asc" / "dec" label somewhere (e.g. another section in the dropdown).

Ah yeah! This was mostly for Ian to see it working and then we'll think about how to add it to the sort. Good feedback to have in tho

DafyddLlyr · 2025-01-13T16:25:55Z

editor.planx.uk/src/ui/editor/SortFlowsSelect.tsx

+  const addToSearchParams = (sortKey: SortTypes) => {
+    navigation.navigate(
+      {
+        pathname: window.location.pathname,
+        search: `?sort=${sortKey}`,
+      },
+      {
+        replace: true,
+      },
+    );
+  };


Adding this to the URL is great to see 👍

Two thoughts -

We should add direction also (now)

This could be a great hook once we hit a second use case (useSearchParamState()?) (future)

Just seen this is listed still a to do on the PR description 👌

editor.planx.uk/src/ui/editor/SortFlowsSelect.tsx

DafyddLlyr · 2025-01-13T16:43:32Z

editor.planx.uk/src/ui/editor/SortFlowsSelect.tsx

+
+type SortDirection = "asc" | "desc";
+type SortKeys = keyof FlowSummary;
+type SortNestedKeys = keyof PublishedFlowSummary | keyof FlowSummaryOperations;


I think a lot of the complexity in this component is coming from the fact we have a slightly tricky object to work with initially returned by our GetFlows query.

We can't totally unnest / flatten natively via a GraphQL query, but don't be shy about transforming a database object to a more appropriate model for use in the frontend.

For example, if our FlowSummary had properties publishedAt or status, we can simplify a whole lot of things in this component.

Alternatively, we can use key paths as assessors - type-fest and lodash can really help here.

For example, we could use "publishedFlows[0].publishedAt" as a key using lodash's get()

This was a great one and I did have to add type-fest to the editor to get the method

I think we've had it in and out a few times as a dependency, we also use it in the API and I think planx-core.

DafyddLlyr · 2025-01-13T16:49:36Z

editor.planx.uk/src/ui/editor/SortFlowsSelect.tsx

+  flows: FlowSummary[];
+  setFlows: React.Dispatch<React.SetStateAction<FlowSummary[] | null>>;
+}) => {
+  const [sortBy, setSortBy] = useState<SortObject>(sortArray[0]);


Naming here is a little tricky to understand - it's not super easy to work out at glance if this an object or array?

Often we can leverage the type system to avoid pseudo-Hungarian notation for variables.

A simpler approach may be to just store the sortKey as a string and then maintain a map/lookup object for reference when we need display values.

I've taken the approach of slugifying the displayName and using that as a lookup key, since I had to keep the sortKey as part of an object connected with the name we use for displaying it in the select.

There's a chance here that I've Wiley Coyote'd myself and ran off a cliff when the answer was much simpler, but it met what I wanted in terms of how you can define your sort options and how they are displayed in one place, and the component handles the logic of sorting / finding them.

DafyddLlyr · 2025-01-13T16:53:01Z

editor.planx.uk/src/ui/editor/SortFlowsSelect.tsx

+        if (!b[sortKey][0]) return -1;
+
+        const aValue = a[sortKey][0][nestedKey];
+        const bValue = b[sortKey][0][nestedKey];


Thanks for the the insightful comment on how you get here 👍 This makes things easier to review and understand - much appreciated!

First off - a useEffect() seems fine for this in order to maintain that reactivity - no real issues here.

The logic here is pretty complex and dense - I think removing the entire concept of nestedKeys (see previous comments) would help out a lot here as there's just a single level to deal with.

editor.planx.uk/src/ui/editor/SortFlowsSelect.tsx

RODO94 · 2025-01-15T14:50:36Z

editor.planx.uk/src/pages/Team.tsx

+        <SortControl<FlowSummary>
+          records={flows}
+          setRecords={setFlows}
+          sortOptions={sortArray}


Switched naming to something more generic and reusable across the comp and remove refs to flow in the naming as well

RODO94 · 2025-01-15T14:52:41Z

editor.planx.uk/src/ui/editor/SortControl.tsx

+
+  const navigation = useNavigation();
+  const route = useCurrentRoute();
+  const selectedDisplaySlug = slugify(selectedSort.displayName);


This is probably the part I feel most unsure about, but I'm comfortable that it delivers a unifying parameter I can use to search for sortOptions while adding it to the search params in a nice way for the user to see and read. My issue lay in the lack of readability in the key naming for deeply nested keys, and the fact displayName could really contain any sort of spaces and the like.

Yeah it's a good question - fieldName is guaranteed to be unique, but not necessarily readable.

There's no guarantee that slugify(selectedSort.displayName) doesn't return duplicates through right? It's unlikely but possible.

This might just be a data structure question. If we were to make sortOptions something like this our problems go away -

sortOptions: Record<string, SortableFields<T>> ... const sortArray: Record<string, SortableFields<FlowSummary>> = { name: { displayName: "Name", fieldName: "name" }, lastUpdated: { displayName: "Last updated", fieldName: "updatedAt" }, lastPublished: { displayName: "Last published", fieldName: `publishedFlows.0.publishedAt`, }, }

This would -

Force a unique key

Provide us with a map already

What do you think?

Yeah this was an iteration I considered, I was thinking this is dealing with a complexity which is within the component (so not obvious when you add it to your table comp) so it is better to handle it within the component?

Imagining someone building this into their component and being confused why you have to add so many names, ultimately the same thing three times.

I can see the risk of duplicates but thought it was quite edge casey and resolvable.

But, I'm happy to integrate the change, I can see the benefits of the safety, so you don't need to try find your error in the naming.

Yeah this is a fair point - lets aim to keep the complexity within the component I think 👍

Your version is clearer and simpler for consumers. The risk of duplicates is very very low!

editor.planx.uk/src/ui/editor/SortControl.tsx

editor.planx.uk/src/pages/FlowEditor/lib/store/editor.ts

DafyddLlyr · 2025-01-15T15:09:51Z

editor.planx.uk/src/pages/Team.tsx

@@ -351,6 +364,13 @@ const Team: React.FC = () => {
        </Box>
        {showAddFlowButton && <AddFlowButton flows={flows} />}
      </Box>
+      {hasFeatureFlag("SORT_FLOWS") && flows && (


We can maybe drop the flows condition by dropping this into the next block which is already checking the more accurate teamHasFlows.

I'll add this in when I combine it with the Filters block

DafyddLlyr · 2025-01-15T15:15:53Z

editor.planx.uk/src/ui/editor/SortControl.tsx

+
+  const navigation = useNavigation();
+  const route = useCurrentRoute();
+  const selectedDisplaySlug = slugify(selectedSort.displayName);


Yeah it's a good question - fieldName is guaranteed to be unique, but not necessarily readable.

There's no guarantee that slugify(selectedSort.displayName) doesn't return duplicates through right? It's unlikely but possible.

This might just be a data structure question. If we were to make sortOptions something like this our problems go away -

sortOptions: Record<string, SortableFields<T>> ... const sortArray: Record<string, SortableFields<FlowSummary>> = { name: { displayName: "Name", fieldName: "name" }, lastUpdated: { displayName: "Last updated", fieldName: "updatedAt" }, lastPublished: { displayName: "Last published", fieldName: `publishedFlows.0.publishedAt`, }, }

This would -

Force a unique key

Provide us with a map already

What do you think?

editor.planx.uk/src/pages/Team.tsx

editor.planx.uk/src/ui/editor/SortControl.tsx

refine component and sort style style refinement on layout add status to getFlows refine type use on sort keys

DafyddLlyr

Thanks - changes here look great, and we've ended up with a solution which is much for robust and expandable - love to see it!

Not manually tested due to failing CI (due to cache issue - please run gh cache delete --all and then re-run CI).

DafyddLlyr · 2025-01-17T13:03:47Z

editor.planx.uk/src/pages/Team.tsx

+    {
+      displayName: "Name",
+      fieldName: "name",
+      directionNames: { asc: "A - Z", desc: "Z - A" },


Nice! When we have more use cases for this we may find we want these display values centralised within the component, and just allow consumers to define by type (e.g. direction: "alphabetical" | "date"). This is perfect for now 👍

RODO94 · 2025-01-17T13:22:10Z

@DafyddLlyr manual testing is probably best with the filters included

RODO94 force-pushed the rory/order-toggle-button branch from f235e5b to 73f0b7c Compare January 10, 2025 16:13

RODO94 commented Jan 10, 2025

View reviewed changes

RODO94 marked this pull request as ready for review January 13, 2025 11:29

RODO94 requested a review from a team January 13, 2025 11:29

DafyddLlyr requested changes Jan 13, 2025

View reviewed changes

jessicamcinchak reviewed Jan 14, 2025

View reviewed changes

editor.planx.uk/src/ui/editor/SortFlowsSelect.tsx Outdated Show resolved Hide resolved

RODO94 force-pushed the rory/order-toggle-button branch from f235879 to 9b7aa3f Compare January 14, 2025 14:54

RODO94 commented Jan 15, 2025

View reviewed changes

editor.planx.uk/src/ui/editor/SortControl.tsx Show resolved Hide resolved

RODO94 requested a review from DafyddLlyr January 15, 2025 14:54

DafyddLlyr reviewed Jan 15, 2025

View reviewed changes

DafyddLlyr assigned RODO94 Jan 17, 2025

RODO94 added 3 commits January 17, 2025 11:42

initial work on ordering

0d896fc

refine component and sort style style refinement on layout add status to getFlows refine type use on sort keys

add search params to sort

7081351

add changes and type-fest to editor

8c48a5b

RODO94 force-pushed the rory/order-toggle-button branch from 5587d15 to 8c48a5b Compare January 17, 2025 11:43

add help description and new sort for direciton

90e8b19

DafyddLlyr approved these changes Jan 17, 2025

View reviewed changes

RODO94 merged commit 9cde7ae into main Jan 17, 2025
12 checks passed

RODO94 deleted the rory/order-toggle-button branch January 17, 2025 13:23

RODO94 mentioned this pull request Jan 22, 2025

feat: add filters to Team page #4176

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add select for sorting flow list #4137

feat: add select for sorting flow list #4137

RODO94 commented Jan 10, 2025 •

edited

Loading

RODO94 Jan 10, 2025

RODO94 Jan 10, 2025

RODO94 Jan 10, 2025 •

edited

Loading

DafyddLlyr Jan 13, 2025

github-actions bot commented Jan 10, 2025 •

edited

Loading

DafyddLlyr left a comment

DafyddLlyr Jan 13, 2025

RODO94 Jan 13, 2025

DafyddLlyr Jan 13, 2025

DafyddLlyr Jan 13, 2025

DafyddLlyr Jan 13, 2025

DafyddLlyr Jan 13, 2025 •

edited

Loading

RODO94 Jan 15, 2025

DafyddLlyr Jan 15, 2025

DafyddLlyr Jan 13, 2025

RODO94 Jan 15, 2025

DafyddLlyr Jan 13, 2025

RODO94 Jan 15, 2025

RODO94 Jan 15, 2025

DafyddLlyr Jan 15, 2025

RODO94 Jan 15, 2025

DafyddLlyr Jan 16, 2025

DafyddLlyr Jan 15, 2025

RODO94 Jan 17, 2025

DafyddLlyr Jan 15, 2025

DafyddLlyr left a comment

DafyddLlyr Jan 17, 2025

RODO94 commented Jan 17, 2025

		import TrendingDownIcon from "@mui/icons-material/TrendingDown";
		import TrendingUpIcon from "@mui/icons-material/TrendingUp";

feat: add select for sorting flow list #4137

feat: add select for sorting flow list #4137

Conversation

RODO94 commented Jan 10, 2025 • edited Loading

What does this PR do?

Approach

What's included?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RODO94 Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 10, 2025 • edited Loading

DafyddLlyr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DafyddLlyr Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DafyddLlyr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RODO94 commented Jan 17, 2025

RODO94 commented Jan 10, 2025 •

edited

Loading

RODO94 Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

DafyddLlyr Jan 13, 2025 •

edited

Loading