Making it harder to delete all rows from lists by XingY · Pull Request #7471 · LabKey/platform

XingY · 2026-03-03T20:31:44Z

Rationale

This PR aims to reduce the risk of accidental “delete all rows” operations on Lists by removing the delete-all button from the list grid itself, replacing it with a lists webpart menu action with explicit confirmation flow for truncating list data within the current container.

Related Pull Requests

Changes

Removed the “Delete All Rows” button from list grids and removed the helper that generated it.
Added “Delete All Data from List” action with a new confirmation JSP that summarizes impacted lists and row counts.
Tightened container scoping in list related-data cleanup and updated attachment download resolution to use the correct data container.

Copilot

Pull request overview

This PR aims to reduce the risk of accidental “delete all rows” operations on Lists by removing the one-click delete-all UI and replacing it with an Admin-only, explicit confirmation flow for truncating list data within the current container.

Changes:

Removed the “Delete All Rows” button from list grids and removed the helper that generated it.
Added an Admin-only “Delete All Data from List” action with a new confirmation JSP that summarizes impacted lists and row counts.
Tightened container scoping in list related-data cleanup and updated attachment download resolution to use the correct data container.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`list/src/org/labkey/list/view/truncateListData.jsp`	New confirmation view for truncating list data and showing per-list row counts.
`list/src/org/labkey/list/view/ListQueryView.java`	Removes the grid-level “Delete All Rows” button.
`list/src/org/labkey/list/model/ListQueryUpdateService.java`	Scopes related-data cleanup to the target container when enumerating entityIds.
`list/src/org/labkey/list/model/ListManagerSchema.java`	Reworks delete UI into an Admin-only menu that includes “Delete All Data from List”.
`list/src/org/labkey/list/model/ListDefinitionImpl.java`	Adds helper to resolve the correct container for list-item attachment downloads.
`list/src/org/labkey/list/controllers/ListController.java`	Adds `TruncateListDataAction` and updates attachment download to use resolved data container.
`api/src/org/labkey/api/query/QueryView.java`	Removes `createDeleteAllRowsButton()` implementation (API surface change).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-03T20:43:35Z

list/src/org/labkey/list/view/truncateListData.jsp

+            long count = (table != null) ? new TableSelector(table).getRowCount() : 0;
+        %>
+        <li>
+            <%= simpleLink(listDef.getName(), listDef.urlFor(ListController.GridAction.class, listDef.getContainer())) %>


The list link points to the list definition’s container (listDef.getContainer()), but the operation is truncating rows in the current container. This link is likely to show a different folder’s data than what will be deleted. Consider linking to the grid in the current container so the user can inspect the rows that will actually be removed.

Suggested change

<%= simpleLink(listDef.getName(), listDef.urlFor(ListController.GridAction.class, listDef.getContainer())) %>

<%= simpleLink(listDef.getName(), listDef.urlFor(ListController.GridAction.class, currentContainer)) %>

Copilot · 2026-03-03T20:43:36Z

list/src/org/labkey/list/view/truncateListData.jsp

+            TableInfo table = listQuerySchema.getTable(listDef.getName());
+            long count = (table != null) ? new TableSelector(table).getRowCount() : 0;
+        %>


Row counts are computed via listQuerySchema.getTable(listDef.getName()), which resolves the list by name in the current container. If multiple selected lists share the same name but are defined in different containers, the displayed count can be for the wrong list. Consider deriving the TableInfo from the selected ListDefinition (e.g., use the list’s definition container to resolve the correct list, and apply an explicit container filter for the current container when counting rows).

Copilot · 2026-03-03T20:43:36Z

api/src/org/labkey/api/query/QueryView.java

        return null;
    }



This removes the public QueryView.createDeleteAllRowsButton() method from the API module. Even though there are no in-repo references, this is a binary/source breaking change for external modules that compile against the LabKey API. Consider keeping the method (deprecated) and altering its behavior/visibility, or providing a compatible replacement path so downstream modules don’t break on upgrade.

Copilot · 2026-03-03T20:43:36Z

list/src/org/labkey/list/model/ListDefinitionImpl.java

