Remove snapshot filters from Entities and Submissions list by sadiqkhoja · Pull Request #1459 · getodk/central-frontend

sadiqkhoja · 2026-01-07T18:01:28Z

Closes getodk/central#1557

Changes:

Removed snapshot filters from the Entities and Submissions list.
If user end up on a page where there are no rows on the page then pagination component will show Rows 0 of N, N being the total number of records.
Since we don't need to keep the consistency between page navigation, I have removed deletedSubmissions and deletedEntities variable from the corresponding resource. The result of that is that when user deletes a row, then navigates to another page and then returns to the original page, they would see 250 (or whatever page size) records. Previously we used to show 250 - deleted records on the original page.
Since I am no longer maintaining state of deleted records, a generic message There are no Entities/Submissions to show will be displayed if all rows are deleted by the user. Previously we used to show All Entities/Submissions on the page have been deleted.

Known issues / questions

When user deletes a row, then total count on the top navigation tab is updated but total count in the pagination control is not updated. Additionally, the text "Rows 1-250 of N" is not updated. Second part is a bit complicated because we should drop with the row ranges when number of pages are less than 10, should we just subtract 1 from the range of the current page like 251-499 would be the second option if row is deleted from the second page? Should I fix this in the current PR or in a separate PR?
There was a suggest to show toast message if the count is changed between page navigation, I haven't implemented that. Wouldn't that be quite annoying if Submissions/Entities are being created continuously?

What has been done to verify that this works as intended?

All tests are passing.

Why is this the best possible solution? Were any other approaches considered?

It's mostly removal of code.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

All functionality around filters, pagination, search, delete and bulk delete should be tested on Submissions and Entity list page.

Does this change require updates to user documentation? If so, please file an issue here and include the link below.

No

Before submitting this PR, please make sure you have:

run npm run test and npm run lint and confirmed all checks still pass OR confirm CircleCI build passes
verified that any code or assets from external sources are properly credited in comments or that everything is internally sourced

…Submissions list

sadiqkhoja · 2026-01-12T19:43:04Z

Met with @lognaturel and @nicoleorlowski, agreed upon following changes:

refresh the data when using bulk delete
Update total count on the pagination control
- Subtract from the second number of the range on non-bulk delete
Remove page dropdown (shown when there are less than 10 pages) from the pagination control

Separate issue for:

Adding another pagination control at the top
Remove/reposition timestamp of last data refreshe

* removed page dropdown which was shown when number of pages were less than 10 * update count and range in the pagination control when a row is deleted * refresh the data after bulk delete

matthew-white

Still taking a look, but I thought I'd leave a few questions/comments so far.

matthew-white · 2026-02-11T01:48:55Z

src/components/entity/list.vue

-        this.dataset.entities -= uuids.length;
-
-
        this.bulkDeletedEntities = [...this.selectedEntities];


For this.bulkDeletedEntities, I think we could now just store the entity UUIDs. That seems to be the only piece of data that's needed anymore. Just thinking to reduce the amount of entity data in memory.

matthew-white · 2026-02-11T01:50:35Z

src/components/entity/list.vue

        this.bulkDeletedEntities = [...this.selectedEntities];
-        this.odataEntities.value = this.odataEntities.value.filter(e => !this.selectedEntities.has(e));
        this.alert.success(this.$tcn('alert.bulkDelete', this.selectedEntities.size))
          .cta(this.$t('action.undo'), () => this.requestBulkRestore(uuids));


This is maybe outside the scope of this PR, but this.requestBulkRestore() doesn't seem to take an argument.

matthew-white · 2026-02-11T01:55:30Z

src/components/entity/list.vue

I understand a lot of the code removal here, but not 100% why this line was removed. Is it because the watcher on alert.state does this already?

matthew-white · 2026-02-11T02:14:07Z

src/components/pagination.vue

-              {{ text }}
-            </option>
-          </select>
+          <template v-if="empty">0</template>


Suggested change

<template v-if="empty">0</template>

