fix(condo): DOMA-13432 added commission validation#7823
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughFeeDistributionJsonSchema now restricts ChangesFee distribution schema validation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 338fde28d0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| module.exports = { | ||
| FEE_DISTRIBUTION_SCHEMA_FIELD, | ||
| FEE_DISTRIBUTION_QUERY_LIST, | ||
| FeeDistributionSchemaJsonValidator, |
There was a problem hiding this comment.
AGENTS.md asks not to export internal constants/utilities solely for tests; this adds the compiled AJV validator to the production module's public API just so FeeDistribution.spec.js can import it. That exposes an implementation detail that other code can start depending on, so please test through FEE_DISTRIBUTION_SCHEMA_FIELD.hooks.validateInput or move validation into a deliberately public helper instead.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/condo/domains/acquiring/schema/fields/json/FeeDistribution.spec.js (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
faker.datatype.uuid()instead of a hardcoded UUID.As per coding guidelines,
**/*.{test.js,spec.js,spec.ts}files should "generate UUIDs withfaker.datatype.uuid()" rather than hardcoded literals.♻️ Proposed fix
+const { faker } = require('`@faker-js/faker`') + const { FeeDistributionSchemaJsonValidator } = require('./FeeDistribution') ... - category: '123e4567-e89b-12d3-a456-426614174000', + category: faker.datatype.uuid(), ... - category: '123e4567-e89b-12d3-a456-426614174000', + category: faker.datatype.uuid(),Also applies to: 23-23
🤖 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 `@apps/condo/domains/acquiring/schema/fields/json/FeeDistribution.spec.js` at line 16, The test data in FeeDistribution.spec.js is using hardcoded UUID literals instead of generated values. Update the affected fixture/object assignments in this spec to use faker.datatype.uuid() for the category and any other UUID fields, keeping the existing test setup aligned with the UUID generation guideline while preserving the same structure around the relevant test data.Source: Coding guidelines
🤖 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.
Inline comments:
In `@apps/condo/domains/acquiring/schema/fields/json/FeeDistribution.js`:
- Around line 33-35: The percent validation in FeeDistribution currently only
enforces numeric-string shape in the schema field definition and still allows
arbitrarily large values. Update the validation for the percent property in
FeeDistribution so it also enforces an upper bound such as 100, using a
secondary/custom check or format-based validation since AJV range keywords won’t
apply to a string type. Keep the existing pattern for format, but ensure the
commission validation in the same schema rejects out-of-range values.
---
Nitpick comments:
In `@apps/condo/domains/acquiring/schema/fields/json/FeeDistribution.spec.js`:
- Line 16: The test data in FeeDistribution.spec.js is using hardcoded UUID
literals instead of generated values. Update the affected fixture/object
assignments in this spec to use faker.datatype.uuid() for the category and any
other UUID fields, keeping the existing test setup aligned with the UUID
generation guideline while preserving the same structure around the relevant
test data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82c65ce1-e546-48de-8fb6-325f40e8b64a
📒 Files selected for processing (2)
apps/condo/domains/acquiring/schema/fields/json/FeeDistribution.jsapps/condo/domains/acquiring/schema/fields/json/FeeDistribution.spec.js
|
1 participant
Summary by CodeRabbit
Bug Fixes
percent,minAmount, andmaxAmountonly accept properly formatted numeric strings.Tests
percent,minAmount, andmaxAmount.