+        }
+        catch (IllegalStateException e)
+        {
+            /* more than one row matches */


Swallowing the IllegalStateException when multiple rows match the same EntityId makes diagnosing data issues difficult and turns a real match into a NotFound for downloads. Consider logging at least a warning/debug with the list id/entityId, or handling deterministically (e.g., limit/order) if duplicates are expected to be possible.

Suggested change

/* more than one row matches */

// More than one row matches the specified EntityId; log for diagnosis and return null as before

Logger logger = LogManager.getLogger(ListDefinitionImpl.class);

logger.warn("Multiple list items match EntityId '{}' when resolving download container. List: '{}', Container: '{}'. Returning null.",

entityId, getName(), getContainer().getPath(), e);

Copilot · 2026-03-03T20:43:37Z

list/src/org/labkey/list/controllers/ListController.java

+    @RequiresPermission(AdminPermission.class)
+    public static class TruncateListDataAction extends ConfirmAction<ListDeletionForm>
+    {


PR description still contains the template placeholders for rationale and change list. Please update the PR description with the actual rationale/behavioral change summary (and any manual testing notes) before merging so reviewers/release notes have accurate context.

Copilot · 2026-03-03T20:43:37Z

list/src/org/labkey/list/view/truncateListData.jsp

+            });
+%>
+<labkey:errors></labkey:errors>
+<p>Are you sure you want to delete all data in <%= h(currentPath) %> from the following Lists? This action cannot be undone and will result in an empty list.</p>


The confirmation text says truncating will "result in an empty list", but this action deletes rows only from the current folder/container (not necessarily all rows across the project/shared list). Suggest rewording to explicitly state it will delete all rows for this list in the current folder/container path shown.

Suggested change

<p>Are you sure you want to delete all data in <%= h(currentPath) %> from the following Lists? This action cannot be undone and will result in an empty list.</p>

<p>Are you sure you want to delete all data in <%= h(currentPath) %> from the following Lists? This action cannot be undone and will delete all rows for these lists in the folder/container path shown, but will not affect rows in other folders/containers.</p>

labkey-nicka · 2026-03-03T22:45:35Z

list/src/org/labkey/list/controllers/ListController.java

+        public boolean handlePost(ListDeletionForm form, BindException errors)
+        {
+            Container containerDataToDelete = getContainer();
+            for (Pair<Integer, Container> pair : form.getListContainerMap())


Seems like this whole loop should be wrapped in a transaction.

labkey-nicka · 2026-03-03T22:53:30Z

list/src/org/labkey/list/model/ListDefinitionImpl.java

    }

-    public boolean hasListItemForEntityId(String entityId, User user)
+    public Container getListItemContainerForDownload(String entityId, User user, Class<? extends Permission> permissionClass)


Annotate with @Nullable

XingY added 2 commits March 3, 2026 12:29

Making it harder to delete all rows from lists

aec9635

CRLF

a7feb9e

This was referenced Mar 3, 2026

Making it harder to delete all rows from lists LabKey/testAutomation#2904

Open

Making it harder to delete all rows from lists #7466

Closed

XingY requested a review from Copilot March 3, 2026 20:34

Copilot started reviewing on behalf of XingY March 3, 2026 20:34 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

labkey-nicka assigned XingY Mar 3, 2026

labkey-nicka self-requested a review March 3, 2026 21:24

labkey-nicka approved these changes Mar 3, 2026

View reviewed changes

code review changes

ccd92fc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making it harder to delete all rows from lists#7471

Making it harder to delete all rows from lists#7471
XingY wants to merge 3 commits intorelease26.3-SNAPSHOTfrom
26.3_fb_truncateList

XingY commented Mar 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

labkey-nicka Mar 3, 2026

Uh oh!

labkey-nicka Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	<%= simpleLink(listDef.getName(), listDef.urlFor(ListController.GridAction.class, listDef.getContainer())) %>
	<%= simpleLink(listDef.getName(), listDef.urlFor(ListController.GridAction.class, currentContainer)) %>

-            /* more than one row matches */
+            // More than one row matches the specified EntityId; log for diagnosis and return null as before
+            Logger logger = LogManager.getLogger(ListDefinitionImpl.class);
+            logger.warn("Multiple list items match EntityId '{}' when resolving download container. List: '{}', Container: '{}'. Returning null.",
+                entityId, getName(), getContainer().getPath(), e);

	<p>Are you sure you want to delete all data in <%= h(currentPath) %> from the following Lists? This action cannot be undone and will result in an empty list.</p>
	<p>Are you sure you want to delete all data in <%= h(currentPath) %> from the following Lists? This action cannot be undone and will delete all rows for these lists in the folder/container path shown, but will not affect rows in other folders/containers.</p>

Conversation

XingY commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale

Related Pull Requests

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

labkey-nicka Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

labkey-nicka Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

XingY commented Mar 3, 2026 •

edited

Loading