Skip to content

Conversation

@yelyzavetakhokhlova
Copy link

@yelyzavetakhokhlova yelyzavetakhokhlova commented Nov 26, 2025

PR Checklist

  • Have you verified that the PR is pointing to the correct target branch? (develop for features/bugfixes, other if mentioned in the task)
  • Have you verified that your branch is consistent with the target branch and has no conflicts? (if not, make a rebase under the target branch)
  • Have you checked that everything works within the branch according to the task description and tested it locally?
  • Have you run the linter (npm run lint) prior to submission? Enable the git hook on commit in your IDE to run it and format the code automatically.
  • Have you run the tests locally and added/updated them if needed?
  • Have you checked that app can be built (npm run build)?
  • Have you checked that no new circular dependencies appreared with your changes? (the webpack plugin reports circular dependencies within the dev npm script)
  • Have you made sure that all the necessary pipelines has been successfully completed?
  • If the task requires translations to be updated, have you done this by running the manage:translations script?
  • Have you added the link to the PR in the Jira ticket comments?

Visuals

Screen.Recording.2025-11-26.at.20.40.39.mov

Summary by CodeRabbit

  • New Features

    • Added batch delete functionality for test cases with confirmation modal and impact warnings regarding associated Test Plans
  • Refactor

    • Enhanced test case selection tracking and streamlined data refresh mechanisms

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 26, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Introduces batch delete modal for test cases with folder counter updates. Refactors test case selection state from flat array to richer SelectedTestCaseRow type. Renames bulkUpdateTestCases URL generator to testCasesBatch. Extracts test case refetch logic into a dedicated hook. Updates affected components to use new selection and refetch APIs.

Changes

