Trashmodalunit test by Prashant-thakur77 · Pull Request #5816 · learningequality/studio

Prashant-thakur77 · 2026-04-07T11:19:52Z

Summary

Migrates trashModal.spec.js from @vue/test-utils to @testing-library/vue, as part of the broader effort to make Studio's frontend tests more user-centric and resilient to implementation changes.
Manual verification:

Logged in as a@a.com, opened a published channel, removed resources, and verified the trash modal behavior .
All the test cases passed.

Screencast.From.2026-04-07.16-36-39.webm

References

Closes #5790
See #5789

Reviewer guidance

1)Only trashModal.spec.js was changed.
2) Reviewer need to check if all the needed testcases are present and testing-library is properly used.

AI usage

I used Claude to help plan and implement the migration.I checked other Testing-library implementations and verified the tests written and updated wherever needed.I followed the guidance given in the issue description and its parent issue description.I again checked if all the test cases are passing properly.

learning-equality-bot · 2026-04-07T11:20:27Z

👋 Hi @Prashant-thakur77, thanks for contributing!

For the review process to begin, please verify that the following is satisfied:

Contribution is aligned with our contributing guidelines
Pull request description has correctly filled AI usage section & follows our AI guidance:
AI guidance

State explicitly whether you didn't use or used AI & how.

If you used it, ensure that the PR is aligned with Using AI as well as our DEEP framework. DEEP asks you:
- Disclose — Be open about when you've used AI for support.
- Engage critically — Question what is generated. Review code for correctness and unnecessary complexity.
- Edit — Review and refine AI output. Remove unnecessary code and verify it still works after your edits.
- Process sharing — Explain how you used the AI so others can learn.
Examples of good disclosures:

"I used Claude Code to implement the component, prompting it to follow the pattern in ComponentX. I reviewed the generated code, removed unnecessary error handling, and verified the tests pass."

"I brainstormed the approach with Gemini, then had it write failing tests for the feature. After reviewing the tests, I used Claude Code to generate the implementation. I refactored the output to reduce verbosity and ran the full test suite."

Also check that issue requirements are satisfied & you ran pre-commit locally.

Pull requests that don't follow the guidelines will be closed.

Reviewer assignment can take up to 2 weeks.

Prashant-thakur77 · 2026-04-07T11:20:39Z

Hlo @MisRob ,I have resolved the issue,do review when you have time.

learning-equality-bot · 2026-04-11T05:35:33Z

📢✨ Before we assign a reviewer, we'll turn on @rtibblesbot to pre-review. Its comments are generated by an LLM, and should be evaluated accordingly.

learning-equality-bot · 2026-04-11T05:35:55Z

📢✨ Before we assign a reviewer, we'll invite community pre-review. See the community review guidance for both authors and reviewers.

AllanOXDi

Hi @Prashant-thakur77, thanks for working on this. I left a few clean up

AllanOXDi · 2026-04-15T15:39:00Z

+    .mockResolvedValue({});
+  jest.spyOn(TrashModal.methods, 'loadAncestors').mockResolvedValue();
+  jest.spyOn(TrashModal.methods, 'removeContentNodes').mockResolvedValue();
+  const loadNodesSpy = jest.spyOn(TrashModal.methods, 'loadNodes');


This works but is non-obvious; add a comment explaining that real execution is intentional.

AllanOXDi · 2026-04-15T15:41:55Z

