perf: Move collection sorting to frontend (#3475)

* perf: Move collection sorting to frontend, on demand, memoized

* fix: Add default

app/components/Sidebar/components/DocumentLink.tsx

@@ -258,19 +258,18 @@ function InnerDocumentLink(
   });
 
   const nodeChildren = React.useMemo(() => {
-    if (
-      collection &&
+    const insertDraftDocument =
       activeDocument?.isDraft &&
       activeDocument?.isActive &&
-      activeDocument?.parentDocumentId === node.id
-    ) {
-      return sortNavigationNodes(
-        [activeDocument?.asNavigationNode, ...node.children],
-        collection.sort
-      );
-    }
+      activeDocument?.parentDocumentId === node.id;
 
-    return node.children;
+    return collection && insertDraftDocument
+      ? sortNavigationNodes(
+          [activeDocument?.asNavigationNode, ...node.children],
+          collection.sort,
+          false
+        )
+      : node.children;
   }, [
     activeDocument?.isActive,
     activeDocument?.isDraft,

app/components/Sidebar/components/useCollectionDocuments.ts

@@ -12,19 +12,19 @@ export default function useCollectionDocuments(
       return [];
     }
 
-    if (
+    const insertDraftDocument =
       activeDocument?.isActive &&
       activeDocument?.isDraft &&
       activeDocument?.collectionId === collection.id &&
-      !activeDocument?.parentDocumentId
-    ) {
-      return sortNavigationNodes(
-        [activeDocument.asNavigationNode, ...collection.documents],
-        collection.sort
-      );
-    }
+      !activeDocument?.parentDocumentId;
 
-    return collection.documents;
+    return insertDraftDocument
+      ? sortNavigationNodes(
+          [activeDocument.asNavigationNode, ...collection.sortedDocuments],
+          collection.sort,
+          false
+        )
+      : collection.sortedDocuments;
   }, [
     activeDocument?.isActive,
     activeDocument?.isDraft,
@@ -32,7 +32,7 @@ export default function useCollectionDocuments(
     activeDocument?.parentDocumentId,
     activeDocument?.asNavigationNode,
     collection,
-    collection?.documents,
+    collection?.sortedDocuments,
     collection?.id,
     collection?.sort,
   ]);

app/models/Collection.ts

@@ -1,5 +1,6 @@
 import { trim } from "lodash";
 import { action, computed, observable } from "mobx";
+import { sortNavigationNodes } from "@shared/utils/collections";
 import CollectionsStore from "~/stores/CollectionsStore";
 import Document from "~/models/Document";
 import ParanoidModel from "~/models/ParanoidModel";
@@ -95,6 +96,11 @@ export default class Collection extends ParanoidModel {
     );
   }
 
+  @computed
+  get sortedDocuments() {
+    return sortNavigationNodes(this.documents, this.sort);
+  }
+
   @action
   updateDocument(document: Document) {
     const travelNodes = (nodes: NavigationNode[]) =>
@@ -130,7 +136,7 @@ export default class Collection extends ParanoidModel {
     };
 
     if (this.documents) {
-      travelNodes(this.documents);
+      travelNodes(this.sortedDocuments);
     }
 
     return result;

app/scenes/Collection.tsx

@@ -230,7 +230,7 @@ function CollectionScene() {
                       collectionId: collection.id,
                       parentDocumentId: null,
                       sort: collection.sort.field,
-                      direction: "ASC",
+                      direction: collection.sort.direction,
                     }}
                     showParentDocuments
                   />

app/stores/DocumentsStore.ts

@@ -147,7 +147,7 @@ export default class DocumentsStore extends BaseStore<Document> {
 
     return compact([
       ...drafts,
-      ...collection.documents.map((node) => this.get(node.id)),
+      ...collection.sortedDocuments.map((node) => this.get(node.id)),
     ]);
   }
 
@@ -303,27 +303,31 @@ export default class DocumentsStore extends BaseStore<Document> {
   };
 
   @action