Cohort / File(s) Change Summary
URL and export naming
app/src/common/urls.js, app/src/controllers/testCase/index.ts, app/src/controllers/testCase/selectors.ts
Renamed bulkUpdateTestCases(projectKey) to testCasesBatch(projectKey) in URL generator. Re-exported updateFolderCounterAction from controllers. Strengthened typing of testCasesSelector to return TestCase[] explicitly.
Selection state refactoring
app/src/pages/inside/common/testCaseList/testCaseList.tsx, app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/allTestCasesPage.tsx, app/src/pages/inside/testPlansPage/testPlanDetailsPage/testPlanFolders/allTestCasesPage/allTestCasesPage.tsx
Replaced flat selectedRowIds: number[] state with richer selectedRows: SelectedTestCaseRow[] model. Updated TestCaseList props from handleSelectedRowIds to selectedRows and handleSelectedRows. Removed xor utility in favor of explicit selection computation. Added SelectedTestCaseRow interface export.
Batch delete modal feature
app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/*
New modal component with hook for batch deletion of test cases. Includes localized messages, folder counter delta mapping, loading states, and success/error notifications. Exports key, modal component, and hooks for integration.
Test case refetch hook
app/src/pages/inside/testCaseLibraryPage/hooks/useRefetchCurrentTestCases.ts
New hook useRefetchCurrentTestCases that encapsulates conditional dispatch logic for fetching test cases by folder or all test cases based on current URL state and pagination.
Refetch hook adoption
app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDuplicateToFolderModal/useBatchDuplicateToFolder.ts, app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/editTestCaseModal/useUpdateTestCase.ts, app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/changePriorityModal.tsx
Replaced individual action dispatches with unified useRefetchCurrentTestCases hook. Updated BulkUpdateTestCasesPayload to remove folderId field; changed URL reference to testCasesBatch. Simplified Redux selector usage.
Formatting
app/src/pages/inside/testCaseLibraryPage/testCaseFolders/modals/duplicateFolderModal/useDuplicateFolder.ts
Minor whitespace adjustment.

Sequence Diagram

sequenceDiagram
    participant User
    participant UI as TestCaseList / Modal
    participant SelectionState as allTestCasesPage State
    participant BatchDelete as useBatchDeleteTestCases
    participant API
    participant Redux
    participant Notifications

    User->>UI: Click Delete (or select rows)
    UI->>SelectionState: handleSelectedRows(selectedRows)
    SelectionState->>SelectionState: Update selectedRows: SelectedTestCaseRow[]
    SelectionState->>SelectionState: Compute folderDeltasMap via countBy
    User->>UI: Confirm delete in modal
    UI->>BatchDelete: batchDelete(testCaseIds, folderDeltasMap)
    BatchDelete->>BatchDelete: Show loading spinner
    BatchDelete->>API: DELETE /project/{key}/tms/test-case/batch
    API-->>BatchDelete: Success response
    BatchDelete->>Redux: Dispatch updateFolderCounterAction (per folder)
    BatchDelete->>Redux: Dispatch useRefetchCurrentTestCases()
    Redux->>API: Fetch updated test cases
    API-->>Redux: Updated list
    BatchDelete->>SelectionState: onClearSelection()
    SelectionState->>SelectionState: Clear selectedRows
    BatchDelete->>Notifications: Show success notification
    BatchDelete->>UI: Hide modal & spinner
    UI->>User: Updated list displayed
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Areas requiring extra attention:

  • Selection state refactoring across multiple components (TestCaseList, allTestCasesPage in two locations) — ensure consistency in the new SelectedTestCaseRow model and prop handling
  • Batch delete modal integration with folder counter updates (countBy logic, folderDeltasMap computation) — verify delta calculations align with actual deletions
  • Hook consolidation in useUpdateTestCase.ts and related files — confirm that the unified refetch hook correctly handles both folder-specific and global test case fetches
  • Type safety improvements in testCasesSelector — validate that TestCase[] typing doesn't break existing consumers

Possibly related PRs

Suggested reviewers

  • AmsterGet
  • allaprischepa
  • maria-hambardzumian
  • ViktorSoroka07
  • OleksandrDragun

Poem

🐰 A tale of selected rows and folders freed,
Batch deletes now bloom where selections lead,
From flat arrays to richer types we'd say,
Refetch hooks simplify the testing way—
In modals bright, the counter updates dance,
CodeRabbit hops through change with swift advance!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Implement batch test case removal' clearly and directly summarizes the main objective of the changeset, which introduces batch deletion functionality for test cases.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yelyzavetakhokhlova
Copy link
Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Nov 26, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@yelyzavetakhokhlova yelyzavetakhokhlova force-pushed the test-library/batch-test-case-removal branch from 3db6d96 to 31469b4 Compare November 26, 2025 18:55
@codecov-commenter
Copy link

codecov-commenter commented Nov 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.59%. Comparing base (ad78371) to head (328d946).
⚠️ Report is 2 commits behind head on feature/test-library.

Additional details and impacted files
@@                  Coverage Diff                  @@
##           feature/test-library    #4759   +/-   ##
=====================================================
  Coverage                 72.59%   72.59%           
=====================================================
  Files                        79       79           
  Lines                       905      905           
  Branches                    124      124           
=====================================================
  Hits                        657      657           
  Misses                      224      224           
  Partials                     24       24           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yelyzavetakhokhlova
Copy link
Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Nov 26, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (6)
app/src/controllers/testCase/selectors.ts (1)

72-73: testCasesSelector return type and fallback are consistent; optional consistency refactor

Returning TestCase[] and defaulting to [] is a good, null‑safe pattern and matches the updated state typing.

If you want to keep selectors consistent with others that go through testCaseSelector, you could optionally refactor as:

-export const testCasesSelector = (state: RootState): TestCase[] =>
-  state.testCase?.testCases?.list || [];
+export const testCasesSelector = (state: RootState): TestCase[] =>
+  testCaseSelector(state).testCases?.list || [];

Purely a style/consistency tweak; current code is fine.

app/src/pages/inside/testPlansPage/testPlanDetailsPage/testPlanFolders/allTestCasesPage/allTestCasesPage.tsx (1)

29-29: Consider the architectural implications of cross-feature imports.

The import of SelectedTestCaseRow from testCaseLibraryPage into testPlansPage creates a dependency between these features. If SelectedTestCaseRow is truly shared across multiple features, consider moving it to a common types location (e.g., pages/inside/common/types or types/testCase) to better reflect its shared nature and reduce coupling.

app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/useBatchDeleteTestCases.ts (2)

35-43: Consider adding input validation for edge cases.

The batchDelete function doesn't validate that testCaseIds is non-empty before making the API call. While the UI likely prevents this, defensive validation would make the hook more robust and prevent unnecessary API calls.

Apply this pattern:

 const batchDelete = useCallback(
   async (testCaseIds: number[], folderDeltasMap: Record<number, number>) => {
+    if (testCaseIds.length === 0) {
+      return;
+    }
+
     showSpinner();

61-64: Generic error notification lacks context.

The error handler displays a generic "errorOccurredTryAgain" message without logging the error or providing specific context about the batch delete operation. This makes debugging difficult in production.

Consider logging the error and providing more context:

 } catch (error: unknown) {
+  console.error('Batch delete test cases failed:', error);
   showErrorNotification({
-    messageId: 'errorOccurredTryAgain',
+    messageId: 'testCasesBatchDeleteFailed',
   });
 }
app/src/pages/inside/common/testCaseList/testCaseList.tsx (2)

90-104: Selection logic correctly tracks folder IDs.

The refactored handleRowSelect properly finds the test case to extract folderId and maintains the new SelectedTestCaseRow structure. The toggle logic with .some() and .filter() is clear and correct.

Optional performance consideration: For large test case lists, the testCases.find() on Line 91 could become a bottleneck. Consider creating a Map of id → testCase if performance becomes an issue.


106-122: Select all logic is correct but could be optimized.

The handleSelectAll function properly toggles selection for the current page and maintains the SelectedTestCaseRow structure. However, there are minor optimization opportunities:

  • Lines 107-109: Creates currentPageTestCaseIds array, then checks each ID with .includes() (O(n²) in worst case)
  • Lines 116-118: Filters and maps separately (two iterations)

Consider this optimization:

 const handleSelectAll = () => {
-  const currentPageTestCaseIds = currentData.map(({ id }) => id);
-  const isAllCurrentPageSelected = currentPageTestCaseIds.every((testCaseId) =>
-    selectedRowIds.includes(testCaseId),
-  );
+  const selectedIdSet = new Set(selectedRowIds);
+  const isAllCurrentPageSelected = currentData.every(({ id }) => selectedIdSet.has(id));

   const newSelectedRows = isAllCurrentPageSelected
-    ? selectedRows.filter((row) => !currentPageTestCaseIds.includes(row.id))
+    ? selectedRows.filter((row) => !currentData.some(tc => tc.id === row.id))
     : [
         ...selectedRows,
-        ...currentData
-          .filter((testCase) => !selectedRowIds.includes(testCase.id))
-          .map((testCase) => ({ id: testCase.id, folderId: testCase.testFolder.id })),
+        ...currentData
+          .filter((testCase) => !selectedIdSet.has(testCase.id))
+          .map((testCase) => ({ id: testCase.id, folderId: testCase.testFolder.id })),
       ];
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ad78371 and 31469b4.

📒 Files selected for processing (17)
  • app/src/common/urls.js (1 hunks)
  • app/src/controllers/testCase/index.ts (1 hunks)
  • app/src/controllers/testCase/selectors.ts (2 hunks)
  • app/src/pages/inside/common/testCaseList/testCaseList.tsx (6 hunks)
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/allTestCasesPage.tsx (7 hunks)
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/batchDeleteTestCasesModal.scss (1 hunks)
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/batchDeleteTestCasesModal.tsx (1 hunks)
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/index.ts (1 hunks)
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/messages.ts (1 hunks)
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/useBatchDeleteTestCases.ts (1 hunks)
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/useBatchDeleteTestCasesModal.tsx (1 hunks)
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDuplicateToFolderModal/useBatchDuplicateToFolder.ts (4 hunks)
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/changePriorityModal.tsx (1 hunks)
  • app/src/pages/inside/testCaseLibraryPage/hooks/useRefetchCurrentTestCases.ts (1 hunks)
  • app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/editTestCaseModal/useUpdateTestCase.ts (3 hunks)
  • app/src/pages/inside/testCaseLibraryPage/testCaseFolders/modals/duplicateFolderModal/useDuplicateFolder.ts (1 hunks)
  • app/src/pages/inside/testPlansPage/testPlanDetailsPage/testPlanFolders/allTestCasesPage/allTestCasesPage.tsx (3 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: yelyzavetakhokhlova
Repo: reportportal/service-ui PR: 4615
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/useCreateTestCase.ts:22-23
Timestamp: 2025-09-22T11:43:56.813Z
Learning: In the test case creation modal (app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/), testFolderId is intentionally hardcoded to 85 as a temporary measure until folder selection functionality is implemented.
📚 Learning: 2025-09-22T11:43:56.813Z
Learnt from: yelyzavetakhokhlova
Repo: reportportal/service-ui PR: 4615
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/useCreateTestCase.ts:22-23
Timestamp: 2025-09-22T11:43:56.813Z
Learning: In the test case creation modal (app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/), testFolderId is intentionally hardcoded to 85 as a temporary measure until folder selection functionality is implemented.

Applied to files:

  • app/src/controllers/testCase/index.ts
  • app/src/pages/inside/testCaseLibraryPage/testCaseFolders/modals/duplicateFolderModal/useDuplicateFolder.ts
  • app/src/pages/inside/testCaseLibraryPage/hooks/useRefetchCurrentTestCases.ts
  • app/src/pages/inside/common/testCaseList/testCaseList.tsx
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/changePriorityModal.tsx
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/allTestCasesPage.tsx
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDuplicateToFolderModal/useBatchDuplicateToFolder.ts
  • app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/editTestCaseModal/useUpdateTestCase.ts
  • app/src/pages/inside/testPlansPage/testPlanDetailsPage/testPlanFolders/allTestCasesPage/allTestCasesPage.tsx
📚 Learning: 2025-07-24T11:26:08.671Z
Learnt from: allaprischepa
Repo: reportportal/service-ui PR: 4487
File: app/src/pages/organization/projectTeamPage/projectTeamListTable/projectTeamListTable.jsx:105-105
Timestamp: 2025-07-24T11:26:08.671Z
Learning: In the ReportPortal service-ui project, the react-hooks/exhaustive-deps ESLint rule is strictly enforced, requiring ALL dependencies used within useEffect, useMemo, and useCallback hooks to be included in the dependency array, even stable references like formatMessage from useIntl(). This prevents ESLint errors and maintains code consistency.

Applied to files:

  • app/src/pages/inside/common/testCaseList/testCaseList.tsx
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/allTestCasesPage.tsx
📚 Learning: 2025-08-14T12:06:14.147Z
Learnt from: allaprischepa
Repo: reportportal/service-ui PR: 4543
File: app/src/pages/inside/projectSettingsPageContainer/content/analyzerContainer/autoAnalysis/autoAnalysis.jsx:17-17
Timestamp: 2025-08-14T12:06:14.147Z
Learning: In the reportportal/service-ui project, webpack uses ProvidePlugin to automatically provide React as a global variable, so explicit `import React from 'react'` is not needed in .jsx files for JSX to work. The project uses classic JSX runtime with automatic React injection via webpack.

Applied to files:

  • app/src/pages/inside/common/testCaseList/testCaseList.tsx
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/allTestCasesPage.tsx
📚 Learning: 2025-06-09T17:12:07.281Z
Learnt from: Guria
Repo: reportportal/service-ui PR: 4385
File: app/src/components/testCaseList/testCaseNameCell/testCaseNameCell.tsx:30-42
Timestamp: 2025-06-09T17:12:07.281Z
Learning: In React components, static objects like icon mappings that don't depend on props or state should be defined outside the component function to avoid unnecessary re-creation on every render, improving performance.

Applied to files:

  • app/src/pages/inside/common/testCaseList/testCaseList.tsx
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/allTestCasesPage.tsx
📚 Learning: 2025-06-18T14:36:19.317Z
Learnt from: yelyzavetakhokhlova
Repo: reportportal/service-ui PR: 4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:75-86
Timestamp: 2025-06-18T14:36:19.317Z
Learning: In ReportPortal TypeScript components, noop handlers are sometimes used to satisfy TypeScript requirements for required props, not because functionality is missing. This is a common pattern when the actual form handling is managed by wrapper components.

Applied to files:

  • app/src/pages/inside/common/testCaseList/testCaseList.tsx
  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/allTestCasesPage.tsx
📚 Learning: 2025-09-04T09:25:36.100Z
Learnt from: allaprischepa
Repo: reportportal/service-ui PR: 4589
File: app/src/pages/inside/common/modals/renameOrganizationModal/renameOrganizationModal.tsx:90-95
Timestamp: 2025-09-04T09:25:36.100Z
Learning: In the ReportPortal service-ui project, it's a common approach to use the `dirty` prop from redux-form to control modal closing behavior - specifically to prohibit modal closing when changes were made in the form. This pattern should be maintained for consistency across the project.

Applied to files:

  • app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/changePriorityModal.tsx
📚 Learning: 2025-06-26T13:05:48.078Z
Learnt from: allaprischepa
Repo: reportportal/service-ui PR: 4434
File: app/src/controllers/organization/projects/sagas.js:141-172
Timestamp: 2025-06-26T13:05:48.078Z
Learning: In the ReportPortal service-ui codebase, the JSON Patch operations for project renaming use a custom path format where the `path` property is specified as 'name' (without leading forward slash), which differs from RFC 6902 standard but matches their API implementation requirements.

Applied to files:

  • app/src/common/urls.js
🧬 Code graph analysis (8)
app/src/pages/inside/testCaseLibraryPage/hooks/useRefetchCurrentTestCases.ts (4)
app/src/controllers/pages/typed-selectors.ts (1)
  • urlFolderIdSelector (55-63)
app/src/controllers/testCase/selectors.ts (1)
  • testCasesPageSelector (75-76)
app/src/pages/inside/testCaseLibraryPage/utils.ts (1)
  • getTestCaseRequestParams (40-57)
app/src/controllers/testCase/index.ts (2)
  • getTestCaseByFolderIdAction (20-20)
  • getAllTestCasesAction (19-19)
app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/useBatchDeleteTestCasesModal.tsx (2)
app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/index.ts (2)
  • useBatchDeleteTestCasesModal (17-17)
  • BATCH_DELETE_TEST_CASES_MODAL_KEY (18-18)
app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/batchDeleteTestCasesModal.tsx (2)
  • BatchDeleteTestCasesModalData (38-42)
  • BATCH_DELETE_TEST_CASES_MODAL_KEY (36-36)
app/src/pages/inside/common/testCaseList/testCaseList.tsx (1)
app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/allTestCasesPage.tsx (1)
  • SelectedTestCaseRow (67-70)
app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/useBatchDeleteTestCases.ts (3)
app/src/pages/inside/testCaseLibraryPage/hooks/useRefetchCurrentTestCases.ts (1)
  • useRefetchCurrentTestCases (28-47)
app/src/common/utils/fetch.ts (1)
  • fetch (61-92)
app/src/controllers/testCase/index.ts (1)
  • updateFolderCounterAction (21-21)
app/src/controllers/testCase/selectors.ts (1)
app/src/pages/inside/testCaseLibraryPage/types.ts (1)
  • TestCase (76-94)
app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDuplicateToFolderModal/useBatchDuplicateToFolder.ts (1)
app/src/pages/inside/testCaseLibraryPage/hooks/useRefetchCurrentTestCases.ts (1)
  • useRefetchCurrentTestCases (28-47)
app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/editTestCaseModal/useUpdateTestCase.ts (1)
app/src/pages/inside/testCaseLibraryPage/hooks/useRefetchCurrentTestCases.ts (1)
  • useRefetchCurrentTestCases (28-47)
app/src/pages/inside/testPlansPage/testPlanDetailsPage/testPlanFolders/allTestCasesPage/allTestCasesPage.tsx (1)
app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/allTestCasesPage.tsx (1)
  • SelectedTestCaseRow (67-70)
🪛 GitHub Check: SonarCloud Code Analysis
app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/batchDeleteTestCasesModal.tsx

[warning] 96-96: Move this component definition out of the parent component and pass data as props.

See more on https://sonarcloud.io/project/issues?id=reportportal_service-ui&issues=AZrBeVHwUWt__NWe_Q1s&open=AZrBeVHwUWt__NWe_Q1s&pullRequest=4759

🔇 Additional comments (17)
app/src/controllers/testCase/index.ts (1)

21-21: Export verified and properly defined.

The updateFolderCounterAction is correctly defined in actionCreators.ts (line 186) and properly exported in index.ts (line 21). The export statement is syntactically correct and aligns with the PR objective to implement batch test case removal with folder counter updates. No issues found.

app/src/controllers/testCase/selectors.ts (1)

28-40: Stronger typing of testCases.list to TestCase[] looks good

Aligning testCases.list with the TestCase interface improves type safety and eliminates the need for casts at call sites. This should make downstream selector/consumer code clearer without affecting runtime behavior.

app/src/pages/inside/testCaseLibraryPage/testCaseFolders/modals/duplicateFolderModal/useDuplicateFolder.ts (1)

51-52: LGTM!

The formatting change (added empty line) improves readability with no functional impact.

app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/batchDeleteTestCasesModal.scss (1)

17-23: LGTM!

The SCSS styling for bold text emphasis is properly structured and aligns with the modal's UI requirements.

app/src/pages/inside/testCaseLibraryPage/hooks/useRefetchCurrentTestCases.ts (1)

28-47: LGTM! Effective refactoring that centralizes refetch logic.

The hook properly consolidates test case refetch logic that was previously duplicated across multiple modal hooks. The conditional dispatching based on urlFolderId and the dependency array are correct.

app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDuplicateToFolderModal/useBatchDuplicateToFolder.ts (1)

28-28: LGTM! Clean refactoring using the new centralized hook.

The refactoring successfully replaces the conditional dispatch logic with the useRefetchCurrentTestCases hook, reducing code duplication and improving maintainability.

Also applies to: 44-44, 71-73, 91-91

app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/useBatchDeleteTestCasesModal.tsx (1)

24-28: LGTM!

The modal hook follows the standard pattern and is properly typed with BatchDeleteTestCasesModalData.

app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/index.ts (1)

17-18: LGTM!

The barrel export file properly consolidates the batch delete modal exports.

app/src/common/urls.js (1)

379-379: All references to the old bulkUpdateTestCases URL generator have been successfully updated. The testCasesBatch URL generator is correctly used in useUpdateTestCase.ts (line 78), and all call sites in changePriorityModal.tsx are using the updated function properly. No orphaned references remain.

app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/changePriorityModal.tsx (1)

47-53: Based on my verification of the codebase, I can now provide the rewritten review comment.

The payload structure is correctly implemented and intentionally designed — no backend compatibility issues identified.

The BulkUpdateTestCasesPayload type in useUpdateTestCase.ts explicitly defines testCaseIds and priority as the only fields. The removal of folderId from bulk operations appears to be intentional architecture: single test case updates (UpdateTestCasePayload) include testFolderId, while bulk updates deliberately exclude it. The payload sent from changePriorityModal.tsx matches the type definition exactly, and there are no type errors or compile-time issues. Since this is the current type definition in the codebase and the code compiles successfully, the backend integration for this simplified payload structure is already in place.

app/src/pages/inside/testPlansPage/testPlanDetailsPage/testPlanFolders/allTestCasesPage/allTestCasesPage.tsx (1)

53-66: Selection state maintained despite selectable={false}.

The component maintains selectedRows state and passes selection props to TestCaseList, but Line 68 sets selectable={false}. Verify this is intentional—if selection UI is hidden but state tracking is needed for other purposes, consider adding a comment explaining why.

app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/allTestCasesPage.tsx (2)

67-70: LGTM! Well-structured interface for richer selection tracking.

The SelectedTestCaseRow interface correctly captures both id and folderId, enabling accurate folder counter updates during batch operations. This is a solid foundation for the batch delete feature.


86-91: Good optimization with useMemo for derived selection IDs.

The derived selectedRowIds is correctly memoized to prevent unnecessary re-renders when passing to child components.

app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/batchDeleteTestCasesModal.tsx (1)

46-60: Modal integration and loading state management look solid.

The hook integration properly handles success callbacks, modal dismissal, and selection clearing. The loading overlay is correctly positioned to block interaction during the operation.

app/src/pages/inside/testCaseLibraryPage/allTestCasesPage/batchDeleteTestCasesModal/messages.ts (1)

19-29: LGTM! Well-structured localization messages.

The message definitions follow react-intl best practices with proper pluralization and semantic formatting. The bold emphasis appropriately highlights the count and irreversible nature of the action.

app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/editTestCaseModal/useUpdateTestCase.ts (2)

25-25: Excellent refactoring to use centralized refetch logic.

Replacing manual action dispatching with the useRefetchCurrentTestCases hook reduces code duplication and ensures consistent refetch behavior across the application. This aligns well with the DRY principle.

Also applies to: 42-42, 85-85


27-36: Payload type updates align with API evolution.

The changes from folderId?: string to testFolderId: number and removal of folderId from bulk updates reflect a cleaner, more type-safe API contract. The required testFolderId ensures the field is always present when needed.

@yelyzavetakhokhlova yelyzavetakhokhlova force-pushed the test-library/batch-test-case-removal branch from 31469b4 to 328d946 Compare November 27, 2025 11:50
@sonarqubecloud
Copy link

@yelyzavetakhokhlova yelyzavetakhokhlova requested a review from a team November 27, 2025 12:46
@yelyzavetakhokhlova yelyzavetakhokhlova merged commit 2a8a144 into feature/test-library Nov 27, 2025
9 checks passed
@yelyzavetakhokhlova yelyzavetakhokhlova deleted the test-library/batch-test-case-removal branch November 27, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants