From f110ffdafd3dc7afc9918f18631bceeb24b5ea71 Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Thu, 27 Jan 2022 19:51:37 +0200 Subject: [PATCH] (core) Follow chain of same-record links for getDefaultColValues Summary: When two widgets are linked by same-record linking, and the source of that link is also filter-linked, then it will pick up default values from its own filter-link source, but the same-record-link target didn't. This fixes that so that default values are filled in intuitively. Moved the logic of linkingState, linkingFilter, and getDefaultColValues from BaseView.js to LinkingState.ts and ViewSectionRec.ts. In particular getDefaultColValues is now a property of LinkingState which may be copied from the source view section for a same-record link. Note that `ViewSectionRec.linkingFilter` no longer uses `computerBuilder` and thus doesn't ignore dependencies inside LinkingState any more. I couldn't figure out how to make `linkingFilter` a `pureComputed` (otherwise I get recursion errors) that ignores dependencies. In any case, it's now important to have a dependency on `srcSection.linkingState()` for `getDefaultColValues` to work correctly, so I think this is for the best. Test Plan: Added a new nbrowser test and fixture. Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3238 --- app/client/components/BaseView.js | 49 ++++------------- app/client/components/DetailView.js | 2 +- app/client/components/LinkingState.ts | 42 ++++++++++++--- app/client/models/entities/ViewSectionRec.ts | 56 ++++++++++++++++---- app/common/ActiveDocAPI.ts | 2 +- 5 files changed, 91 insertions(+), 60 deletions(-) diff --git a/app/client/components/BaseView.js b/app/client/components/BaseView.js index c9124697..ce54761d 100644 --- a/app/client/components/BaseView.js +++ b/app/client/components/BaseView.js @@ -4,7 +4,6 @@ var moment = require('moment-timezone'); var {getSelectionDesc} = require('app/common/DocActions'); var {nativeCompare, roundDownToMultiple, waitObs} = require('app/common/gutil'); var gristTypes = require('app/common/gristTypes'); -var koUtil = require('../lib/koUtil'); var tableUtil = require('../lib/tableUtil'); var {DataRowModel} = require('../models/DataRowModel'); var {DynamicQuerySet} = require('../models/QuerySet'); @@ -15,7 +14,6 @@ var {Cursor} = require('./Cursor'); var FieldBuilder = require('../widgets/FieldBuilder'); var commands = require('./commands'); var BackboneEvents = require('backbone').Events; -const {LinkingState} = require('./LinkingState'); const {ClientColumnGetters} = require('app/client/models/ClientColumnGetters'); const {reportError, reportSuccess} = require('app/client/models/errors'); const {urlState} = require('app/client/models/gristUrlState'); @@ -24,8 +22,6 @@ const {copyToClipboard} = require('app/client/lib/copyToClipboard'); const {setTestState} = require('app/client/lib/testState'); const {ExtraRows} = require('app/client/models/DataTableModelWithDiff'); const {createFilterMenu} = require('app/client/ui/ColumnFilterMenu'); -const {LinkConfig} = require('app/client/ui/selectBy'); -const {encodeObject} = require("app/plugin/objtypes"); /** * BaseView forms the basis for ViewSection classes. @@ -127,36 +123,9 @@ function BaseView(gristDoc, viewSectionModel, options) { //-------------------------------------------------- // Prepare logic for linking with other sections. - // Linking state maintains .filterFunc and .cursorPos observables which we use for - // auto-scrolling and filtering. - this._linkingState = this.autoDispose(koUtil.computedBuilder(() => { - let v = this.viewSection; - let src = v.linkSrcSection(); - if (!src.getRowId()) { - return null; - } - try { - const config = new LinkConfig(v); - return LinkingState.create.bind(LinkingState, null, this.gristDoc, config); - } catch (err) { - console.warn(`Can't create LinkingState: ${err.message}`); - return null; - } - })); - - this._linkingFilter = this.autoDispose(ko.computed(() => { - const linking = this._linkingState(); - const result = linking && linking.filterColValues ? linking.filterColValues() : {filters: {}}; - result.operations = result.operations || {}; - for (const key in result.filters) { - result.operations[key] = result.operations[key] || 'in'; - } - return result; - })); - // A computed for the rowId of the row selected by section linking. this.linkedRowId = this.autoDispose(ko.computed(() => { - let linking = this._linkingState(); + let linking = this.viewSection.linkingState(); return linking && linking.cursorPos ? linking.cursorPos() : null; }).extend({deferred: true})); @@ -165,7 +134,7 @@ function BaseView(gristDoc, viewSectionModel, options) { // Indicated whether editing the section should be disabled given the current linking state. this.disableEditing = this.autoDispose(ko.computed(() => { - const linking = this._linkingState(); + const linking = this.viewSection.linkingState(); return linking && linking.disableEditing(); })); @@ -219,7 +188,7 @@ function BaseView(gristDoc, viewSectionModel, options) { // dependency changes. this.autoDispose(ko.computed(() => { this._isLoading(true); - const linkingFilter = this._linkingFilter(); + const linkingFilter = this.viewSection.linkingFilter(); this._queryRowSource.makeQuery(linkingFilter.filters, linkingFilter.operations, (err) => { if (this.isDisposed()) { return; } if (err) { reportError(err); } @@ -228,7 +197,7 @@ function BaseView(gristDoc, viewSectionModel, options) { })); // Reset cursor to the first row when filtering changes. - this.autoDispose(this._linkingFilter.subscribe((x) => this.onLinkFilterChange())); + this.autoDispose(this.viewSection.linkingFilter.subscribe((x) => this.onLinkFilterChange())); // When sorting changes, reset the cursor to the first row. (The alternative of moving the // cursor to stay at the same record is sometimes better, but sometimes more annoying.) @@ -408,11 +377,11 @@ BaseView.prototype._parsePasteForView = function(data, fields) { }; BaseView.prototype._getDefaultColValues = function() { - const {filters, operations} = this._linkingFilter.peek(); - return _.mapObject( - _.pick(filters, (value, key) => value.length > 0 && key !== "id"), - (value, key) => operations[key] === "intersects" ? encodeObject(value) : value[0] - ); + const linkingState = this.viewSection.linkingState.peek(); + if (!linkingState) { + return {}; + } + return linkingState.getDefaultColValues(); }; /** diff --git a/app/client/components/DetailView.js b/app/client/components/DetailView.js index f617223d..c5577f7b 100644 --- a/app/client/components/DetailView.js +++ b/app/client/components/DetailView.js @@ -321,7 +321,7 @@ DetailView.prototype.buildTitleControls = function() { // Note that the controls should still be visible with a filter link. const showControls = ko.computed(() => { if (!this._isSingle || this.recordLayout.layoutEditor()) { return false; } - const linkingState = this._linkingState(); + const linkingState = this.viewSection.linkingState(); return !(linkingState && Boolean(linkingState.cursorPos)); }); return dom('div', diff --git a/app/client/components/LinkingState.ts b/app/client/components/LinkingState.ts index 05b4068c..c71d3d9b 100644 --- a/app/client/components/LinkingState.ts +++ b/app/client/components/LinkingState.ts @@ -1,6 +1,6 @@ -import {GristDoc} from "app/client/components/GristDoc"; import {DataRowModel} from "app/client/models/DataRowModel"; import * as DataTableModel from "app/client/models/DataTableModel"; +import {DocModel} from 'app/client/models/DocModel'; import {ColumnRec} from "app/client/models/entities/ColumnRec"; import {TableRec} from "app/client/models/entities/TableRec"; import {ViewSectionRec} from "app/client/models/entities/ViewSectionRec"; @@ -9,9 +9,12 @@ import {LinkConfig} from "app/client/ui/selectBy"; import {ClientQuery, QueryOperation} from "app/common/ActiveDocAPI"; import {isList, isRefListType} from "app/common/gristTypes"; import * as gutil from "app/common/gutil"; +import {encodeObject} from 'app/plugin/objtypes'; import {Disposable} from "grainjs"; import * as ko from "knockout"; -import * as _ from "underscore"; +import mapValues = require('lodash/mapValues'); +import pickBy = require('lodash/pickBy'); +import identity = require('lodash/identity'); /** @@ -31,7 +34,7 @@ function isSummaryOf(summary: TableRec, detail: TableRec): boolean { gutil.isSubset(summary.summarySourceColRefs(), detail.summarySourceColRefs())); } -type FilterColValues = Pick; +export type FilterColValues = Pick; /** * Maintains state useful for linking sections, i.e. auto-filtering and auto-scrolling. @@ -65,18 +68,21 @@ export class LinkingState extends Disposable { // {[colId]: colValues} mapping, with a dependency on srcSection.activeRowId() public readonly filterColValues?: ko.Computed; + // Get default values for a new record so that it continues to satisfy the current linking filters + public readonly getDefaultColValues: () => any; + private _srcSection: ViewSectionRec; private _srcTableModel: DataTableModel; private _srcCol: ColumnRec; private _srcColId: string | undefined; - constructor(gristDoc: GristDoc, linkConfig: LinkConfig) { + constructor(docModel: DocModel, linkConfig: LinkConfig) { super(); const {srcSection, srcCol, srcColId, tgtSection, tgtCol, tgtColId} = linkConfig; this._srcSection = srcSection; this._srcCol = srcCol; this._srcColId = srcColId; - this._srcTableModel = gristDoc.getTableModel(srcSection.table().tableId()); + this._srcTableModel = docModel.dataTables[srcSection.table().tableId()]; const srcTableData = this._srcTableModel.tableData; if (tgtColId) { @@ -103,10 +109,11 @@ export class LinkingState extends Disposable { const colId = col.colId(); const srcValue = srcTableData.getValue(srcRowId as number, colId); result.filters[colId] = [srcValue]; + result.operations[colId] = 'in'; if (isDirectSummary) { const tgtColType = col.type(); if (tgtColType === 'ChoiceList' || tgtColType.startsWith('RefList:')) { - result.operations![colId] = 'intersects'; + result.operations[colId] = 'intersects'; } } } @@ -116,12 +123,33 @@ export class LinkingState extends Disposable { // TODO: We should move the cursor, but don't currently it for summaries. For that, we need a // column or map representing the inverse of summary table's "group" column. } else { - const srcValueFunc = srcColId ? this._makeSrcCellGetter() : _.identity; + const srcValueFunc = srcColId ? this._makeSrcCellGetter() : identity; if (srcValueFunc) { this.cursorPos = this.autoDispose(ko.computed(() => srcValueFunc(srcSection.activeRowId()) as RowId )); } + + if (!srcColId) { + // This is a same-record link: copy getDefaultColValues from the source if possible + const getDefaultColValues = srcSection.linkingState()?.getDefaultColValues; + if (getDefaultColValues) { + this.getDefaultColValues = getDefaultColValues; + } + } + } + + if (!this.getDefaultColValues) { + this.getDefaultColValues = () => { + if (!this.filterColValues) { + return {}; + } + const {filters, operations} = this.filterColValues.peek(); + return mapValues( + pickBy(filters, (value: any[], key: string) => value.length > 0 && key !== "id"), + (value, key) => operations[key] === "intersects" ? encodeObject(value) : value[0] + ); + }; } } diff --git a/app/client/models/entities/ViewSectionRec.ts b/app/client/models/entities/ViewSectionRec.ts index 64c873e9..0a677328 100644 --- a/app/client/models/entities/ViewSectionRec.ts +++ b/app/client/models/entities/ViewSectionRec.ts @@ -1,16 +1,27 @@ import * as BaseView from 'app/client/components/BaseView'; -import { ColumnRec, FilterRec, TableRec, ViewFieldRec, ViewRec } from 'app/client/models/DocModel'; +import {CursorPos} from 'app/client/components/Cursor'; +import {FilterColValues, LinkingState} from 'app/client/components/LinkingState'; +import {KoArray} from 'app/client/lib/koArray'; +import { + ColumnRec, + DocModel, + FilterRec, + IRowModel, + recordSet, + refRecord, + TableRec, + ViewFieldRec, + ViewRec +} from 'app/client/models/DocModel'; import * as modelUtil from 'app/client/models/modelUtil'; +import {RowId} from 'app/client/models/rowset'; +import {LinkConfig} from 'app/client/ui/selectBy'; +import {getWidgetTypes} from 'app/client/ui/widgetTypes'; import {AccessLevel, ICustomWidget} from 'app/common/CustomWidget'; +import {arrayRepeat} from 'app/common/gutil'; +import {Sort} from 'app/common/SortSpec'; +import {Computed} from 'grainjs'; import * as ko from 'knockout'; -import { CursorPos, } from 'app/client/components/Cursor'; -import { KoArray, } from 'app/client/lib/koArray'; -import { DocModel, IRowModel, recordSet, refRecord, } from 'app/client/models/DocModel'; -import { RowId, } from 'app/client/models/rowset'; -import { getWidgetTypes, } from 'app/client/ui/widgetTypes'; -import { arrayRepeat, } from 'app/common/gutil'; -import { Sort, } from 'app/common/SortSpec'; -import { Computed, } from 'grainjs'; import defaults = require('lodash/defaults'); // Represents a section of user views, now also known as a "page widget" (e.g. a view may contain @@ -98,10 +109,16 @@ export interface ViewSectionRec extends IRowModel<"_grist_Views_section"> { linkSrcCol: ko.Computed; linkTargetCol: ko.Computed; - activeRowId: ko.Observable; // May be null when there are no rows. + // Linking state maintains .filterFunc and .cursorPos observables which we use for + // auto-scrolling and filtering. + linkingState: ko.Computed; + + linkingFilter: ko.Computed; + + activeRowId: ko.Observable; // May be null when there are no rows. // If the view instance for section is instantiated, it will be accessible here. - viewInstance: ko.Observable; + viewInstance: ko.Observable; // Describes the most recent cursor position in the section. Only rowId and fieldIndex are used. lastCursorPos: CursorPos; @@ -425,6 +442,23 @@ export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel): this.activeRowId = ko.observable(null); + this.linkingState = this.autoDispose(ko.pureComputed(() => { + if (!this.linkSrcSection().getRowId()) { + return null; + } + try { + const config = new LinkConfig(this); + return new LinkingState(docModel, config); + } catch (err) { + console.warn(`Can't create LinkingState: ${err.message}`); + return null; + } + })); + + this.linkingFilter = this.autoDispose(ko.pureComputed(() => { + return this.linkingState()?.filterColValues?.() || {filters: {}, operations: {}}; + })); + // If the view instance for this section is instantiated, it will be accessible here. this.viewInstance = ko.observable(null); diff --git a/app/common/ActiveDocAPI.ts b/app/common/ActiveDocAPI.ts index 073332e4..ff485494 100644 --- a/app/common/ActiveDocAPI.ts +++ b/app/common/ActiveDocAPI.ts @@ -99,7 +99,7 @@ interface BaseQuery { * Allows filtering with more complex operations. */ export interface ClientQuery extends BaseQuery { - operations?: { + operations: { [colId: string]: QueryOperation; }; }