Refactored Advertisements test from Jest to Vitest #2486

[khushipatil1523]

What kind of change does this PR introduce?

refactoring

Issue Number: #2484

Fixes ##2484

Did you add tests for your changes?

Vitest tes passed 👍

Summary by CodeRabbit

• New Features

  • Introduced a new configuration for the Vitest testing framework to enhance testing capabilities.
  • Added exclusion settings in TypeScript configuration to streamline the compilation process.

• Bug Fixes

  • Updated assertions in multiple test cases to improve clarity and consistency in toast notification checks.

• Documentation

  • Enhanced metadata in the project configuration for better documentation and structure.

• Chores

  • Updated various dependencies to their latest versions for improved performance and security.
  • Modified pre-commit hooks to streamline the development workflow.

[coderabbitai]

Walkthrough

This pull request introduces a comprehensive migration from Jest to Vitest for testing in the Talawa Admin project. The changes span multiple files, including configuration files like jest.config.js, tsconfig.json, and package.json, as well as numerous test files across different components and screens. The primary focus is on updating test assertions, mocking frameworks, and testing configurations to align with Vitest's syntax and features.

Changes

File	Change Summary
.husky/pre-commit	Commented out formatting and linting commands, kept npm test
jest.config.js	Updated coverage collection and test matching configurations
package.json	Updated dependencies, added Vitest-related scripts and dependencies
tsconfig.json	Added exclude property for compilation
vitest.config.ts	New configuration file for Vitest testing framework
Multiple test files	Replaced toBeCalledWith with toHaveBeenCalledWith for toast notifications

Possibly related issues

• Refactor : src/components/Advertisements/Advertisements.test.tsx from Jest to Vitest #2486: Refactoring Advertisements.test.tsx from Jest to Vitest
• Refactor : src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.test.tsx from Jest to Vitest #2485: Refactoring AdvertisementRegister.test.tsx from Jest to Vitest
• Refactor : src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.test.tsx from Jest to Vitest #2484: Refactoring AdvertisementEntry.test.tsx from Jest to Vitest
• Refactor: src/screens/UserPortal/Campaigns from Jest to Vitest #2571: Refactoring UserPortal/Campaigns tests from Jest to Vitest
• Refactor: src/screens/Requests from Jest to Vitest #2569: Refactoring Requests tests from Jest to Vitest

Possibly related PRs

