diff --git a/app/client/components/BaseView.ts b/app/client/components/BaseView.ts index 63b890256d..3fdf8c8bbd 100644 --- a/app/client/components/BaseView.ts +++ b/app/client/components/BaseView.ts @@ -34,13 +34,13 @@ import {closeRegisteredMenu} from 'app/client/ui2018/menus'; import type {CommentText} from 'app/client/widgets/MentionTextBox'; import {BuildEditorOptions, createAllFieldWidgets, FieldBuilder} from 'app/client/widgets/FieldBuilder'; import {BulkColValues, CellValue, DocAction, UserAction} from 'app/common/DocActions'; +import {DocStateComparison} from 'app/common/DocState'; import {DismissedPopup} from 'app/common/Prefs'; import {SortFunc} from 'app/common/SortFunc'; import {Sort} from 'app/common/SortSpec'; import * as gristTypes from 'app/common/gristTypes'; import {IGristUrlState} from 'app/common/gristUrls'; import {arrayRepeat, nativeCompare, roundDownToMultiple, waitObs} from 'app/common/gutil'; -import {DocStateComparison} from 'app/common/UserAPI'; import {CursorPos, UIRowId} from 'app/plugin/GristAPI'; import {Events as BackboneEvents} from 'backbone'; diff --git a/app/client/components/DocComm.ts b/app/client/components/DocComm.ts index 4ff810e0ff..879bd31520 100644 --- a/app/client/components/DocComm.ts +++ b/app/client/components/DocComm.ts @@ -51,6 +51,7 @@ export class DocComm extends Disposable implements ActiveDocAPI { public stopTiming = this._wrapMethod("stopTiming"); public getAssistantState = this._wrapMethod("getAssistantState"); public listActiveUserProfiles = this._wrapMethod("listActiveUserProfiles"); + public applyProposal = this._wrapMethod("applyProposal"); public changeUrlIdEmitter = this.autoDispose(new Emitter()); diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index c38500b4e5..d63746e3a9 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -71,6 +71,7 @@ import {delay} from 'app/common/delay'; import {DisposableWithEvents} from 'app/common/DisposableWithEvents'; import {isSchemaAction, UserAction} from 'app/common/DocActions'; import {OpenLocalDocResult} from 'app/common/DocListAPI'; +import {DocStateComparison} from 'app/common/DocState'; import {isList, isListType, isRefListType} from 'app/common/gristTypes'; import {HashLink, IDocPage, isViewDocPage, parseUrlId, SpecialDocPage, ViewDocPage} from 'app/common/gristUrls'; import {undef, waitObs} from 'app/common/gutil'; @@ -79,7 +80,7 @@ import type {UserOrgPrefs} from 'app/common/Prefs'; import {StringUnion} from 'app/common/StringUnion'; import {TableData} from 'app/common/TableData'; import {getGristConfig} from 'app/common/urlUtils'; -import {AttachmentTransferStatus, DocAPI, DocStateComparison, ExtendedUser} from 'app/common/UserAPI'; +import {AttachmentTransferStatus, DocAPI, ExtendedUser} from 'app/common/UserAPI'; import {AttachedCustomWidgets, IAttachedCustomWidget, IWidgetType, WidgetType} from 'app/common/widgetTypes'; import {CursorPos} from 'app/plugin/GristAPI'; import { diff --git a/app/client/lib/UrlState.ts b/app/client/lib/UrlState.ts index 609d7a6576..7054722a2c 100644 --- a/app/client/lib/UrlState.ts +++ b/app/client/lib/UrlState.ts @@ -125,10 +125,16 @@ export class UrlState extends Disposable { * Applies to an element to create a smart link, e.g. dom('a', setLinkUrl({ws: wsId})). It * both sets the href (e.g. to allow the link to be opened to a new tab), AND intercepts plain * clicks on it to "follow" the link without reloading the page. + * + * If a "beforeChange" option is passed in, it will be run just before changing the URL. */ public setLinkUrl( urlState: IUrlState|UpdateFunc, - options?: {replace?: boolean, avoidReload?: boolean} + options?: { + replace?: boolean, + avoidReload?: boolean, + beforeChange?: () => void; + } ): DomElementMethod[] { return [ dom.attr('href', (use) => this.makeUrl(urlState, use)), @@ -136,6 +142,7 @@ export class UrlState extends Disposable { // Only override plain-vanilla clicks. if (ev.shiftKey || ev.metaKey || ev.ctrlKey || ev.altKey) { return; } ev.preventDefault(); + options?.beforeChange?.(); return this.pushUrl(urlState, options); }), ]; diff --git a/app/client/models/DataTableModelWithDiff.ts b/app/client/models/DataTableModelWithDiff.ts index 5ee0030ce0..e86ebfa4f2 100644 --- a/app/client/models/DataTableModelWithDiff.ts +++ b/app/client/models/DataTableModelWithDiff.ts @@ -8,10 +8,9 @@ import { TableData } from 'app/client/models/TableData'; import { createEmptyTableDelta, getTableIdAfter, getTableIdBefore, TableDelta } from 'app/common/ActionSummary'; import { DisposableWithEvents } from 'app/common/DisposableWithEvents'; import { CellVersions, UserAction } from 'app/common/DocActions'; -import { GristObjCode } from 'app/plugin/GristData'; +import { DocStateComparisonDetails } from 'app/common/DocState'; import { CellDelta } from 'app/common/TabularDiff'; -import { DocStateComparisonDetails } from 'app/common/UserAPI'; -import { CellValue } from 'app/plugin/GristData'; +import { CellValue, GristObjCode } from 'app/plugin/GristData'; // A special row id, representing omitted rows. const ROW_ID_SKIP = -1; diff --git a/app/client/ui/DocumentSettings.ts b/app/client/ui/DocumentSettings.ts index b720983bf7..e0e9c13d4f 100644 --- a/app/client/ui/DocumentSettings.ts +++ b/app/client/ui/DocumentSettings.ts @@ -19,7 +19,7 @@ import {openFilePicker} from 'app/client/ui/FileDialog'; import {buildNotificationsConfig} from 'app/client/ui/Notifications'; import {hoverTooltip, showTransientTooltip, withInfoTooltip} from 'app/client/ui/tooltips'; import {bigBasicButton, bigPrimaryButton} from 'app/client/ui2018/buttons'; -import {cssRadioCheckboxOptions, radioCheckboxOption} from 'app/client/ui2018/checkbox'; +import {cssRadioCheckboxOptions, labeledSquareCheckbox, radioCheckboxOption} from 'app/client/ui2018/checkbox'; import {colors, mediaSmall, theme} from 'app/client/ui2018/cssVars'; import {icon} from 'app/client/ui2018/icons'; import {cssLink} from 'app/client/ui2018/links'; @@ -62,6 +62,10 @@ export class DocSettingsPage extends Disposable { private _timezone = this._docInfo.timezone; private _locale: KoSaveableObservable = this._docInfo.documentSettingsJson.prop('locale'); private _currency: KoSaveableObservable = this._docInfo.documentSettingsJson.prop('currency'); + private _acceptProposals = Observable.create( + this, + Boolean(this._gristDoc.docPageModel.currentDoc.get()?.options?.proposedChanges?.acceptProposals) + ); constructor(private _gristDoc: GristDoc) { super(); @@ -72,6 +76,7 @@ export class DocSettingsPage extends Disposable { const isTimingOn = this._gristDoc.isTimingOn; const isDocOwner = isOwner(docPageModel.currentDoc.get()); const isDocEditor = isOwnerOrEditor(docPageModel.currentDoc.get()); + const proposedChangesEnabled = docPageModel.appModel?.experiments?.isEnabled('proposedChangesPage'); return cssContainer({tabIndex: '-1'}, dom.create(AdminSection, t('Document Settings'), [ @@ -112,6 +117,31 @@ export class DocSettingsPage extends Disposable { ), disabled: isDocOwner ? false : t('Only available to document owners'), }), + proposedChangesEnabled ? dom.create(AdminSectionItem, { + id: 'acceptProposals', + name: t('Proposals'), + description: t('Allow others to propose changes'), + value: labeledSquareCheckbox( + this._acceptProposals, + t("Show proposals"), + dom.on('click', () => { + // A tiny delay just to make reload feel less jarring. + setTimeout(async () => { + const docId = docPageModel.currentDocId.get(); + if (!docId) { + // Should never happen, don't bother translating. + reportError(new Error('Document not found')); + return; + } + const acceptProposals = this._acceptProposals.get(); + await docPageModel.appModel.api.updateDoc(docId, {options: {proposedChanges: {acceptProposals}}}); + window.location.reload(); + }, 250); + }), + testId('accept-proposals'), + ), + disabled: isDocOwner ? false : t('Only available to document owners'), + }) : null, ]), dom.create(buildNotificationsConfig, this._gristDoc.docApi, docPageModel.currentDoc.get()), diff --git a/app/client/ui/Experiments.ts b/app/client/ui/Experiments.ts index a5db45a49a..83c7737099 100644 --- a/app/client/ui/Experiments.ts +++ b/app/client/ui/Experiments.ts @@ -13,6 +13,7 @@ const G = getBrowserGlobals('document', 'window'); const EXPERIMENTS = { newRecordButton: () => t('New record button'), + proposedChangesPage: () => t('Proposed changes page'), }; type Experiment = keyof typeof EXPERIMENTS; diff --git a/app/client/ui/ProposedChangesPage.ts b/app/client/ui/ProposedChangesPage.ts index cd88d6c077..654905a44d 100644 --- a/app/client/ui/ProposedChangesPage.ts +++ b/app/client/ui/ProposedChangesPage.ts @@ -1,19 +1,34 @@ -import { ActionContext, ActionLogPart, computeContext, showCell } from 'app/client/components/ActionLog'; +import { ActionLogPart, computeContext, showCell } from 'app/client/components/ActionLog'; +import { cssBannerLink } from 'app/client/components/Banner'; import { GristDoc } from 'app/client/components/GristDoc'; import { makeT } from 'app/client/lib/localization'; +import { getTimeFromNow } from 'app/client/lib/timeUtils'; +import { urlState } from 'app/client/models/gristUrlState'; import { docListHeader } from 'app/client/ui/DocMenuCss'; -import { replaceTrunkWithFork } from 'app/client/ui/MakeCopyMenu'; import { buildOriginalUrlId } from 'app/client/ui/ShareMenu'; -import { bigPrimaryButton } from 'app/client/ui2018/buttons'; -import { mediaSmall, testId, theme, vars } from 'app/client/ui2018/cssVars'; -import { ActionSummary } from 'app/common/ActionSummary'; -import { parseUrlId } from 'app/common/gristUrls'; -import { DocStateComparison, DocStateComparisonDetails } from 'app/common/UserAPI'; -import { Disposable, dom, Observable, styled } from 'grainjs'; +import { basicButton, bigPrimaryButton, primaryButton } from 'app/client/ui2018/buttons'; +import { labeledSquareCheckbox } from 'app/client/ui2018/checkbox'; +import { colors, mediaSmall, theme, vars } from 'app/client/ui2018/cssVars'; +import { icon } from 'app/client/ui2018/icons'; +import { loadingSpinner } from 'app/client/ui2018/loaders'; +import { + DocStateComparison, + DocStateComparisonDetails, + removeMetadataChangesFromDetails, +} from 'app/common/DocState'; +import { buildUrlId, parseUrlId } from 'app/common/gristUrls'; +import { isLongerThan } from 'app/common/gutil'; +import { Proposal } from 'app/common/UserAPI'; +import { + Computed, Disposable, dom, makeTestId, MutableObsArray, + obsArray, Observable, styled +} from 'grainjs'; import * as ko from 'knockout'; const t = makeT('ProposedChangesPage'); +const testId = makeTestId('test-proposals-'); + /** * This is a page to show the differences between the current document * (the "fork") and an original document (the "trunk"). The differences @@ -21,28 +36,190 @@ const t = makeT('ProposedChangesPage'); * weak, so this page inherits that weakness. * TODO: improve the rendering of differences. * - * The page is assumed to be shown when working on an unsaved copy. It - * could be useful to show in other circumstances, but the goal is to - * use it for a crowdsourcing workflow. At that point, the page will - * have a "Propose Changes" button rather than just what it has now, - * a "Replace Original" button. */ export class ProposedChangesPage extends Disposable { - // This page shows information pulled from an API call, which - // takes a little time to fetch. This flag tracks whether - // everything needed has been fetched. - public readonly isInitialized = Observable.create(this, false); + public body: ProposedChangesTrunkPage | ProposedChangesForkPage; + public readonly isInitialized: Observable = Observable.create(this, false); + + constructor(public gristDoc: GristDoc) { + super(); + const urlId = this.gristDoc.docPageModel.currentDocId.get(); + const parts = parseUrlId(urlId || ''); + const isFork = Boolean(urlId && parts.trunkId && parts.forkId); + + this.body = this.autoDispose( + isFork ? + ProposedChangesForkPage.create(null, gristDoc) : + ProposedChangesTrunkPage.create(null, gristDoc)); + + const loader = this.body.load(); + loader.then(result => { + if (result) { this.isInitialized.set(true); } + }).catch(reportError); + isLongerThan(loader, 100).then((slow) => slow && this.isInitialized.set("slow")).catch(() => {}); + } + + public buildDom() { + const content = cssContainer( + cssHeader(this.body.title(), betaTag('Beta')), + dom.maybe(this.isInitialized, (init) => { + if (init === 'slow') { + return loadingSpinner(); + } else { + return this.body.buildDom(); + } + }) + ); + // Common pattern on other pages to avoid clipboard deactivation. + return dom('div.clipboard', + {tabIndex: "-1"}, + content); + } +} + +export class ProposedChangesTrunkPage extends Disposable { + private _proposalsObs: MutableObsArray = this.autoDispose(obsArray()); + private _proposals?: Proposal[]; + + private _proposalCount = Computed.create( + this, this._proposalsObs, + (_owner, ps) => ps.length, + ); + private _showDismissed = Observable.create(this, false); + + constructor(public gristDoc: GristDoc) { + super(); + } + + public async load() { + const urlId = this.gristDoc.docPageModel.currentDocId.get(); + if (!urlId) { return; } + const proposals = await this.gristDoc.appModel.api.getDocAPI(urlId).getProposals(); + if (this.isDisposed()) { return; } + this._proposals = proposals.proposals.filter(p => p.status.status !== 'retracted'); + this._proposalsObs.splice(0); + this._proposalsObs.push(...this._proposals); + return true; + } + + public title() { + return t('Proposed Changes'); + } + + public buildDom() { + return this.buildTrunkDom(); + } + + public buildTrunkDom() { + if (!this._proposals) { return null; } + const isReadOnly = this.gristDoc.docPageModel.currentDoc.get()?.isReadonly; + return [ + dom.domComputed(this._showDismissed, showDismissed => { + return [ + dom.maybe((use) => use(this._proposalCount) === 0, () => { + return [ + dom('p', 'There are no proposed changes.'), + ]; + }), + isReadOnly ? [ + dom('p', 'Would you like to propose some changes?'), + bigPrimaryButton( + t("Work on a Copy"), + dom.on('click', async () => { + const {urlId} = await this.gristDoc.docComm.fork(); + await urlState().pushUrl({doc: urlId}); + }), + testId('fork'), + ), + ] : null, + dom.maybe(this._proposalsObs, proposals => { + if (proposals.some(p => p.status.status === 'dismissed')) { + if (isReadOnly) { return null; } + return labeledSquareCheckbox(this._showDismissed, 'Show dismissed proposals'); + } + }), + // this seems silly, shouldn't there be something special for arrays + dom.maybe(this._proposalsObs, proposals => { + return proposals.map((proposal, idx) => { + const details = proposal.comparison.comparison?.details; + if (!details) { return null; } + const applied = proposal.status.status === 'applied'; + const dismissed = proposal.status.status === 'dismissed'; + if (dismissed && !showDismissed) { return null; } + const name = `# ${proposal.shortId}`; + return [ + cssProposalHeader( + proposal.srcDoc.id !== 'hidden' ? + cssBannerLink( + name, + urlState().setLinkUrl({ + doc: buildUrlId({ + trunkId: this.gristDoc.docId(), + forkId: proposal.srcDoc.id, + ...(proposal.srcDoc.creator.anonymous ? {} : { + forkUserId: proposal.srcDoc.creator.id, + }) + }), + docPage: 'proposals', + }) + ) : name, + ' | ', + proposal.srcDoc.creator.name || proposal.srcDoc.creator.email, ' | ', + getProposalActionSummary(proposal), + testId('header'), + ), + renderComparisonDetails(this.gristDoc, details), + proposal.status.status === 'dismissed' ? 'DISMISSED' : null, + isReadOnly ? null : cssDataRow( + primaryButton( + applied ? t("Reapply") : t("Apply"), + dom.on('click', async () => { + const outcome = await this.gristDoc.docComm.applyProposal(proposal.shortId); + this._proposalsObs.splice(idx, 1, outcome.proposal); + // For the moment, send debug information to console + for (const change of outcome.log.changes) { + if (change.fail) { + reportError(new Error(change.msg)); + } + console.log(change); + } + }), + testId('apply'), + ), + ' ', + (isReadOnly || proposal.status.status === 'dismissed') ? null : basicButton( + t("Dismiss"), + dom.on('click', async () => { + const result = await this.gristDoc.docComm.applyProposal(proposal.shortId, { + dismiss: true, + }); + this._proposalsObs.splice(idx, 1, result.proposal); + }), + testId('dismiss'), + ), + ), + ]; + }); + }) + ]; + }) + ]; + } +} + +export class ProposedChangesForkPage extends Disposable { // This will hold a comparison between this document and another version. private _comparison?: DocStateComparison; - // This holds any extra context known about the comparison. Computed on - // request. It is managed by ActionLogPart. - private _context: ko.Observable = ko.observable({}); + private _proposalObs: Observable = Observable.create(this, null); constructor(public gristDoc: GristDoc) { super(); - this.load(); + } + + public title() { + return t('Propose Changes'); } /** @@ -50,75 +227,112 @@ export class ProposedChangesPage extends Disposable { * TODO: make sure the comparison remains live either by recomputing * or by concatenating incoming changes to the computed difference. */ - public load() { + public async load() { const urlId = this.gristDoc.docPageModel.currentDocId.get(); + if (!urlId) { return; } const parts = parseUrlId(urlId || ''); - if (urlId && parts.trunkId && parts.forkId) { - const comparisonUrlId = parts.trunkId; - this.gristDoc.appModel.api.getDocAPI(urlId).compareDoc( - comparisonUrlId, { detail: true } - ).then(comparison => { - if (this.isDisposed()) { return; } - this._comparison = comparison; - this.isInitialized.set(true); - }).catch(reportError); - } else if (urlId) { - // TODO: bring in handling for trunk view. The idea is that it - // would list proposed changes from forks. I've omitted it for now - // to tackle that part separately. We just avoid showing the page - // when not on a fork. - throw new Error('ProposedChangesPage opened unexpectedly'); - } - } - - public buildDom() { - return cssContainer( - cssHeader(t('Proposed Changes'), betaTag('Beta')), - cssDataRow( - dom.maybe(this.isInitialized, () => { - const details = this._comparison?.details; - if (details) { - return this._renderComparisonDetails(details); - } - })), - cssControlRow( - bigPrimaryButton( - t("Replace Original"), - dom.on('click', () => { - const docModel = this.gristDoc.docPageModel; - const doc = docModel.currentDoc.get(); - if (doc) { - const origUrlId = buildOriginalUrlId(doc.id, doc.isSnapshot); - replaceTrunkWithFork(doc, docModel, origUrlId).catch(reportError); - } - }), - testId('replace'), - ) - ) + const comparisonUrlId = parts.trunkId; + const comparison = await this.gristDoc.appModel.api.getDocAPI(urlId).compareDoc( + comparisonUrlId, { detail: true } ); + if (this.isDisposed()) { return; } + this._comparison = comparison; + const proposals = await this.gristDoc.appModel.api.getDocAPI(urlId).getProposals({ + outgoing: true + }); + if (this.isDisposed()) { return; } + this._proposalObs.set(proposals.proposals[0] || null); + return true; } - private _renderComparisonDetails(details: DocStateComparisonDetails) { - // In the comparison, the current doc is the left (or local) - // document, and the trunk is the right (or remote) document. - // We want to look at the changes from their most recent - // common ancestor and the current doc, which are the - // "leftChanges". - const actionSummary = details.leftChanges; - const part = new ActionLogPartInProposal(this.gristDoc, actionSummary); + public buildDom() { + const details = this._comparison?.details; + const isSnapshot = this.gristDoc.docPageModel.isSnapshot.get(); + const docId = this.gristDoc.docId(); + const origUrlId = buildOriginalUrlId(docId, isSnapshot); + const isReadOnly = this.gristDoc.docPageModel.currentDoc.get()?.isReadonly; + const maybeHasChanges = + Object.keys(details?.leftChanges.tableDeltas || {}).length !== 0 || + details?.leftChanges.tableRenames.length !== 0; + const trunkAcceptsProposals = + this.gristDoc.docPageModel.currentDoc?.get()?.options?.proposedChanges?.acceptProposals; return [ + dom.maybe(() => !trunkAcceptsProposals, () => { + return cssWarningMessage( + cssWarningIcon('Warning'), + t(`The original document isn't asking for proposed changes.`) + ); + }), dom('p', - t('This is a list of changes relative to the original document.'), + t('This is a list of changes relative to the {{originalDocument}}.', { + originalDocument: cssBannerLink( + t('original document'), + urlState().setLinkUrl({ + doc: origUrlId, + docPage: 'proposals', + }, { + beforeChange: () => this.gristDoc.docPageModel.clearUnsavedChanges() + }), + dom.on('click', () => { + this.gristDoc.docPageModel.clearUnsavedChanges(); + }), + ) + }), ), - part.renderTabularDiffs(actionSummary, "", this._context), + dom.maybe(() => !maybeHasChanges, () => { + return dom('p', t('No changes found to propose. Please make some edits.')); + }), + cssDataRow( + details ? renderComparisonDetails(this.gristDoc, details) : null, + ), + dom.domComputed(this._proposalObs, proposal => { + return [ + dom('p', + getProposalActionSummary(proposal), + testId('status'), + ), + (isReadOnly || !maybeHasChanges) ? null : cssControlRow( + bigPrimaryButton( + (proposal?.updatedAt && + proposal?.status?.status === undefined) ? t('Update Change') : t('Propose Change'), + dom.on('click', async () => { + const urlId = this.gristDoc.docPageModel.currentDocId.get(); + await this.gristDoc.appModel.api.getDocAPI(urlId!).makeProposal(); + await this.update(); + }), + testId('propose'), + ), + (proposal?.updatedAt && (proposal?.status.status !== 'retracted')) ? bigPrimaryButton( + t("Retract Change"), + dom.on('click', async () => { + const urlId = this.gristDoc.docPageModel.currentDocId.get(); + await this.gristDoc.appModel.api.getDocAPI(urlId!).makeProposal({retracted: true}); + await this.update(); + }), + testId('retract'), + ) : null + ) + ]; + }) ]; } + + public async update() { + const urlId = this.gristDoc.docPageModel.currentDocId.get(); + if (!urlId) { return; } + const proposals = await this.gristDoc.appModel.api.getDocAPI(urlId).getProposals({ + outgoing: true + }); + if (this.isDisposed()) { return; } + this._proposalObs.set(proposals.proposals[0] || null); + } } + class ActionLogPartInProposal extends ActionLogPart { public constructor( private _gristDoc: GristDoc, - private _summary: ActionSummary + private _details: DocStateComparisonDetails ) { super(_gristDoc); } @@ -132,7 +346,7 @@ class ActionLogPartInProposal extends ActionLogPart { } public async getContext() { - return computeContext(this._gristDoc, this._summary); + return computeContext(this._gristDoc, this._details.leftChanges); } } @@ -177,3 +391,58 @@ export const betaTag = styled('span', ` font-size: ${vars.xsmallFontSize}; color: ${theme.accentText}; `); + +const cssProposalHeader = styled('h3', ` + padding: 5px; + padding-top: 20px; + padding-bottom: 5px; + border-top-left-radius: 20px; + border-bottom-right-radius: 20px; + background-color: ${theme.accessRulesTableHeaderBg}; + color: ${theme.accessRulesTableHeaderFg}; +`); + +export const cssWarningMessage = styled('div', ` + margin-top: 8px; + display: flex; + align-items: center; + column-gap: 8px; +`); + +export const cssWarningIcon = styled(icon, ` + --icon-color: ${colors.warning}; + flex-shrink: 0; +`); + + +function getProposalActionSummary(proposal: Proposal|null) { + return proposal?.updatedAt ? dom.text( + proposal?.status.status === 'retracted' ? + t("Retracted {{at}}", {at: getTimeFromNow(proposal.updatedAt)}) : + proposal?.status.status === 'dismissed' ? + t("Dismissed {{at}}", {at: getTimeFromNow(proposal.updatedAt)}) : + proposal?.status.status === 'applied' && proposal.appliedAt ? + t("Applied {{at}}", {at: getTimeFromNow(proposal.appliedAt)}) : + t("Proposed {{at}}", {at: getTimeFromNow(proposal.updatedAt)}), + ) : null; +} + + +function renderComparisonDetails(gristDoc: GristDoc, origDetails: DocStateComparisonDetails) { + // The change we want to render is based on a calculation + // done on the fork document. The calculation treated the + // fork as the local/left document, and the trunk as the + // remote/right document. + const {details, leftHadMetadata} = removeMetadataChangesFromDetails(origDetails); + // We want to look at the changes from their most recent + // common ancestor and the current doc. + const part = new ActionLogPartInProposal(gristDoc, details); + // This holds any extra context known about the comparison. Computed on + // request. It is managed by ActionLogPart. + // TODO: does this need ownership of some kind for disposal? + const context = ko.observable({}); + return [ + leftHadMetadata ? dom('p', "(some changes we can't deal with yet were ignored)") : null, + part.renderTabularDiffs(details.leftChanges, "", context), + ]; +} diff --git a/app/client/ui/ShareMenu.ts b/app/client/ui/ShareMenu.ts index aabe163d06..b6f7156590 100644 --- a/app/client/ui/ShareMenu.ts +++ b/app/client/ui/ShareMenu.ts @@ -49,6 +49,7 @@ export function buildShareMenuButton(pageModel: DocPageModel): DomContents { // available (a user quick enough to open the menu in this state would have to re-open it). return dom.maybe(pageModel.currentDoc, (doc) => { const saveCopy = () => handleSaveCopy({pageModel, doc, modalTitle: t("Save Document")}); + console.log("HIYA", doc); if (doc.isSnapshot) { const backToCurrent = () => urlState().pushUrl({doc: buildOriginalUrlId(doc.id, true)}); return shareButton(t("Back to Current"), () => [ @@ -72,6 +73,18 @@ export function buildShareMenuButton(pageModel: DocPageModel): DomContents { menuExports(doc, pageModel), ], {buttonAction: saveCopy}); } else if (doc.isFork) { + if (doc.options?.proposedChanges?.acceptProposals) { + return shareButton(t("Propose Changes"), () => [ + menuManageUsers(doc, pageModel), + menuSaveCopy({pageModel, doc, saveActionTitle: t("Save Copy")}), + menuOriginal(doc, pageModel), + menuExports(doc, pageModel), + ], {buttonAction: async () => { + await urlState().pushUrl({ + docPage: 'proposals' + }); + }}); + } // For forks, the main actions are "Replace Original" and "Save Copy". When "Replace // Original" is unavailable (for samples, forks of public docs, etc), we'll consider "Save // Copy" primary and keep it as an action button on top. Otherwise, show a tag without a diff --git a/app/client/ui/Tools.ts b/app/client/ui/Tools.ts index 13d0ab88ee..374bea8db0 100644 --- a/app/client/ui/Tools.ts +++ b/app/client/ui/Tools.ts @@ -30,7 +30,7 @@ import {stretchedLink} from 'app/client/ui2018/stretchedLink'; import {unstyledButton} from 'app/client/ui2018/unstyled'; import {buildOpenAssistantButton} from 'app/client/widgets/AssistantPopup'; import {isOwner} from 'app/common/roles'; -import {Disposable, dom, makeTestId, Observable, observable, styled} from 'grainjs'; +import { computed, Disposable, dom, makeTestId, Observable, observable, styled} from 'grainjs'; import noop from 'lodash/noop'; const testId = makeTestId('test-tools-'); @@ -40,6 +40,14 @@ export function tools(owner: Disposable, gristDoc: GristDoc, leftPanelOpen: Obse const docPageModel = gristDoc.docPageModel; const isDocOwner = isOwner(docPageModel.currentDoc.get()); const isOverridden = Boolean(docPageModel.userOverride.get()); + const canMakeProposal = computed((use) => { + return use(docPageModel.isFork) && !use(docPageModel.isBareFork) && !use(docPageModel.isPrefork) && + !use(docPageModel.isSnapshot); + }); + // If we are on a fork, currentDoc options are actually for the trunk. + const trunkAcceptsProposals = computed((use) => { + return use(docPageModel.currentDoc)?.options?.proposedChanges?.acceptProposals; + }); const canViewAccessRules = observable(false); function updateCanViewAccessRules() { canViewAccessRules.set((isDocOwner && !isOverridden) || @@ -97,12 +105,15 @@ export function tools(owner: Disposable, gristDoc: GristDoc, leftPanelOpen: Obse cssPageButton(cssPageIcon('Log'), cssLinkText(t("Document History")), testId('log'), dom.on('click', () => gristDoc.showTool('docHistory'))) ), - dom.maybe(docPageModel.isFork, () => { + dom.maybe( + (use) => use(trunkAcceptsProposals), () => { return cssPageEntry( cssPageEntry.cls('-selected', (use) => use(gristDoc.activeViewId) === 'proposals'), cssPageLink( - cssPageIcon('EyeShow'), - cssLinkText(t("Proposed Changes")), + cssPageIcon('MobileChat'), + dom.domComputed(canMakeProposal, (proposable) => { + return cssLinkText(proposable ? t("Propose Changes") : t("Proposed Changes")); + }), testId('proposals'), urlState().setLinkUrl({docPage: 'proposals'}) ) diff --git a/app/client/ui/TopBar.ts b/app/client/ui/TopBar.ts index 306c8b0313..32bd6344bb 100644 --- a/app/client/ui/TopBar.ts +++ b/app/client/ui/TopBar.ts @@ -110,6 +110,15 @@ export function createTopBarDoc(owner: MultiHolder, appModel: AppModel, pageMode isPublic: Computed.create(owner, doc, (use, _doc) => Boolean(_doc && _doc.public)), isTemplate: pageModel.isTemplate, isAnonymous, + isProposable: Computed.create( + owner, gristDoc.docPageModel.currentDoc, + (_use, currentDoc) => Boolean(currentDoc?.options?.proposedChanges?.acceptProposals) + ), + isReadonly: pageModel.isReadonly, + proposeChanges: async () => { + const {urlId} = await gristDoc.docComm.fork(); + await urlState().pushUrl({doc: urlId}); + }, }), dom.hide(use => use(isSearchOpen) && use(isNarrowScreenObs())), ) diff --git a/app/client/ui2018/breadcrumbs.ts b/app/client/ui2018/breadcrumbs.ts index ec98bd0adf..9e06e6fd84 100644 --- a/app/client/ui2018/breadcrumbs.ts +++ b/app/client/ui2018/breadcrumbs.ts @@ -91,6 +91,7 @@ export function docBreadcrumbs( docNameSave: (val: string) => Promise, pageNameSave: (val: string) => Promise, cancelRecoveryMode: () => Promise, + proposeChanges?: () => Promise, isDocNameReadOnly?: BindableValue, isPageNameReadOnly?: BindableValue, isFork: Observable, @@ -102,6 +103,8 @@ export function docBreadcrumbs( isPublic?: Observable, isTemplate?: Observable, isAnonymous?: boolean, + isProposable?: Observable, + isReadonly?: Observable, } ): Element { const shouldShowWorkspace = !(options.isTemplate && options.isAnonymous); @@ -145,7 +148,11 @@ export function docBreadcrumbs( return cssTag(t("snapshot"), testId('snapshot-tag')); } if (use(options.isFork) && !use(options.isTutorialFork)) { - return cssTag(t("unsaved"), testId('unsaved-tag')); + if (options.isProposable && use(options.isProposable)) { + return cssTag(t("proposing changes"), testId('proposing-changes-tag')); + } else { + return cssTag(t("unsaved"), testId('unsaved-tag')); + } } if (use(options.isRecoveryMode)) { return cssAlertTag(t("recovery mode"), @@ -157,6 +164,18 @@ export function docBreadcrumbs( return cssTag(t("fiddle"), tooltip({title: t(`You may make edits, but they will create a new copy and will not affect the original document.`)}), testId('fiddle-tag')); } + if (options.isProposable && use(options.isProposable)) { + if (options.isReadonly && use(options.isReadonly)) { + return cssAlertTag('', + dom('a', dom.on('click', () => options.proposeChanges?.()), + 'propose changes ', icon('Pencil')), + testId('propose-changes-tag')); + } else { + return cssTag(t("direct edit"), tooltip({ + title: 'Work on a copy if you want to propose changes' + }), testId('direct-tag')); + } + } }), separator(' / ', testId('bc-separator'), diff --git a/app/common/ActiveDocAPI.ts b/app/common/ActiveDocAPI.ts index 78abfec606..c34ce11808 100644 --- a/app/common/ActiveDocAPI.ts +++ b/app/common/ActiveDocAPI.ts @@ -1,8 +1,9 @@ import {ActionGroup} from 'app/common/ActionGroup'; import {BulkAddRecord, CellValue, TableDataAction, UserAction} from 'app/common/DocActions'; +import {DocStateComparison} from 'app/common/DocState'; import {PredicateFormulaProperties} from 'app/common/PredicateFormula'; import {FetchUrlOptions, UploadResult} from 'app/common/uploads'; -import {DocStateComparison, PermissionData, UserAccessData} from 'app/common/UserAPI'; +import {PermissionData, Proposal, UserAccessData} from 'app/common/UserAPI'; import {ParseOptions} from 'app/plugin/FileParserAPI'; import {AccessTokenOptions, AccessTokenResult, UIRowId} from 'app/plugin/GristAPI'; import {IMessage} from 'grain-rpc'; @@ -540,5 +541,24 @@ export interface ActiveDocAPI { * Lists users that currently have the doc open. * This list varies based on the requesting user's permissions. */ - listActiveUserProfiles(): Promise + listActiveUserProfiles(): Promise; + + applyProposal(proposalId: number, option?: { + dismiss?: boolean, + }): Promise; +} + +export interface ApplyProposalResult { + proposal: Proposal; + log: PatchLog; +} + +export interface PatchLog { + changes: PatchItem[]; + applied: boolean; +} + +export interface PatchItem { + msg: string; + fail?: boolean; } diff --git a/app/common/DocState.ts b/app/common/DocState.ts new file mode 100644 index 0000000000..9a0690c964 --- /dev/null +++ b/app/common/DocState.ts @@ -0,0 +1,79 @@ +import { ActionSummary, createEmptyActionSummary } from 'app/common/ActionSummary'; + +/** + * Information about a single document state. + */ +export interface DocState { + n: number; // a sequential identifier + h: string; // a hash identifier +} + +/** + * A list of document states. Most recent is first. + */ +export interface DocStates { + states: DocState[]; +} + +/** + * A comparison between two documents, called "left" and "right". + * The comparison is based on the action histories in the documents. + * If those histories have been truncated, the comparison may report + * two documents as being unrelated even if they do in fact have some + * shared history. + */ +export interface DocStateComparison { + left: DocState; // left / local document + right: DocState; // right / remote document + parent: DocState|null; // most recent common ancestor of left and right + // summary of the relationship between the two documents. + // same: documents have the same most recent state + // left: the left document has actions not yet in the right + // right: the right document has actions not yet in the left + // both: both documents have changes (possible divergence) + // unrelated: no common history found + summary: 'same' | 'left' | 'right' | 'both' | 'unrelated'; + // optionally, details of what changed may be included. + details?: DocStateComparisonDetails; +} + +/** + * Detailed comparison between document versions. For now, this + * is provided as a pair of ActionSummary objects, relative to + * the most recent common ancestor. + */ +export interface DocStateComparisonDetails { + leftChanges: ActionSummary; + rightChanges: ActionSummary; +} + +export function removeMetadataChangesFromDetails(details: DocStateComparisonDetails) { + const {summary: leftChanges, hadMetadata: leftHadMetadata} = removeMetadataChangesFromSummary(details.leftChanges); + const {summary: rightChanges, hadMetadata: rightHadMetadata} = removeMetadataChangesFromSummary(details.rightChanges); + return { + details: { + leftChanges, + rightChanges, + }, + leftHadMetadata, + rightHadMetadata, + }; +} + +export function removeMetadataChangesFromSummary(summary: ActionSummary) { + const result = createEmptyActionSummary(); + result.tableRenames = summary.tableRenames; + const tables = Object.keys(summary.tableDeltas); + const metaTables = new Set(); + for (const table of tables) { + if (table.startsWith('_grist_')) { + metaTables.add(table); + continue; + } + result.tableDeltas[table] = summary.tableDeltas[table]; + } + return { + summary: result, + hadMetadata: metaTables.size > 0, + }; +} diff --git a/app/common/UserAPI.ts b/app/common/UserAPI.ts index 64b9e8b01b..c2c42aae0b 100644 --- a/app/common/UserAPI.ts +++ b/app/common/UserAPI.ts @@ -1,4 +1,3 @@ -import {ActionSummary} from 'app/common/ActionSummary'; import {ApplyUAResult, ForkResult, FormulaTimingInfo, PermissionDataWithExtraUsers, QueryFilters, TimingStatus} from 'app/common/ActiveDocAPI'; import {AssistanceRequest, AssistanceResponse} from 'app/common/Assistance'; @@ -9,6 +8,7 @@ import {ICustomWidget} from 'app/common/CustomWidget'; import {BulkColValues, TableColValues, TableRecordValue, TableRecordValues, TableRecordValuesWithoutIds, UserAction} from 'app/common/DocActions'; import {DocCreationInfo, OpenDocMode} from 'app/common/DocListAPI'; +import {DocStateComparison} from 'app/common/DocState'; import {OrgUsageSummary} from 'app/common/DocUsage'; import {Features, Product} from 'app/common/Features'; import {isClient} from 'app/common/gristUrls'; @@ -150,6 +150,7 @@ export interface DocumentOptions { appearance?: DocumentAppearance|null; // Whether search engines should index this document. Defaults to `false`. allowIndex?: boolean; + proposedChanges?: ProposedChanges|null; } export interface TutorialMetadata { @@ -175,6 +176,11 @@ export interface DocumentProperties extends CommonProperties { options: DocumentOptions|null; } +export interface ProposedChanges { + mayHaveProposals?: boolean; + acceptProposals?: boolean; +} + export const documentPropertyKeys = [ ...commonPropertyKeys, 'isPinned', @@ -199,6 +205,29 @@ export interface Fork { options: DocumentOptions|null; } +export interface ProposalComparison { + comparison?: DocStateComparison; +} + +export interface ProposalStatus { + status?: 'applied' | 'retracted' | 'dismissed'; +} + +export interface Proposal { + shortId: number; + comparison: ProposalComparison; + status: ProposalStatus; + createdAt: string; // ISO date string + updatedAt: string; // ISO date string + appliedAt: string|null; // ISO date string + srcDoc: Document & { + creator: FullUser + }, + destDoc: Document & { + creator: FullUser + }, +} + // Non-core options for a user. export interface UserOptions { // Whether signing in with Google is allowed. Defaults to true if unset. @@ -338,53 +367,6 @@ export interface DocSnapshots { snapshots: DocSnapshot[]; // snapshots, freshest first. } -/** - * Information about a single document state. - */ -export interface DocState { - n: number; // a sequential identifier - h: string; // a hash identifier -} - -/** - * A list of document states. Most recent is first. - */ -export interface DocStates { - states: DocState[]; -} - -/** - * A comparison between two documents, called "left" and "right". - * The comparison is based on the action histories in the documents. - * If those histories have been truncated, the comparison may report - * two documents as being unrelated even if they do in fact have some - * shared history. - */ -export interface DocStateComparison { - left: DocState; // left / local document - right: DocState; // right / remote document - parent: DocState|null; // most recent common ancestor of left and right - // summary of the relationship between the two documents. - // same: documents have the same most recent state - // left: the left document has actions not yet in the right - // right: the right document has actions not yet in the left - // both: both documents have changes (possible divergence) - // unrelated: no common history found - summary: 'same' | 'left' | 'right' | 'both' | 'unrelated'; - // optionally, details of what changed may be included. - details?: DocStateComparisonDetails; -} - -/** - * Detailed comparison between document versions. For now, this - * is provided as a pair of ActionSummary objects, relative to - * the most recent common ancestor. - */ -export interface DocStateComparisonDetails { - leftChanges: ActionSummary; - rightChanges: ActionSummary; -} - export interface CopyDocOptions { documentName: string; asTemplate?: boolean; @@ -615,6 +597,13 @@ export interface DocAPI { * If there is one store available it means that external storage is configured and can be used by this document. */ getAttachmentStores(): Promise<{stores: AttachmentStoreDesc[]}>; + + makeProposal(options?: { + retracted?: boolean, + }): Promise; + getProposals(options?: { + outgoing?: boolean + }): Promise<{proposals: Proposal[]}>; } // Operations that are supported by a doc worker. @@ -1321,6 +1310,24 @@ export class DocAPIImpl extends BaseAPI implements DocAPI { }); } + public async makeProposal(options?: { + retracted?: boolean, + }) { + await this.requestJson(`${this._url}/propose`, { + method: 'POST', + body: JSON.stringify(options || {}), + }); + } + + public async getProposals(options?: { + outgoing?: boolean, + }) { + const result = await this.requestJson(`${this._url}/proposals?outgoing=${Boolean(options?.outgoing)}`, { + method: 'GET', + }); + return result; + } + private _getRecords(tableId: string, endpoint: 'data' | 'records', options?: GetRowsParams): Promise { const url = new URL(`${this._url}/tables/${tableId}/${endpoint}`); if (options?.filters) { diff --git a/app/common/gutil.ts b/app/common/gutil.ts index 388a8e1eb9..297ea3499d 100644 --- a/app/common/gutil.ts +++ b/app/common/gutil.ts @@ -948,6 +948,10 @@ export function isAffirmative(parameter: any): boolean { return ['1', 'on', 'true', 'yes'].includes(String(parameter).toLowerCase()); } +export function isNotAffirmative(parameter: any): boolean { + return ['0', 'off', 'false', 'no'].includes(String(parameter).toLowerCase()); +} + /** * Returns whether a value is neither null nor undefined, with a type guard for the return type. * diff --git a/app/gen-server/entity/Document.ts b/app/gen-server/entity/Document.ts index 76236e9b3c..928f50b148 100644 --- a/app/gen-server/entity/Document.ts +++ b/app/gen-server/entity/Document.ts @@ -9,6 +9,7 @@ import {AclRuleDoc} from "app/gen-server/entity/AclRule"; import {Alias} from "app/gen-server/entity/Alias"; import {Resource} from "app/gen-server/entity/Resource"; import {Secret} from "app/gen-server/entity/Secret"; +import {User} from "app/gen-server/entity/User"; import {Workspace} from "app/gen-server/entity/Workspace"; // Acceptable ids for use in document urls. @@ -79,6 +80,10 @@ export class Document extends Resource { @Column({name: 'created_by', type: 'integer', nullable: true}) public createdBy: number|null; + @ManyToOne(_type => User) + @JoinColumn({name: 'created_by'}) + public creator: User; + @Column({name: 'trunk_id', type: 'text', nullable: true}) public trunkId: string|null; @@ -152,6 +157,22 @@ export class Document extends Resource { } } } + if (props.options.proposedChanges !== undefined) { + if (props.options.proposedChanges === null) { + this.options.proposedChanges = null; + } else { + this.options.proposedChanges = this.options.proposedChanges || {}; + // Merge individual properties, once there are more than one, following + // the example of appearance and tutorial above. + // TODO: add a helper? Seems a common pattern now. + if (props.options.proposedChanges.mayHaveProposals !== undefined) { + this.options.proposedChanges.mayHaveProposals = props.options.proposedChanges.mayHaveProposals; + } + if (props.options.proposedChanges.acceptProposals !== undefined) { + this.options.proposedChanges.acceptProposals = props.options.proposedChanges.acceptProposals; + } + } + } if (props.options.allowIndex !== undefined) { this.options.allowIndex = props.options.allowIndex; } diff --git a/app/gen-server/entity/Proposal.ts b/app/gen-server/entity/Proposal.ts new file mode 100644 index 0000000000..e16b9cdb5c --- /dev/null +++ b/app/gen-server/entity/Proposal.ts @@ -0,0 +1,82 @@ +import { ProposalComparison, ProposalStatus } from 'app/common/UserAPI'; +import { Document } from 'app/gen-server/entity/Document'; +import { nativeValues } from 'app/gen-server/lib/values'; +import { BaseEntity, Column, Entity, JoinColumn, ManyToOne, PrimaryColumn } from 'typeorm'; + +/** + * + * A table for tracking proposed changes to documents. + * + * For a "githubby" feal, proposals are identified with a document and a + * short incrementing integer that is unique for that document, like the PR + * number associated with repositories. + * + * The "comparison" column contains changes in the format used by the + * /compare endpoint. This could be tweaked in future. That format represents + * an overall diff, but doesn't give details of individual steps. + * + * The "comparison" column shouldn't be allowed to get too big, ideally. + * Any large changes should be externalized - either computed on demand, + * or placed in an S3-like store. + * + * Each proposal has a source and destination document. They are assumed to + * share a common ancestor. The source contains the proposed changes. + * Users of the destination document will be offered those changes, and there + * should be a way to merge them into the destination document. + * + * Currently, only a single proposal is permitted between a given + * source/destination pair. This should perhaps be relaxed, to be + * future proof, although the UI would be likely to still be + * constrained in this way for now. + * + * The 'status' of the proposal is a bit simplistic for now. Here are some + * states of proposals: + * - dismissed + * - retracted + * - applied + * If this feature were to grow, probably status would need to be events + * in a full timeline, but it doesn't make sense to invest in that now, and + * some kind of summary state would be needed anyway. + * + * There's at least one security problem with proposals. The source + * document id may be a "secret" if it was created by an anonymous + * user, in the sense that anyone who knows the id could edit + * it. Something to bear in mind. Some proposal endpoints do some + * censoring but may not be the right way to go. + * + */ + +@Entity({name: 'proposals'}) +export class Proposal extends BaseEntity { + @Column({name: 'short_id', type: Number}) + public shortId: number; + + @Column({name: 'comparison', type: nativeValues.jsonEntityType, nullable: true}) + public comparison: ProposalComparison; + + @Column({name: 'status', type: nativeValues.jsonEntityType, nullable: true}) + public status: ProposalStatus; + + @PrimaryColumn({name: 'src_doc_id', type: String}) + public srcDocId: string; + + @ManyToOne(_type => Document, { onDelete: 'CASCADE' }) + @JoinColumn({name: 'src_doc_id'}) + public srcDoc: Document; + + @PrimaryColumn({name: 'dest_doc_id', type: String}) + public destDocId: string; + + @ManyToOne(_type => Document, { onDelete: 'CASCADE' }) + @JoinColumn({name: 'dest_doc_id'}) + public destDoc: Document; + + @Column({name: 'created_at', type: Date, default: () => "CURRENT_TIMESTAMP"}) + public createdAt: Date; + + @Column({name: 'updated_at', type: Date, default: () => "CURRENT_TIMESTAMP"}) + public updatedAt: Date; + + @Column({name: 'applied_at', type: Date}) + public appliedAt: Date|null; +} diff --git a/app/gen-server/lib/DocApiForwarder.ts b/app/gen-server/lib/DocApiForwarder.ts index 06a543bf22..ac3bf8dd31 100644 --- a/app/gen-server/lib/DocApiForwarder.ts +++ b/app/gen-server/lib/DocApiForwarder.ts @@ -80,6 +80,8 @@ export class DocApiForwarder { app.use('/api/docs/:docId/timing/start', withDoc); app.use('/api/docs/:docId/timing/stop', withDoc); app.use('/api/docs/:docId/forms/:vsId', withDoc); + app.use('/api/docs/:docId/propose', withDoc); + app.use('/api/docs/:docId/proposals', withDoc); app.use('/api/docs/:docId/copy', withoutDoc); app.use('^/api/docs$', withoutDoc); diff --git a/app/gen-server/lib/homedb/HomeDBManager.ts b/app/gen-server/lib/homedb/HomeDBManager.ts index 037556d8f8..dd8b670567 100644 --- a/app/gen-server/lib/homedb/HomeDBManager.ts +++ b/app/gen-server/lib/homedb/HomeDBManager.ts @@ -3,6 +3,7 @@ import {ApiError, LimitType} from 'app/common/ApiError'; import {mapGetOrSet, mapSetOrClear, MapWithTTL} from 'app/common/AsyncCreate'; import {ConfigKey, ConfigValue} from 'app/common/Config'; import {getDataLimitInfo} from 'app/common/DocLimits'; +import {DocStateComparison} from 'app/common/DocState'; import {createEmptyOrgUsageSummary, DocumentUsage, OrgUsageSummary} from 'app/common/DocUsage'; import {normalizeEmail} from 'app/common/emails'; import { @@ -30,9 +31,10 @@ import { PermissionData, PermissionDelta, PREVIEWER_EMAIL, + ProposalStatus, UserAccessData, UserOptions, - WorkspaceProperties + WorkspaceProperties, } from 'app/common/UserAPI'; import {AclRule, AclRuleDoc, AclRuleOrg, AclRuleWs} from 'app/gen-server/entity/AclRule'; import {Alias} from 'app/gen-server/entity/Alias'; @@ -51,6 +53,7 @@ import { personalFreeFeatures, Product } from 'app/gen-server/entity/Product'; +import {Proposal} from 'app/gen-server/entity/Proposal'; import {Secret} from 'app/gen-server/entity/Secret'; import {Share} from 'app/gen-server/entity/Share'; import {User} from 'app/gen-server/entity/User'; @@ -1860,7 +1863,10 @@ export class HomeDBManager { public async updateDocument( scope: DocScope, props: Partial, - transaction?: EntityManager + transaction?: EntityManager, + options?: { + allowSpecialPermit?: boolean, + } ): Promise>> { const notifications: Array<() => Promise> = []; const markPermissions = Permissions.SCHEMA_EDIT; @@ -1875,6 +1881,7 @@ export class HomeDBManager { query = this._doc(scope, { manager, markPermissions, + allowSpecialPermit: options?.allowSpecialPermit, }); } const queryResult = await verifyEntity(query); @@ -3414,6 +3421,114 @@ export class HomeDBManager { return new Map(records.map(r => [r.userId, r.prefs])); } + public setProposal(options: { + srcDocId: string, + destDocId: string, + comparison: DocStateComparison + retracted?: boolean + }) { + return this._connection.transaction(async manager => { + const maxRow = await manager.createQueryBuilder() + .from(Proposal, 'proposals') + .select("MAX(proposals.short_id)", "max") + .where("proposals.dest_doc_id = :docId", { docId: options.destDocId }) + .getRawOne<{ max: number }>(); + const shortId = (maxRow?.max || 0) + 1; + const status: ProposalStatus = options?.retracted ? { status: 'retracted' } : {}; + await manager.createQueryBuilder() + .insert() + .into(Proposal, ['srcDocId', 'destDocId', 'comparison', 'shortId', 'status', 'updatedAt', 'appliedAt']) + .values({ + srcDocId: options.srcDocId, + destDocId: options.destDocId, + comparison: {comparison: options.comparison}, + status, + appliedAt: null, + shortId, + }) + .orUpdate(['comparison', 'status', 'updated_at', 'applied_at'], ['src_doc_id', 'dest_doc_id']) + .execute(); + this.unwrapQueryResult(await this.updateDocument({ + urlId: options.destDocId, + userId: this.getPreviewerUserId(), + specialPermit: { + docId: options.destDocId, + } + }, { + options: { + proposedChanges: { + mayHaveProposals: true, + }, + }, + }, manager, { + allowSpecialPermit: true, + })); + // typeorm is super annoying... + const proposal = await manager.createQueryBuilder() + .from(Proposal, 'proposals') + .select('proposals') + .where("proposals.dest_doc_id = :destDocId", { destDocId: options.destDocId }) + .andWhere("proposals.src_doc_id = :srcDocId", { srcDocId: options.srcDocId }) + .getOneOrFail(); + const proposal2 = this._normalizeQueryResults(proposal); + return proposal2; + }); + } + + public async updateProposalStatus(destDocId: string, shortId: number, + status: ProposalStatus) { + const timestamp = new Date(); + const result = await this._connection.createQueryBuilder() + .update(Proposal) + .set({ + status, + updatedAt: timestamp, + ...(status.status === 'applied') ? { + appliedAt: timestamp + } : {} + }) + .where('shortId = :shortId', {shortId}) + .andWhere('destDocId = :destDocId', {destDocId}) + .execute(); + return result; + } + + public async getProposals(options: { + srcDocId?: string, + destDocId?: string, + shortId?: number, + }): Promise { + const result = await this._connection.createQueryBuilder() + .select('proposals') + .from(Proposal, 'proposals') + .leftJoinAndSelect('proposals.srcDoc', 'src_doc') + .leftJoinAndSelect('src_doc.creator', 'src_creator') + .leftJoinAndSelect('src_creator.logins', 'src_logins') + .leftJoinAndSelect('proposals.destDoc', 'dest_doc') + .leftJoinAndSelect('dest_doc.creator', 'dest_creator') + .leftJoinAndSelect('dest_creator.logins', 'dest_logins') + .where(options) + .orderBy('proposals.short_id', 'DESC') + .getMany(); + const result2 = this._normalizeQueryResults(result); + return result2; + } + + public async getProposal(destDocId: string, shortId: number, + transaction?: EntityManager): Promise { + const result = await (transaction || this._connection).createQueryBuilder() + .select('proposals') + .from(Proposal, 'proposals') + .leftJoinAndSelect('proposals.srcDoc', 'src_doc') + .leftJoinAndSelect('src_doc.creator', 'src_creator') + .leftJoinAndSelect('src_creator.logins', 'src_logins') + .where('proposals.shortId = :shortId', {shortId}) + .andWhere('proposals.destDocId = :destDocId', {destDocId}) + .getOne(); + const result2 = this._normalizeQueryResults(result); + return result2; + } + /** * Run an operation in an existing transaction if available, otherwise create * a new transaction for it. diff --git a/app/gen-server/migration/1759256005608-Proposals.ts b/app/gen-server/migration/1759256005608-Proposals.ts new file mode 100644 index 0000000000..43df309c1b --- /dev/null +++ b/app/gen-server/migration/1759256005608-Proposals.ts @@ -0,0 +1,84 @@ +import { nativeValues } from 'app/gen-server/lib/values'; +import * as sqlUtils from 'app/gen-server/sqlUtils'; +import { MigrationInterface, QueryRunner, Table, TableIndex, TableUnique } from 'typeorm'; + +export class Proposals1759256005608 implements MigrationInterface { + + public async up(queryRunner: QueryRunner): Promise { + const dbType = queryRunner.connection.driver.options.type; + const datetime = sqlUtils.datetime(dbType); + const now = sqlUtils.now(dbType); + await queryRunner.createTable(new Table({ + name: 'proposals', + columns: [ + { + name: 'short_id', + type: 'integer', + }, + { + name: 'src_doc_id', + type: "varchar", + isPrimary: true, + }, + { + name: 'dest_doc_id', + type: "varchar", + isPrimary: true, + }, + { + name: 'comparison', + type: nativeValues.jsonType, + }, + { + name: 'status', + type: nativeValues.jsonType, + }, + { + name: "created_at", + type: datetime, + default: now + }, + { + name: "updated_at", + type: datetime, + default: now + }, + { + name: "applied_at", + type: datetime, + isNullable: true + }, + ], + foreignKeys: [ + { + columnNames: ['src_doc_id'], + referencedColumnNames: ['id'], + referencedTableName: 'docs', + onDelete: 'CASCADE', + }, + { + columnNames: ['dest_doc_id'], + referencedColumnNames: ['id'], + referencedTableName: 'docs', + onDelete: 'CASCADE', + }, + ], + })); + await queryRunner.createIndex('proposals', new TableIndex({ + name: 'proposals__dest_doc_id__short_id', + columnNames: ['dest_doc_id', 'short_id'], + isUnique: true, + })); + await queryRunner.createUniqueConstraint( + 'proposals', + new TableUnique({ + name: 'proposals__dest_doc_id__src_doc_id', + columnNames: ['dest_doc_id', 'src_doc_id'], + }) + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.dropTable('proposals'); + } +} diff --git a/app/server/lib/ActionHistory.ts b/app/server/lib/ActionHistory.ts index 35ac9d3e0a..e4328bf61f 100644 --- a/app/server/lib/ActionHistory.ts +++ b/app/server/lib/ActionHistory.ts @@ -16,8 +16,8 @@ import {LocalActionBundle} from 'app/common/ActionBundle'; import {ActionGroup, MinimalActionGroup} from 'app/common/ActionGroup'; import {createEmptyActionSummary} from 'app/common/ActionSummary'; -import {DocState} from 'app/common/UserAPI'; import {summarizeAction} from 'app/common/ActionSummarizer'; +import {DocState} from 'app/common/DocState'; export interface ActionGroupOptions { // If set, inspect the action in detail in order to include a summary of diff --git a/app/server/lib/ActionHistoryImpl.ts b/app/server/lib/ActionHistoryImpl.ts index 19e6d9be29..19c0209653 100644 --- a/app/server/lib/ActionHistoryImpl.ts +++ b/app/server/lib/ActionHistoryImpl.ts @@ -3,8 +3,8 @@ */ import {LocalActionBundle} from 'app/common/ActionBundle'; import {ActionGroup, MinimalActionGroup} from 'app/common/ActionGroup'; +import {DocState} from 'app/common/DocState'; import * as marshaller from 'app/common/marshal'; -import {DocState} from 'app/common/UserAPI'; import {reportTimeTaken} from 'app/server/lib/reportTimeTaken'; import * as crypto from 'crypto'; import keyBy = require('lodash/keyBy'); diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 82ab1ef30d..707701809e 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -17,6 +17,7 @@ import {ActionSummary} from 'app/common/ActionSummary'; import { AclResources, AclTableDescription, + ApplyProposalResult, ApplyUAExtendedOptions, ApplyUAOptions, ApplyUAResult, @@ -28,6 +29,7 @@ import { ImportResult, ISuggestionWithValue, MergeOptions, + PatchLog, PermissionDataWithExtraUsers, QueryResult, ServerQuery, @@ -54,6 +56,11 @@ import { import {DocData} from 'app/common/DocData'; import {getDataLimitInfo, getDataLimitRatio, getSeverity} from 'app/common/DocLimits'; import {DocSnapshots} from 'app/common/DocSnapshot'; +import { + DocState, + DocStateComparison, + removeMetadataChangesFromDetails +} from 'app/common/DocState'; import {DocumentSettings} from 'app/common/DocumentSettings'; import { DataLimitInfo, @@ -87,9 +94,7 @@ import { AttachmentTransferStatus, CreatableArchiveFormats, DocReplacementOptions, - DocState, - DocStateComparison, - NEW_DOCUMENT_CODE + NEW_DOCUMENT_CODE, } from 'app/common/UserAPI'; import {convertFromColumn} from 'app/common/ValueConverter'; import {guessColInfo} from 'app/common/ValueGuesser'; @@ -164,6 +169,7 @@ import {createAttachmentsIndex, DocStorage, REMOVE_UNUSED_ATTACHMENTS_DELAY} fro import {expandQuery, getFormulaErrorForExpandQuery} from 'app/server/lib/ExpandedQuery'; import {GranularAccess, GranularAccessForBundle} from 'app/server/lib/GranularAccess'; import {OnDemandActions} from 'app/server/lib/OnDemandActions'; +import {Patch } from 'app/server/lib/Patch'; import {findOrAddAllEnvelope, Sharing} from 'app/server/lib/Sharing'; import {Cancelable} from 'lodash'; import cloneDeep = require('lodash/cloneDeep'); @@ -898,6 +904,37 @@ export class ActiveDoc extends EventEmitter { return this._activeDocImport.generateImportDiff(hiddenTableId, transformRule, mergeOptions); } + public async applyProposal(docSession: OptDocSession, proposalId: number, options?: { + dismiss?: boolean, + }): Promise { + const urlId = this.docName; + const proposal = await this._getHomeDbManagerOrFail().getProposal(urlId, proposalId); + if (!proposal) { + throw new ApiError('Proposal not found', 404); + } + const origDetails = proposal.comparison.comparison?.details; + if (!origDetails) { + throw new ApiError('Proposal details not found', 500); + } + let result: PatchLog = {changes: [], applied: false}; + if (!options?.dismiss) { + const patch = new Patch(this, docSession); + const {details} = removeMetadataChangesFromDetails(origDetails); + result = await patch.applyChanges(details); + if (result.applied) { + await this._getHomeDbManagerOrFail().updateProposalStatus(urlId, proposalId, { + status: 'applied' + }); + } + } else { + await this._getHomeDbManagerOrFail().updateProposalStatus(urlId, proposalId, { + status: 'dismissed' + }); + } + const proposal2 = await this._getHomeDbManagerOrFail().getProposal(urlId, proposalId); + return {proposal: proposal2 as any, log: result}; + } + /** * Close the current document. */ diff --git a/app/server/lib/ActiveDocImport.ts b/app/server/lib/ActiveDocImport.ts index 9f527ae4a0..61108a2c0f 100644 --- a/app/server/lib/ActiveDocImport.ts +++ b/app/server/lib/ActiveDocImport.ts @@ -10,10 +10,10 @@ import {ApplyUAResult, DataSourceTransformed, ImportOptions, ImportResult, Impor TransformRuleMap} from 'app/common/ActiveDocAPI'; import {ApiError} from 'app/common/ApiError'; import {BulkColValues, CellValue, fromTableDataAction, UserAction} from 'app/common/DocActions'; +import {DocStateComparison} from 'app/common/DocState'; import {isBlankValue} from 'app/common/gristTypes'; import * as gutil from 'app/common/gutil'; import {localTimestampToUTC} from 'app/common/RelativeDates'; -import {DocStateComparison} from 'app/common/UserAPI'; import {guessColInfoForImports} from 'app/common/ValueGuesser'; import {ParseFileResult, ParseOptions} from 'app/plugin/FileParserAPI'; import {GristColumn, GristTable} from 'app/plugin/GristTable'; diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 3fbb94a3b2..6e6b2213eb 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -12,6 +12,11 @@ import { UserAction } from 'app/common/DocActions'; import {DocData} from 'app/common/DocData'; +import { + DocState, + DocStateComparison, + DocStates, +} from 'app/common/DocState'; import { extractTypeFromColType, getReferencedTableId, @@ -32,9 +37,7 @@ import { ArchiveUploadResult, CreatableArchiveFormats, DocReplacementOptions, - DocState, - DocStateComparison, - DocStates, + FullUser, NEW_DOCUMENT_CODE } from 'app/common/UserAPI'; import {Document} from "app/gen-server/entity/Document"; @@ -1344,62 +1347,13 @@ export class DocWorkerApi { nullable: true, isValid: (n) => n > 0, }); - const docSession = docSessionFromRequest(req); - const {states} = await this._getStates(docSession, activeDoc); - const ref = await fetch(this._grist.getHomeInternalUrl(`/api/docs/${req.params.docId2}/states`), { - headers: { - ...getTransitiveHeaders(req, { includeOrigin: false }), - 'Content-Type': 'application/json', - } + const docId2 = req.params.docId2; + const comp = await this._compareDoc(req, activeDoc, { + showDetails, + docId2, + maxRows, }); - if (!ref.ok) { - res.status(ref.status).send(await ref.text()); - return; - } - const states2: DocState[] = (await ref.json()).states; - const left = states[0]; - const right = states2[0]; - if (!left || !right) { - // This should not arise unless there's a bug. - throw new Error('document with no history'); - } - const rightHashes = new Set(states2.map(state => state.h)); - const parent = states.find(state => rightHashes.has(state.h )) || null; - const leftChanged = parent && parent.h !== left.h; - const rightChanged = parent && parent.h !== right.h; - const summary = leftChanged ? (rightChanged ? 'both' : 'left') : - (rightChanged ? 'right' : (parent ? 'same' : 'unrelated')); - const comparison: DocStateComparison = { - left, right, parent, summary - }; - if (showDetails && parent) { - // Calculate changes from the parent to the current version of this document. - const leftChanges = ( - await this._getChanges(activeDoc, { - states, - leftHash: parent.h, - rightHash: "HEAD", - maxRows, - }) - ).details!.rightChanges; - - // Calculate changes from the (common) parent to the current version of the other document. - let url = `/api/docs/${req.params.docId2}/compare?left=${parent.h}`; - if (maxRows !== undefined) { - url += `&maxRows=${maxRows}`; - } - const rightChangesReq = await fetch(this._grist.getHomeInternalUrl(url), { - headers: { - ...getTransitiveHeaders(req, { includeOrigin: false }), - 'Content-Type': 'application/json', - } - }); - const rightChanges = (await rightChangesReq.json()).details!.rightChanges; - - // Add the left and right changes as details to the result. - comparison.details = { leftChanges, rightChanges }; - } - res.json(comparison); + res.json(comp); })); // Give details about what changed between two versions of a document. @@ -1423,6 +1377,66 @@ export class DocWorkerApi { ); })); + this._app.post('/api/docs/:docId/propose', canEdit, withDoc(async (activeDoc, req, res) => { + const urlId = activeDoc.docName; + const parts = parseUrlId(urlId || ''); + const retracted = Boolean(req.body.retracted); + if (urlId && parts.trunkId && parts.forkId) { + const comparisonUrlId = parts.trunkId; + const comp = await this._compareDoc(req, activeDoc, { + showDetails: true, + docId2: comparisonUrlId, + maxRows: null, + }); + await this._dbManager.setProposal({ + srcDocId: parts.forkId, + destDocId: parts.trunkId, + comparison: comp, + retracted + }); + res.json(comp); + return; + } + res.json(null); + })); + + this._app.get('/api/docs/:docId/proposals', canView, withDoc(async (activeDoc, req, res) => { + const docSession = docSessionFromRequest(req); + if (!await activeDoc.canCopyEverything(docSession)) { + throw new ApiError('access denied', 400); + } + const parsed = parseUrlId(activeDoc.docName); + const docId = parsed.forkId || parsed.trunkId; + const isOutgoing = optBooleanParam(req.query.outgoing, 'outgoing'); + const options = isOutgoing ? { + srcDocId: docId, + } : { + destDocId: docId, + }; + const result = await this._dbManager.getProposals(options); + const owner = await activeDoc.isOwner(docSession); + if (!owner) { + for (const proposal of result) { + const creator = proposal.srcDoc.creator as any as FullUser; + if (creator.anonymous) { + proposal.srcDocId = 'hidden'; + proposal.srcDoc.id = 'hidden'; + } + } + } + await sendReply(req, res, { + data: { proposals: result }, + status: 200 + }); + })); + + this._app.post('/api/docs/:docId/proposals/:proposalId/apply', canEdit, withDoc(async (activeDoc, req, res) => { + const proposalId = integerParam(req.params.proposalId, 'proposalId'); + const docSession = docSessionFromRequest(req); + const changes = activeDoc.applyProposal(docSession, proposalId); + await sendReply(req, res, {data: {proposalId, changes}, status: 200}); + })); + // Do an import targeted at a specific workspace. Although the URL fits ApiServer, this // endpoint is handled only by DocWorker, so is handled here. // This endpoint either uploads a new file to import, or accepts an existing uploadId. @@ -2564,6 +2578,71 @@ export class DocWorkerApi { }, }); } + + private async _compareDoc(req: RequestWithLogin, activeDoc: ActiveDoc, + options: { + showDetails: boolean, + docId2: string, + maxRows: number|null|undefined, + }) { + const {showDetails, docId2, maxRows} = options; + const docSession = docSessionFromRequest(req); + const {states} = await this._getStates(docSession, activeDoc); + const ref = await fetch(this._grist.getHomeInternalUrl(`/api/docs/${docId2}/states`), { + headers: { + ...getTransitiveHeaders(req, { includeOrigin: false }), + 'Content-Type': 'application/json', + } + }); + if (!ref.ok) { + throw new ApiError(await ref.text(), ref.status); + } + const states2: DocState[] = (await ref.json()).states; + const left = states[0]; + const right = states2[0]; + if (!left || !right) { + // This should not arise unless there's a bug. + throw new Error('document with no history'); + } + const rightHashes = new Set(states2.map(state => state.h)); + const parent = states.find(state => rightHashes.has(state.h )) || null; + const leftChanged = parent && parent.h !== left.h; + const rightChanged = parent && parent.h !== right.h; + const summary = leftChanged ? (rightChanged ? 'both' : 'left') : + (rightChanged ? 'right' : (parent ? 'same' : 'unrelated')); + const comparison: DocStateComparison = { + left, right, parent, summary + }; + if (showDetails && parent) { + // Calculate changes from the parent to the current version of this document. + const leftChanges = ( + await this._getChanges(activeDoc, { + states, + leftHash: parent.h, + rightHash: "HEAD", + maxRows, + }) + ).details!.rightChanges; + + // Calculate changes from the (common) parent to the current version of the other document. + let url = `/api/docs/${docId2}/compare?left=${parent.h}`; + if (maxRows !== undefined) { + url += `&maxRows=${maxRows}`; + } + const rightChangesReq = await fetch(this._grist.getHomeInternalUrl(url), { + headers: { + ...getTransitiveHeaders(req, { includeOrigin: false }), + 'Content-Type': 'application/json', + } + }); + const rightChanges = (await rightChangesReq.json()).details!.rightChanges; + + // Add the left and right changes as details to the result. + comparison.details = { leftChanges, rightChanges }; + } + + return comparison; + } } export function addDocApiRoutes( diff --git a/app/server/lib/DocWorker.ts b/app/server/lib/DocWorker.ts index 48633cf361..a5a9762ac9 100644 --- a/app/server/lib/DocWorker.ts +++ b/app/server/lib/DocWorker.ts @@ -137,6 +137,7 @@ export class DocWorker { stopTiming: activeDocMethod.bind(null, 'owners', 'stopTiming'), getAssistantState: activeDocMethod.bind(null, 'owners', 'getAssistantState'), listActiveUserProfiles: activeDocMethod.bind(null, null, 'listActiveUserProfiles'), + applyProposal: activeDocMethod.bind(null, 'owners', 'applyProposal'), }); } diff --git a/app/server/lib/HashUtil.ts b/app/server/lib/HashUtil.ts index 43ab4ccb3a..254329b7cd 100644 --- a/app/server/lib/HashUtil.ts +++ b/app/server/lib/HashUtil.ts @@ -1,4 +1,4 @@ -import { DocState } from 'app/common/UserAPI'; +import { DocState } from 'app/common/DocState'; /** * diff --git a/app/server/lib/Patch.ts b/app/server/lib/Patch.ts new file mode 100644 index 0000000000..462c774903 --- /dev/null +++ b/app/server/lib/Patch.ts @@ -0,0 +1,184 @@ +/** + * + * A really bad tiny implementation of daff (tabular diff tool) to apply changes. + * Very incomplete and naive and bad. + * + */ + +import { TableDelta } from 'app/common/ActionSummary'; +import { PatchItem, PatchLog } from 'app/common/ActiveDocAPI'; +import { UserAction } from 'app/common/DocActions'; +import { DocStateComparisonDetails } from 'app/common/DocState'; +import { ActiveDoc } from 'app/server/lib/ActiveDoc'; +import { OptDocSession } from 'app/server/lib/DocSession'; + +export class Patch { + private _otherId: number|undefined; + private _linkId: number|undefined; + + public constructor(public gristDoc: ActiveDoc, public docSession: OptDocSession) {} + + public async applyChanges(details: DocStateComparisonDetails): Promise { + const changes: PatchItem[] = []; + try { + const summary = details.leftChanges; + if (summary.tableRenames.length > 0) { + throw new Error('table-level changes cannot be handled yet'); + } + for (const [, delta] of Object.entries(summary.tableDeltas)) { + if (delta.columnRenames.length > 0) { + throw new Error('column-level changes cannot be handled yet'); + } + } + for (const [tableId, delta] of Object.entries(summary.tableDeltas)) { + if (tableId.startsWith('_grist_')) { + continue; + } + if (delta.columnRenames.length > 0) { + changes.push({ + msg: 'column renames ignored', + }); + } + if (delta.removeRows.length > 0) { + changes.push(...await this.removeRows(tableId, delta)); + } + if (delta.updateRows.length > 0) { + changes.push(...await this.updateRows(tableId, delta)); + } + if (delta.addRows.length > 0) { + changes.push(...await this.addRows(tableId, delta)); + } + } + } catch (e) { + changes.push({ + msg: String(e), + fail: true, + }); + } + const applied = changes.some(change => !change.fail); + return {changes, applied}; + } + + public async change(delta: TableDelta, tableId: string, rowId: number, colId: string, + pre: any, post: any): Promise { + await this.applyUserActions([ + ['UpdateRecord', tableId, rowId, { [colId]: post }], + ]); + // delta.accepted ||= {}; + // delta.accepted.updateRows ||= []; + // delta.accepted.updateRows.push(rowId); + return { + msg: 'did an update', + }; + } + + public async applyUserActions(actions: UserAction[]) { + const result = await this.gristDoc.applyUserActions( + this.docSession, actions, { + otherId: this._otherId, + linkId: this._linkId, + } + ); + if (!this._otherId) { + this._otherId = result.actionNum; + } + this._linkId = result.actionNum; + } + + public async doAdd(delta: TableDelta, tableId: string, rowId: number, rec: Record): Promise { + if (rec.manualSort) { + delete rec.manualSort; + } + await this.applyUserActions([ + ['AddRecord', tableId, null, rec], + ]); + // delta.accepted ||= {}; + // delta.accepted.addRows ||= []; + // delta.accepted.addRows.push(rowId); + return { + msg: 'did an add', + }; + } + + public async doRemove(delta: TableDelta, tableId: string, rowId: number, + rec: Record): Promise { + await this.applyUserActions([ + ['RemoveRecord', tableId, rowId], + ]); + //delta.accepted ||= {}; + //delta.accepted.removeRows ||= []; + //delta.accepted.removeRows.push(rowId); + return { + msg: 'did a remove', + }; + } + + public async updateRows(tableId: string, delta: TableDelta): Promise { + const changes: PatchItem[] = []; + const rows = remaining(delta.updateRows, undefined); //, delta.accepted?.updateRows); + const columnDeltas = delta.columnDeltas; + for (const row of rows) { + for (const [colId, columnDelta] of Object.entries(columnDeltas)) { + const cellDelta = columnDelta[row]; + if (!cellDelta) { + changes.push({ + msg: 'there is a row that does not exist anymore', + }); + continue; + } + const pre = cellDelta[0]?.[0]; + const post = cellDelta[1]?.[0]; + changes.push(await this.change(delta, tableId, row, colId, pre, post)); + } + } + return changes; + } + + public async addRows(tableId: string, delta: TableDelta): Promise { + const changes: PatchItem[] = []; + const rows = remaining(delta.addRows, undefined); //delta.accepted?.addRows); + const columnDeltas = delta.columnDeltas; + for (const row of rows) { + const rec: Record = {}; + for (const [colId, columnDelta] of Object.entries(columnDeltas)) { + const cellDelta = columnDelta[row]; + if (!cellDelta) { + changes.push({ + msg: 'there is a row that does not exist anymore', + }); + continue; + } + rec[colId] = cellDelta[1]?.[0]; + } + changes.push(await this.doAdd(delta, tableId, row, rec)); + } + return changes; + } + + public async removeRows(tableId: string, delta: TableDelta): Promise { + const changes: PatchItem[] = []; + const rows = remaining(delta.removeRows, undefined); //delta.accepted?.removeRows); + const columnDeltas = delta.columnDeltas; + for (const row of rows) { + const rec: Record = {}; + for (const [colId, columnDelta] of Object.entries(columnDeltas)) { + const cellDelta = columnDelta[row]; + if (!cellDelta) { + changes.push({ + msg: 'there is a row that does not exist anymore', + }); + continue; + } + rec[colId] = cellDelta[0]?.[0]; + } + changes.push(await this.doRemove(delta, tableId, row, rec)); + } + return changes; + } +} + + +function remaining(proposed: number[], accepted: number[]|undefined): number[] { + const a = new Set(accepted); + return proposed.filter(n => !a.has(n)); +} diff --git a/app/server/lib/requestUtils.ts b/app/server/lib/requestUtils.ts index 452db86559..f530458d14 100644 --- a/app/server/lib/requestUtils.ts +++ b/app/server/lib/requestUtils.ts @@ -360,6 +360,8 @@ export function optBooleanParam(p: any, name: string): boolean|undefined { export function booleanParam(p: any, name: string): boolean { if (typeof p === 'boolean') { return p; } + if (gutil.isAffirmative(p)) { return true; } + if (gutil.isNotAffirmative(p)) { return false; } throw new ApiError(`${name} parameter should be a boolean: ${p}`, 400); } diff --git a/test/nbrowser/ProposedChangesPage.ts b/test/nbrowser/ProposedChangesPage.ts index c2a6e410c2..761244f3e9 100644 --- a/test/nbrowser/ProposedChangesPage.ts +++ b/test/nbrowser/ProposedChangesPage.ts @@ -6,22 +6,63 @@ describe('ProposedChangesPage', function() { this.timeout(60000); const cleanup = setupTestSuite(); - it('show comparison and functions as expected', async function() { + it('can be enabled experimentally for a document', async function() { const session = await gu.session().teamSite.login(); - // const api = session.createHomeApi(); await session.tempDoc(cleanup, 'Hello.grist'); - // Open the share menu and click item to work on a copy. + await driver.find('.test-tools-settings').click(); + const url = await driver.getCurrentUrl(); + await driver.get(url + '?experiment=proposedChangesPage'); + await driver.findWait('.test-modal-confirm', 2000).click(); + await driver.findWait('.test-modal-confirm', 2000).click(); + assert.match( + await driver.findWait('#admin-panel-item-description-acceptProposals', 2000).getText(), + /Allow others to propose changes/); + assert.equal( + await driver.find('input.test-settings-accept-proposals').getAttribute('checked'), + null + ); + await driver.find('input.test-settings-accept-proposals').click(); + await driver.findWait('.test-tools-proposals', 2000); + assert.equal( + await driver.find('input.test-settings-accept-proposals').getAttribute('checked'), + 'true' + ); + }); + + it('show comparison and functions as expected', async function() { + // Load a test document. + const session = await gu.session().teamSite.login(); + const doc = await session.tempDoc(cleanup, 'Hello.grist'); + + // Turn on feature. + const api = session.createHomeApi(); + await api.updateDoc(doc.id, { + options: { + proposedChanges: { + acceptProposals: true + } + } + }); + + // Put something known in the first cell. await gu.getCell('A', 1).click(); await gu.waitAppFocus(); await gu.enterCell('test1'); + + // Work on a copy. await driver.find('.test-tb-share').click(); await driver.find('.test-work-on-copy').click(); await gu.waitForServer(); + + // Change the content of the first cell. await gu.getCell('A', 1).click(); await gu.waitAppFocus(); await gu.enterCell('test2'); + + assert.equal(await driver.find('.test-tools-proposals').getText(), + 'Propose Changes'); await driver.find('.test-tools-proposals').click(); - await driver.findContentWait('.test-main-content', /Proposed Changes/, 2000); + await driver.findContentWait('.test-main-content', /Propose Changes/, 2000); await driver.findWait('.action_log_table', 2000); assert.lengthOf(await driver.findAll('.action_log_table'), 1); assert.equal( @@ -43,14 +84,40 @@ describe('ProposedChangesPage', function() { await driver.find('.action_log_table tr:nth-of-type(2)').getText(), '→ 1\ntest1test2\nTEST1TEST2' ); - await driver.find('.test-replace').click(); - const confirmButton = driver.findWait('.test-modal-confirm', 3000); - assert.equal(await confirmButton.getText(), 'Update'); - await confirmButton.click(); - - // check we're no longer on a fork, but still have the change made on the fork. - await gu.waitForUrl(/^[^~]*$/, 6000); - await gu.waitForDocToLoad(); + assert.match(await driver.find('.test-proposals-propose').getText(), /Propose/); + await driver.find('.test-proposals-propose').click(); + await driver.findContentWait('.test-proposals-status', /Proposed/, 2000); + assert.match(await driver.find('.test-proposals-propose').getText(), /Update/); + await driver.findWait('.test-proposals-retract', 2000).click(); + await driver.findContentWait('.test-proposals-status', /Retracted/, 2000); + assert.match(await driver.find('.test-proposals-propose').getText(), /Propose/); + await driver.find('.test-proposals-propose').click(); + await driver.findContentWait('.test-proposals-status', /Proposed/, 2000); + + await driver.findContentWait('span', /original document/, 2000).click(); + + assert.match( + await driver.findContentWait('.test-proposals-header', /# 1/, 2000).getText(), + /Proposed/ + ); + + assert.lengthOf(await driver.findAll('.test-proposals-header'), 1); + + await driver.findContent('span.action_log_cell_add', /test2/).click(); + + await driver.findContentWait('.test-widget-title-text', /TABLE1/, 2000); + assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), 'test1'); + assert.equal(await driver.find('.test-tools-proposals').getText(), + 'Proposed Changes'); + await driver.find('.test-tools-proposals').click(); + + await driver.findWait('.test-proposals-apply', 2000).click(); + await gu.waitForServer(); + await driver.findContent('span.action_log_cell_add', /test2/).click(); + await driver.findContentWait('.test-widget-title-text', /TABLE1/, 2000); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), 'test2'); + + // There's a formula column error, can't write to it. + // Need to deal with this (and other column types) earlier... }); }); diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 30ec7f2688..ce969d8aba 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -15,11 +15,12 @@ import * as PluginApi from 'app/plugin/grist-plugin-api'; import { BaseAPI } from 'app/common/BaseAPI'; import {csvDecodeRow} from 'app/common/csvFormat'; import { AccessLevel } from 'app/common/CustomWidget'; +import { DocStateComparison } from 'app/common/DocState'; import { decodeUrl } from 'app/common/gristUrls'; import { FullUser, UserProfile } from 'app/common/LoginSessionAPI'; import { resetOrg } from 'app/common/resetOrg'; import { TestState } from 'app/common/TestState'; -import { Organization as APIOrganization, DocStateComparison, +import { Organization as APIOrganization, UserAPI, UserAPIImpl, Workspace } from 'app/common/UserAPI'; import { Organization } from 'app/gen-server/entity/Organization'; import { Product } from 'app/gen-server/entity/Product'; diff --git a/test/server/lib/ActionHistory.ts b/test/server/lib/ActionHistory.ts index 40e6a9c380..79e0d471d8 100644 --- a/test/server/lib/ActionHistory.ts +++ b/test/server/lib/ActionHistory.ts @@ -1,6 +1,6 @@ import {LocalActionBundle} from 'app/common/ActionBundle'; import {ActionGroup, MinimalActionGroup} from 'app/common/ActionGroup'; -import {DocState} from 'app/common/UserAPI'; +import {DocState} from 'app/common/DocState'; import {ActionGroupOptions, ActionHistory, ActionHistoryUndoInfo, asActionGroup, asMinimalActionGroup} from 'app/server/lib/ActionHistory'; import {ActionHistoryImpl, computeActionHash} from 'app/server/lib/ActionHistoryImpl'; diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index d95fa0481c..e5c4eead16 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -1,10 +1,11 @@ /* eslint-disable @typescript-eslint/no-shadow */ import {ActionSummary} from 'app/common/ActionSummary'; import {BulkColValues, UserAction} from 'app/common/DocActions'; +import {DocState} from 'app/common/DocState'; import {SHARE_KEY_PREFIX} from 'app/common/gristUrls'; import {arrayRepeat} from 'app/common/gutil'; import {WebhookSummary} from 'app/common/Triggers'; -import {DocAPI, DocState, UserAPIImpl} from 'app/common/UserAPI'; +import {DocAPI, UserAPIImpl} from 'app/common/UserAPI'; import {AddOrUpdateRecord, Record as ApiRecord, ColumnsPut, RecordWithStringId} from 'app/plugin/DocApiTypes'; import {CellValue, GristObjCode} from 'app/plugin/GristData'; import {Deps} from 'app/server/lib/ActiveDoc'; diff --git a/test/server/lib/HashUtil.ts b/test/server/lib/HashUtil.ts index 321242d1b9..9ad42bfdc9 100644 --- a/test/server/lib/HashUtil.ts +++ b/test/server/lib/HashUtil.ts @@ -1,4 +1,4 @@ -import { DocState } from 'app/common/UserAPI'; +import { DocState } from 'app/common/DocState'; import { HashUtil } from 'app/server/lib/HashUtil'; import { assert } from 'chai';