<template v-if="empty">{{ $n(0, 'default') }}</template>

Wondering whether an Arabic locale would display a different numeral for 0?

Introduce a new key to handle 0 case. We need to say Row 0 of N, here default pluralization scheme fails; we can't say "Rows 0"

matthew-white · 2026-02-11T02:16:38Z

src/components/pagination.vue

    type: Number,
    required: true
  },
+  removed: {


It helped me once I understood that this is the number removed specifically from the current page. How about adding a code comment along those lines?

matthew-white · 2026-02-11T02:17:59Z

test/components/submission/filters.spec.js

-        .beforeEachResponse((_, { url }) => {
-          const filters = new URL(url, window.location.origin).searchParams.get('$filter').split(' and ');
-
-          const start = filters[2].split(' ge ')[1];
-          start.should.equal('1970-01-01T00:00:00.000Z');
-          DateTime.fromISO(start).zoneName.should.equal(Settings.defaultZoneName);
-
-          const end = filters[3].split(' le ')[1];
-          end.should.equal('1970-01-02T23:59:59.999Z');
-          DateTime.fromISO(end).zoneName.should.equal(Settings.defaultZoneName);
-        })


I feel like these are still useful assertions and aren't just about the snapshot filter. Maybe the indices on filters here just need to be changed for the test to pass? Similar to the changes for entity/filters.spec.js.

matthew-white · 2026-02-11T02:31:11Z

src/components/pagination.vue

    type: Array,
    required: true
  },
+  empty: Boolean,


Can this be derived from count, removed, page, and size? Something like count.value === 0 || props.page > lastPage.value? If it can't be, I think a code comment explaining the case that it's capturing would be helpful.

let's compute empty using count, removed, sizeOfCurrentPage

also add a unit test

matthew-white · 2026-02-11T02:33:27Z

src/components/pagination.vue

-  return result;
-});
+
 const sizeOfCurrentPage = computed(() => {


Should this use props.empty somehow? Something like:

if (props.empty) return 0;

I have removed props.empty. I was thinking to if (props.count === 0) return 0; but we don't show the pagination component if the count is 0

matthew-white · 2026-02-11T02:33:42Z

src/components/pagination.vue

-              {{ text }}
-            </option>
-          </select>
+          <template v-if="empty">0</template>


Is there a test of the case where props.empty is true? I did a quick search for "Row 0", but I didn't find any matches. I might have missed the relevant tests though.

matthew-white · 2026-02-11T02:36:29Z

src/components/pagination.vue

I see that there isn't any pagination.spec.js. Instead, the component is tested implicitly via tests of SubmissionList and EntityList. I was probably the one who decided not to add a pagination.spec.js, but with the introduction of new props, part of me is wondering whether it'd improve testing or make the component's contract clearer to add such a file. I'm not necessarily advocating for that, but I'm wondering what you think of that idea.

matthew-white · 2026-02-11T04:56:39Z

src/components/entity/list.vue

From the PR description:

Since I am no longer maintaining state of deleted records, a generic message There are no Entities/Submissions to show will be displayed if all rows are deleted by the user. Previously we used to show All Entities/Submissions on the page have been deleted.

Mostly this makes sense to me, but I'm wondering about the case where the table is filtered. In that case, it won't say, "There are no Entities to show.", it'll say "There are no matching Entities." That feels a little less accurate/clear to me. I guess you could argue that "matching" includes the page number, but I wanted to ask about it.

Also, I know that removedEntities is now just a number, but a set, but the only way it was used here before was removedEntities.size. Could the number be used in lieu of removedEntities.size? I know they're not exactly the same, since the number now just refers to the current page. For that reason, the first if no longer works, but doesn't the second if still work?

Agree. Also add a test that message is correct if table is filtered and all rows are deleted.

matthew-white · 2026-02-11T05:01:42Z

src/components/submission/list.vue

          if (confirm != null) this.confirmDelete = confirm;

-          this.odata.removedSubmissions.add(instanceId);
+          this.odata.removedSubmissions += 1;


I feel like there's a slight race condition here, though it probably doesn't matter.

Delete a submission, checking "Delete immediately before confirmation".

Delete a second submission.

While the delete request is in progress, navigate to another page.

If the new page data is received before the success response from the delete, then removedSubmissions will be incremented for the current page when it shouldn't be.

My initial thought is that nothing needs to change, since removedSubmissions isn't used in any critical way. Mostly I just wanted to mention it as food for thought.

sadiqkhoja · 2026-02-12T20:21:31Z

src/components/pagination.vue

-              {{ text }}
-            </option>
-          </select>
+          <template v-if="empty">0</template>


Introduce a new key to handle 0 case. We need to say Row 0 of N, here default pluralization scheme fails; we can't say "Rows 0"

sadiqkhoja · 2026-02-12T20:22:31Z

src/components/pagination.vue

@@ -35,13 +35,7 @@ except according to the terms contained in the LICENSE file.
      <i18n-t tag="div" keypath="rows" :plural="sizeOfCurrentPage"


this can be replaced with computed ref. We no longer need i18n-t as select element is removed.

sadiqkhoja · 2026-02-12T20:25:30Z

src/components/pagination.vue

    type: Array,
    required: true
  },
+  empty: Boolean,


let's compute empty using count, removed, sizeOfCurrentPage

sadiqkhoja · 2026-02-12T20:26:04Z

src/components/pagination.vue

    type: Array,
    required: true
  },