• feat!: Migrate Testing Framework from Jest to Vitest #2457: Migration of testing framework from Jest to Vitest
• Refactored src/components/AddOn/support/components/Action/Action.test.tsx from Jest to Vitest #2482: Refactoring tests for the Action component from Jest to Vitest
• Refactor: Adding vitest to Avatar component #2622: Adding tests for the Avatar component
• Refactor: src/screens/UserPortal/Volunteer/UpcomingEvents from Jest to Vitest #2623: Migrating tests for the UserScreen component to Vitest
• refactored: src/screens/OrgSettings from Jest to Vitest(fixes: #2567) #2625: Refactoring tests for the OrgSettings component to Vitest

Poem

🐰 Hop, hop, testing we go!
From Jest to Vitest, watch our code flow
Assertions sharp, mocks precise
Our tests now dance with Vitest's might
Quality code, our rabbit's delight! 🧪

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[github-actions]

Our Pull Request Approval Process

Thanks for contributing!

Testing Your Code

Remember, your PRs won't be reviewed until these criteria are met:

1. We don't merge PRs with poor code quality.
   1. Follow coding best practices such that CodeRabbit.ai approves your PR.
2. We don't merge PRs with failed tests.
   1. When tests fail, click on the Details link to learn more.
   2. Write sufficient tests for your changes (CodeCov Patch Test). Your testing level must be better than the target threshold of the repository
   3. Tests may fail if you edit sensitive files. Ask to add the ignore-sensitive-files-pr label if the edits are necessary.
3. We cannot merge PRs with conflicting files. These must be fixed.

Our policies make our code better.

Reviewers

Do not assign reviewers. Our Queue Monitors will review your PR and assign them.
When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

1. comments in this PR or
2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

1. arbitrators of future discussions with other contributors about the validity of your changes
2. point of contact for evaluating the validity of your work
3. person who verifies matching issues by others that should be closed.
4. person who gives general guidance in fixing your tests

CONTRIBUTING.md

Read our CONTRIBUTING.md file. Most importantly:

1. PRs with issues not assigned to you will be closed by the reviewer
2. Fix the first comment in the PR so that each issue listed automatically closes

Other

1. 🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise.
2. Read the CONTRIBUTING.md file make

[github-actions]

Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.

[varshith257]

@khushipatil1523 I think it's your first contribution here. Welcome!

We don't accept PRs against main branch. You need to upstream your local branch with develop-postgres branch and submit PR against it. If you need any help feel free to reach out to Community in Slack.

You can follow this

> git remote add upstream https://github.com/PalisadoesFoundation/talawa-admin.git

> git fetch upstream

> git checkout -b refactor/test-jest-to-vitest-2486 upstream/develop-postgres

Now make changes in this branch and submit a new PR
Closing

[coderabbitai]

Actionable comments posted: 6

🔭 Outside diff range comments (19)

src/components/UserPortal/StartPostModal/StartPostModal.test.tsx (1)

Uncomment and update the success toast assertion to match implementation

The implementation shows that a success toast is displayed with the translation key 'postNowVisibleInFeed', but the test has a commented assertion with a hardcoded message. The test should be updated to assert the translated message.

• Update test assertion in src/components/UserPortal/StartPostModal/StartPostModal.test.tsx to:

expect(toast.success).toHaveBeenCalledWith(t('postNowVisibleInFeed'));

🔗 Analysis chain

Line range hint 153-156: Consider uncommenting success toast assertion

There's a commented-out success toast assertion. Consider either removing it completely or uncommenting and fixing it if it's needed for complete test coverage.

---

Let me gather more context about the specific test file to understand the toast usage pattern.

---

Let me check the implementation file to see if there's any success toast being shown after post creation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if success toasts are tested in other files
rg "toast.success.*toHaveBeenCalledWith" -A 2

Length of output: 14521

---

Script:

#!/bin/bash
# Check the test file content around the commented toast assertion
rg -B 5 -A 5 "Your post is now visible in the feed" src/components/UserPortal/StartPostModal/StartPostModal.test.tsx

# Check the implementation file to see if there's a success toast being shown
rg "toast.success.*visible.*feed" src/components/UserPortal/StartPostModal/StartPostModal.tsx

Length of output: 562

---

Script:

#!/bin/bash
# Check the implementation file for any success toast after post creation
rg "toast\." -A 2 src/components/UserPortal/StartPostModal/StartPostModal.tsx

Length of output: 310

src/components/OrgSettings/AgendaItemCategories/OrganizationAgendaCategory.test.tsx (2)

Line range hint 1-14: Update Jest imports and configurations for Vitest

The file still uses Jest-specific imports and configurations. For complete migration to Vitest:

-import 'jest-localstorage-mock';
-import 'jest-location-mock';
+import { beforeEach, afterEach, describe, expect, test, vi } from 'vitest';

---

Line range hint 27-33: Replace Jest mock with Vitest mock

Update the mock implementation to use Vitest's mocking system:

-jest.mock('react-toastify', () => ({
+vi.mock('react-toastify', () => ({
  toast: {
-    success: jest.fn(),
-    error: jest.fn(),
+    success: vi.fn(),
+    error: vi.fn(),
  },
}));

src/screens/OrganizationTags/OrganizationTags.test.tsx (1)

Line range hint 44-50: Update Jest mock with Vitest mock

Update the mock implementation:

-jest.mock('react-toastify', () => ({
+vi.mock('react-toastify', () => ({
  toast: {
-    success: jest.fn(),
-    error: jest.fn(),
+    success: vi.fn(),
+    error: vi.fn(),
  },
}));

src/components/UserPortal/Register/Register.test.tsx (1)

Line range hint 52-58: Update Jest mock with Vitest mock

Update the mock implementation:

-jest.mock('react-toastify', () => ({
+vi.mock('react-toastify', () => ({
  toast: {
-    success: jest.fn(),
-    warn: jest.fn(),
-    error: jest.fn(),
+    success: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
  },
}));

src/screens/ManageTag/ManageTag.test.tsx (1)

Line range hint 1-295: Complete the Jest to Vitest migration

While the toast assertion has been updated, there are several Jest-specific patterns that should be migrated to Vitest:

Apply these changes for complete migration:

-import 'jest-location-mock';
+import 'vitest-location-mock';

-jest.mock('react-toastify', () => ({
+vi.mock('react-toastify', () => ({
   toast: {
-    success: jest.fn(),
-    error: jest.fn(),
+    success: vi.fn(),
+    error: vi.fn(),
   },
 }));

 beforeEach(() => {
-  jest.mock('react-router-dom', () => ({
-    ...jest.requireActual('react-router-dom'),
+  vi.mock('react-router-dom', () => ({
+    ...vi.importActual('react-router-dom'),
     useParams: () => ({ orgId: 'orgId' }),
   }));
 });

 afterEach(() => {
-  jest.clearAllMocks();
+  vi.clearAllMocks();
 });

src/screens/SubTags/SubTags.test.tsx (1)

Line range hint 1-326: Migrate remaining Jest patterns to Vitest

Similar to ManageTag.test.tsx, this file needs complete migration to Vitest patterns.

Apply these changes:

-import 'jest-location-mock';
+import 'vitest-location-mock';

-jest.mock('react-toastify', () => ({
+vi.mock('react-toastify', () => ({
   toast: {
-    success: jest.fn(),
-    error: jest.fn(),
+    success: vi.fn(),
+    error: vi.fn(),
   },
 }));

 beforeEach(() => {
-  jest.mock('react-router-dom', () => ({
-    ...jest.requireActual('react-router-dom'),
+  vi.mock('react-router-dom', () => ({
+    ...vi.importActual('react-router-dom'),
     useParams: () => ({ orgId: 'orgId' }),
   }));
 });

 afterEach(() => {
-  jest.clearAllMocks();
+  vi.clearAllMocks();
 });

src/components/AgendaCategory/AgendaCategoryContainer.test.tsx (1)

Update test configuration and mocks for Vitest environment

The test file needs to be updated to align with Vitest's configuration and mocking patterns. Based on the setup files, the project is currently using Jest but has a Vitest configuration in place.

• Replace jest-localstorage-mock with the built-in Vitest DOM environment which includes localStorage
• Update the mock syntax from jest.mock to vi.mock
• Update the test timeout from jest.setTimeout to vi.setConfig({ testTimeout: 15000 })
• Remove jest-location-mock as the jsdom environment in Vitest provides location object

-import 'jest-localstorage-mock';
-import 'jest-location-mock';

-jest.mock('react-toastify', () => ({
+vi.mock('react-toastify', () => ({
   toast: {
-    success: jest.fn(),
-    error: jest.fn(),
+    success: vi.fn(),
+    error: vi.fn(),
   },
 }));

🔗 Analysis chain

Line range hint 1-369: Migrate Jest patterns and update localStorage mock for Vitest

The file needs complete migration to Vitest patterns, including the localStorage mock.

Apply these changes:

-import 'jest-localstorage-mock';
+import 'vitest-localstorage-mock';
-import 'jest-location-mock';
+import 'vitest-location-mock';

-jest.mock('react-toastify', () => ({
+vi.mock('react-toastify', () => ({
   toast: {
-    success: jest.fn(),
-    error: jest.fn(),
+    success: vi.fn(),
+    error: vi.fn(),
   },
 }));

 async function wait(ms = 100): Promise<void> {
   await act(async () => {
     return new Promise((resolve) => setTimeout(resolve, ms));
   });
 }

Run this script to check if the vitest-localstorage-mock package is available:

---

Let me check if there are any existing test setup files or configurations that might help us understand the testing environment better.

---

Let me check the content of the test setup files and vitest configuration to understand the current testing environment setup.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if vitest-localstorage-mock is listed in package.json
rg -l "vitest-localstorage-mock" package.json || echo "Package not found in dependencies"

Length of output: 122

---

Script:

#!/bin/bash
# Look for test setup files and vitest configuration
fd -e ts -e js -e json "vitest.config|test.setup|setupTests" 

# Check package.json for testing-related dependencies
rg "devDependencies|dependencies" package.json -A 10

Length of output: 977

---

Script:

#!/bin/bash
# Check content of setup files
echo "=== setupTests.ts ==="
cat src/setupTests.ts

echo -e "\n=== vitest.config.ts ==="
cat vitest.config.ts

Length of output: 1979

src/components/AgendaItems/AgendaItemsContainer.test.tsx (1)

Line range hint 285-285: Uncomment and update the toast assertion

The commented assertion should be uncommented and updated to use toHaveBeenCalledWith for consistency with other toast assertions in the file.

Apply this change:

-    // expect(toast.success).toBeCalledWith(translations.agendaItemUpdated);
+    expect(toast.success).toHaveBeenCalledWith(translations.agendaItemUpdated);

src/components/RecurrenceOptions/RecurrenceOptions.test.tsx (2)

Line range hint 1-11: Complete the Jest to Vitest migration for imports and testing utilities.

The test file still uses Jest-specific imports and testing utilities. For a complete migration to Vitest:

Apply these changes to use Vitest equivalents:

-import { MockedProvider } from '@apollo/react-testing';
+import { MockedProvider } from '@apollo/client/testing';
-import { act, render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { render, screen, fireEvent, waitFor } from '@testing-library/react';
-import 'jest-location-mock';
+import { beforeEach, describe, expect, test, vi } from 'vitest';

---

Line range hint 26-33: Update the mock implementation to use Vitest's mocking system.

The test uses Jest's mocking syntax which should be updated to Vitest's equivalent.

Apply this change:

-jest.mock('react-toastify', () => ({
+vi.mock('react-toastify', () => ({
   toast: {
-    success: jest.fn(),
-    warning: jest.fn(),
-    error: jest.fn(),
+    success: vi.fn(),
+    warning: vi.fn(),
+    error: vi.fn(),
   },
 }));

src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.test.tsx (2)

Line range hint 1-19: Complete the Jest to Vitest migration for testing framework imports.

The test file needs to be updated to use Vitest-specific imports and testing utilities.

Apply these changes:

-import React, { act } from 'react';
+import React from 'react';
-import { MockedProvider } from '@apollo/react-testing';
+import { MockedProvider } from '@apollo/client/testing';
+import { beforeEach, describe, expect, test, vi } from 'vitest';

---

Line range hint 26-34: Update mock implementation to use Vitest's mocking system.

The test uses Jest's mocking syntax which should be updated to Vitest's equivalent.

Apply this change:

-jest.mock('react-toastify', () => ({
+vi.mock('react-toastify', () => ({
   toast: {
-    success: jest.fn(),
-    warning: jest.fn(),
-    error: jest.fn(),
+    success: vi.fn(),
+    warning: vi.fn(),
+    error: vi.fn(),
   },
 }));

src/components/UserPortal/PostCard/PostCard.test.tsx (2)

Line range hint 1-11: Complete the Jest to Vitest migration for testing framework imports.

The test file needs to be updated to use Vitest-specific imports and testing utilities.

Apply these changes:

-import React, { act } from 'react';
+import React from 'react';
-import { MockedProvider } from '@apollo/react-testing';
+import { MockedProvider } from '@apollo/client/testing';
+import { beforeEach, describe, expect, test, vi } from 'vitest';

---

Line range hint 23-31: Update mock implementation to use Vitest's mocking system.

The test uses Jest's mocking syntax which should be updated to Vitest's equivalent.

Apply this change:

-jest.mock('react-toastify', () => ({
+vi.mock('react-toastify', () => ({
   toast: {
-    error: jest.fn(),
-    info: jest.fn(),
-    success: jest.fn(),
+    error: vi.fn(),
+    info: vi.fn(),
+    success: vi.fn(),
   },
 }));

src/components/AddOn/core/AddOnRegister/AddOnRegister.test.tsx (2)

Line range hint 1-1: Migration needed: Replace Jest mocking with Vitest

The test file still uses Jest's mocking system. For Vitest migration, replace jest.mock() with Vitest's vi.mock():

- jest.mock('react-toastify', () => ({
+ vi.mock('react-toastify', () => ({
  toast: {
-   success: jest.fn(),
+   success: vi.fn(),
  },
}));

- const mockNavigate = jest.fn();
+ const mockNavigate = vi.fn();

Also applies to: 76-77

---

Line range hint 182-196: Improve test reliability: Avoid fixed timeouts

The test waits for 3 seconds which makes the test suite slow and potentially flaky. Instead of using a fixed timeout, consider using waitFor to check for the actual condition:

- await wait(3000); // Waiting for 3 seconds to reload the page
- expect(mockNavigate).toHaveBeenCalledWith(0);
+ await waitFor(() => {
+   expect(mockNavigate).toHaveBeenCalledWith(0);
+ }, {
+   timeout: 3000
+ });

src/screens/OrganizationDashboard/OrganizationDashboard.test.tsx (2)

Line range hint 3-3: Migration needed: Update test setup for Vitest

Replace Jest-specific mocks with Vitest equivalents:

- import 'jest-location-mock';
+ import { beforeEach, afterEach, vi } from 'vitest';

- jest.mock('react-toastify', () => ({
+ vi.mock('react-toastify', () => ({
  toast: {
-   success: jest.fn(),
-   warn: jest.fn(),
-   error: jest.fn(),
+   success: vi.fn(),
+   warn: vi.fn(),
+   error: vi.fn(),
  },
}));

Also applies to: 31-41

---

Line range hint 179-199: Improve test isolation: Avoid modifying React internals

The current approach of mocking React's useState directly could lead to unexpected behavior. Consider refactoring to test the component's behavior rather than its implementation details:

test('event data should update correctly', async () => {
  mockId = '123';
  render(
    <MockedProvider addTypename={false} link={link1}>
      <BrowserRouter>
        <Provider store={store}>
          <I18nextProvider i18n={i18nForTest}>
            <OrganizationDashboard />
          </I18nextProvider>
        </Provider>
      </BrowserRouter>
    </MockedProvider>
  );
  
  // Wait for initial render
  await waitFor(() => {
    expect(screen.getByTestId('cardItem')).toBeInTheDocument();
  });
  
  // Trigger event update
  // Assert updated UI
});

🧹 Nitpick comments (11)

vitest.config.ts (1)

16-17: Consider adding additional test environment setup.

While the basic environment is configured, you might want to add setup files for common test utilities and mocks.

     globals: true,
     environment: 'jsdom',
+    setupFiles: ['./src/test/setup.ts'],
+    environmentOptions: {
+      jsdom: {
+        resources: 'usable',
+      },
+    },

jest.config.js (1)

80-81: Fix formatting issues in coverage configuration

The coverage configuration has formatting issues that need to be addressed.

-  coverageDirectory:'./coverage/jest',
-  coverageReporters:['text','html','text-summary','lcov'],
+  coverageDirectory: './coverage/jest',
+  coverageReporters: ['text', 'html', 'text-summary', 'lcov'],

🧰 Tools

🪛 eslint

[error] 80-80: Insert ·

(prettier/prettier)

---

[error] 81-81: Replace ['text','html','text-summary', with ·['text',·'html',·'text-summary',·

(prettier/prettier)

src/components/UserPortal/StartPostModal/StartPostModal.test.tsx (1)

147-150: Consider removing redundant empty assertion

The test includes a redundant assertion not.toHaveBeenCalledWith() without parameters. Consider removing it as the subsequent assertion adequately tests the toast behavior.

-    expect(toast.error).not.toHaveBeenCalledWith();
     expect(toast.info).toHaveBeenCalledWith(
       'Processing your post. Please wait.',
     );

src/components/OrgSettings/AgendaItemCategories/OrganizationAgendaCategory.test.tsx (1)

Line range hint 193-236: Uncomment and implement the error test case

The commented-out test case for error handling should be implemented to ensure complete test coverage. This is particularly important for error scenarios.

Would you like me to help implement this test case with the correct Vitest syntax?

src/components/AgendaCategory/AgendaCategoryContainer.test.tsx (1)

Line range hint 1-1: Consider a systematic approach for Jest to Vitest migration

While the toast assertions have been updated to use toHaveBeenCalledWith, there are several other Jest-specific patterns that need migration across multiple files. Consider:

1. Creating a migration guide for the team
2. Using codemods or scripts to automate the migration
3. Updating test utilities and mock files consistently

Key patterns to migrate project-wide:

• Replace jest.fn() with vi.fn()
• Replace jest.mock() with vi.mock()
• Replace jest.clearAllMocks() with vi.clearAllMocks()
• Update mock imports (e.g., jest-location-mock to vitest-location-mock)
• Replace jest.requireActual() with vi.importActual()

Would you like me to help create a migration guide or codemod script for these changes?

src/screens/UserPortal/Events/Events.test.tsx (1)

339-341: Consider using translations for toast messages

While the toast assertions have been correctly updated to use toHaveBeenCalledWith, the success message is hardcoded. Consider using translations for consistency with other components.

Example refactor:

-    expect(toast.success).toHaveBeenCalledWith(
-      'Event created and posted successfully.',
-    );
+    expect(toast.success).toHaveBeenCalledWith(translations.eventCreated);

Also applies to: 382-384

src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.test.tsx (1)

252-252: Assertion method changes are correct but need consistent formatting.

The changes to use toHaveBeenCalledWith are correct, but the formatting varies across assertions. Some use multiple lines while others are single-line.

Consider consistent formatting for all assertions:

-expect(toast.success).toHaveBeenCalledWith('Advertisement created successfully.');
+expect(toast.success).toHaveBeenCalledWith(
+  'Advertisement created successfully.'
+);

Also applies to: 343-343, 468-468, 595-595

src/components/UserPortal/PostCard/PostCard.test.tsx (1)

341-341: Assertion method changes are correct but need consistent formatting.

The changes to use toHaveBeenCalledWith are correct, but the formatting is inconsistent between single-line and multi-line assertions.

Consider using consistent multi-line formatting for all toast assertions for better readability:

-expect(toast.success).toHaveBeenCalledWith('Post updated Successfully');
+expect(toast.success).toHaveBeenCalledWith(
+  'Post updated Successfully'
+);

Also applies to: 391-393

src/components/UsersTableItem/UserTableItem.test.tsx (1)

Line range hint 1-1200: Consider organizing tests using describe blocks for better structure.

The test file has good coverage but could benefit from better organization. Consider grouping related tests under descriptive describe blocks for better readability and maintenance.

Example structure:

 describe('Testing User Table Item', () => {
+  describe('Basic Rendering', () => {
     test('Should render props and text elements test for the page component', async () => {
       // ... existing test
     });
+  });
+
+  describe('Modal Functionality', () => {
     test('Should render elements correctly when JoinedOrgs and BlockedByOrgs are empty', async () => {
       // ... existing test
     });
+  });
+
+  describe('User Management', () => {
     test('Remove user from Organization should function properly', async () => {
       // ... existing test
     });
+  });
 });

src/components/AddOn/core/AddOnRegister/AddOnRegister.test.tsx (1)

Line range hint 45-71: Simplify test setup: Extract common test configuration

Multiple test cases share similar setup code. Consider extracting the common setup into a helper function:

function renderAddOnRegister(props: any, mocks: any[] = []) {
  return render(
    <MockedProvider addTypename={false} mocks={mocks}>
      <BrowserRouter>
        <Provider store={store}>
          <I18nextProvider i18n={i18nForTest}>
            <AddOnRegister {...props} />
          </I18nextProvider>
        </Provider>
      </BrowserRouter>
    </MockedProvider>
  );
}

src/screens/OrganizationDashboard/OrganizationDashboard.test.tsx (1)

Line range hint 44-48: Improve test maintainability: Use test lifecycle hooks properly

The beforeEach hook modifies localStorage directly. Consider using Vitest's beforeEach for better test isolation and cleanup:

beforeEach(() => {
  vi.stubGlobal('localStorage', {
    setItem: vi.fn(),
    getItem: vi.fn(),
    clear: vi.fn()
  });
  
  // Set required items
  localStorage.setItem('FirstName', 'John');
  localStorage.setItem('LastName', 'Doe');
  localStorage.setItem(
    'UserImage',
    'https://api.dicebear.com/5.x/initials/svg?seed=John%20Doe'
  );
});

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3d1f1f0 and 2958074.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (25)

• .husky/pre-commit (1 hunks)
• jest.config.js (3 hunks)
• package.json (6 hunks)
• src/components/AddOn/core/AddOnRegister/AddOnRegister.test.tsx (1 hunks)
• src/components/Advertisements/Advertisements.spec.tsx (3 hunks)
• src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.test.tsx (4 hunks)
• src/components/AgendaCategory/AgendaCategoryContainer.test.tsx (2 hunks)
• src/components/AgendaItems/AgendaItemsContainer.test.tsx (1 hunks)
• src/components/EventListCard/EventListCard.test.tsx (8 hunks)
• src/components/OrgSettings/AgendaItemCategories/OrganizationAgendaCategory.test.tsx (1 hunks)
• src/components/RecurrenceOptions/CustomRecurrence.test.tsx (2 hunks)
• src/components/RecurrenceOptions/RecurrenceOptions.test.tsx (2 hunks)
• src/components/TableLoader/TableLoader.test.tsx (1 hunks)
• src/components/UserPortal/PostCard/PostCard.test.tsx (2 hunks)
• src/components/UserPortal/Register/Register.test.tsx (7 hunks)
• src/components/UserPortal/StartPostModal/StartPostModal.test.tsx (2 hunks)
• src/components/UsersTableItem/UserTableItem.test.tsx (3 hunks)
• src/screens/ManageTag/ManageTag.test.tsx (1 hunks)
• src/screens/OrganizationDashboard/OrganizationDashboard.test.tsx (1 hunks)
• src/screens/OrganizationEvents/OrganizationEvents.test.tsx (3 hunks)
• src/screens/OrganizationTags/OrganizationTags.test.tsx (2 hunks)
• src/screens/SubTags/SubTags.test.tsx (2 hunks)
• src/screens/UserPortal/Events/Events.test.tsx (2 hunks)
• tsconfig.json (1 hunks)
• vitest.config.ts (1 hunks)

🧰 Additional context used

📓 Learnings (2)

src/components/Advertisements/Advertisements.spec.tsx (1)

Learnt from: bitbard3
PR: PalisadoesFoundation/talawa-admin#2588
File: src/components/ChangeLanguageDropdown/ChangeLanguageDropdown.spec.tsx:145-155
Timestamp: 2024-12-02T04:20:11.745Z
Learning: In PRs focused solely on refactoring test cases from Jest to Vitest, avoid suggesting optimizations or changes outside the migration scope.

src/screens/OrganizationTags/OrganizationTags.test.tsx (1)

Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2398
File: src/screens/OrganizationTags/OrganizationTags.test.tsx:184-0
Timestamp: 2024-11-12T10:40:58.655Z
Learning: In the `talawa-admin` project, it's acceptable for test cases in `src/screens/OrganizationTags/OrganizationTags.test.tsx` to test multiple behaviors within a single test function without needing to split them into smaller, focused tests.

🪛 eslint

src/components/Advertisements/Advertisements.spec.tsx

[error] 72-72: import() type annotations are forbidden.

(@typescript-eslint/consistent-type-imports)

vitest.config.ts

[error] 1-1: Definition for rule '@typescript-eslint/ban-types' was not found.

(@typescript-eslint/ban-types)

jest.config.js

[error] 13-13: Insert ⏎··

(prettier/prettier)

---

[error] 80-80: Insert ·

(prettier/prettier)

---

[error] 81-81: Replace ['text','html','text-summary', with ·['text',·'html',·'text-summary',·

(prettier/prettier)

🪛 Biome (1.9.4)

package.json

[error] 70-70: The key test:vitest was already declared.

This where a duplicated key was declared again.

If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.

(lint/suspicious/noDuplicateObjectKeys)

🔇 Additional comments (19)

tsconfig.json (1)

21-22: LGTM!

The TypeScript configuration changes appropriately exclude build artifacts and the Vitest configuration file. The includes and compiler options are well-configured for the Vitest environment.

vitest.config.ts (2)

18-33: LGTM! Comprehensive coverage configuration.

The coverage configuration is well-structured with:

• Appropriate provider and output directory
• Comprehensive exclusion patterns
• Multiple report formats including HTML and LCOV

---

9-11: Verify if additional node polyfills are needed.

The configuration only includes the 'events' polyfill. Depending on your codebase, you might need additional Node.js built-in modules.

✅ Verification successful

The 'events' polyfill is sufficient for the current codebase

The codebase scan shows that only the following Node.js built-in modules are used:

• fs (in setup and test files)
• path (in git hooks)
• util (in test setup)

These files are either test files or development scripts that don't need polyfills in the browser environment. The current configuration with only the 'events' polyfill is appropriate as it's likely needed by some of the runtime dependencies.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Node.js built-in module usage in test files
rg -t ts -t tsx -t js -t jsx "require\(['\"](?:fs|path|crypto|buffer|stream|util)" src/

Length of output: 119

---

Script:

#!/bin/bash
# Search for Node.js built-in module usage with ripgrep, fixing file type flags
rg -t typescript -t javascript "require\(['\"](?:fs|path|crypto|buffer|stream|util)" .

# Also search for ES6 import syntax
rg -t typescript -t javascript "from ['\"](?:fs|path|crypto|buffer|stream|util)['\"]" .

# Search specifically for events module usage to understand why it's included
rg -t typescript -t javascript "(?:require\(['\"]events['\"]|from ['\"]events['\"])" .

Length of output: 386

---

Script:

#!/bin/bash
# Search for Node.js built-in module usage without file type flags
rg "(?:require\(['\"]|from ['\""])(?:fs|path|crypto|buffer|stream|util|events)['\"]" .

# Look for package.json to check dependencies that might need node polyfills
fd "package.json" -x cat {}

Length of output: 269

---

Script:

#!/bin/bash
# Search for Node.js built-in module usage with simpler patterns
rg "from ['\"]events['\"]" .
rg "from ['\"]fs['\"]" .
rg "from ['\"]path['\"]" .
rg "from ['\"]crypto['\"]" .
rg "from ['\"]buffer['\"]" .
rg "from ['\"]stream['\"]" .
rg "from ['\"]util['\"]" .

# Check package.json for dependencies
cat package.json

Length of output: 6481

src/components/TableLoader/TableLoader.test.tsx (1)

76-76: LGTM: Assertion updated to align with Vitest's preferred syntax

The change from toThrowError() to toThrow() is correct and follows Vitest's recommended assertion style.

jest.config.js (1)

3-13: LGTM: Coverage exclusions are well-defined

The coverage exclusions are appropriate, excluding test files, build artifacts, and type definitions.

🧰 Tools

🪛 eslint

[error] 13-13: Insert ⏎··

(prettier/prettier)

src/components/UserPortal/StartPostModal/StartPostModal.test.tsx (1)

132-134: LGTM: Toast error assertion updated to preferred syntax

The change from toBeCalledWith to toHaveBeenCalledWith follows the recommended assertion syntax.

src/components/OrgSettings/AgendaItemCategories/OrganizationAgendaCategory.test.tsx (1)

188-190: LGTM: Toast assertion updated correctly

The update to use toHaveBeenCalledWith is correct and aligns with Vitest's preferred syntax.

src/screens/OrganizationTags/OrganizationTags.test.tsx (1)

226-228: LGTM: Toast assertions updated correctly

The updates to use toHaveBeenCalledWith are correct and consistent with Vitest's syntax.

Also applies to: 245-247

src/components/UserPortal/Register/Register.test.tsx (1)

107-107: LGTM: Toast assertions updated consistently

All toast assertions have been correctly updated to use toHaveBeenCalledWith, maintaining consistency throughout the test suite.

Also applies to: 127-127, 148-148, 172-172, 198-198, 226-228, 263-265

src/components/AgendaItems/AgendaItemsContainer.test.tsx (1)

374-376: LGTM: Toast assertion updated correctly

The update to use toHaveBeenCalledWith aligns with Vitest's preferred assertion syntax.

src/screens/OrganizationEvents/OrganizationEvents.test.tsx (1)

285-285: LGTM: Toast assertions consistently updated

All toast assertions have been properly updated to use toHaveBeenCalledWith, maintaining consistency across the test suite.

Also applies to: 374-376, 455-455

src/components/RecurrenceOptions/RecurrenceOptions.test.tsx (1)

456-456: Assertion method changes align with modern testing practices.

The change from toBeCalledWith to toHaveBeenCalledWith is correct and follows modern testing best practices. This change improves readability and is the preferred method in both Jest and Vitest.

Also applies to: 578-578

src/components/Advertisements/Advertisements.spec.tsx (2)

3-3: LGTM: Vitest imports added correctly

The import of Vitest testing utilities has been properly added.

---

56-62: LGTM: Plugin helper mock updated to use Vitest syntax

The mock implementation has been correctly updated to use Vitest's vi instead of Jest's jest.

src/components/RecurrenceOptions/CustomRecurrence.test.tsx (1)

584-584: LGTM: Toast assertions updated to Vitest syntax

The toast assertions have been correctly updated from Jest's toBeCalledWith to Vitest's toHaveBeenCalledWith.

Also applies to: 712-712

src/components/EventListCard/EventListCard.test.tsx (1)

365-365: LGTM: Toast assertions consistently updated throughout the file

All toast assertions have been properly updated from Jest's toBeCalledWith to Vitest's toHaveBeenCalledWith, maintaining consistency across test cases.

Also applies to: 418-418, 462-462, 699-699, 765-765, 826-826, 866-866, 924-924

src/components/UsersTableItem/UserTableItem.test.tsx (2)

502-502: LGTM! Assertion method updates improve readability.

The change from toBeCalledWith to toHaveBeenCalledWith is a good improvement as it makes the assertions more readable and aligns with Vitest's preferred syntax.

Also applies to: 506-507

---

696-696: LGTM! Consistent assertion updates.

The changes maintain consistency with the updated assertion style throughout the test suite.

Also applies to: 723-724

src/screens/OrganizationDashboard/OrganizationDashboard.test.tsx (1)

Line range hint 1-200: Verify test framework configuration

Ensure that the Vitest configuration properly handles the test environment setup, including DOM simulation and module mocking.

[coderabbitai]

⚠️ Potential issue

Consider maintaining comprehensive pre-commit checks.

The pre-commit hook has been simplified to only run tests, removing important checks like formatting, linting, and type checking. While this simplifies the pre-commit process, it might lead to inconsistent code quality.

Consider restoring these essential checks:

#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

+npm run format:fix
+npm run lint:fix
+npm run typecheck
npm test

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	npm test
	#!/usr/bin/env sh
	. "$(dirname -- "$0")/_/husky.sh"
	npm run format:fix
	npm run lint:fix
	npm run typecheck
	npm test

[coderabbitai]

🛠️ Refactor suggestion

Consider broadening test file pattern.

The current pattern src/**/*.spec.{js,jsx,ts,tsx} might miss test files using .test. naming convention.

-    include: ['src/**/*.spec.{js,jsx,ts,tsx}'],
+    include: ['src/**/*.{spec,test}.{js,jsx,ts,tsx}'],

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	include: ['src/**/*.spec.{js,jsx,ts,tsx}'],
	include: ['src/**/*.{spec,test}.{js,jsx,ts,tsx}'],

[coderabbitai]

💡 Codebase verification

The .spec files need to be included in the test pattern

The codebase contains active test files using the .spec.{ts,tsx} pattern that contain real test cases using Vitest. These files need to be included in the test pattern to ensure all tests are executed.

• src/Constant/constant.spec.ts: Contains multiple test cases for constants
• src/components/Advertisements/Advertisements.spec.tsx: Contains comprehensive component tests using Vitest

Update the testMatch pattern in jest.config.js to include both patterns:

'<rootDir>/src/**/*.{test,spec}.{js,jsx,ts,tsx}'

🔗 Analysis chain

Consider keeping .spec.tsx pattern for consistency

The testMatch pattern now only includes .test.{js,jsx,ts,tsx} files. Since this PR is migrating from Jest to Vitest, consider keeping the .spec.tsx pattern to avoid missing any existing test files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any .spec files that might be missed
fd -e spec.tsx -e spec.ts -e spec.jsx -e spec.js

Length of output: 134

---

Script:

#!/bin/bash
# Let's check if these .spec files are actually being picked up by the test runner
# by searching for test execution patterns in them
rg -U "describe|test|it\(" src/Constant/constant.spec.ts src/components/Advertisements/Advertisements.spec.tsx

Length of output: 2419

[coderabbitai]

⚠️ Potential issue

Remove duplicate test:vitest script

There's a duplicate test:vitest script entry. The first set of Vitest scripts at lines 70-72 appears to be more complete with watch and coverage options.

Remove the duplicate entry:

-    "test:vitest": "vitest --run"

Also applies to: 86-86

🧰 Tools

🪛 Biome (1.9.4)

[error] 70-70: The key test:vitest was already declared.

This where a duplicated key was declared again.

If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.

(lint/suspicious/noDuplicateObjectKeys)

[coderabbitai]

🛠️ Refactor suggestion

Clean up redundant Jest dependencies

Since we're migrating to Vitest, the following Jest-related dependencies can be removed:

-    "jest": "^27.4.5",
-    "jest-localstorage-mock": "^2.4.19",
-    "jest-location-mock": "^2.0.0",
-    "jest-preview": "^0.3.1",

Committable suggestion skipped: line range outside the PR's diff.

[coderabbitai]

⚠️ Potential issue

Fix TypeScript error in router mock implementation

The router mock implementation contains a TypeScript error with the import type annotation.

Apply this diff to fix the type annotation:

-  async (importOriginal: () => Promise<typeof import('react-router-dom')>) => {
+  async (importOriginal: () => Promise<object>) => {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// let mockID: string | undefined = '1';
	// vi.mock('react-router-dom', () => ({
	// ...vi.importActual('react-router-dom'),
	// useParams: () => ({ orgId: mockID }),
	// }));
	let mockID: string | undefined = '1';
	vi.mock(
	'react-router-dom',
	async (importOriginal: () => Promise<typeof import('react-router-dom')>) => {
	const actual = await importOriginal();
	return {
	...actual,
	useParams: () => ({ orgId: mockID }),
	BrowserRouter: ({ children }: { children: React.ReactNode }) => (
	<div>{children}</div>
	),
	};
	},
	);
	let mockID: string | undefined = '1';
	vi.mock(
	'react-router-dom',
	async (importOriginal: () => Promise<object>) => {
	const actual = await importOriginal();
	return {
	...actual,
	useParams: () => ({ orgId: mockID }),
	BrowserRouter: ({ children }: { children: React.ReactNode }) => (
	<div>{children}</div>
	),
	};
	},
	);

🧰 Tools

🪛 eslint

[error] 72-72: import() type annotations are forbidden.

(@typescript-eslint/consistent-type-imports)

[palisadoes]

We have a policy of unassigning contributors who close PRs without getting validation from our reviewer team. This is because:

1. We start looking for people to review PRs when you submit them.
2. We often contact them and link to the PR. If the PR is closed the whole effort is wasted.
3. The historical thread of reviewer comments is broken when the work is spread across multiple PRs. The quality of our code is affected negatively.

Please be considerate of our volunteers' limited time and our desire to improve our code base.

This policy is stated as a pinned post in all our Talawa repositories. Our YouTube videos explain why this practice is not acceptable to our Community.

In most cases you don’t have to close the PR to trigger the GitHub workflow to run again. Making a new commit and pushing it to your GitHub account will normally be sufficient.

Unfortunately, if this continues we will have to close the offending PR and unassign you from the issue.