Skip to content

Feature #134 closing studies based on selection#135

Open
junaidferoz wants to merge 7 commits intodevfrom
feat-134-close_all_studies_options
Open

Feature #134 closing studies based on selection#135
junaidferoz wants to merge 7 commits intodevfrom
feat-134-close_all_studies_options

Conversation

@junaidferoz
Copy link
Copy Markdown
Collaborator

@junaidferoz junaidferoz commented Mar 26, 2026

Selective Study Closing with Workflow and User Filters

Previously, users could only close all studies at once. This feature adds the ability to selectively close studies by filtering them based on workflow type and/or user, reducing the risk of unintentionally closing studies and providing finer-grained control over bulk operations.

New User Features

  • Workflow filter: Select a specific workflow type when closing studies, or choose "All workflows" to close studies across all workflows.
  • User filter: Optionally filter studies by the user who owns them, with a default "Any user" option for unfiltered behavior.
  • Scoped confirmation: The confirmation dialog now displays which workflow and user filters are applied, along with an approximate count of studies to be closed.

New Dev Features

  • Extended studyCloseBulk handler: Backend now accepts optional workflowId and userId parameters to filter study lists before closing.
  • Server-side validation: All filtering is enforced on the backend to ensure malicious client requests cannot close out-of-scope studies.

Improvements

  • Filtered progress reporting: Progress indicators now reflect only the studies being closed within the selected scope, providing accurate feedback.
  • Enhanced safety: Users can preview exactly which studies will be affected before confirming the bulk close operation

@junaidferoz junaidferoz linked an issue Mar 26, 2026 that may be closed by this pull request
@junaidferoz junaidferoz self-assigned this Mar 26, 2026
@junaidferoz junaidferoz changed the title Dashboard: scoped bulk close for studies (workflow / owner / guest role) [FEATURE] it would be good to be able to select types of studies, to be able to not close all but still as a bulk request Mar 26, 2026
@junaidferoz junaidferoz added the enhancement New feature or request label Mar 26, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines “Bulk Close Studies” so operators can scope bulk-closing to a subset of open studies (by workflow / owner / guest-role), preview the matching count in the UI, and have the backend close exactly the study IDs computed by the client (with a strict mode to avoid falling back to “close everything”).

Changes:

  • Updated the bulk-close modal UI to support workflow/user scoping, show a live match count, and send an explicit study id list (studyIds + studyIdsJson) with bulkCloseUseIdList: true.
  • Updated the backend studyCloseBulk handler to resolve id lists robustly (array/object/JSON) and return closedCount.
  • Adjusted supporting config/ops files (sequelize CLI env loading; additional gitignore entries).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
frontend/src/components/dashboard/study/BulkCloseModal.vue Adds workflow/user/guest scoping UI, match count, and emits bulk close with explicit study id list.
frontend/src/components/dashboard/Study.vue Updates bulk-close button label/tooltip.
backend/webserver/sockets/study.js Adds id-list resolution + strict mode; filters/loads studies and returns closedCount.
backend/db/.sequelizerc Loads repo-root .env (and .env.$ENV) for sequelize-cli runs.
.gitignore Ignores a specific root artifact filename and root logs/.

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

* @returns {Promise<{closedCount: number}>} Number of studies that were closed (updated).
*/
async closeBulk(data, options) {

Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

studyCloseBulk currently performs a destructive operation (closing studies) without any server-side authorization check. Since any socket client can emit this event directly, this should enforce a permission (e.g., await this.isAdmin() or await this.hasAccess("frontend.dashboard.studies.closeAllStudies")) before proceeding, otherwise users can bypass the UI restriction and close studies they shouldn't.

Suggested change
// Enforce server-side authorization for bulk-closing studies.
if (!(await this.isAdmin()) && !(await this.hasAccess("frontend.dashboard.studies.closeAllStudies"))) {
throw new Error("No permission to bulk close studies");
}

Copilot uses AI. Check for mistakes.
Comment on lines +55 to +59
if (!Number.isFinite(id) || seen.has(id)) {
continue;
}
seen.add(id);
const s = await this.models['study'].getById(id, {}, false);
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loadStudiesForBulkCloseByIds issues one getById query per id and awaits them sequentially, which becomes an N+1 bottleneck for large bulk-closes. Consider fetching all studies in one query (e.g. this.models.study.getAllByKeyValues('id', ids) / findAll({ where: { id: { [Op.in]: ids } } })) and then filtering by projectId/template in-memory.

Copilot uses AI. Check for mistakes.
Comment on lines +156 to +160
const resolvedStudyIds = StudySocket.resolveBulkCloseStudyIds(data);
const idListMode = data.bulkCloseUseIdList === true;

let studies;
if (idListMode) {
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several new branching behaviors here (strict bulkCloseUseIdList mode, studyIdsJson parsing, and id-list vs legacy filter selection), but there are no socket tests covering studyCloseBulk in backend/tests/socket.study.test.js. Adding integration tests for the success path (closes exactly the provided ids) and error path (strict mode with missing/invalid id list) would help prevent regressions.

Copilot uses AI. Check for mistakes.
@junaidferoz junaidferoz changed the title [FEATURE] it would be good to be able to select types of studies, to be able to not close all but still as a bulk request Feature #134 closing studies based on selection Mar 30, 2026
@UKPLab UKPLab deleted a comment from Copilot AI Apr 8, 2026
Copy link
Copy Markdown
Collaborator Author

@junaidferoz junaidferoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes based on the review comments. Also metioned which commit is the change made

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Study Closing Type Selection

3 participants