Fix #1912: Added test for SecondaryCard component #2069
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary by CodeRabbit
WalkthroughAdds a new unit test suite for the SecondaryCard React component ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
frontend/__tests__/unit/components/SecondaryCard.test.tsx (3)
15-17: Prefer role-based assertion for semantics (stronger than text matching)Asserting by role ensures we verify the heading semantics and not just text presence.
Apply this diff:
- render(<SecondaryCard title={defaultProps.title} />); - expect(screen.getByText(defaultProps.title)).toBeInTheDocument(); + render(<SecondaryCard title={defaultProps.title} />); + expect( + screen.getByRole('heading', { level: 2, name: defaultProps.title }) + ).toBeInTheDocument();
24-27: Nit: Query the root element directly instead of a descendant selectorQuerying with a class selector can match nested descendants. Grabbing the root and asserting its class is less brittle.
Apply this diff:
- const { container } = render(<SecondaryCard />); - expect(container.querySelector('.mb-8')).toBeInTheDocument(); + const { container } = render(<SecondaryCard />); + const cardElement = container.firstElementChild as HTMLElement; + expect(cardElement).toHaveClass('mb-8');
123-127: Optionally assert icon accessibility state explicitlySince you query with hidden: true, also asserting aria-hidden improves a11y clarity if the icon is meant to be decorative.
Apply this diff:
- const iconElement = screen.getByRole('img', { hidden: true }); - expect(iconElement).toHaveClass('h-5', 'w-5'); + const iconElement = screen.getByRole('img', { hidden: true }); + expect(iconElement).toHaveClass('h-5', 'w-5'); + expect(iconElement).toHaveAttribute('aria-hidden', 'true');
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/__tests__/unit/components/SecondaryCard.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/__tests__/unit/pages/createProgram.test.tsx:70-86
Timestamp: 2025-07-12T17:36:57.255Z
Learning: When testing React page components that use mocked form components, validation logic should be tested at the form component level, not the page level. Page-level tests should focus on authentication, role checking, submission handling, and navigation logic.
📚 Learning: 2025-07-12T17:36:57.255Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/__tests__/unit/pages/createProgram.test.tsx:70-86
Timestamp: 2025-07-12T17:36:57.255Z
Learning: When testing React page components that use mocked form components, validation logic should be tested at the form component level, not the page level. Page-level tests should focus on authentication, role checking, submission handling, and navigation logic.
Applied to files:
frontend/__tests__/unit/components/SecondaryCard.test.tsx
🔇 Additional comments (2)
frontend/__tests__/unit/components/SecondaryCard.test.tsx (2)
5-12: Well-scoped suite that aligns with the issue’s objectivesGood coverage across render variants, conditional UI, accessibility, and styling. The structure is clear and readable.
50-61: No interactive props detected — no event tests requiredI inspected src/components/SecondaryCard.tsx. Props are { title?, icon?, children?, className? } and it renders a non-interactive
(optionaland FontAwesomeIcon). There are no onClick/onKey*/button/anchor/role/tabIndex handlers to test.
- File inspected: src/components/SecondaryCard.tsx — no interactive props found; current prop/children tests are sufficient.
- If you plan to make this component interactive later, add explicit event props (e.g. onClick/onKeyDown) and corresponding user-event tests.
Comment on lines
43
to
47
| it('does not render the icon when icon prop is not provided', () => { | ||
| const { container } = render(<SecondaryCard title={defaultProps.title} />); | ||
| const iconElement = container.querySelector('svg'); | ||
| expect(iconElement).not.toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid coupling to implementation details; assert icon absence via role, not 'svg' tag
Selecting the SVG tag tightly couples the test to FontAwesome’s DOM. Using role keeps it implementation-agnostic.
Apply this diff:
- const { container } = render(<SecondaryCard title={defaultProps.title} />);
- const iconElement = container.querySelector('svg');
- expect(iconElement).not.toBeInTheDocument();
+ render(<SecondaryCard title={defaultProps.title} />);
+ expect(screen.queryByRole('img', { hidden: true })).not.toBeInTheDocument();📝 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
| it('does not render the icon when icon prop is not provided', () => { | |
| const { container } = render(<SecondaryCard title={defaultProps.title} />); | |
| const iconElement = container.querySelector('svg'); | |
| expect(iconElement).not.toBeInTheDocument(); | |
| }); | |
| it('does not render the icon when icon prop is not provided', () => { | |
| render(<SecondaryCard title={defaultProps.title} />); | |
| expect(screen.queryByRole('img', { hidden: true })).not.toBeInTheDocument(); | |
| }); |
🤖 Prompt for AI Agents
In frontend/__tests__/unit/components/SecondaryCard.test.tsx around lines 43–47,
the test currently queries the DOM for an 'svg' tag which couples it to
FontAwesome implementation; change the assertion to use an accessibility role
query instead (e.g., queryByRole('img') or queryByRole with the icon's
accessible name) so the test asserts the icon is absent via role rather than the
'svg' element, replacing the container.querySelector('svg') check with a
testing-library queryByRole and expecting it not to be in the document.
Comment on lines
90
to
98
| it('handles null children gracefully by rendering nothing for the children', () => { | ||
| const { container } = render( | ||
| <SecondaryCard title="Title">{null}</SecondaryCard> | ||
| ); | ||
| const cardElement = container.firstChild; | ||
| const titleElement = screen.getByRole('heading', { name: 'Title' }); | ||
| expect(titleElement).toBeInTheDocument(); | ||
| expect(cardElement.childNodes.length).toBe(1); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Brittle structure assertion; avoid relying on childNodes length
Asserting exact child node counts is fragile to harmless markup changes (e.g., wrappers). Prefer content- or role-based assertions.
Apply this diff:
- const { container } = render(
- <SecondaryCard title="Title">{null}</SecondaryCard>
- );
- const cardElement = container.firstChild;
- const titleElement = screen.getByRole('heading', { name: 'Title' });
- expect(titleElement).toBeInTheDocument();
- expect(cardElement.childNodes.length).toBe(1);
+ const { container } = render(
+ <SecondaryCard title="Title">{null}</SecondaryCard>
+ );
+ const heading = screen.getByRole('heading', { level: 2, name: 'Title' });
+ expect(heading).toBeInTheDocument();
+ // Ensure no extra visible text/content beyond the title
+ expect(container.textContent?.trim()).toBe('Title');📝 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
| it('handles null children gracefully by rendering nothing for the children', () => { | |
| const { container } = render( | |
| <SecondaryCard title="Title">{null}</SecondaryCard> | |
| ); | |
| const cardElement = container.firstChild; | |
| const titleElement = screen.getByRole('heading', { name: 'Title' }); | |
| expect(titleElement).toBeInTheDocument(); | |
| expect(cardElement.childNodes.length).toBe(1); | |
| }); | |
| it('handles null children gracefully by rendering nothing for the children', () => { | |
| const { container } = render( | |
| <SecondaryCard title="Title">{null}</SecondaryCard> | |
| ); | |
| const heading = screen.getByRole('heading', { level: 2, name: 'Title' }); | |
| expect(heading).toBeInTheDocument(); | |
| // Ensure no extra visible text/content beyond the title | |
| expect(container.textContent?.trim()).toBe('Title'); | |
| }); |
🤖 Prompt for AI Agents
frontend/__tests__/unit/components/SecondaryCard.test.tsx around lines 90 to 98:
the test currently asserts a brittle DOM structure by checking
cardElement.childNodes.length; remove that childNodes length assertion and
instead assert via content/role queries that only the title is rendered and no
children content exists (for example, use screen.queryByText or
within(cardElement).queryByRole/queryByText to assert there is no extra content
or region rendered for children when children is null). Ensure the test verifies
presence of the heading and uses queryBy* to assert absence of any child content
rather than relying on node count.
Comment on lines
117
to
121
| it('has the correct classNames for the h2 title element', () => { | ||
| render(<SecondaryCard title={defaultProps.title} />); | ||
| const titleElement = screen.getByText(defaultProps.title); | ||
| expect(titleElement).toHaveClass('mb-4', 'flex', 'flex-row', 'items-center', 'gap-2', 'text-2xl', 'font-semibold'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Target the actual h2 element when asserting classes
getByText may resolve a child node (e.g., span) inside the h2, causing false negatives. Query the heading element directly.
Apply this diff:
- render(<SecondaryCard title={defaultProps.title} />);
- const titleElement = screen.getByText(defaultProps.title);
- expect(titleElement).toHaveClass('mb-4', 'flex', 'flex-row', 'items-center', 'gap-2', 'text-2xl', 'font-semibold');
+ render(<SecondaryCard title={defaultProps.title} />);
+ const heading = screen.getByRole('heading', { level: 2, name: defaultProps.title });
+ expect(heading).toHaveClass(
+ 'mb-4',
+ 'flex',
+ 'flex-row',
+ 'items-center',
+ 'gap-2',
+ 'text-2xl',
+ 'font-semibold'
+ );📝 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
| it('has the correct classNames for the h2 title element', () => { | |
| render(<SecondaryCard title={defaultProps.title} />); | |
| const titleElement = screen.getByText(defaultProps.title); | |
| expect(titleElement).toHaveClass('mb-4', 'flex', 'flex-row', 'items-center', 'gap-2', 'text-2xl', 'font-semibold'); | |
| }); | |
| it('has the correct classNames for the h2 title element', () => { | |
| render(<SecondaryCard title={defaultProps.title} />); | |
| const heading = screen.getByRole('heading', { level: 2, name: defaultProps.title }); | |
| expect(heading).toHaveClass( | |
| 'mb-4', | |
| 'flex', | |
| 'flex-row', | |
| 'items-center', | |
| 'gap-2', | |
| 'text-2xl', | |
| 'font-semibold' | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In frontend/__tests__/unit/components/SecondaryCard.test.tsx around lines 117 to
121, the test uses getByText which can match a child node (like a span) instead
of the h2; replace that lookup with a heading-specific query (e.g.,
getByRole('heading', { level: 2, name: defaultProps.title }) or
getByRole('heading', { name: defaultProps.title, level: 2 }) ) so the assertion
targets the actual h2 element, then run the same toHaveClass assertion against
that heading element.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
frontend/__tests__/unit/components/SecondaryCard.test.tsx (3)
43-47: Avoid coupling to implementation details; assert icon absence via role, not 'svg' tag
Selecting the SVG tag tightly couples the test to FontAwesome’s DOM. Using role keeps it implementation-agnostic.Apply this diff:
- const { container } = render(<SecondaryCard title={defaultProps.title} />) - const iconElement = container.querySelector('svg') - expect(iconElement).not.toBeInTheDocument() + render(<SecondaryCard title={defaultProps.title} />) + expect(screen.queryByRole('img', { hidden: true })).not.toBeInTheDocument()
90-96: Brittle structure assertion; avoid relying on childNodes length
Checking childNodes length is fragile to harmless markup changes. Assert via content/roles instead.Apply this diff:
- const { container } = render(<SecondaryCard title="Title">{null}</SecondaryCard>) - const cardElement = container.firstChild - const titleElement = screen.getByRole('heading', { name: 'Title' }) - expect(titleElement).toBeInTheDocument() - expect(cardElement.childNodes.length).toBe(1) + const { container } = render(<SecondaryCard title="Title">{null}</SecondaryCard>) + const heading = screen.getByRole('heading', { level: 2, name: 'Title' }) + expect(heading).toBeInTheDocument() + // Ensure no extra visible text/content beyond the title + expect(container.textContent?.trim()).toBe('Title')
122-134: Target the actual h2 element when asserting classes
getByText may resolve a child node (e.g., span) inside the h2, causing false negatives. Query the heading element directly.Apply this diff:
- render(<SecondaryCard title={defaultProps.title} />) - const titleElement = screen.getByText(defaultProps.title) - expect(titleElement).toHaveClass( + render(<SecondaryCard title={defaultProps.title} />) + const heading = screen.getByRole('heading', { level: 2, name: defaultProps.title }) + expect(heading).toHaveClass( 'mb-4', 'flex', 'flex-row', 'items-center', 'gap-2', 'text-2xl', 'font-semibold' )
🧹 Nitpick comments (2)
frontend/__tests__/unit/components/SecondaryCard.test.tsx (2)
15-17: Prefer role-based heading queries with explicit level over text or generic heading queriesUsing getByText can match nested spans and generic queryByRole('heading') can mask level mismatches. Query the actual h2 for stability and semantics.
Apply this diff:
@@ - render(<SecondaryCard title={defaultProps.title} />) - expect(screen.getByText(defaultProps.title)).toBeInTheDocument() + render(<SecondaryCard title={defaultProps.title} />) + const heading = screen.getByRole('heading', { level: 2, name: defaultProps.title }) + expect(heading).toBeInTheDocument() @@ - render(<SecondaryCard title={defaultProps.title} />) - const titleElement = screen.getByRole('heading', { name: 'Test Title' }) + render(<SecondaryCard title={defaultProps.title} />) + const titleElement = screen.getByRole('heading', { level: 2, name: 'Test Title' }) expect(titleElement).toBeInTheDocument() @@ - render(<SecondaryCard title={customTitle} />) - expect(screen.getByText(customTitle)).toBeInTheDocument() + render(<SecondaryCard title={customTitle} />) + expect(screen.getByRole('heading', { level: 2, name: customTitle })).toBeInTheDocument() @@ - const titleElement = screen.queryByRole('heading') + const titleElement = screen.queryByRole('heading', { level: 2 }) expect(titleElement).not.toBeInTheDocument()Also applies to: 37-41, 63-68, 83-88
109-120: Prefer firstElementChild for type-safety; consider querying by role/testid
container.firstChild is typed as Node and may include text nodes. firstElementChild avoids type mismatches and DOM-node pitfalls.Apply this diff:
- const { container } = render(<SecondaryCard className={defaultProps.className} />) - const cardElement = container.firstChild + const { container } = render(<SecondaryCard className={defaultProps.className} />) + const cardElement = container.firstElementChild as HTMLElementIf feasible in the component, exposing a data-testid on the root would let you replace container traversal with screen.getByTestId for even clearer intent.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/__tests__/unit/components/SecondaryCard.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-12T17:36:57.255Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/__tests__/unit/pages/createProgram.test.tsx:70-86
Timestamp: 2025-07-12T17:36:57.255Z
Learning: When testing React page components that use mocked form components, validation logic should be tested at the form component level, not the page level. Page-level tests should focus on authentication, role checking, submission handling, and navigation logic.
Applied to files:
frontend/__tests__/unit/components/SecondaryCard.test.tsx
🔇 Additional comments (6)
frontend/__tests__/unit/components/SecondaryCard.test.tsx (6)
51-55: Good coverage for combined title+icon rendering
Asserts both title presence and icon role; aligns with accessibility best practices.
57-60: Children rendering assertion looks solid
Straightforward verification of children content rendering.
70-80: Complex children case is well covered
Verifies nested text and interactive child; good balance of content and role assertions.
100-104: LGTM: semantic heading check
Validates h2 semantics directly via role and level.
136-140: Icon class assertion via role is clean and accessible
Good use of role query with hidden: true and focused class assertion on the icon element.
50-61: No additional event/state tests needed for SecondaryCard
I reviewedfrontend/src/components/SecondaryCard.tsx—it doesn’t define anyon*handlers, internal state, or interactive elements (e.g.<button>, clickable roles). The existing prop‐based rendering tests are sufficient.
kasya
approved these changes
Aug 15, 2025
|
Dhirajsharma2060
pushed a commit
to Dhirajsharma2060/Nest
that referenced
this pull request
Aug 16, 2025
* Fix OWASP#1912: Added test for SecondaryCard component * Added newline at end of SecondaryCard.test.tsx * Fix make check --------- Co-authored-by: Kate Golovanova <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 28, 2025
* test: add unit tests for SortBy component * test: update SortBy component tests for prop naming and accessibility * test: import React in SortBy component tests for consistency * made some chnages * test: update SortBy component tests for improved accessibility and clarity * test: ensure order toggle button functionality in SortBy component tests * feat: add has_full_name filter to top contributors query and model * fix: update regex for user name filtering in RepositoryContributor queryset * Update code * test: enhance RepositoryContributor tests with full name filter validations * refactor: use class constants for regex pattern and mock data in RepositoryContributor tests * test: fix regex filter call in has_full_name filter validation * Update code * Update tests * Update code * Update tests * Update test * feat: periodic job to sync OWASP Employee badges (#1762) * fix: optimize badge assignment logic and improve test coverage for sync_user_badges command * refactor: clean up comments in sync_user_badges test cases for clarity * refactor: remove unused mock_badge fixture from sync_user_badges test * feat: add management command to sync OWASP Staff badges and corresponding tests * refactor: optimize badge assignment logic for OWASP Staff badge sync * Update code * feat: add badges field to User model and update badge sync logic for OWASP staff feat:add badges field to User model and updated badge sync logic OWASP staff * feat: add badges field to User model and update related tests for badge synchronization * test: ensure no badge operations are performed when no employees are eligible * test: enhance user filter logic and improve badge assignment messages in tests suggested by coderabbit * test: refine user filter logic for badge assignment and enhance handling of keyword and positional arguments * test: refactor user filter logic in badge assignment tests for improved clarity and functionality * test: simplify user filter logic in badge assignment tests for improved readability and maintainability * test: simplify user filter logic by extracting helper functions for improved clarity and maintainability * test: update patch paths for badge and user objects in sync_user_badges tests for consistency * Update code * test: update patch paths for badge and user objects in TestSyncUserBadgesCommand for consistency * Fix #1912: Added test for SecondaryCard component (#2069) * Fix #1912: Added test for SecondaryCard component * Added newline at end of SecondaryCard.test.tsx * Fix make check --------- Co-authored-by: Kate Golovanova <[email protected]> * test: add unit tests for GeneralCompliantComponent (#2018) * test: add unit tests for GeneralCompliantComponent * fix(#1835): Refactor tests based on reviewer feedback * Fix make check and revert unnecessary changes * Fix issues --------- Co-authored-by: Kate Golovanova <[email protected]> * Added Light Theme Support for Contribution HeatMap (#2072) * Added Light Theme Support for Contribution HeatMap * Added Light Theme Support for Contribution HeatMap (Updated) * Added Light Theme Support for Contribution HeatMap (Updated) * Update color scheme for the heatmap * Update code --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * feat: add comprehensive unit tests for DisplayIcon component (#2048) * feat: add comprehensive unit tests for DisplayIcon component - 30+ test cases covering rendering, props, edge cases, and accessibility - Mock all dependencies and ensure SonarQube compliance - Test both snake_case and camelCase property conventions * Fixed issues flagged by the bot * Fixed issues flagged by the bot * Fixed issues flagged by the bot and added unhover word in cspell/custom-dict.txt --------- Co-authored-by: Kate Golovanova <[email protected]> * enhance:change-hover-color-action-button (#1978) * enhance:change-hover-color-action-button * Update colors and tests * Add comprehensive hover state check --------- Co-authored-by: Kate <[email protected]> * Test: add unit tests for BarChart component #1801 (#1904) * Add unit tests for BarChart component - Added comprehensive test cases for BarChart component - Tests cover rendering, data handling, and prop validation - Follows project testing standards * Add unit tests for BarChart component * Add unit tests for BarChart component * Fix linting issues * Fixed unit tests for BarChart component * Fixed unit tests for BarChart component By CodeRabbit * Fixed unit tests for BarChart component By CodeRabbit * Fix tests and make check issues * Fixed unit tests for BarChart component * Fix issues and add more tests --------- Co-authored-by: Kate Golovanova <[email protected]> * Project Dasbored Drop Down Test case has been made (#2052) * Project Dasbored Drop Down Test case has been made * Changes made * Made the changes for the code check and test cases * Changes has been made so that it works with eslint * Fix make check issues --------- Co-authored-by: Kate Golovanova <[email protected]> * fix: update command references in badge management tests * Update code * fix: update badge management command to correctly filter users by badge relationship * style: format code for better readability in badge assignment logic * fix: update badge assignment logic to use UserBadge model and remove unnecessary user management calls * fix: include owasp Makefile in backend Makefile * fix: improve badge removal logic for non-OWASP employees * fix: add unique constraint for user and badge in UserBadge model * Update code * Revert volumes * feat: add is_active field to UserBadge model and update badge assignment logic * style: format migration file for userbadge is_active field * test: mock return value for UserBadge get_or_create in badge sync tests * fix: correct docstring for nest_update_badges management command tests * Update code --------- Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: rasesh <[email protected]> Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Karmjeet Chauhan <[email protected]> Co-authored-by: Abhay Bahuguna <[email protected]> Co-authored-by: Rizwaan Ansari <[email protected]> Co-authored-by: Vijesh Shetty <[email protected]> Co-authored-by: Mohd Waqar Ahemed <[email protected]> Co-authored-by: SOHAMPAL23 <[email protected]>
Dishant1804
pushed a commit
to Dishant1804/Nest
that referenced
this pull request
Sep 6, 2025
* test: add unit tests for SortBy component * test: update SortBy component tests for prop naming and accessibility * test: import React in SortBy component tests for consistency * made some chnages * test: update SortBy component tests for improved accessibility and clarity * test: ensure order toggle button functionality in SortBy component tests * feat: add has_full_name filter to top contributors query and model * fix: update regex for user name filtering in RepositoryContributor queryset * Update code * test: enhance RepositoryContributor tests with full name filter validations * refactor: use class constants for regex pattern and mock data in RepositoryContributor tests * test: fix regex filter call in has_full_name filter validation * Update code * Update tests * Update code * Update tests * Update test * feat: periodic job to sync OWASP Employee badges (OWASP#1762) * fix: optimize badge assignment logic and improve test coverage for sync_user_badges command * refactor: clean up comments in sync_user_badges test cases for clarity * refactor: remove unused mock_badge fixture from sync_user_badges test * feat: add management command to sync OWASP Staff badges and corresponding tests * refactor: optimize badge assignment logic for OWASP Staff badge sync * Update code * feat: add badges field to User model and update badge sync logic for OWASP staff feat:add badges field to User model and updated badge sync logic OWASP staff * feat: add badges field to User model and update related tests for badge synchronization * test: ensure no badge operations are performed when no employees are eligible * test: enhance user filter logic and improve badge assignment messages in tests suggested by coderabbit * test: refine user filter logic for badge assignment and enhance handling of keyword and positional arguments * test: refactor user filter logic in badge assignment tests for improved clarity and functionality * test: simplify user filter logic in badge assignment tests for improved readability and maintainability * test: simplify user filter logic by extracting helper functions for improved clarity and maintainability * test: update patch paths for badge and user objects in sync_user_badges tests for consistency * Update code * test: update patch paths for badge and user objects in TestSyncUserBadgesCommand for consistency * Fix OWASP#1912: Added test for SecondaryCard component (OWASP#2069) * Fix OWASP#1912: Added test for SecondaryCard component * Added newline at end of SecondaryCard.test.tsx * Fix make check --------- Co-authored-by: Kate Golovanova <[email protected]> * test: add unit tests for GeneralCompliantComponent (OWASP#2018) * test: add unit tests for GeneralCompliantComponent * fix(OWASP#1835): Refactor tests based on reviewer feedback * Fix make check and revert unnecessary changes * Fix issues --------- Co-authored-by: Kate Golovanova <[email protected]> * Added Light Theme Support for Contribution HeatMap (OWASP#2072) * Added Light Theme Support for Contribution HeatMap * Added Light Theme Support for Contribution HeatMap (Updated) * Added Light Theme Support for Contribution HeatMap (Updated) * Update color scheme for the heatmap * Update code --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * feat: add comprehensive unit tests for DisplayIcon component (OWASP#2048) * feat: add comprehensive unit tests for DisplayIcon component - 30+ test cases covering rendering, props, edge cases, and accessibility - Mock all dependencies and ensure SonarQube compliance - Test both snake_case and camelCase property conventions * Fixed issues flagged by the bot * Fixed issues flagged by the bot * Fixed issues flagged by the bot and added unhover word in cspell/custom-dict.txt --------- Co-authored-by: Kate Golovanova <[email protected]> * enhance:change-hover-color-action-button (OWASP#1978) * enhance:change-hover-color-action-button * Update colors and tests * Add comprehensive hover state check --------- Co-authored-by: Kate <[email protected]> * Test: add unit tests for BarChart component OWASP#1801 (OWASP#1904) * Add unit tests for BarChart component - Added comprehensive test cases for BarChart component - Tests cover rendering, data handling, and prop validation - Follows project testing standards * Add unit tests for BarChart component * Add unit tests for BarChart component * Fix linting issues * Fixed unit tests for BarChart component * Fixed unit tests for BarChart component By CodeRabbit * Fixed unit tests for BarChart component By CodeRabbit * Fix tests and make check issues * Fixed unit tests for BarChart component * Fix issues and add more tests --------- Co-authored-by: Kate Golovanova <[email protected]> * Project Dasbored Drop Down Test case has been made (OWASP#2052) * Project Dasbored Drop Down Test case has been made * Changes made * Made the changes for the code check and test cases * Changes has been made so that it works with eslint * Fix make check issues --------- Co-authored-by: Kate Golovanova <[email protected]> * fix: update command references in badge management tests * Update code * fix: update badge management command to correctly filter users by badge relationship * style: format code for better readability in badge assignment logic * fix: update badge assignment logic to use UserBadge model and remove unnecessary user management calls * fix: include owasp Makefile in backend Makefile * fix: improve badge removal logic for non-OWASP employees * fix: add unique constraint for user and badge in UserBadge model * Update code * Revert volumes * feat: add is_active field to UserBadge model and update badge assignment logic * style: format migration file for userbadge is_active field * test: mock return value for UserBadge get_or_create in badge sync tests * fix: correct docstring for nest_update_badges management command tests * Update code --------- Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: rasesh <[email protected]> Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Karmjeet Chauhan <[email protected]> Co-authored-by: Abhay Bahuguna <[email protected]> Co-authored-by: Rizwaan Ansari <[email protected]> Co-authored-by: Vijesh Shetty <[email protected]> Co-authored-by: Mohd Waqar Ahemed <[email protected]> Co-authored-by: SOHAMPAL23 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed change
Resolves #1912
Added unit tests for the SecondaryCard component to ensure expected behavior, edge case handling, and rendering logic.
Checklist
make check-testlocally; all checks and tests passed.