From ab4155a81f61763272715476ad32769e695684c8 Mon Sep 17 00:00:00 2001 From: Matthew White Date: Mon, 20 Nov 2023 12:35:13 -0500 Subject: [PATCH 1/3] Show conflicts in entity feed Closes getodk/central#531. Closes getodk/central#532. Closes getodk/central-frontend#816. --- src/assets/scss/_variables.scss | 8 + src/assets/scss/app.scss | 2 +- src/components/entity/diff.vue | 87 +++++++--- src/components/entity/diff/head.vue | 161 ++++++++++++++++++ src/components/entity/diff/row.vue | 104 ++++++++++++ src/components/entity/diff/table.vue | 136 ++++++++++++++++ src/components/entity/feed-entry.vue | 5 +- src/components/entity/show.vue | 1 + src/components/entity/version-link.vue | 64 ++++++++ src/components/feed-entry.vue | 7 +- src/locales/en.json5 | 6 +- src/request-data/entity.js | 8 +- test/components/entity/activity.spec.js | 12 +- test/components/entity/diff.spec.js | 91 +++++++---- test/components/entity/diff/head.spec.js | 77 +++++++++ test/components/entity/diff/row.spec.js | 172 ++++++++++++++++++++ test/components/entity/diff/table.spec.js | 111 +++++++++++++ test/components/entity/feed-entry.spec.js | 17 +- test/components/entity/version-link.spec.js | 61 +++++++ test/data/entities.js | 8 +- transifex/strings_en.json | 83 +++++++++- 21 files changed, 1145 insertions(+), 76 deletions(-) create mode 100644 src/components/entity/diff/head.vue create mode 100644 src/components/entity/diff/row.vue create mode 100644 src/components/entity/diff/table.vue create mode 100644 src/components/entity/version-link.vue create mode 100644 test/components/entity/diff/head.spec.js create mode 100644 test/components/entity/diff/row.spec.js create mode 100644 test/components/entity/diff/table.spec.js create mode 100644 test/components/entity/version-link.spec.js diff --git a/src/assets/scss/_variables.scss b/src/assets/scss/_variables.scss index 4c364d08d..21357c4ee 100644 --- a/src/assets/scss/_variables.scss +++ b/src/assets/scss/_variables.scss @@ -37,7 +37,9 @@ $color-info-light: #d9edf7; $color-info-dark: darken($color-info, 10%); $color-warning: #f29e00; $color-warning-light: #f5c93b; +$color-warning-dark: #e06f0b; $color-danger: #de2a11; +$color-danger-lighter: #dd5454; // Between $color-danger and $color-danger-light $color-danger-light: #ffe6e6; $color-danger-dark: darken($color-danger, 10%); @@ -80,6 +82,9 @@ $padding-top-table-data: 8px; // Panels $box-shadow-panel-main: 0 0 24px rgba(0, 0, 0, 0.25), 0 35px 115px rgba(0, 0, 0, 0.28); +// Navs +$padding-left-nav-tab: 7px; + $color-subpanel-background: #eee; $color-subpanel-active: #ddd; $color-subpanel-border: #e6e6e6; @@ -92,6 +97,9 @@ $ease-extreme-out: cubic-bezier(.05, .9, 0, 1); //////////////////////////////////////////////////////////////////////////////// // COMPONENTS +// FeedEntry +$hpadding-feed-entry: 15px; + // Modal $padding-modal-body: 15px; $padding-left-modal-header: 15px; diff --git a/src/assets/scss/app.scss b/src/assets/scss/app.scss index 835196873..a1ed5d019 100644 --- a/src/assets/scss/app.scss +++ b/src/assets/scss/app.scss @@ -780,7 +780,7 @@ becomes more complicated. border-bottom: 2px solid transparent; border-radius: 0; color: $color-text; - padding: 7px 8px 6px; + padding: $padding-left-nav-tab 8px 6px; } &:focus { diff --git a/src/components/entity/diff.vue b/src/components/entity/diff.vue index b3825e158..32cd902f4 100644 --- a/src/components/entity/diff.vue +++ b/src/components/entity/diff.vue @@ -10,37 +10,78 @@ including this file, may be copied, modified, propagated, or distributed except according to the terms contained in the LICENSE file. --> - - + + + + +{ + "en": { + "noChange": "There are no changes to show." + } +} + diff --git a/src/components/entity/diff/head.vue b/src/components/entity/diff/head.vue new file mode 100644 index 000000000..89e57cb4c --- /dev/null +++ b/src/components/entity/diff/head.vue @@ -0,0 +1,161 @@ + + + + + + + + +{ + "en": { + // {version} and {baseVersion} are version numbers. + "introduction": "This Submission update was applied to version {version} of this Entity, but it was created based on version {baseVersion}.", + "hardConflict": { + "description": "Other updates had already written to the same properties." + }, + "softConflict": { + "title": "Parallel Update", + "description": "No parallel update before this one touched the same properties." + }, + "tab": { + // A comparison between two versions of an Entity, from the point of view + // of the data collector (the author) + "baseDiff": "Author’s View", + // A comparison between two versions of an Entity, from the point of view + // of the Central server + "serverDiff": "Central’s View", + // This text is shown above a comparison of two versions of an Entity. + // {version} is a short version identifier, for example, "v3". + "updating": "(updating {version})" + } + } +} + diff --git a/src/components/entity/diff/row.vue b/src/components/entity/diff/row.vue new file mode 100644 index 000000000..64e82ec9d --- /dev/null +++ b/src/components/entity/diff/row.vue @@ -0,0 +1,104 @@ + + + + + + + + +{ + "en": { + // This is shown for an Entity property that is part of a conflict between + // versions of the Entity + "conflictingProp": "Another update already wrote to this property." + } +} + diff --git a/src/components/entity/diff/table.vue b/src/components/entity/diff/table.vue new file mode 100644 index 000000000..fdafbf795 --- /dev/null +++ b/src/components/entity/diff/table.vue @@ -0,0 +1,136 @@ + + + + + + + + +{ + "en": { + "header": { + // The value of an Entity property in an older version of the Entity + "oldValue": "Old value", + // The value of an Entity property in a newer version of the Entity + "newValue": "New value", + }, + // This is shown when Central displays a comparison of two versions of an + // Entity. + "comparing": "Comparing", + // {version} is a short identifier of an Entity version, for example, "v3". + // {source} indicates who or what created the Entity version, for example, + // "Update by Alice". + "version": "{version} ({source})" + } +} + diff --git a/src/components/entity/feed-entry.vue b/src/components/entity/feed-entry.vue index bcf266251..7e99d9734 100644 --- a/src/components/entity/feed-entry.vue +++ b/src/components/entity/feed-entry.vue @@ -91,13 +91,13 @@ except according to the terms contained in the LICENSE file. + + +{ + "en": { + "submission": "Submission {instanceName}", + "api": "Update by {name}" + } +} + diff --git a/src/components/feed-entry.vue b/src/components/feed-entry.vue index 730593a1a..9be2fa2fe 100644 --- a/src/components/feed-entry.vue +++ b/src/components/feed-entry.vue @@ -38,11 +38,14 @@ defineProps({ $margin-bottom: 20px; .feed-entry { + @include clearfix; box-shadow: 0 7px 18px rgba(0, 0, 0, 0.05); margin-bottom: $margin-bottom; } -.feed-entry-heading, .feed-entry-body .markdown-view { padding: 10px 15px; } +.feed-entry-heading, .feed-entry-body .markdown-view { + padding: 10px $hpadding-feed-entry; +} .feed-entry-heading { background-color: #fff; @@ -80,7 +83,7 @@ $margin-bottom: 20px; } .feed-entry-body { - background-color: #f9f9f9; + background-color: $color-page-background; .markdown-view > p:last-child { margin: 0 0 0px; } } diff --git a/src/locales/en.json5 b/src/locales/en.json5 index 435a9588e..f1abe3d13 100644 --- a/src/locales/en.json5 +++ b/src/locales/en.json5 @@ -357,6 +357,8 @@ "areYouSure": "Are you sure you wish to proceed?", // This is a title shown above a section of the page. "basicInfo": "Basic Information", + "conflict": "Conflict", + "conflicts": "Conflicts", // This is a title shown above a section of the page. "currentDraft": "Your Current Draft", // This is a title shown above a section of the page. @@ -397,7 +399,9 @@ "propertiesCount": "{inform} of {count} property | {inform} of {count} properties", "new": "New!", "totalEntities": "Total Entities", - "conflicts": "Conflicts" + // A short identifier of an Entity version. "v" is short for "version". + // {version} is a version number. + "versionShort": "v{version}" }, "mixin": { "request": { diff --git a/src/request-data/entity.js b/src/request-data/entity.js index 77d07427d..6d2f2f9d8 100644 --- a/src/request-data/entity.js +++ b/src/request-data/entity.js @@ -19,6 +19,12 @@ export default () => { transformResponse: ({ data }) => reactive(data) })); const audits = createResource('audits'); - const entityVersions = createResource('entityVersions'); + const entityVersions = createResource('entityVersions', () => ({ + transformResponse: ({ data }) => { + for (const version of data) + version.conflictingProperties = new Set(version.conflictingProperties); + return data; + } + })); return { entity, audits, entityVersions }; }; diff --git a/test/components/entity/activity.spec.js b/test/components/entity/activity.spec.js index 019048ac1..785cfc5f4 100644 --- a/test/components/entity/activity.spec.js +++ b/test/components/entity/activity.spec.js @@ -11,21 +11,23 @@ import { mockRouter } from '../../util/router'; import { testRequestData } from '../../util/request-data'; import { wait } from '../../util/util'; -const mountComponent = (options = undefined) => - mount(EntityActivity, mergeMountOptions(options, { +const mountComponent = (options = undefined) => { + const entity = testData.extendedEntities.last(); + return mount(EntityActivity, mergeMountOptions(options, { global: { - provide: { projectId: '1', datasetName: 'trees' } + provide: { projectId: '1', datasetName: 'trees', uuid: entity.uuid } }, container: { requestData: testRequestData([useEntity], { - entity: testData.extendedEntities.last(), + entity, audits: testData.extendedAudits.sorted() .filter(({ action }) => action.startsWith('entity.')), entityVersions: testData.extendedEntityVersions.sorted() }), - router: mockRouter('/projects/1/entity-lists/trees/entities/e') + router: mockRouter(`/projects/1/entity-lists/trees/entities/${entity.uuid}`) } })); +}; // Creates an entity with three versions, including a conflict, then resolves // the conflict. const resolveConflict = () => { diff --git a/test/components/entity/diff.spec.js b/test/components/entity/diff.spec.js index 2859cd0b0..154491c0d 100644 --- a/test/components/entity/diff.spec.js +++ b/test/components/entity/diff.spec.js @@ -1,56 +1,89 @@ -import DiffItem from '../../../src/components/diff-item.vue'; +import { last } from 'ramda'; + import EntityDiff from '../../../src/components/entity/diff.vue'; +import EntityDiffHead from '../../../src/components/entity/diff/head.vue'; +import EntityDiffTable from '../../../src/components/entity/diff/table.vue'; import useEntity from '../../../src/request-data/entity'; +import createTestContainer from '../../util/container'; import testData from '../../data'; +import { mockRouter } from '../../util/router'; import { mount } from '../../util/lifecycle'; import { testRequestData } from '../../util/request-data'; -const mountComponent = () => mount(EntityDiff, { - props: { entityVersion: testData.extendedEntityVersions.last() }, - container: { +const mountComponent = () => { + const { uuid } = testData.extendedEntities.last(); + const container = createTestContainer({ requestData: testRequestData([useEntity], { entityVersions: testData.extendedEntityVersions.sorted() - }) - } -}); + }), + router: mockRouter(`/projects/1/entity-lists/trees/entities/${uuid}`) + }); + const entityVersion = last(container.requestData.localResources.entityVersions); + return mount(EntityDiff, { + global: { + provide: { projectId: '1', datasetName: 'trees', uuid, entityVersion } + }, + container + }); +}; describe('EntityDiff', () => { - it('renders a DiffItem for each change', () => { - testData.extendedEntities.createPast(1, { - label: 'dogwood', - data: { height: '1', circumference: '2' } + describe('conflict class', () => { + it('has the correct class if the version is a hard conflict', () => { + testData.extendedEntities.createPast(1, { label: 'foo' }); + testData.extendedEntityVersions.createPast(1, { label: 'bar' }); + testData.extendedEntityVersions.createPast(1, { + baseVersion: 1, + label: 'baz', + conflictingProperties: ['label'] + }); + mountComponent().classes('hard-conflict').should.be.true(); }); - testData.extendedEntityVersions.createPast(1, { - label: 'Dogwood', - data: { height: '3', circumference: '4' } + + it('has the correct class if the version is a soft conflict', () => { + testData.extendedEntities.createPast(1); + testData.extendedEntityVersions.createPast(2, { baseVersion: 1 }); + mountComponent().classes('soft-conflict').should.be.true(); + }); + + it('has the correct class if the version is not a conflict', () => { + testData.extendedEntities.createPast(1); + testData.extendedEntityVersions.createPast(1); + mountComponent().classes().should.eql(['entity-diff']); }); - const component = mountComponent(); - component.findAllComponents(DiffItem).length.should.equal(3); }); - it('passes the correct props to the DiffItem for a property change', () => { + it('renders EntityDiffHead if the version is a conflict', () => { + testData.extendedEntities.createPast(1); + testData.extendedEntityVersions.createPast(2, { baseVersion: 1 }); + mountComponent().findComponent(EntityDiffHead).exists().should.be.true(); + }); + + it('passes the correct diff to EntityDiffTable', async () => { testData.extendedEntities.createPast(1, { + label: 'dogwood', data: { height: '1' } }); + testData.extendedEntityVersions.createPast(1, { label: 'Dogwood' }); testData.extendedEntityVersions.createPast(1, { + baseVersion: 1, + label: 'Dogwood', data: { height: '2' } }); const component = mountComponent(); - const props = component.getComponent(DiffItem).props(); - props.new.should.equal('2'); - props.old.should.equal('1'); - props.path.should.eql(['height']); + const table = component.getComponent(EntityDiffTable); + // Base diff + table.props().diff.should.eql(['label', 'height']); + await component.get('.entity-diff-head li:nth-child(2) a').trigger('click'); + // Server diff + table.props().diff.should.eql(['height']); }); - it('passes the correct props to the DiffItem for a label change', () => { - testData.extendedEntities.createPast(1, { label: 'dogwood' }); - testData.extendedEntityVersions.createPast(1, { label: 'Dogwood' }); - const component = mountComponent(); - const props = component.getComponent(DiffItem).props(); - props.new.should.equal('Dogwood'); - props.old.should.equal('dogwood'); - props.path.should.eql(['label']); + it('shows a message for an empty diff', () => { + testData.extendedEntities.createPast(1); + testData.extendedEntityVersions.createPast(1); + mountComponent().get('.empty-table-message').should.be.visible(); }); }); diff --git a/test/components/entity/diff/head.spec.js b/test/components/entity/diff/head.spec.js new file mode 100644 index 000000000..0fef31a78 --- /dev/null +++ b/test/components/entity/diff/head.spec.js @@ -0,0 +1,77 @@ +import EntityDiffHead from '../../../../src/components/entity/diff/head.vue'; + +import testData from '../../../data'; +import { mergeMountOptions, mount } from '../../../util/lifecycle'; + +const mountComponent = (options = undefined) => + mount(EntityDiffHead, mergeMountOptions(options, { + props: { modelValue: 'baseDiff' }, + global: { + provide: { entityVersion: testData.extendedEntityVersions.last() } + } + })); + +describe('EntityDiffHead', () => { + describe('hard conflict', () => { + beforeEach(() => { + testData.extendedEntities.createPast(1, { label: 'foo' }); + testData.extendedEntityVersions.createPast(1, { label: 'bar' }); + testData.extendedEntityVersions.createPast(1, { + baseVersion: 1, + label: 'baz', + conflictingProperties: ['label'] + }); + }); + + it('shows the correct icon', () => { + mountComponent().find('.icon-warning').exists().should.be.true(); + }); + + it('shows the correct text', () => { + const text = mountComponent().get('p').text(); + text.should.equal('Conflict\u00a0This Submission update was applied to version 2 of this Entity, but it was created based on version 1. Other updates had already written to the same properties.'); + }); + }); + + describe('soft conflict', () => { + beforeEach(() => { + testData.extendedEntities.createPast(1); + testData.extendedEntityVersions.createPast(2, { baseVersion: 1 }); + }); + + it('shows the correct icon', () => { + mountComponent().find('.icon-info-circle').exists().should.be.true(); + }); + + it('shows the correct text', () => { + const text = mountComponent().get('p').text(); + text.should.equal('Parallel Update\u00a0This Submission update was applied to version 2 of this Entity, but it was created based on version 1. No parallel update before this one touched the same properties.'); + }); + }); + + it('shows the correct versions in the tabs', () => { + testData.extendedEntities.createPast(1); + testData.extendedEntityVersions.createPast(2, { baseVersion: 1 }); + const text = mountComponent().findAll('li a').map(a => a.text()); + text.should.eql([ + 'Author’s View (updating v1)', + 'Central’s View (updating v2)' + ]); + }); + + it('activates the tab that corresponds to the modelValue prop', () => { + testData.extendedEntities.createPast(1); + testData.extendedEntityVersions.createPast(2, { baseVersion: 1 }); + const li = mountComponent().findAll('li.active'); + li.length.should.equal(1); + li[0].text().should.startWith('Author'); + }); + + it('emits an update:modelValue event after a click on a tab', async () => { + testData.extendedEntities.createPast(1); + testData.extendedEntityVersions.createPast(2, { baseVersion: 1 }); + const component = mountComponent(); + await component.get('li:nth-child(2) a').trigger('click'); + component.emitted('update:modelValue').should.eql([['serverDiff']]); + }); +}); diff --git a/test/components/entity/diff/row.spec.js b/test/components/entity/diff/row.spec.js new file mode 100644 index 000000000..265cb8d0e --- /dev/null +++ b/test/components/entity/diff/row.spec.js @@ -0,0 +1,172 @@ +import { last } from 'ramda'; + +import EntityDiffRow from '../../../../src/components/entity/diff/row.vue'; + +import useEntity from '../../../../src/request-data/entity'; + +import createTestContainer from '../../../util/container'; +import testData from '../../../data'; +import { mergeMountOptions, mount } from '../../../util/lifecycle'; +import { testRequestData } from '../../../util/request-data'; + +const mountComponent = (options) => { + const container = createTestContainer({ + requestData: testRequestData([useEntity], { + entityVersions: testData.extendedEntityVersions.sorted() + }) + }); + const { entityVersions } = container.requestData.localResources; + return mount(EntityDiffRow, mergeMountOptions(options, { + global: { + provide: { entityVersion: last(entityVersions) } + }, + props: { oldVersion: entityVersions[0] } + })); +}; + +describe('EntityDiffRow', () => { + describe('first column', () => { + it('shows the correct text for a label', () => { + testData.extendedEntities.createPast(1, { label: 'dogwood' }); + testData.extendedEntityVersions.createPast(1, { label: 'Dogwood' }); + const row = mountComponent({ + props: { name: 'label' } + }); + row.get('td').text().should.equal('Label'); + }); + + it('renders a property correctly', async () => { + testData.extendedEntities.createPast(1, { + data: { height: '1' } + }); + testData.extendedEntityVersions.createPast(1, { + data: { height: '2' } + }); + const row = mountComponent({ + props: { name: 'height' } + }); + const td = row.get('td'); + td.text().should.equal('height'); + await td.should.have.textTooltip(); + td.classes('property').should.be.true(); + }); + }); + + describe('old value', () => { + it('shows an old label', () => { + testData.extendedEntities.createPast(1, { label: 'dogwood' }); + testData.extendedEntityVersions.createPast(1, { label: 'Dogwood' }); + const row = mountComponent({ + props: { name: 'label' } + }); + row.get('td:nth-child(2)').text().should.equal('dogwood'); + }); + + it('shows the old value of a property', () => { + testData.extendedEntities.createPast(1, { + data: { height: '111111' } + }); + testData.extendedEntityVersions.createPast(1, { + data: { height: '222222' } + }); + const row = mountComponent({ + props: { name: 'height' } + }); + row.get('td:nth-child(2)').text().should.equal('111111'); + }); + + it('renders correctly if the old value is an empty string', () => { + testData.extendedEntities.createPast(1, { + data: { height: '' } + }); + testData.extendedEntityVersions.createPast(1, { + data: { height: '1' } + }); + const row = mountComponent({ + props: { name: 'height' } + }); + const td = row.get('td:nth-child(2)'); + td.text().should.equal('(empty)'); + td.classes('empty').should.be.true(); + }); + + it('renders correctly if the old value does not exist', () => { + testData.extendedDatasets.createPast(1, { + properties: [{ name: 'height' }], + entities: 1 + }); + testData.extendedEntities.createPast(1, { data: {} }); + testData.extendedEntityVersions.createPast(1, { + data: { height: '1' } + }); + const row = mountComponent({ + props: { name: 'height' } + }); + const td = row.get('td:nth-child(2)'); + td.text().should.equal('(empty)'); + td.classes('empty').should.be.true(); + }); + }); + + describe('new value', () => { + it('shows a new label', () => { + testData.extendedEntities.createPast(1, { label: 'dogwood' }); + testData.extendedEntityVersions.createPast(1, { label: 'Dogwood' }); + const row = mountComponent({ + props: { name: 'label' } + }); + row.get('td:nth-child(4)').text().should.equal('Dogwood'); + }); + + it('shows the new value of a property', () => { + testData.extendedEntities.createPast(1, { + data: { height: '111111' } + }); + testData.extendedEntityVersions.createPast(1, { + data: { height: '222222' } + }); + const row = mountComponent({ + props: { name: 'height' } + }); + row.get('td:nth-child(4)').text().should.equal('222222'); + }); + + it('renders correctly if the new value is an empty string', () => { + testData.extendedEntities.createPast(1, { + data: { height: '1' } + }); + testData.extendedEntityVersions.createPast(1, { + data: { height: '' } + }); + const row = mountComponent({ + props: { name: 'height' } + }); + const td = row.get('td:nth-child(4)'); + td.text().should.equal('(empty)'); + td.classes('empty').should.be.true(); + }); + }); + + it('renders correctly if the property is conflicting', async () => { + testData.extendedEntities.createPast(1, { + data: { height: '1' } + }); + testData.extendedEntityVersions.createPast(1, { + data: { height: '2' } + }); + testData.extendedEntityVersions.createPast(1, { + baseVersion: 1, + data: { height: '3' }, + conflictingProperties: ['height'] + }); + const row = mountComponent({ + props: { name: 'height' } + }); + row.get('td').classes('conflicting').should.be.true(); + const iconContainer = row.get('td:nth-child(3) span'); + iconContainer.find('.icon-exclamation-circle').exists().should.be.true(); + await iconContainer.should.have.tooltip('Another update already wrote to this property.'); + const text = row.get('.sr-only').text(); + await text.should.equal('Another update already wrote to this property.'); + }); +}); diff --git a/test/components/entity/diff/table.spec.js b/test/components/entity/diff/table.spec.js new file mode 100644 index 000000000..58a9641e3 --- /dev/null +++ b/test/components/entity/diff/table.spec.js @@ -0,0 +1,111 @@ +import { last } from 'ramda'; + +import EntityDiffRow from '../../../../src/components/entity/diff/row.vue'; +import EntityDiffTable from '../../../../src/components/entity/diff/table.vue'; +import EntityVersionLink from '../../../../src/components/entity/version-link.vue'; + +import useEntity from '../../../../src/request-data/entity'; + +import createTestContainer from '../../../util/container'; +import testData from '../../../data'; +import { mount } from '../../../util/lifecycle'; +import { mockRouter } from '../../../util/router'; +import { testRequestData } from '../../../util/request-data'; + +const mountComponent = (options = undefined) => { + const { uuid } = testData.extendedEntities.last(); + const container = createTestContainer({ + requestData: testRequestData([useEntity], { + entityVersions: testData.extendedEntityVersions.sorted() + }), + router: mockRouter(`/projects/1/entity-lists/trees/entities/${uuid}`) + }); + const entityVersion = last(container.requestData.localResources.entityVersions); + return mount(EntityDiffTable, { + props: { diff: entityVersion[options?.props?.diff ?? 'baseDiff'] }, + global: { + provide: { projectId: '1', datasetName: 'trees', uuid, entityVersion } + }, + container + }); +}; + +describe('EntityDiffTable', () => { + describe('Comparing row', () => { + const createConflict = () => { + const users = Array.from(testData.extendedUsers + .createPast(1, { displayName: 'Alice' }) + .createPast(1, { displayName: 'Bob' }) + .createPast(1, { displayName: 'David' })); + testData.extendedEntities.createPast(1, { creator: users[0] }); + testData.extendedEntityVersions + .createPast(1, { baseVersion: 1, creator: users[1] }) + .createPast(1, { baseVersion: 1, creator: users[2] }); + }; + + it('does not show the row if the version is not a conflict', () => { + testData.extendedEntities.createPast(1); + testData.extendedEntityVersions.createPast(1); + mountComponent().find('.conflicting').exists().should.be.false(); + }); + + it('shows the old version number', () => { + createConflict(); + const table = mountComponent(); + const td = table.get('td:nth-child(2)'); + td.text().should.equal('v1 (Update by Alice)'); + td.getComponent(EntityVersionLink).props().version.version.should.equal(1); + }); + + it('shows the correct version number for the server diff', () => { + createConflict(); + const table = mountComponent({ + props: { diff: 'serverDiff' } + }); + table.get('td:nth-child(2)').text().should.equal('v2 (Update by Bob)'); + }); + + it('shows the new version number', () => { + createConflict(); + const table = mountComponent(); + const td = table.get('td:last-child'); + td.text().should.equal('v3 (Update by David)'); + td.getComponent(EntityVersionLink).props().version.version.should.equal(3); + }); + }); + + it('renders an EntityDiffRow for each change', () => { + testData.extendedEntities.createPast(1, { + label: 'dogwood', + data: { height: '1', circumference: '2' } + }); + testData.extendedEntityVersions.createPast(1, { + label: 'Dogwood', + data: { height: '3', circumference: '4' } + }); + const table = mountComponent(); + table.findAllComponents(EntityDiffRow).length.should.equal(3); + }); + + it('passes correct props to EntityDiffRow for a label change', () => { + testData.extendedEntities.createPast(1, { label: 'dogwood' }); + testData.extendedEntityVersions.createPast(1, { label: 'Dogwood' }); + const table = mountComponent(); + const props = table.getComponent(EntityDiffRow).props(); + props.oldVersion.version.should.equal(1); + props.name.should.equal('label'); + }); + + it('passes correct props to EntityDiffRow for a property change', () => { + testData.extendedEntities.createPast(1, { + data: { height: '1' } + }); + testData.extendedEntityVersions.createPast(1, { + data: { height: '2' } + }); + const table = mountComponent(); + const props = table.getComponent(EntityDiffRow).props(); + props.oldVersion.version.should.equal(1); + props.name.should.equal('height'); + }); +}); diff --git a/test/components/entity/feed-entry.spec.js b/test/components/entity/feed-entry.spec.js index aed8b0906..4f026c71a 100644 --- a/test/components/entity/feed-entry.spec.js +++ b/test/components/entity/feed-entry.spec.js @@ -13,10 +13,11 @@ import { mockRouter } from '../../util/router'; import { mergeMountOptions, mount } from '../../util/lifecycle'; import { testRequestData } from '../../util/request-data'; -const mountComponent = (options) => - mount(EntityFeedEntry, mergeMountOptions(options, { +const mountComponent = (options) => { + const entity = testData.extendedEntities.last(); + return mount(EntityFeedEntry, mergeMountOptions(options, { global: { - provide: { projectId: '1', datasetName: 'trees' } + provide: { projectId: '1', datasetName: 'trees', uuid: entity.uuid } }, props: { entry: testData.extendedAudits.last(), @@ -25,13 +26,14 @@ const mountComponent = (options) => : null }, container: { - router: mockRouter('/projects/1/entity-lists/trees/entities/e'), + router: mockRouter(`/projects/1/entity-lists/trees/entities/${entity.uuid}`), requestData: testRequestData([useEntity], { - entity: testData.extendedEntities.last(), + entity, entityVersions: testData.extendedEntityVersions.sorted() }) } })); +}; describe('EntityFeedEntry', () => { beforeEach(() => { @@ -303,7 +305,7 @@ describe('EntityFeedEntry', () => { const component = mountComponent({ props: updateEntityFromSubmission({ deleted: true }) }); - const links = component.findAllComponents(RouterLinkStub); + const links = component.get('.feed-entry-title').findAllComponents(RouterLinkStub); links.length.should.equal(0); }); }); @@ -314,8 +316,7 @@ describe('EntityFeedEntry', () => { action: 'entity.update.version', details: {} }); - const diff = mountComponent().getComponent(EntityDiff); - diff.props().entityVersion.version.should.equal(2); + mountComponent().findComponent(EntityDiff).exists().should.be.true(); }); describe('entity.update.resolve audit event', () => { diff --git a/test/components/entity/version-link.spec.js b/test/components/entity/version-link.spec.js new file mode 100644 index 000000000..626d50d6c --- /dev/null +++ b/test/components/entity/version-link.spec.js @@ -0,0 +1,61 @@ +import { RouterLinkStub } from '@vue/test-utils'; + +import EntityVersionLink from '../../../src/components/entity/version-link.vue'; + +import testData from '../../data'; +import { mockRouter } from '../../util/router'; +import { mount } from '../../util/lifecycle'; + +const mountComponent = () => mount(EntityVersionLink, { + props: { + uuid: testData.extendedEntities.last().uuid, + version: testData.extendedEntityVersions.last() + }, + global: { + provide: { + projectId: '1', + datasetName: testData.extendedDatasets.last().name + } + }, + container: { router: mockRouter('/') } +}); + +describe('EntityVersionLink', () => { + it('links to the correct location', () => { + testData.extendedDatasets.createPast(1, { name: 'á' }); + testData.extendedEntities.createPast(1, { uuid: 'e' }); + testData.extendedEntityVersions.createPast(1); + const { to } = mountComponent().getComponent(RouterLinkStub).props(); + to.should.equal('/projects/1/entity-lists/%C3%A1/entities/e#v2'); + }); + + it('shows the correct text if the entity source is the API', () => { + testData.extendedUsers.createPast(1, { displayName: 'Alice' }); + testData.extendedEntities.createPast(1); + mountComponent().text().should.equal('Update by Alice'); + }); + + describe('entity source is a submission', () => { + it('shows the instance name if the submission has one', () => { + const { submission, submissionCreate } = testData.extendedEntities + .createSourceSubmission('submission.create', { + meta: { instanceName: 'Some Name' } + }); + testData.extendedEntities.createPast(1, { + source: { submission, submissionCreate } + }); + mountComponent().text().should.equal('Submission Some Name'); + }); + + it('falls back to the instance ID', () => { + const { submissionCreate } = testData.extendedEntities + .createSourceSubmission('submission.create', { instanceId: 's' }); + testData.extendedEntities.createPast(1, { + // Don't specify source.submission, matching the response if the + // submission is deleted. + source: { submissionCreate } + }); + mountComponent().text().should.equal('Submission s'); + }); + }); +}); diff --git a/test/data/entities.js b/test/data/entities.js index e556e1cae..d811b1db0 100644 --- a/test/data/entities.js +++ b/test/data/entities.js @@ -19,7 +19,7 @@ const diffVersions = (dataReceived, previousVersion) => { const diff = Object.keys(dataReceived).filter(name => name !== 'label' && dataReceived[name] !== previousVersion.data[name]); if (dataReceived.label != null && dataReceived.label !== previousVersion.label) - diff.push('label'); + diff.unshift('label'); return diff; }; @@ -34,6 +34,7 @@ const entityVersions = dataStore({ label = undefined, data = {}, conflictingProperties = undefined, + source = {}, creator = extendedUsers.first(), // Internal option for the `entities` store. This is an entity that is in @@ -100,6 +101,7 @@ const entityVersions = dataStore({ baseDiff: diffVersions(dataReceived, baseVersion), serverDiff: diffVersions(dataReceived, lastVersion), resolved: false, + source, creatorId: creator.id, creator: toActor(creator), createdAt @@ -125,6 +127,7 @@ entities = dataStore({ version = 1, label = faker.random.word(), data = undefined, + source = undefined, creator: creatorOption = undefined }) => { if (extendedDatasets.size === 0) { @@ -157,10 +160,11 @@ entities = dataStore({ _entity: entity, label, data: data ?? randomData(dataset.properties), + source, creator }); for (let i = 2; i <= version; i += 1) - createVersion({ _entity: entity, creator }); + createVersion({ _entity: entity, source, creator }); return entity; }, diff --git a/transifex/strings_en.json b/transifex/strings_en.json index a91222ee9..6aa559077 100644 --- a/transifex/strings_en.json +++ b/transifex/strings_en.json @@ -778,6 +778,12 @@ "string": "Basic Information", "developer_comment": "This is a title shown above a section of the page." }, + "conflict": { + "string": "Conflict" + }, + "conflicts": { + "string": "Conflicts" + }, "currentDraft": { "string": "Your Current Draft", "developer_comment": "This is a title shown above a section of the page." @@ -864,8 +870,9 @@ "totalEntities": { "string": "Total Entities" }, - "conflicts": { - "string": "Conflicts" + "versionShort": { + "string": "v{version}", + "developer_comment": "A short identifier of an Entity version. \"v\" is short for \"version\". {version} is a version number." } }, "mixin": { @@ -1544,6 +1551,70 @@ "developer_comment": "This is a title shown above a section of the page." } }, + "EntityDiff": { + "noChange": { + "string": "There are no changes to show." + } + }, + "EntityDiffHead": { + "introduction": { + "string": "This Submission update was applied to version {version} of this Entity, but it was created based on version {baseVersion}.", + "developer_comment": "{version} and {baseVersion} are version numbers." + }, + "hardConflict": { + "description": { + "string": "Other updates had already written to the same properties." + } + }, + "softConflict": { + "title": { + "string": "Parallel Update" + }, + "description": { + "string": "No parallel update before this one touched the same properties." + } + }, + "tab": { + "baseDiff": { + "string": "Author’s View", + "developer_comment": "A comparison between two versions of an Entity, from the point of view of the data collector (the author)" + }, + "serverDiff": { + "string": "Central’s View", + "developer_comment": "A comparison between two versions of an Entity, from the point of view of the Central server" + }, + "updating": { + "string": "(updating {version})", + "developer_comment": "This text is shown above a comparison of two versions of an Entity. {version} is a short version identifier, for example, \"v3\"." + } + } + }, + "EntityDiffRow": { + "conflictingProp": { + "string": "Another update already wrote to this property.", + "developer_comment": "This is shown for an Entity property that is part of a conflict between versions of the Entity" + } + }, + "EntityDiffTable": { + "header": { + "oldValue": { + "string": "Old value", + "developer_comment": "The value of an Entity property in an older version of the Entity" + }, + "newValue": { + "string": "New value", + "developer_comment": "The value of an Entity property in a newer version of the Entity" + } + }, + "comparing": { + "string": "Comparing", + "developer_comment": "This is shown when Central displays a comparison of two versions of an Entity." + }, + "version": { + "string": "{version} ({source})", + "developer_comment": "{version} is a short identifier of an Entity version, for example, \"v3\". {source} indicates who or what created the Entity version, for example, \"Update by Alice\"." + } + }, "EntityFeedEntry": { "title": { "submission": { @@ -1737,6 +1808,14 @@ } } }, + "EntityVersionLink": { + "submission": { + "string": "Submission {instanceName}" + }, + "api": { + "string": "Update by {name}" + } + }, "FieldKeyList": { "action": { "create": { From 4195c46d5d5206ee06874ec524f4c42435f68bd7 Mon Sep 17 00:00:00 2001 From: Matthew White Date: Mon, 20 Nov 2023 20:18:05 -0500 Subject: [PATCH 2/3] Make small changes after code review --- src/components/entity/diff.vue | 3 ++- src/components/entity/diff/row.vue | 2 +- src/components/entity/diff/table.vue | 2 +- test/components/entity/diff.spec.js | 6 ++++++ test/components/entity/diff/row.spec.js | 12 +++++------ test/components/entity/diff/table.spec.js | 26 ++++++----------------- test/components/entity/feed-entry.spec.js | 2 +- 7 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/components/entity/diff.vue b/src/components/entity/diff.vue index 32cd902f4..8229f21ef 100644 --- a/src/components/entity/diff.vue +++ b/src/components/entity/diff.vue @@ -12,7 +12,8 @@ except according to the terms contained in the LICENSE file.