VIBE-209 Bug Fix for Non Match Column Values Not Being Populated

[junaidiqbalmoj]

Jira link

https://tools.hmcts.net/jira/browse/VIBE-209

Change description

Bug Fix for Non Match Column Values Not Being Populated

Summary by CodeRabbit

• Bug Fixes

  • Simplified success message text for blob ingestion when location data is not found in reference data, removing technical qualifiers for improved clarity.

• Tests

  • Updated test expectations and extended coverage to reflect simplified messages and asynchronous validation logic.

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

[coderabbitai]

Walkthrough

The pull request modifies blob ingestion functionality by removing the "(no_match=true)" suffix from user-facing success messages and updating the location lookup in validateBlobRequest from a synchronous call to an awaited asynchronous call. Documentation, tests, and implementation files are updated consistently to reflect these changes.

Changes

Cohort / File(s)	Summary
Message text updates docs/tickets/VIBE-209/plan.md, libs/api/src/blob-ingestion/repository/service.ts	Removed "(no_match=true)" suffix from success response messages when location is not found, simplifying user-facing text.
Service test expectations libs/api/src/blob-ingestion/repository/service.test.ts	Updated test assertions to expect the new message format without the "(no_match=true)" suffix; artifact creation and other assertions remain unchanged.
Async location lookup libs/api/src/blob-ingestion/validation.ts, libs/api/src/blob-ingestion/validation.test.ts	Changed location lookup from synchronous to awaited asynchronous call in validateBlobRequest; updated mocks to return Promises and added new tests verifying correct handling of awaited results for both non-existent and existing locations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas requiring attention:
  • Verify that adding await to getLocationById in validation.ts maintains the original behavior and doesn't introduce unintended async handling issues
  • Confirm new async tests in validation.test.ts adequately cover both truthy and falsy location resolution paths
  • Ensure consistency of message text changes across all affected files

Possibly related PRs

• VIBE-180 Upload Reference Data #106: Both modify callers to handle getLocationById as an async Promise by changing synchronous lookups to awaited calls and updating related tests.

Suggested reviewers

• KianKwa

Poem

🐰 A rabbit hops through async calls,
With promises that never stall,
Messages trimmed of clutter bright,
No-match flags fade from sight!
Clean and swift, the code now stands tall. ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions fixing 'Non Match Column Values' but the changes only involve removing '(no_match=true)' text from success messages and fixing async/await in validation logic, not populating missing column values.	Update the title to accurately reflect the actual changes, such as 'VIBE-209: Remove no_match qualifier from success messages and fix async location validation' or similar.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch vibe-209-bug-fix

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 52d2a47 and cd7ac0f.

📒 Files selected for processing (5)

