diff --git a/app/components/Sidebar/components/DocumentLink.tsx b/app/components/Sidebar/components/DocumentLink.tsx index a68afb20f..21f7120b2 100644 --- a/app/components/Sidebar/components/DocumentLink.tsx +++ b/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, diff --git a/app/components/Sidebar/components/useCollectionDocuments.ts b/app/components/Sidebar/components/useCollectionDocuments.ts index d3b7e7aa8..42af1eedf 100644 --- a/app/components/Sidebar/components/useCollectionDocuments.ts +++ b/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, ]); diff --git a/app/models/Collection.ts b/app/models/Collection.ts index 65bba03c7..09b50c65d 100644 --- a/app/models/Collection.ts +++ b/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; diff --git a/app/scenes/Collection.tsx b/app/scenes/Collection.tsx index d4242e06f..2c81ea2f1 100644 --- a/app/scenes/Collection.tsx +++ b/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 /> diff --git a/app/stores/DocumentsStore.ts b/app/stores/DocumentsStore.ts index f224c4126..75bf6846e 100644 --- a/app/stores/DocumentsStore.ts +++ b/app/stores/DocumentsStore.ts @@ -147,7 +147,7 @@ export default class DocumentsStore extends BaseStore { 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 { }; @action - fetchArchived = async (options?: PaginationParams): Promise => { + fetchArchived = async (options?: PaginationParams): Promise => { return this.fetchNamedPage("archived", options); }; @action - fetchDeleted = async (options?: PaginationParams): Promise => { + fetchDeleted = async (options?: PaginationParams): Promise => { return this.fetchNamedPage("deleted", options); }; @action - fetchRecentlyUpdated = async (options?: PaginationParams): Promise => { + fetchRecentlyUpdated = async ( + options?: PaginationParams + ): Promise => { return this.fetchNamedPage("list", options); }; @action - fetchTemplates = async (options?: PaginationParams): Promise => { + fetchTemplates = async (options?: PaginationParams): Promise => { return this.fetchNamedPage("list", { ...options, template: true }); }; @action - fetchAlphabetical = async (options?: PaginationParams): Promise => { + fetchAlphabetical = async ( + options?: PaginationParams + ): Promise => { return this.fetchNamedPage("list", { sort: "title", direction: "ASC", @@ -334,7 +338,7 @@ export default class DocumentsStore extends BaseStore { @action fetchLeastRecentlyUpdated = async ( options?: PaginationParams - ): Promise => { + ): Promise => { return this.fetchNamedPage("list", { sort: "updatedAt", direction: "ASC", @@ -343,7 +347,9 @@ export default class DocumentsStore extends BaseStore { }; @action - fetchRecentlyPublished = async (options?: PaginationParams): Promise => { + fetchRecentlyPublished = async ( + options?: PaginationParams + ): Promise => { return this.fetchNamedPage("list", { sort: "publishedAt", direction: "DESC", @@ -352,22 +358,24 @@ export default class DocumentsStore extends BaseStore { }; @action - fetchRecentlyViewed = async (options?: PaginationParams): Promise => { + fetchRecentlyViewed = async ( + options?: PaginationParams + ): Promise => { return this.fetchNamedPage("viewed", options); }; @action - fetchStarred = (options?: PaginationParams): Promise => { + fetchStarred = (options?: PaginationParams): Promise => { return this.fetchNamedPage("starred", options); }; @action - fetchDrafts = (options?: PaginationParams): Promise => { + fetchDrafts = (options?: PaginationParams): Promise => { return this.fetchNamedPage("drafts", options); }; @action - fetchOwned = (options?: PaginationParams): Promise => { + fetchOwned = (options?: PaginationParams): Promise => { return this.fetchNamedPage("list", options); }; diff --git a/server/migrations/20220430043135-collection-sort-backfill.js b/server/migrations/20220430043135-collection-sort-backfill.js new file mode 100644 index 000000000..8b18a8fc3 --- /dev/null +++ b/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 + } +}; diff --git a/server/models/Collection.ts b/server/models/Collection.ts index ebab9b59a..0679782d4 100644 --- a/server/models/Collection.ts +++ b/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), }; }; diff --git a/server/presenters/collection.ts b/server/presenters/collection.ts index df65d602f..3c646cc37 100644 --- a/server/presenters/collection.ts +++ b/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({ diff --git a/shared/utils/collections.ts b/shared/utils/collections.ts index 38ea4379a..16be536b7 100644 --- a/shared/utils/collections.ts +++ b/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, })); };