+  empty: Boolean,


also add a unit test

sadiqkhoja · 2026-02-12T20:27:25Z

src/components/pagination.vue

Create corresponding spec for unit testing this component.

sadiqkhoja · 2026-02-12T20:50:21Z

src/components/entity/list.vue

Agree. Also add a test that message is correct if table is filtered and all rows are deleted.

matthew-white

I still need to review pagination.spec.js, but that's mostly it. It's looking great! I'm dropping a few comments now, but I'm planning to finish review tomorrow.

matthew-white · 2026-02-18T07:24:10Z

src/components/submission/list.vue

Similar to EntityList, how about adding this if back?

Not sure why this is being shown without any diff context. I'm talking about the second if removed from emptyMessage.

matthew-white · 2026-02-18T07:27:59Z

test/data/entities.js

+extendedEntities.delete = (index) => {
+  const entity = entities.get(index);
+  if (entity == null) throw new Error('entity not found');
+  entities.update(index, { deletedAt: new Date() });


Suggested change

entities.update(index, { deletedAt: new Date() });

entities.update(index, { deletedAt: new Date().toISOString() });

Since it's supposed to match the API response, I think it should be a string, not a Date.

matthew-white · 2026-02-18T07:31:51Z

src/components/pagination.vue

-      <i18n-t tag="div" keypath="rows" :plural="sizeOfCurrentPage"
-        class="form-group">
+      <div class="form-group">{{ pageRange }}</div>
+      <!-- <i18n-t tag="div" keypath="rows" :plural="sizeOfCurrentPage"


I see that this is commented out, but it'd be great to remove the <i18n-t> block entirely if possible.

matthew-white

Looks great! 💯 I love the new tests in pagination.spec.js.

matthew-white · 2026-02-22T23:17:17Z

src/components/pagination.vue

 -->
 <template>
-  <div class="pagination">
+  <div v-if="count > 0" class="pagination">


Within the template, is count kind of ambiguous? It's not clear to me whether it will be evaluated as the prop or as the computed ref. It might not be a bad idea to rename or alias the computed ref so that there's no ambiguity.

matthew-white · 2026-02-22T23:28:00Z

src/components/pagination.vue

I think these .form-control styles can be removed now that the <select> element has been removed.

matthew-white · 2026-02-23T00:46:42Z

src/components/entity/list.vue

-
-
-        this.bulkDeletedEntities = [...this.selectedEntities];
+        this.bulkDeletedEntities = [...this.selectedEntities].map(e => e.__id);


Suggested change

this.bulkDeletedEntities = [...this.selectedEntities].map(e => e.__id);

this.bulkDeletedEntities = uuids;

I think uuids can be reused so we don't need to .map() again.

matthew-white · 2026-02-23T00:47:43Z

src/components/entity/list.vue

-
-        this.bulkDeletedEntities = [...this.selectedEntities];
+        this.bulkDeletedEntities = [...this.selectedEntities].map(e => e.__id);
        this.odataEntities.value = this.odataEntities.value.filter(e => !this.selectedEntities.has(e));


Do we need this line anymore? Since we're calling this.refresh(), I'm thinking that this.odataEntities will be cleared. For comparison, onSuccess in requestBulkRestore() no longer modifies this.odataEntities.value.

* Fixes getodk/central#1557: Remove snapshot filters from Entities and Submissions list * Simplified pagination control * removed page dropdown which was shown when number of pages were less than 10 * update count and range in the pagination control when a row is deleted * refresh the data after bulk delete * PR Feedbacks

sadiqkhoja force-pushed the fixes/remove-snapshot-filters branch from 3a23949 to f485773 Compare January 7, 2026 18:09

Fixes getodk/central#1557: Remove snapshot filters from Entities and …

969065d

…Submissions list

sadiqkhoja force-pushed the fixes/remove-snapshot-filters branch from f485773 to 969065d Compare January 7, 2026 19:55

sadiqkhoja force-pushed the fixes/remove-snapshot-filters branch 2 times, most recently from 60cf462 to c7b7474 Compare January 27, 2026 20:43

Simplified pagination control

937a6e4

* removed page dropdown which was shown when number of pages were less than 10 * update count and range in the pagination control when a row is deleted * refresh the data after bulk delete

sadiqkhoja force-pushed the fixes/remove-snapshot-filters branch from c7b7474 to 937a6e4 Compare January 27, 2026 20:57

sadiqkhoja requested a review from matthew-white January 28, 2026 02:15

matthew-white reviewed Feb 11, 2026

View reviewed changes

sadiqkhoja commented Feb 12, 2026

View reviewed changes

PR Feedback

68c04d9

matthew-white reviewed Feb 18, 2026

View reviewed changes

PR Feedback

34da70c

matthew-white mentioned this pull request Feb 23, 2026

Replace filtering based on client timestamps getodk/central#1557

Closed

matthew-white approved these changes Feb 23, 2026

View reviewed changes

sadiqkhoja added 2 commits February 24, 2026 10:35

PR Feedback

8346081

Merge branch 'master' into fixes/remove-snapshot-filters

74f0126

sadiqkhoja merged commit 3ecfbc7 into getodk:master Feb 24, 2026
8 of 9 checks passed

		this.dataset.entities -= uuids.length;


		this.bulkDeletedEntities = [...this.selectedEntities];

	<template v-if="empty">0</template>
	<template v-if="empty">{{ $n(0, 'default') }}</template>

		@@ -35,13 +35,7 @@ except according to the terms contained in the LICENSE file.
		<i18n-t tag="div" keypath="rows" :plural="sizeOfCurrentPage"

	entities.update(index, { deletedAt: new Date() });
	entities.update(index, { deletedAt: new Date().toISOString() });

	this.bulkDeletedEntities = [...this.selectedEntities].map(e => e.__id);
	this.bulkDeletedEntities = uuids;

Conversation

sadiqkhoja commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What has been done to verify that this works as intended?

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Does this change require updates to user documentation? If so, please file an issue here and include the link below.

Before submitting this PR, please make sure you have:

Uh oh!

sadiqkhoja commented Jan 12, 2026

Uh oh!

matthew-white left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthew-white left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthew-white left a comment

Choose a reason for hiding this comment

Uh oh!

matthew-white Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sadiqkhoja commented Jan 7, 2026 •

edited

Loading

matthew-white Feb 22, 2026 •

edited

Loading