+    jest.spyOn(TrashModal.methods, 'loadChildren').mockReturnValue(new Promise(() => {}));
+  } else {
+    jest.spyOn(TrashModal.methods, 'loadChildren').mockResolvedValue({
+      more: items === testChildren ? null : { parent: TRASH_ID, page: 2 },


using (===) to decide pagination is not quite right "show more unless the exact same array object was passed." Use an explicit hasMore parameter instead.

AllanOXDi · 2026-04-15T17:13:28Z

-      expect(wrapper.vm.selected).toEqual([]);
-      expect(wrapper.vm.previewNodeId).toBe(null);
+      await waitFor(() => {
+        expect(loadContentNodesSpy).toHaveBeenCalledWith({ parent: TRASH_ID, page: 2 });


The assertion uses toHaveBeenCalledWith, which passes if any call matches, so the initial call and the "Show more" call are both included. This passes correctly but could be more precise with toHaveBeenLastCalledWith to explicitly assert the pagination call was the most recent one.

AllanOXDi · 2026-04-15T17:15:27Z

+
+      await user.click(showMoreBtn);

-    it('moveNoves should clear selected and previewNodeId', async () => {


The original had a test for clicking an item, setting previewNodeId (which opens ResourceDrawer). This has been dropped entirely. It should either be added back or explicitly noted in the PR description as an intentional omission.

I’ve restored the missing coverage in line with Testing Library patterns:

Reintroduced the test that verifies clicking an item sets previewNodeId and opens the ResourceDrawer.

Reworked the previous moveNodes test from the diff to follow user interactions instead of directly invoking wrapper.vm. The test now selects an item, opens the preview drawer, completes the “Move here” flow, and asserts that selections are cleared and the drawer is closed.
What are your thoughts on this apporach @AllanOXDi .

AllanOXDi · 2026-04-15T17:18:33Z

 import { RouteNames } from '../../../constants';
+import TrashModal from '../TrashModal';

 const store = factory();


Move inside makeWrapper so each test gets a clean store.

AllanOXDi · 2026-04-15T17:25:44Z

+  const utils = render(
+    TrashModal,
+    {
+      store,


Passing the shared module-level store into render. See line 9, the shared store's dispatch spy in the deletion test ( see line 200) may not be fully cleaned up before subsequent tests run.

Fixed as part of moving store into makeWrapper

AllanOXDi · 2026-04-15T17:28:36Z

+      expect(screen.getByTestId('delete')).toBeDisabled();
+      expect(screen.getByTestId('restore')).toBeDisabled();
+
+      await user.click(screen.getAllByRole('checkbox')[1]);


A bit brittle. [1] assumes the second checkbox in DOM order is the first item checkbox (index 0 being select-all). If any component renders an additional checkbox before the list, this clicks the wrong element. Use screen.getAllByTestId('checkbox')[0] instead.

Done. Changed to within(screen.getAllByTestId('checkbox')[0]).getByRole('checkbox').
Note: data-test="checkbox" is on the k-checkbox-container div, not the input, so within() is needed to reach the actual checkbox input for clicking.

AllanOXDi · 2026-04-15T17:36:52Z

+
+      await waitFor(() => {
+        screen
+          .getAllByRole('checkbox')


Relies on the select-all being first in DOM order. Works today but fragile for the same reason as line 117.

AllanOXDi · 2026-04-15T17:38:28Z

+
+    it('successful deletion triggers snackbar and reloads nodes', async () => {
+      jest.spyOn(TrashModal.methods, 'deleteContentNodes').mockResolvedValue();
+      const dispatchSpy = jest.spyOn(store, 'dispatch').mockImplementation(() => Promise.resolve());


Since makeWrapper is called after this line, the spy is active during mount. But because the store is shared (line 9), this mock intercepts dispatch calls in other tests if cleanup races. The fix is again to move store into makeWrapper.

Prashant-thakur77 · 2026-04-16T11:02:06Z

Hlo @AllanOXDi I have done the requested changes,do look at them when you have time .Please do tell if any other changes are required.

rtibbles assigned AllanOXDi Apr 7, 2026

Prashant-thakur77 force-pushed the trashmodalunit-test branch from 08ea025 to 90f477b Compare April 7, 2026 18:56

AllanOXDi assigned rtibblesbot Apr 8, 2026

MisRob unassigned rtibblesbot Apr 11, 2026

MisRob requested review from AllanOXDi and rtibblesbot April 11, 2026 05:34

MisRob added the community-review label Apr 11, 2026

AllanOXDi requested changes Apr 15, 2026

View reviewed changes

Prashant-thakur77 added 9 commits April 16, 2026 15:48

test: Made changes to trashModal test file to use vue testing library

a95ff09

Updated the test file

cfb8bf8

Updated test file again

19ab7bc

Added necessary stubs

95e1bdf

Finalizing

b46e073

Updated test file

378faf9

Updated test file

fa90400

Refined testcases

03c1662

Updated the test based on reviwer feedback

812d4fc

Prashant-thakur77 force-pushed the trashmodalunit-test branch from 90f477b to 812d4fc Compare April 16, 2026 11:00


		await user.click(showMoreBtn);

		it('moveNoves should clear selected and previewNodeId', async () => {

Uh oh!

Conversation

Prashant-thakur77 commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

References

Reviewer guidance

AI usage

Uh oh!

learning-equality-bot bot commented Apr 7, 2026

Uh oh!

Prashant-thakur77 commented Apr 7, 2026

Uh oh!

learning-equality-bot bot commented Apr 11, 2026

Uh oh!

learning-equality-bot bot commented Apr 11, 2026

Uh oh!

AllanOXDi 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!

AllanOXDi Apr 15, 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!

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!

Prashant-thakur77 Apr 16, 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!

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!

Prashant-thakur77 Apr 16, 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!

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!

Prashant-thakur77 commented Apr 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Prashant-thakur77 commented Apr 7, 2026 •

edited

Loading

AllanOXDi Apr 15, 2026 •

edited

Loading

Prashant-thakur77 Apr 16, 2026 •

edited

Loading

Prashant-thakur77 Apr 16, 2026 •

edited

Loading