refactor(datasets): add comprehensive unit tests for dataset API hooks and fix api guards #36175

sadpandajoe · 2025-11-18T23:16:51Z

SUMMARY

This PR adds comprehensive unit test coverage for the dataset API hooks used in the filter configuration flow, plus fixes two critical bugs discovered during testing.

New Test Coverage (35 tests)

src/hooks/apiResources/datasets.test.ts (21 tests)

getDatasetId utility function - ID extraction from string/numeric formats
createVerboseMap utility function - verbose name mapping from columns/metrics
useDatasetDrillInfo hook - REST API path, caching behavior, error handling
Extension path coverage - drill-by extension integration, error handling, malformed payloads
Edge cases - NaN IDs, empty strings, malformed datasource IDs

src/features/datasets/hooks/useDatasetLists.test.ts (14 tests)

Pagination - single page, multi-page fetching
Error handling - API errors, network timeouts, persistent failures
Guard conditions - schema null/undefined, database undefined
Schema encoding - special characters in schema names

Bug Fixes

Pagination infinite loop (useDatasetLists.ts:68)
- Problem: Error handler in pagination loop didn't break, causing infinite retries on persistent API errors
- Fix: Added break statement in catch block
- Risk: CRITICAL - could cause browser hangs/crashes
Missing database guard (useDatasetLists.ts:82)
- Problem: API calls fired with database: undefined when db not selected
- Fix: Added db?.id !== undefined check to guard condition
- Risk: MEDIUM - API spam with malformed filters

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

- Add ESLint disable comments for describe blocks (following ChartList pattern) - Fix TypeScript mock typing using jest.mocked() helper - Update supersetGetCache.delete mock to be properly typed - Fix mockDb.owners type to be tuple [number] - Add 'as any' casts for test fixtures to avoid strict type checking - All 30 unit tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Improvements to existing unit tests for dataset hooks and API resources. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

- Add break statement in pagination loop error handler to prevent infinite retries on persistent API errors - Add db?.id guard to prevent API calls with undefined database ID - Add 5 new tests covering extension fallback, db undefined, and pagination error scenarios - Update existing error tests to expect single API call (not retry) Test Results: 35/35 passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Copilot

Pull Request Overview

This pull request adds comprehensive unit test coverage for dataset API hooks and fixes two critical bugs that could cause browser hangs and unnecessary API calls.

Key Changes:

Added 35 comprehensive unit tests across two test files (datasets.test.ts and useDatasetLists.test.ts)
Fixed pagination infinite loop bug by adding break statement in error handler
Fixed missing database guard to prevent API calls with undefined database ID

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
superset-frontend/src/hooks/apiResources/datasets.test.ts	New test file with 21 tests covering getDatasetId, createVerboseMap, useDatasetDrillInfo hook, extension integration, and edge cases
superset-frontend/src/features/datasets/hooks/useDatasetLists.test.ts	New test file with 14 tests covering pagination, error handling, guard conditions, and schema encoding
superset-frontend/src/features/datasets/hooks/useDatasetLists.ts	Critical bug fixes: added break statement in pagination error handler (line 68) and database ID guard check (line 82)

superset-frontend/src/hooks/apiResources/datasets.test.ts

bito-code-review

Code Review Agent Run #7e8d58

Actionable Suggestions - 1

superset-frontend/src/features/datasets/hooks/useDatasetLists.test.ts - 1
- Incorrect URL encoding expectation in test · Line 378-380

Review Details

Files reviewed - 3 · Commit Range: 7c68474..1699185
- superset-frontend/src/features/datasets/hooks/useDatasetLists.test.ts
- superset-frontend/src/features/datasets/hooks/useDatasetLists.ts
- superset-frontend/src/hooks/apiResources/datasets.test.ts
Files skipped - 0
Tools
- Eslint (Linter) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by

bito-code-review · 2025-11-18T23:24:56Z

superset-frontend/src/features/datasets/hooks/useDatasetLists.test.ts

+  // Verify the encoded schema is present in the URL (double-encoded by rison)
+  // Schema 'sales analytics' -> encodeURIComponent -> 'sales%20analytics' -> rison.encode_uri -> 'sales%2520analytics'
+  expect(callArg).toContain('sales%2520analytics');


Incorrect URL encoding expectation in test

The test expects double-encoded schema in the URL, but rison does not encode percent signs, leading to incorrect validation. Update to expect 'sales%20analytics' instead of 'sales%2520analytics' to match actual rison behavior. This affects the useDatasetsList hook's API endpoint construction, where schema encoding is critical for proper URL formation and API calls.

