Merge pull request #616 from getodk/ktuite/duplicate-form-names

Showing form IDs next to duplicated form names

src/components/form/row.vue

@@ -15,6 +15,7 @@ except according to the terms contained in the LICENSE file.
       <link-if-can :to="primaryFormPath(form)">
         {{ form.nameOrId() }}
       </link-if-can>
+      <span v-if="showIdForDuplicateName" class="duplicate-form-id">({{ form.xmlFormId }})</span>
     </td>
 
     <td v-for="reviewState of visibleReviewStates" :key="reviewState" class="review-state">
@@ -81,6 +82,8 @@ except according to the terms contained in the LICENSE file.
 </template>
 
 <script>
+import { mapGetters } from 'vuex';
+
 import DateTime from '../date-time.vue';
 import EnketoFill from '../enketo/fill.vue';
 import EnketoPreview from '../enketo/preview.vue';
@@ -112,7 +115,8 @@ export default {
   computed: {
     // The component assumes that this data will exist when the component is
     // created.
-    ...requestData(['project']),
+    ...requestData(['project', 'forms', 'deletedForms']),
+    ...mapGetters(['duplicateFormNames']),
     visibleReviewStates: () => ['received', 'hasIssues', 'edited'],
     urlFilterEncode() {
       return new Map()
@@ -126,6 +130,10 @@ export default {
         this.form.xmlFormId,
         'draft/testing'
       );
+    },
+    showIdForDuplicateName() {
+      const name = this.form.nameOrId().toLocaleLowerCase();
+      return this.duplicateFormNames.has(name);
     }
   }
 };
@@ -147,6 +155,11 @@ export default {
     font-size: 18px;
   }
 
