LEGLINK-907: Fix validation bug in tenant create/update - #1790
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughFacility creation and update validation now require ChangesFacility validation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
DotNet/Tenant/Controllers/FacilityController.cs (1)
222-224: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover the new request contract with focused XUnit tests.
Add tests for both
StoreFacilityandPutFacilitythat verify:
- Missing
VendorVersionIdreturns400with the exact message.- The manager is not called when validation fails.
- A valid
VendorVersionIdreaches the success path.Vendor = nullwith a validVendorVersionIdsucceeds.The success test in
DotNet/ServiceTests/IntegrationTests/Tenant/FacilityControllerTests.csLines [144]-[171] still supplies both fields. It does not prove that the oldVendorfield is optional. Use Moq for dependencies. Do not use network activity in unit tests.As per path instructions, add focused XUnit tests for each modified conditional branch and use mocks instead of network activity.
Also applies to: 327-329
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DotNet/Tenant/Controllers/FacilityController.cs` around lines 222 - 224, Add focused XUnit tests for StoreFacility and PutFacility covering missing VendorVersionId with the exact 400 message and verifying the manager is not called, plus valid VendorVersionId success cases including Vendor = null. Use Moq for controller dependencies, avoid network activity, and update existing success coverage so it proves Vendor is optional.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@DotNet/Tenant/Controllers/FacilityController.cs`:
- Around line 222-224: Add focused XUnit tests for StoreFacility and PutFacility
covering missing VendorVersionId with the exact 400 message and verifying the
manager is not called, plus valid VendorVersionId success cases including Vendor
= null. Use Moq for controller dependencies, avoid network activity, and update
existing success coverage so it proves Vendor is optional.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 83ed1c9f-f07d-4516-bd87-cf62bdb3b3c9
📒 Files selected for processing (1)
DotNet/Tenant/Controllers/FacilityController.cs
Remove 'no vendor' negative test cases and their step name constants. Deleted Create400NoVendor and Update400NoVendor from DotNet/Automation.UI/Services/ApiHealth/TestSuites/ApiEndPointLibrary.cs and removed the corresponding test steps from DotNet/Automation.UI/Services/ApiHealth/TestSuites/TenantServiceTestSuite.cs. Cleans up unused step names and obsolete negative test cases; no other behavior changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🛠️ Description of Changes
Bug fix where tenant model validation was requiring the old vendor field.
🧪 Testing Performed
Tested the tenant create/update endpoints with Postman.
🧑🔬 Unit Testing
📓 Documentation Updated
Please update any relevant sections in the project documentation that were impacted by the changes in the PR.