Code suggestion

Check the AI-generated fix before applying

Suggested change

// Verify the encoded schema is present in the URL (double-encoded by rison)

// Schema 'sales analytics' -> encodeURIComponent -> 'sales%20analytics' -> rison.encode_uri -> 'sales%2520analytics'

expect(callArg).toContain('sales%2520analytics');

// Verify the encoded schema is present in the URL (encoded by rison)

// Schema 'sales analytics' -> encodeURIComponent -> 'sales%20analytics' -> rison.encode_uri -> 'sales%20analytics'

expect(callArg).toContain('sales%20analytics');

Code Review Run #7e8d58

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

Yes, avoid them

Replace toBeNaN() with Number.isNaN() check since toBeNaN is not a standard Jest matcher. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Remove all describe() nesting following Kent C. Dodds' "Avoid Nesting When You're Testing" principles: - Remove 3 describe blocks (useDatasetDrillInfo, getDatasetId malformed, extension path) - Move beforeEach/afterEach to module level - Create setupExtensionMock() helper for extension tests - All 21 tests now flat with descriptive names Test structure is cleaner and more maintainable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

…s and fix api guards (apache#36175) Co-authored-by: Claude <[email protected]>

pull-request-size bot added the size/XL label Nov 18, 2025

sadpandajoe and others added 3 commits November 18, 2025 15:17

test: enhance dataset API hook unit tests

f8d38ce

Improvements to existing unit tests for dataset hooks and API resources. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

sadpandajoe force-pushed the feat/dataset-unit-tests branch from 3fcbe5a to 1699185 Compare November 18, 2025 23:17

sadpandajoe requested a review from Copilot November 18, 2025 23:18

Copilot started reviewing on behalf of sadpandajoe November 18, 2025 23:19 View session

Copilot finished reviewing on behalf of sadpandajoe November 18, 2025 23:20

Copilot AI reviewed Nov 18, 2025

View reviewed changes

superset-frontend/src/hooks/apiResources/datasets.test.ts Outdated Show resolved Hide resolved

bito-code-review bot reviewed Nov 18, 2025

View reviewed changes

sadpandajoe and others added 2 commits November 18, 2025 16:55

fix(datasets): use valid Jest matcher for NaN assertion

16b5de9

Replace toBeNaN() with Number.isNaN() check since toBeNaN is not a standard Jest matcher. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

sadpandajoe force-pushed the feat/dataset-unit-tests branch from 071e9fd to 011624c Compare November 19, 2025 23:22

sadpandajoe requested a review from Copilot November 20, 2025 00:11

Copilot started reviewing on behalf of sadpandajoe November 20, 2025 00:11 View session

Copilot finished reviewing on behalf of sadpandajoe November 20, 2025 00:13

Copilot AI reviewed Nov 20, 2025

View reviewed changes

betodealmeida approved these changes Nov 21, 2025

View reviewed changes

sadpandajoe merged commit 9d40c24 into master Nov 21, 2025
81 of 85 checks passed

sadpandajoe deleted the feat/dataset-unit-tests branch November 21, 2025 21:26

LuisSanchez pushed a commit to LuisSanchez/superset that referenced this pull request Nov 25, 2025

refactor(datasets): add comprehensive unit tests for dataset API hook…

69005d4

…s and fix api guards (apache#36175) Co-authored-by: Claude <[email protected]>

kshi020302 pushed a commit to jl141/superset that referenced this pull request Nov 30, 2025

refactor(datasets): add comprehensive unit tests for dataset API hook…

7168719

…s and fix api guards (apache#36175) Co-authored-by: Claude <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(datasets): add comprehensive unit tests for dataset API hooks and fix api guards #36175

refactor(datasets): add comprehensive unit tests for dataset API hooks and fix api guards #36175

Uh oh!

sadpandajoe commented Nov 18, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

bito-code-review bot left a comment •

edited

Loading

Uh oh!

bito-code-review bot Nov 18, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

refactor(datasets): add comprehensive unit tests for dataset API hooks and fix api guards #36175

refactor(datasets): add comprehensive unit tests for dataset API hooks and fix api guards #36175

Uh oh!

Conversation

sadpandajoe commented Nov 18, 2025

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

bito-code-review bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Code Review Agent Run #7e8d58

Uh oh!

bito-code-review bot Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bito-code-review bot left a comment •

edited

Loading