+  .duplicate-form-id {
+    font-family: $font-family-monospace;
+    padding-left: 6px;
+  }
+
   .review-state, .total-submissions, .not-published {
     a { @include text-link; }
     text-align: right;

src/components/form/trash-row.vue

@@ -13,6 +13,7 @@ except according to the terms contained in the LICENSE file.
   <tr class="form-trash-row">
     <td class="name">
       <span class="form-name">{{ form.nameOrId() }}</span>
+      <span v-if="showIdForDuplicateName" class="duplicate-form-id">({{ form.xmlFormId }})</span>
     </td>
     <td class="deleted">
       <i18n-t tag="div" keypath="deletedDate" class="deleted-date">
@@ -50,6 +51,8 @@ except according to the terms contained in the LICENSE file.
 </template>
 
 <script>
+import { mapGetters } from 'vuex';
+
 import DateTime from '../date-time.vue';
 import Form from '../../presenters/form';
 import { requestData } from '../../store/modules/request';
@@ -67,7 +70,8 @@ export default {
   computed: {
     // The component assumes that this data will exist when the component is
     // created.
-    ...requestData(['forms']),
+    ...requestData(['forms', 'deletedForms']),
+    ...mapGetters(['duplicateFormNames']),
     activeFormIds() {
       // returns ids of existing forms to disable restoring deleted
       // forms with conflicting ids (also prevented on backend)
@@ -82,6 +86,10 @@ export default {
       if (this.disabled)
         return this.$t('disabled.conflict');
       return null;
+    },
+    showIdForDuplicateName() {
+      const name = this.form.nameOrId().toLocaleLowerCase();
+      return this.duplicateFormNames.has(name);
     }
   },
   methods: {
@@ -108,6 +116,11 @@ export default {
     font-size: 18px;
   }
 
+  .duplicate-form-id {
+    font-family: $font-family-monospace;
+    padding-left: 6px;
+  }
+
   .deleted {
     width: 300px;
     text-align: right;

src/components/project/form-row.vue

@@ -26,6 +26,7 @@ except according to the terms contained in the LICENSE file.
           {{ form.nameOrId() }}
         </template>
       </template>
+      <span v-if="showIdForDuplicateName" class="duplicate-form-id">({{ form.xmlFormId }})</span>
     </td>
     <template v-if="form.publishedAt != null">
       <td v-for="reviewState of visibleReviewStates" :key="reviewState" class="review-state">
@@ -84,6 +85,8 @@ except according to the terms contained in the LICENSE file.
 </template>
 
 <script>
+import { mapGetters } from 'vuex';
+
 import DateTime from '../date-time.vue';
 import Form from '../../presenters/form';
 import Project from '../../presenters/project';
@@ -111,6 +114,7 @@ export default {
     return { reviewStateIcon };
   },
   computed: {
+    ...mapGetters(['duplicateFormNamesPerProject']),
     canLinkToFormOverview() {
       return this.project.permits('form.update');
     },
@@ -137,6 +141,13 @@ export default {
     enketoPath() {
       const encodedId = encodeURIComponent(this.form.enketoId);
       return `${enketoBasePath}/${encodedId}`;
+    },
+    showIdForDuplicateName() {
+      const formNames = this.duplicateFormNamesPerProject[this.project.id];
+      if (formNames) {
+        return formNames.has(this.form.nameOrId().toLocaleLowerCase());
+      }
+      return false;
     }
   }
 };
@@ -153,6 +164,11 @@ export default {
     a { @include text-link; }
   }
 
+  .duplicate-form-id {
+    font-family: $font-family-monospace;
+    padding-left: 6px;
+  }
+
   .review-state, .total-submissions, .not-published {
     text-align: right;
     padding-right: 10px;

src/store/modules/request/keys.js

@@ -197,6 +197,38 @@ const dataGetters = {
         result.push(audit);
     }
     return result;
+  },
+
+  // Returns a set containing just the form names that appear more than once
+  // in a project. Used on project overview to show form ID next to form name
+  // when form names are duplicated.
+  duplicateFormNames: ({ data: { forms, deletedForms } }) => {
+    const allForms = [...forms || [], ...deletedForms || []];
+    const seenNames = new Set();
+    const dupeNames = new Set();
+    for (const form of allForms) {
+      const formName = form.nameOrId().toLocaleLowerCase();
+      if (seenNames.has(formName)) dupeNames.add(formName);
+      seenNames.add(formName);
+    }
+    return dupeNames;
+  },
+
+  // Returns an object of Sets containing duplicate project names for use
+  // by the Project list page.
+  duplicateFormNamesPerProject: ({ data: { projects } }) => {
+    if (projects == null) return {};
+    const dupeNamesByProject = {};
+    for (const project of projects) {
+      const seenNames = new Set();
+      dupeNamesByProject[project.id] = new Set();
+      for (const form of project.formList) {
+        const formName = form.nameOrId().toLocaleLowerCase();
+        if (seenNames.has(formName)) dupeNamesByProject[project.id].add(formName);
+        seenNames.add(formName);
+      }
+    }
+    return dupeNamesByProject;
   }
 };
 export const getters = dataGetters;

test/components/form/trash-row.spec.js

@@ -46,21 +46,21 @@ describe('FormTrashRow', () => {
     beforeEach(mockLogin);
 
     it('shows the submission count', () => {
-      const formData = { submissions: 12345 };
+      const formData = { name: 'a form', submissions: 12345 };
       const cell = mountComponent(formData).get('.total-submissions');
       cell.text().should.equal('12,345');
       cell.find('span').attributes().title.should.equal('Total');
     });
 
     it('shows the submission count when 0', () => {
-      const formData = { submissions: 0 };
+      const formData = { name: 'a form', submissions: 0 };
       const text = mountComponent(formData).get('.total-submissions').text();
       text.should.equal('0');
     });
 
     it('shows the time since last submission', () => {
       const lastSubmission = new Date().toISOString();
-      const formData = { submissions: 1, lastSubmission };
+      const formData = { name: 'a form', submissions: 1, lastSubmission };
       const row = mountComponent(formData);
       const cell = row.get('.last-submission');
       cell.text().should.match(/ago$/);
@@ -71,7 +71,7 @@ describe('FormTrashRow', () => {
     });
 
     it('does not render time if there is no last submission', () => {
-      const formData = { submissions: 0 };
+      const formData = { name: 'a form', submissions: 0 };
       const cell = mountComponent(formData).get('.last-submission');
       cell.text().should.equal('(none)');
       cell.find('span').attributes().title.should.equal('Latest Submission');
@@ -82,7 +82,7 @@ describe('FormTrashRow', () => {
     beforeEach(mockLogin);
 
     it('shows the undelete button', () => {
-      const button = mountComponent({}).get('.form-trash-row-restore-button');
+      const button = mountComponent({ name: 'foo' }).get('.form-trash-row-restore-button');
       button.element.tagName.should.equal('BUTTON');
       button.element.disabled.should.be.false();
     });

test/components/project/list.spec.js

@@ -314,5 +314,31 @@ describe('ProjectList', () => {
       blocks[1].findAllComponents(FormRow).length.should.equal(2);
     });
   });
+
+  describe('duplicate form names', () => {
+    beforeEach(mockLogin);
+
+    it('calculates duplicate form names per project', () => {
+      createProjectsWithForms(
+        [{ name: 'Project 1' }, { name: 'Project 2' }],
+        [
+          [{ name: 'A', xmlFormId: 'a_1' }, { name: 'A', xmlFormId: 'a_2' }, { name: 'B', xmlFormId: 'b_1' }],
+          [{ name: 'C', xmlFormId: 'c_1' }, { name: 'c', xmlFormId: 'c_2' }, { name: 'B', xmlFormId: 'b_1' }]
+        ]
+      );
+      const component = mountComponent();
+      const blocks = component.findAllComponents(ProjectHomeBlock);
+      blocks[0].findAllComponents(FormRow).map((row) => row.find('.form-name').text()).should.eql([
+        'A(a_1)',
+        'A(a_2)',
+        'B'
+      ]);
+      blocks[1].findAllComponents(FormRow).map((row) => row.find('.form-name').text()).should.eql([
+        'B',
+        'c(c_2)',
+        'C(c_1)'
+      ]);
+    });
+  });
 });
 

test/components/project/overview.spec.js

@@ -1,4 +1,6 @@
 import ProjectOverviewDescription from '../../../src/components/project/overview/description.vue';
+import FormList from '../../../src/components/form/list.vue';
+import FormRow from '../../../src/components/form/row.vue';
 import FormTrashList from '../../../src/components/form/trash-list.vue';
 import FormTrashRow from '../../../src/components/form/trash-row.vue';
 
@@ -101,4 +103,56 @@ describe('ProjectOverview', () => {
         });
     });
   });
+
+  describe('duplicate form names', () => {
+    it('shows form ID in parentheses next to form names that are duplicated', async () => {
+      mockLogin();
+      testData.extendedForms.createPast(1, { name: 'Same Name', xmlFormId: 'foo' });
+      testData.extendedForms.createPast(1, { name: 'Same Name', xmlFormId: 'bar' });
+      testData.extendedForms.createPast(1, { name: 'Different Name', xmlFormId: 'baz' });
+      const app = await load('/projects/1', {}, {});
+      const rows = app.getComponent(FormList).findAllComponents(FormRow);
+      rows.map((row) => row.find('.name').text()).should.eql([
+        'Different Name',
+        'Same Name(foo)',
+        'Same Name(bar)'
+      ]);
+    });
+
+    it('considers different capitalizations to be the same and shows form ID', async () => {
+      mockLogin();
+      testData.extendedForms.createPast(1, { name: 'Same Name', xmlFormId: 'foo' });
+      testData.extendedForms.createPast(1, { name: 'same NAME', xmlFormId: 'bar' });
+      const app = await load('/projects/1', {}, {});
+      const rows = app.getComponent(FormList).findAllComponents(FormRow);
+      rows.map((row) => row.find('.name').text()).should.eql([
+        'same NAME(bar)',
+        'Same Name(foo)'
+      ]);
+    });
+
+    it('shows form ID even when form is in separate closed table', async () => {
+      mockLogin();
+      testData.extendedForms.createPast(1, { name: 'Same Name', xmlFormId: 'foo', state: 'closed' });
+      testData.extendedForms.createPast(1, { name: 'Same Name', xmlFormId: 'bar', state: 'open' });
+      const app = await load('/projects/1', {}, {});
+      const rows = app.getComponent(FormList).findAllComponents(FormRow);
+      rows.map((row) => row.find('.name').text()).should.eql([
+        'Same Name(bar)',
+        'Same Name(foo)'
+      ]);
+    });
+
+    it('shows form ID even when duplicate form name is in trash', async () => {
+      mockLogin();
+      testData.extendedForms.createPast(1, { name: 'Same Name', xmlFormId: 'foo' });
+      const app = await load('/projects/1', {}, {
+        deletedForms: () => [{ name: 'Same Name', xmlFormId: 'bar', deletedAt: new Date().toISOString() }]
+      });
+      const formList = app.getComponent(FormList);
+      formList.find('.duplicate-form-id').text().should.equal('(foo)');
+      const trashList = app.getComponent(FormTrashList);
+      trashList.find('.duplicate-form-id').text().should.equal('(bar)');
+    });
+  });
 });