-  fetchArchived = async (options?: PaginationParams): Promise<any> => {
+  fetchArchived = async (options?: PaginationParams): Promise<Document[]> => {
     return this.fetchNamedPage("archived", options);
   };
 
   @action
-  fetchDeleted = async (options?: PaginationParams): Promise<any> => {
+  fetchDeleted = async (options?: PaginationParams): Promise<Document[]> => {
     return this.fetchNamedPage("deleted", options);
   };
 
   @action
-  fetchRecentlyUpdated = async (options?: PaginationParams): Promise<any> => {
+  fetchRecentlyUpdated = async (
+    options?: PaginationParams
+  ): Promise<Document[]> => {
     return this.fetchNamedPage("list", options);
   };
 
   @action
-  fetchTemplates = async (options?: PaginationParams): Promise<any> => {
+  fetchTemplates = async (options?: PaginationParams): Promise<Document[]> => {
     return this.fetchNamedPage("list", { ...options, template: true });
   };
 
   @action
-  fetchAlphabetical = async (options?: PaginationParams): Promise<any> => {
+  fetchAlphabetical = async (
+    options?: PaginationParams
+  ): Promise<Document[]> => {
     return this.fetchNamedPage("list", {
       sort: "title",
       direction: "ASC",
@@ -334,7 +338,7 @@ export default class DocumentsStore extends BaseStore<Document> {
   @action
   fetchLeastRecentlyUpdated = async (
     options?: PaginationParams
-  ): Promise<any> => {
+  ): Promise<Document[]> => {
     return this.fetchNamedPage("list", {
       sort: "updatedAt",
       direction: "ASC",
@@ -343,7 +347,9 @@ export default class DocumentsStore extends BaseStore<Document> {
   };
 
   @action
-  fetchRecentlyPublished = async (options?: PaginationParams): Promise<any> => {
+  fetchRecentlyPublished = async (
+    options?: PaginationParams
+  ): Promise<Document[]> => {
     return this.fetchNamedPage("list", {
       sort: "publishedAt",
       direction: "DESC",
@@ -352,22 +358,24 @@ export default class DocumentsStore extends BaseStore<Document> {
   };
 
   @action
-  fetchRecentlyViewed = async (options?: PaginationParams): Promise<any> => {
+  fetchRecentlyViewed = async (
+    options?: PaginationParams
+  ): Promise<Document[]> => {
     return this.fetchNamedPage("viewed", options);
   };
 
   @action
-  fetchStarred = (options?: PaginationParams): Promise<any> => {
+  fetchStarred = (options?: PaginationParams): Promise<Document[]> => {
     return this.fetchNamedPage("starred", options);
   };
 
   @action
-  fetchDrafts = (options?: PaginationParams): Promise<any> => {
+  fetchDrafts = (options?: PaginationParams): Promise<Document[]> => {
     return this.fetchNamedPage("drafts", options);
   };
 
   @action
-  fetchOwned = (options?: PaginationParams): Promise<any> => {
+  fetchOwned = (options?: PaginationParams): Promise<Document[]> => {
     return this.fetchNamedPage("list", options);
   };
 

server/migrations/20220430043135-collection-sort-backfill.js

@@ -0,0 +1,29 @@
+'use strict';
+
+module.exports = {
+  up: async (queryInterface) => {
+    let again = 1;
+  
+    while (again) {
+      console.log("Backfilling collection sort…");
+      const [, metadata] = await queryInterface.sequelize.query(`
+WITH rows AS (
+  SELECT id FROM collections WHERE "sort" IS NULL ORDER BY id LIMIT 1000
+)
+UPDATE collections
+SET "sort" = :sort::jsonb
+WHERE EXISTS (SELECT * FROM rows WHERE collections.id = rows.id)
+  `, {
+        replacements: {
+          sort: JSON.stringify({ field: "title", direction: "asc" }),
+        }
+      });
+
+      again = metadata.rowCount;
+    }
+  },
+
+  down: async () => {
+    // cannot be undone
+  }
+};

server/models/Collection.ts

@@ -161,6 +161,7 @@ class Collection extends ParanoidModel {
   @Column
   sharing: boolean;
 
+  @Default({ field: "title", direction: "asc" })
   @Column({
     type: DataType.JSONB,
     validate: {
@@ -184,7 +185,7 @@ class Collection extends ParanoidModel {
       },
     },
   })
-  sort: Sort | null;
+  sort: Sort;
 
   // getters
 
@@ -362,10 +363,6 @@ class Collection extends ParanoidModel {
     if (!this.documentStructure) {
       return null;
     }
-    const sort: Sort = this.sort || {
-      field: "title",
-      direction: "asc",
-    };
 
     let result!: NavigationNode | undefined;
 
@@ -399,7 +396,7 @@ class Collection extends ParanoidModel {
 
     return {
       ...result,
-      children: sortNavigationNodes(result.children, sort),
+      children: sortNavigationNodes(result.children, this.sort),
     };
   };
 

server/presenters/collection.ts

@@ -1,9 +1,8 @@
-import { sortNavigationNodes } from "@shared/utils/collections";
 import { APM } from "@server/logging/tracing";
 import Collection from "@server/models/Collection";
 
 function present(collection: Collection) {
-  const data = {
+  return {
     id: collection.id,
     url: collection.url,
     urlId: collection.urlId,
@@ -20,21 +19,6 @@ function present(collection: Collection) {
     deletedAt: collection.deletedAt,
     documents: collection.documentStructure,
   };
-
-  // Handle the "sort" field being empty here for backwards compatability
-  if (!data.sort) {
-    data.sort = {
-      field: "title",
-      direction: "asc",
-    };
-  }
-
-  data.documents = sortNavigationNodes(
-    collection.documentStructure || [],
-    data.sort
-  );
-
-  return data;
 }
 
 export default APM.traceFunction({

shared/utils/collections.ts

@@ -8,7 +8,8 @@ type Sort = {
 
 export const sortNavigationNodes = (
   documents: NavigationNode[],
-  sort: Sort
+  sort: Sort,
+  sortChildren = true
 ): NavigationNode[] => {
   // "index" field is manually sorted and is represented by the documentStructure
   // already saved in the database, no further sort is needed
@@ -22,6 +23,8 @@ export const sortNavigationNodes = (
 
   return orderedDocs.map((document) => ({
     ...document,
-    children: sortNavigationNodes(document.children, sort),
+    children: sortChildren
+      ? sortNavigationNodes(document.children, sort, sortChildren)
+      : document.children,
   }));
 };