• docs/tickets/VIBE-209/plan.md (2 hunks)
• libs/api/src/blob-ingestion/repository/service.test.ts (1 hunks)
• libs/api/src/blob-ingestion/repository/service.ts (1 hunks)
• libs/api/src/blob-ingestion/validation.test.ts (2 hunks)
• libs/api/src/blob-ingestion/validation.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g., userId, caseDetails, documentId). Booleans must use is/has/can prefix (e.g., isActive, hasAccess, canEdit).
Classes and interfaces must use PascalCase (e.g., UserService, CaseRepository). DO NOT use I prefix for interfaces.
Constants must use SCREAMING_SNAKE_CASE (e.g., MAX_FILE_SIZE, DEFAULT_TIMEOUT).
Module ordering: constants outside function scope at the top, exported functions next, other functions ordered by usage, interfaces and types at the bottom.
TypeScript strict mode must be enabled. No any type without justification. Use explicit types for all variables and function parameters.
Always add .js extension to relative imports (e.g., import { foo } from "./bar.js"), even when importing TypeScript files. This is required for ESM with Node.js 'nodenext' module resolution.
Use workspace aliases (@hmcts/*) for imports between packages instead of relative paths.
Only export functions that are intended to be used outside the module. Do not export functions solely for testing purposes.
Only add comments when they provide meaningful explanation of why something is done, not what is done. Code should be self-documenting.
Favor functional style. Don't use classes unless you have shared state.
Data should be immutable by default. Use const and avoid mutations to ensure predictable state.
Functions should have no side effects. Avoid modifying external state or relying on mutable data.

Files:

• libs/api/src/blob-ingestion/repository/service.ts
• libs/api/src/blob-ingestion/repository/service.test.ts
• libs/api/src/blob-ingestion/validation.test.ts
• libs/api/src/blob-ingestion/validation.ts

**/*.{ts,tsx,js,mjs}

📄 CodeRabbit inference engine (CLAUDE.md)

DO NOT use CommonJS. Use import/export, never require()/module.exports. Only ES modules are allowed.

Files:

• libs/api/src/blob-ingestion/repository/service.ts
• libs/api/src/blob-ingestion/repository/service.test.ts
• libs/api/src/blob-ingestion/validation.test.ts
• libs/api/src/blob-ingestion/validation.ts

**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.ts: Do not create generic types.ts files. Colocate types with the appropriate code file where they are used.
Do not create generic files like utils.ts. Be specific with naming (e.g., object-properties.ts, date-formatting.ts).

Files:

• libs/api/src/blob-ingestion/repository/service.ts
• libs/api/src/blob-ingestion/repository/service.test.ts
• libs/api/src/blob-ingestion/validation.test.ts
• libs/api/src/blob-ingestion/validation.ts

**/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Unit/integration test files must be co-located with source files as *.test.ts and use Vitest with describe, it, and expect.

Files:

• libs/api/src/blob-ingestion/repository/service.test.ts
• libs/api/src/blob-ingestion/validation.test.ts

🧠 Learnings (4)

📓 Common learnings

Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 136
File: VIBE-209-specification.md:67-76
Timestamp: 2025-11-27T09:50:32.707Z
Learning: In the CaTH blob ingestion API (VIBE-209), when a court_id/location_id is not found in the Court Master Reference Data, the API returns 200 OK with no_match=true rather than a 404 error. This allows ingestion to proceed and enables later admin mapping of the location.

📚 Learning: 2025-11-27T09:50:32.707Z

Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 136
File: VIBE-209-specification.md:67-76
Timestamp: 2025-11-27T09:50:32.707Z
Learning: In the CaTH blob ingestion API (VIBE-209), when a court_id/location_id is not found in the Court Master Reference Data, the API returns 200 OK with no_match=true rather than a 404 error. This allows ingestion to proceed and enables later admin mapping of the location.

Applied to files:

• libs/api/src/blob-ingestion/repository/service.test.ts
• docs/tickets/VIBE-209/plan.md

📚 Learning: 2025-12-03T13:55:34.702Z

Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to **/*.test.ts : Unit/integration test files must be co-located with source files as `*.test.ts` and use Vitest with `describe`, `it`, and `expect`.

Applied to files:

• libs/api/src/blob-ingestion/validation.test.ts

📚 Learning: 2025-11-27T09:48:13.010Z

Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 136
File: libs/api/src/blob-ingestion/validation.ts:156-163
Timestamp: 2025-11-27T09:48:13.010Z
Learning: In libs/api/src/blob-ingestion/validation.ts, the permissive date validation in isValidISODate and isValidISODateTime functions is expected behavior and should not be flagged for stricter validation.

Applied to files:

• libs/api/src/blob-ingestion/validation.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: E2E Tests

🔇 Additional comments (6)

libs/api/src/blob-ingestion/validation.ts (1)

124-124: Excellent bug fix - missing await was causing locationExists to always be true.

The addition of await here correctly resolves the issue where getLocationById returns a Promise. Without the await, !!Promise would always evaluate to true, causing locationExists to be incorrectly set even when the location didn't exist in the reference data. This fix ensures the actual resolved value is checked.

docs/tickets/VIBE-209/plan.md (1)

507-507: LGTM - cleaner user-facing message.

The message text is simplified by removing the (no_match=true) suffix while the no_match boolean flag remains in the response object, so no information is lost. This makes the API response more professional and easier to read.

Also applies to: 1414-1414

libs/api/src/blob-ingestion/repository/service.ts (1)

97-97: LGTM - message formatting improved.

The success message is cleaner without the redundant suffix. The no_match flag is still explicitly returned on line 96, so API consumers can programmatically check the status while getting a more user-friendly message.

libs/api/src/blob-ingestion/repository/service.test.ts (1)

91-91: LGTM - test updated to match new message format.

The test expectation correctly reflects the updated success message. All other assertions remain unchanged, confirming that the functional behavior (setting no_match to true and creating the artefact with noMatch: true) is preserved.

libs/api/src/blob-ingestion/validation.test.ts (2)

7-12: LGTM - mock correctly returns Promises to match async signature.

The updated mock now returns Promise.resolve() values instead of direct values, properly simulating the asynchronous behavior of getLocationById. This ensures tests accurately reflect the real function behavior.

---

227-256: Excellent test coverage for the bug fix.

These two new tests provide comprehensive coverage for the async location lookup:

1. Lines 227-239: Validates that non-existent locations (resolving to undefined) correctly set locationExists to false. The comment on line 236 helpfully explains this would have failed before the fix due to Promise truthiness.

2. Lines 241-256: Validates that existing locations correctly set locationExists to true when the Promise resolves to a location object.

Both tests directly verify the bug fix and prevent regression.

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

🎭 Playwright E2E Test Results

145 tests   145 ✅  9m 33s ⏱️
 22 suites    0 💤
  1 files      0 ❌

Results for commit cd7ac0f.