From cbca7f60fedf2ca5d23ff52021febae11f5c44b1 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 19 Feb 2025 07:44:15 -0500 Subject: [PATCH] Move document history to `revisions.list` API (#8497) * Revert "Revert "Move document history to `revisions.list` API (#8458)" (#8495)" This reverts commit 2116041cd5776c8d3b884fdf1e22d9297c09a304. * fix: check all events for latest ad-hoc revision * view revision list for deleted docs * rename --------- Co-authored-by: hmacr --- app/components/EventListItem.tsx | 60 +++++-- app/components/PaginatedEventList.tsx | 18 +-- app/components/PaginatedList.tsx | 11 +- app/scenes/Document/components/History.tsx | 177 ++++++++++++++++----- app/stores/RevisionsStore.ts | 2 +- server/routes/api/events/events.test.ts | 39 +++++ server/routes/api/events/events.ts | 39 +++-- server/routes/api/events/schema.ts | 15 +- server/routes/api/revisions/revisions.ts | 1 + shared/utils/EventHelper.ts | 8 +- 10 files changed, 284 insertions(+), 86 deletions(-) diff --git a/app/components/EventListItem.tsx b/app/components/EventListItem.tsx index 4c54151e2a..e882693a6e 100644 --- a/app/components/EventListItem.tsx +++ b/app/components/EventListItem.tsx @@ -14,8 +14,9 @@ import { useLocation } from "react-router-dom"; import styled, { css } from "styled-components"; import EventBoundary from "@shared/components/EventBoundary"; import { s, hover } from "@shared/styles"; +import { RevisionHelper } from "@shared/utils/RevisionHelper"; import Document from "~/models/Document"; -import Event from "~/models/Event"; +import User from "~/models/User"; import { Avatar, AvatarSize } from "~/components/Avatar"; import Item, { Actions, Props as ItemProps } from "~/components/List/Item"; import Time from "~/components/Time"; @@ -25,21 +26,47 @@ import RevisionMenu from "~/menus/RevisionMenu"; import Logger from "~/utils/Logger"; import { documentHistoryPath } from "~/utils/routeHelpers"; -type Props = { - document: Document; - event: Event; - latest?: boolean; +export type RevisionEvent = { + name: "revisions.create"; + latest: boolean; }; -const EventListItem = ({ event, latest, document, ...rest }: Props) => { +export type DocumentEvent = { + name: + | "documents.publish" + | "documents.unpublish" + | "documents.archive" + | "documents.unarchive" + | "documents.delete" + | "documents.restore" + | "documents.add_user" + | "documents.remove_user" + | "documents.move"; + user?: User; +}; + +export type Event = { id: string; actor: User; createdAt: string } & ( + | RevisionEvent + | DocumentEvent +); + +type Props = { + document: Document; + event: Event; +}; + +const EventListItem = ({ event, document, ...rest }: Props) => { const { t } = useTranslation(); const { revisions } = useStores(); const location = useLocation(); const sidebarContext = useLocationSidebarContext(); + const revisionLoadedRef = React.useRef(false); const opts = { userName: event.actor.name, }; const isRevision = event.name === "revisions.create"; + const isDerivedFromDocument = + event.id === RevisionHelper.latestId(document.id); let meta, icon, to: LocationDescriptor | undefined; const ref = React.useRef(null); @@ -50,15 +77,21 @@ const EventListItem = ({ event, latest, document, ...rest }: Props) => { }; const prefetchRevision = async () => { - if (event.name === "revisions.create" && event.modelId) { - await revisions.fetch(event.modelId); + if ( + !document.isDeleted && + event.name === "revisions.create" && + !isDerivedFromDocument && + !revisionLoadedRef.current + ) { + await revisions.fetch(event.id, { force: true }); + revisionLoadedRef.current = true; } }; switch (event.name) { case "revisions.create": icon = ; - meta = latest ? ( + meta = event.latest ? ( <> {t("Current version")} · {event.actor.name} @@ -66,7 +99,10 @@ const EventListItem = ({ event, latest, document, ...rest }: Props) => { t("{{userName}} edited", opts) ); to = { - pathname: documentHistoryPath(document, event.modelId || "latest"), + pathname: documentHistoryPath( + document, + isDerivedFromDocument ? "latest" : event.id + ), state: { sidebarContext, retainScrollPosition: true, @@ -161,9 +197,9 @@ const EventListItem = ({ event, latest, document, ...rest }: Props) => { } actions={ - isRevision && isActive && event.modelId && !latest ? ( + isRevision && isActive && !event.latest ? ( - + ) : undefined } diff --git a/app/components/PaginatedEventList.tsx b/app/components/PaginatedEventList.tsx index ebfbe5e8fc..c7a5bdc025 100644 --- a/app/components/PaginatedEventList.tsx +++ b/app/components/PaginatedEventList.tsx @@ -1,16 +1,13 @@ import * as React from "react"; import styled from "styled-components"; import Document from "~/models/Document"; -import Event from "~/models/Event"; import PaginatedList from "~/components/PaginatedList"; -import EventListItem from "./EventListItem"; +import EventListItem, { type Event } from "./EventListItem"; type Props = { - events: Event[]; + events: Event[]; document: Document; - fetch: ( - options: Record | undefined - ) => Promise[]>; + fetch: (options: Record | undefined) => Promise; options?: Record; heading?: React.ReactNode; empty?: React.ReactNode; @@ -32,13 +29,8 @@ const PaginatedEventList = React.memo(function PaginatedEventList({ heading={heading} fetch={fetch} options={options} - renderItem={(item: Event, index) => ( - + renderItem={(item: Event) => ( + )} renderHeading={(name) => {name}} {...rest} diff --git a/app/components/PaginatedList.tsx b/app/components/PaginatedList.tsx index 1784b2177f..ea7b9f86eb 100644 --- a/app/components/PaginatedList.tsx +++ b/app/components/PaginatedList.tsx @@ -60,7 +60,7 @@ class PaginatedList extends React.PureComponent< fetchCounter = 0; @observable - renderCount = 15; + renderCount = Pagination.defaultLimit; @observable offset = 0; @@ -108,13 +108,16 @@ class PaginatedList extends React.PureComponent< ...this.props.options, }); + if (this.offset !== 0) { + this.renderCount += limit; + } + if (results && (results.length === 0 || results.length < limit)) { this.allowLoadMore = false; } else { this.offset += limit; } - this.renderCount += limit; this.isFetchingInitial = false; } catch (err) { this.error = err; @@ -248,7 +251,9 @@ class PaginatedList extends React.PureComponent< }} {this.allowLoadMore && ( - +
+ +
)} ); diff --git a/app/scenes/Document/components/History.tsx b/app/scenes/Document/components/History.tsx index caf5a3d258..7e77d9de5a 100644 --- a/app/scenes/Document/components/History.tsx +++ b/app/scenes/Document/components/History.tsx @@ -1,12 +1,16 @@ +import orderBy from "lodash/orderBy"; import { observer } from "mobx-react"; import * as React from "react"; import { useTranslation } from "react-i18next"; import { useHistory, useRouteMatch } from "react-router-dom"; import styled from "styled-components"; +import { Pagination } from "@shared/constants"; import { RevisionHelper } from "@shared/utils/RevisionHelper"; import Document from "~/models/Document"; -import Event from "~/models/Event"; +import EventModel from "~/models/Event"; +import Revision from "~/models/Revision"; import Empty from "~/components/Empty"; +import { DocumentEvent, type Event } from "~/components/EventListItem"; import PaginatedEventList from "~/components/PaginatedEventList"; import useKeyDown from "~/hooks/useKeyDown"; import { useLocationSidebarContext } from "~/hooks/useLocationSidebarContext"; @@ -14,21 +18,148 @@ import useStores from "~/hooks/useStores"; import { documentPath } from "~/utils/routeHelpers"; import Sidebar from "./SidebarLayout"; -const EMPTY_ARRAY: Event[] = []; +const DocumentEvents = [ + "documents.publish", + "documents.unpublish", + "documents.archive", + "documents.unarchive", + "documents.delete", + "documents.restore", + "documents.add_user", + "documents.remove_user", + "documents.move", +]; function History() { - const { events, documents } = useStores(); + const { events, documents, revisions } = useStores(); const { t } = useTranslation(); const match = useRouteMatch<{ documentSlug: string }>(); const history = useHistory(); const sidebarContext = useLocationSidebarContext(); const document = documents.getByUrl(match.params.documentSlug); - const eventsInDocument = document - ? events.filter({ documentId: document.id }) - : EMPTY_ARRAY; + const [, setForceRender] = React.useState(0); + const offset = React.useMemo(() => ({ revisions: 0, events: 0 }), []); - const onCloseHistory = () => { + const toEvent = React.useCallback( + (data: Revision | EventModel): Event => { + if (data instanceof Revision) { + return { + id: data.id, + name: "revisions.create", + actor: data.createdBy, + createdAt: data.createdAt, + latest: false, + } satisfies Event; + } + + return { + id: data.id, + name: data.name as DocumentEvent["name"], + actor: data.actor, + user: data.user, + createdAt: data.createdAt, + } satisfies Event; + }, + [] + ); + + const fetchHistory = React.useCallback(async () => { + if (!document) { + return []; + } + + const [revisionsArr, eventsArr] = await Promise.all([ + revisions.fetchPage({ + documentId: document.id, + offset: offset.revisions, + limit: Pagination.defaultLimit, + }), + events.fetchPage({ + events: DocumentEvents, + documentId: document.id, + offset: offset.events, + limit: Pagination.defaultLimit, + }), + ]); + + const pageEvents = orderBy( + [...revisionsArr, ...eventsArr].map(toEvent), + "createdAt", + "desc" + ).slice(0, Pagination.defaultLimit); + + const revisionsCount = pageEvents.filter( + (event) => event.name === "revisions.create" + ).length; + + offset.revisions += revisionsCount; + offset.events += pageEvents.length - revisionsCount; + + // needed to re-render after mobx store and offset is updated + setForceRender((s) => ++s); + + return pageEvents; + }, [document, revisions, events, toEvent, offset]); + + const revisionEvents = React.useMemo(() => { + if (!document) { + return []; + } + + const latestRevisionId = RevisionHelper.latestId(document.id); + return revisions + .filter( + (revision: Revision) => + revision.id !== latestRevisionId && + revision.documentId === document.id + ) + .slice(0, offset.revisions) + .map(toEvent); + }, [document, revisions, offset.revisions, toEvent]); + + const nonRevisionEvents = React.useMemo( + () => + document + ? events + .filter({ documentId: document.id }) + .slice(0, offset.events) + .map(toEvent) + : [], + [document, events, offset.events, toEvent] + ); + + const mergedEvents = React.useMemo(() => { + const merged = orderBy( + [...revisionEvents, ...nonRevisionEvents], + "createdAt", + "desc" + ); + + const latestEvent = merged[0]; + + if (latestEvent && document) { + const latestRevisionEvent = merged.find( + (event) => event.name === "revisions.create" + ); + + if (latestEvent.createdAt !== document.updatedAt) { + merged.unshift({ + id: RevisionHelper.latestId(document.id), + name: "revisions.create", + createdAt: document.updatedAt, + actor: document.updatedBy!, + latest: true, + }); + } else if (latestRevisionEvent) { + latestRevisionEvent.latest = true; + } + } + + return merged; + }, [document, revisionEvents, nonRevisionEvents]); + + const onCloseHistory = React.useCallback(() => { if (document) { history.push({ pathname: documentPath(document), @@ -37,30 +168,7 @@ function History() { } else { history.goBack(); } - }; - - const items = React.useMemo(() => { - if ( - eventsInDocument[0] && - document && - eventsInDocument[0].createdAt !== document.updatedAt - ) { - eventsInDocument.unshift( - new Event( - { - id: RevisionHelper.latestId(document.id), - name: "revisions.create", - documentId: document.id, - createdAt: document.updatedAt, - actor: document.updatedBy, - }, - events - ) - ); - } - - return eventsInDocument; - }, [eventsInDocument, events, document]); + }, [history, document, sidebarContext]); useKeyDown("Escape", onCloseHistory); @@ -69,11 +177,8 @@ function History() { {document ? ( {t("No history yet")}} /> diff --git a/app/stores/RevisionsStore.ts b/app/stores/RevisionsStore.ts index 9e6f07eb21..cbf6768352 100644 --- a/app/stores/RevisionsStore.ts +++ b/app/stores/RevisionsStore.ts @@ -58,7 +58,7 @@ export default class RevisionsStore extends Store { @action fetchPage = async ( - options: PaginationParams | undefined + options: { documentId: string } & (PaginationParams | undefined) ): Promise => { this.isFetching = true; diff --git a/server/routes/api/events/events.test.ts b/server/routes/api/events/events.test.ts index 3896fc6f5e..d845e162d3 100644 --- a/server/routes/api/events/events.test.ts +++ b/server/routes/api/events/events.test.ts @@ -228,6 +228,45 @@ describe("#events.list", () => { expect(body.data[0].id).toEqual(event.id); }); + it("should allow filtering by events param", async () => { + const user = await buildUser(); + const admin = await buildAdmin({ teamId: user.teamId }); + const collection = await buildCollection({ + userId: user.id, + teamId: user.teamId, + }); + const document = await buildDocument({ + userId: user.id, + collectionId: collection.id, + teamId: user.teamId, + }); + // audit event + await buildEvent({ + name: "users.promote", + teamId: user.teamId, + actorId: admin.id, + userId: user.id, + }); + // event viewable in activity stream + const event = await buildEvent({ + name: "documents.publish", + collectionId: collection.id, + documentId: document.id, + teamId: user.teamId, + actorId: user.id, + }); + const res = await server.post("/api/events.list", { + body: { + token: user.getJwtToken(), + events: ["documents.publish"], + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data.length).toEqual(1); + expect(body.data[0].id).toEqual(event.id); + }); + it("should return events with deleted actors", async () => { const user = await buildUser(); const admin = await buildAdmin({ teamId: user.teamId }); diff --git a/server/routes/api/events/events.ts b/server/routes/api/events/events.ts index f8591a73e4..d12207f63f 100644 --- a/server/routes/api/events/events.ts +++ b/server/routes/api/events/events.ts @@ -1,4 +1,5 @@ import Router from "koa-router"; +import intersection from "lodash/intersection"; import { Op, WhereOptions } from "sequelize"; import { EventHelper } from "@shared/utils/EventHelper"; import auth from "@server/middlewares/authentication"; @@ -20,20 +21,35 @@ router.post( async (ctx: APIContext) => { const { user } = ctx.state.auth; const { - sort, - direction, + name, + events, + auditLog, actorId, documentId, collectionId, - name, - auditLog, + sort, + direction, } = ctx.input.body; let where: WhereOptions = { - name: EventHelper.ACTIVITY_EVENTS, teamId: user.teamId, }; + if (auditLog) { + authorize(user, "audit", user.team); + where.name = events + ? intersection(EventHelper.AUDIT_EVENTS, events) + : EventHelper.AUDIT_EVENTS; + } else { + where.name = events + ? intersection(EventHelper.ACTIVITY_EVENTS, events) + : EventHelper.ACTIVITY_EVENTS; + } + + if (name && (where.name as string[]).includes(name)) { + where.name = name; + } + if (actorId) { where = { ...where, actorId }; } @@ -42,15 +58,6 @@ router.post( where = { ...where, documentId }; } - if (auditLog) { - authorize(user, "audit", user.team); - where.name = EventHelper.AUDIT_EVENTS; - } - - if (name && (where.name as string[]).includes(name)) { - where.name = name; - } - if (collectionId) { where = { ...where, collectionId }; @@ -77,7 +84,7 @@ router.post( }; } - const events = await Event.findAll({ + const loadedEvents = await Event.findAll({ where, order: [[sort, direction]], include: [ @@ -94,7 +101,7 @@ router.post( ctx.body = { pagination: ctx.state.pagination, data: await Promise.all( - events.map((event) => presentEvent(event, auditLog)) + loadedEvents.map((event) => presentEvent(event, auditLog)) ), }; } diff --git a/server/routes/api/events/schema.ts b/server/routes/api/events/schema.ts index 04b248e604..624011f160 100644 --- a/server/routes/api/events/schema.ts +++ b/server/routes/api/events/schema.ts @@ -1,8 +1,19 @@ import { z } from "zod"; +import { EventHelper } from "@shared/utils/EventHelper"; import { BaseSchema } from "@server/routes/api/schema"; export const EventsListSchema = BaseSchema.extend({ body: z.object({ + /** Events to retrieve */ + events: z + .array( + z.union([ + z.enum(EventHelper.ACTIVITY_EVENTS), + z.enum(EventHelper.AUDIT_EVENTS), + ]) + ) + .optional(), + /** Id of the user who performed the action */ actorId: z.string().uuid().optional(), @@ -15,7 +26,9 @@ export const EventsListSchema = BaseSchema.extend({ /** Whether to include audit events */ auditLog: z.boolean().default(false), - /** Name of the event to retrieve */ + /** @deprecated, use 'events' parameter instead + * Name of the event to retrieve + */ name: z.string().optional(), /** The attribute to sort the events by */ diff --git a/server/routes/api/revisions/revisions.ts b/server/routes/api/revisions/revisions.ts index dd6e213cbc..f09855d234 100644 --- a/server/routes/api/revisions/revisions.ts +++ b/server/routes/api/revisions/revisions.ts @@ -125,6 +125,7 @@ router.post( const document = await Document.findByPk(documentId, { userId: user.id, + paranoid: false, }); authorize(user, "listRevisions", document); diff --git a/shared/utils/EventHelper.ts b/shared/utils/EventHelper.ts index ce488355ec..18e61735cd 100644 --- a/shared/utils/EventHelper.ts +++ b/shared/utils/EventHelper.ts @@ -1,5 +1,5 @@ export class EventHelper { - public static readonly ACTIVITY_EVENTS = [ + public static ACTIVITY_EVENTS = [ "collections.create", "collections.delete", "collections.move", @@ -20,9 +20,9 @@ export class EventHelper { "users.create", "users.demote", "userMemberships.update", - ]; + ] as const; - public static readonly AUDIT_EVENTS = [ + public static AUDIT_EVENTS = [ "api_keys.create", "api_keys.delete", "authenticationProviders.update", @@ -73,5 +73,5 @@ export class EventHelper { "fileOperations.delete", "webhookSubscriptions.create", "webhookSubscriptions.delete", - ]; + ] as const; }