fix: enabled removal of definitional field on product knowledge update

[nandkishorr]

Proposed Changes

• updated product knowledge updated spec

• updated when definitional is None

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

  • Improved data consistency for product records: the product definition must now be explicitly provided (it may be set blank) and definition fields are reliably cleared when omitted during updates.

• Tests

  • API test payloads updated to include an explicit empty product definition to reflect the new behavior.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Removed the default value from definitional on BaseProductKnowledgeSpec (making the field required in the signature) and added a guard in ProductKnowledgeUpdateSpec.perform_extra_deserialization to set obj.definitional = None when self.definitional is None, ensuring explicit clearing on updates. Tests updated to include "definitional": None in generated payloads.

Changes

Cohort / File(s)	Change Summary
Spec field signature care/emr/resources/inventory/product_knowledge/spec.py	Changed BaseProductKnowledgeSpec.definitional from `ProductDefinitionSpec
Deserialization guard logic care/emr/resources/inventory/product_knowledge/spec.py	In ProductKnowledgeUpdateSpec.perform_extra_deserialization, added a guard: when self.definitional is None, set obj.definitional = None to explicitly clear the field during update deserialization.
Tests payloads care/emr/tests/test_product_knowledge_api.py	Added top-level "definitional": None to payloads in generate_product_knowledge_data and create_update_product_knowledge_data to match the updated spec shape.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Inspect all places that instantiate BaseProductKnowledgeSpec (or subclasses) for reliance on the removed default and adjust callers if needed.
• Verify runtime behavior with your validation library (pydantic/dataclass) to ensure optional-without-default doesn't break instantiation in common patterns.
• Confirm the update-path semantics: clearing definitional when omitted is intended and consistent with API expectations (tests were updated, but double-check other consumers).

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is incomplete and vague. It lacks details about what was changed, why, and how the issue is resolved. The 'Associated Issue' section is missing entirely.	Provide a detailed explanation of the changes, include a link to the associated issue, and clearly explain how this fix enables removal of the definitional field.
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
Title check	✅ Passed	The title accurately describes the main change: enabling removal of the definitional field on product knowledge updates by removing the default value.

✨ 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 nandkishorr/fix/product_knowledge_spec

---

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.}

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.81%. Comparing base (ed21ebe) to head (bdf187a).

Files with missing lines	Patch %	Lines
.../emr/resources/inventory/product_knowledge/spec.py	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3348      +/-   ##
===========================================
- Coverage    73.82%   73.81%   -0.01%     
===========================================
  Files          435      435              
  Lines        19814    19816       +2     
  Branches      2153     2154       +1     
===========================================
+ Hits         14627    14628       +1     
  Misses        4738     4738              
- Partials       449      450       +1     

☔ View full report in Codecov by Sentry.
📢 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.

[vigneshhari]

Just remove the = None here and you should be good to go.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

care/emr/tests/test_product_knowledge_api.py (1)

81-94: Update payload now exercises definitional=None; consider a dedicated behavior assertion

Including "definitional": None in create_update_product_knowledge_data matches the intended “clear the field on update” behavior, so this change is fine as-is. It might be worth adding a focused test that:

• creates a product with a non-null definitional, then
• PATCHes with "definitional": None and asserts the field is actually cleared in the response/DB.

Not mandatory for this PR, but it would prove the behavior you’re trying to fix rather than relying on it indirectly.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 04718e1 and bdf187a.

📒 Files selected for processing (1)

• care/emr/tests/test_product_knowledge_api.py (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.py

📄 CodeRabbit inference engine (.cursorrules)

**/*.py: Prioritize readability and maintainability; follow Django's coding style guide (PEP 8 compliance).
Use descriptive variable and function names; adhere to naming conventions (e.g., lowercase with underscores for functions and variables).

Files:

• care/emr/tests/test_product_knowledge_api.py

**/tests/**/*.py

📄 CodeRabbit inference engine (.cursorrules)

Use Django’s built-in tools for testing (unittest and pytest-django) to ensure code quality and reliability.

Files:

• care/emr/tests/test_product_knowledge_api.py

⏰ 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: Test / Test

🔇 Additional comments (1)

care/emr/tests/test_product_knowledge_api.py (1)

28-51: Explicitly including definitional in create payload looks correct

Adding "definitional": None to generate_product_knowledge_data keeps the test payload shape in sync with the spec change and ensures the API path is at least exercised with an explicit null rather than relying on implicit defaults. Nothing blocking here.