Skip to content

[ENG-867] fix:Ignore unique checks on empty identifiers - #3727

Open
nandkishorr wants to merge 1 commit into
developfrom
ENG-867-ignore-unique-checks-on-empty-identifiers
Open

[ENG-867] fix:Ignore unique checks on empty identifiers#3727
nandkishorr wants to merge 1 commit into
developfrom
ENG-867-ignore-unique-checks-on-empty-identifiers

Conversation

@nandkishorr

@nandkishorr nandkishorr commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Proposed Changes

  • Ignored unique checks on empty identifiers .

Associated Issue

Merge Checklist

  • Tests added/fixed
  • Update docs in /docs
  • Linting Complete
  • Any other necessary step

Only PR's with test cases included and passing lint and test pipelines will be reviewed

@ohcnetwork/care-backend-maintainers @ohcnetwork/care-backend-admins

Summary by CodeRabbit

  • Bug Fixes
    • Empty identifier values no longer trigger uniqueness validation errors.
    • Regex validation behavior remains unchanged.

@nandkishorr nandkishorr self-assigned this Aug 6, 2026
Copilot AI review requested due to automatic review settings August 6, 2026 09:13
@nandkishorr
nandkishorr requested a review from a team as a code owner August 6, 2026 09:13
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c57bec08-4e5c-4d5a-823d-e0bce9fe9350

📥 Commits

Reviewing files that changed from the base of the PR and between fcd51e8 and af2b471.

📒 Files selected for processing (1)
  • care/emr/resources/patient/spec.py

📝 Walkthrough

Walkthrough

The patient identifier validator now checks uniqueness only when an identifier value is present. Regex validation remains unchanged.

Changes

Patient identifier validation

Layer / File(s) Summary
Guard identifier uniqueness checks
care/emr/resources/patient/spec.py
Duplicate checks now run only for non-empty identifier values. Empty values no longer produce uniqueness errors.

Estimated code review effort: 2 (Simple) | ~5 minutes

Suggested reviewers: vigneshhari, copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description states the change, references ENG-867, and includes the required merge checklist; the unused architecture section was correctly omitted.
Title check ✅ Passed The title clearly identifies the fix to ignore uniqueness checks for empty identifiers and includes the associated issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ENG-867-ignore-unique-checks-on-empty-identifiers

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

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates patient identifier validation to avoid running uniqueness validation when the identifier value is empty, aligning behavior with optional identifier fields in the EMR patient resource layer.

Changes:

  • Skip uniqueness validation when value is empty in validate_identifier_config.
  • Preserve existing regex validation behavior for non-empty values.

Comment on lines 87 to 91
queryset = queryset.filter(config__external_id=config["id"])
if obj:
queryset = queryset.exclude(patient=obj)
if config["config"]["unique"] and queryset.exists():
if value and config["config"]["unique"] and queryset.exists():
err = f"Identifier config {config['config']['system']} is not unique"
if obj:
queryset = queryset.exclude(patient=obj)
if config["config"]["unique"] and queryset.exists():
if value and config["config"]["unique"] and queryset.exists():
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 79.45%. Comparing base (fcd51e8) to head (af2b471).

Files with missing lines Patch % Lines
care/emr/resources/patient/spec.py 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3727   +/-   ##
========================================
  Coverage    79.45%   79.45%           
========================================
  Files          480      480           
  Lines        23214    23214           
  Branches      2420     2420           
========================================
  Hits         18445    18445           
  Misses        4165     4165           
  Partials       604      604           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@eeshsaxena eeshsaxena left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Guarding the uniqueness check with value and ... means an empty or missing identifier no longer trips "is not unique" against other empty identifiers, while a real value still gets checked. Since identifier values are strings here, value being falsy covers empty string and None, which is the intended skip. Reads correct.

@vigneshhari

Copy link
Copy Markdown
Member

Missing tests. Also resolve the comments by the AI bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants