Skip to content

WEB-705 Restrict Negative Values for Office Phone, MTN, and Airtel Fi…#3151

Merged
IOhacker merged 1 commit intoopenMF:devfrom
JaySoni1:WEB-705-restrict-negative-values-for-office-phone-mtn-and-airtel-fields-in-datatable-forms
Feb 14, 2026
Merged

WEB-705 Restrict Negative Values for Office Phone, MTN, and Airtel Fi…#3151
IOhacker merged 1 commit intoopenMF:devfrom
JaySoni1:WEB-705-restrict-negative-values-for-office-phone-mtn-and-airtel-fields-in-datatable-forms

Conversation

@JaySoni1
Copy link
Contributor

@JaySoni1 JaySoni1 commented Feb 14, 2026

Changes Made :-

-Restrict negative values for Office Phone, MTN, and Airtel fields in datatable forms .

WEB-705

Before :-

image

After :-

image

Summary by CodeRabbit

  • Bug Fixes
    • Improved input validation for restricted fields (phone numbers, pricing fields) to properly enforce minimum constraints and normalize various field naming formats.

@coderabbitai
Copy link

coderabbitai bot commented Feb 14, 2026

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'pre_merge_checks'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

The change extends the min-validation logic in form field generation by adding detection for restricted fields (mtn, airtel, officephone, office_phone, office phone) through field name normalization, and applies the existing min constraint to these fields alongside the previously supported conditions.

Changes

Cohort / File(s) Summary
Form Field Min-Validation Logic
src/app/core/utils/datatables.ts
Added isRestrictedField detection by normalizing field names and checking against a list of restricted identifiers (mtn, airtel, officephone, office_phone, office phone). Expanded the min-constraint condition to apply when isMinSavingsAmount, isPriceOneShare, or isRestrictedField is true.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

Suggested reviewers

  • alberto-art3ch
🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (34 files):

⚔️ README.md (content)
⚔️ angular.json (content)
⚔️ docker-compose.yml (content)
⚔️ env.sample (content)
⚔️ proxy.conf.js (content)
⚔️ proxy.localhost.conf.js (content)
⚔️ src/app/clients/client-stepper/client-general-step/client-general-step.component.html (content)
⚔️ src/app/clients/client-stepper/client-general-step/client-general-step.component.scss (content)
⚔️ src/app/clients/client-stepper/client-general-step/client-general-step.component.ts (content)
⚔️ src/app/clients/clients.service.ts (content)
⚔️ src/app/clients/edit-client/edit-client.component.html (content)
⚔️ src/app/clients/edit-client/edit-client.component.scss (content)
⚔️ src/app/clients/edit-client/edit-client.component.ts (content)
⚔️ src/app/core/utils/datatables.ts (content)
⚔️ src/app/products/share-products/share-product-stepper/share-product-market-price-step/share-product-market-price-step.component.ts (content)
⚔️ src/app/shared/footer/footer.component.scss (content)
⚔️ src/assets/env.js (content)
⚔️ src/assets/env.template.js (content)
⚔️ src/assets/translations/cs-CS.json (content)
⚔️ src/assets/translations/de-DE.json (content)
⚔️ src/assets/translations/en-US.json (content)
⚔️ src/assets/translations/es-CL.json (content)
⚔️ src/assets/translations/es-MX.json (content)
⚔️ src/assets/translations/fr-FR.json (content)
⚔️ src/assets/translations/it-IT.json (content)
⚔️ src/assets/translations/ko-KO.json (content)
⚔️ src/assets/translations/lt-LT.json (content)
⚔️ src/assets/translations/lv-LV.json (content)
⚔️ src/assets/translations/ne-NE.json (content)
⚔️ src/assets/translations/pt-PT.json (content)
⚔️ src/assets/translations/sw-SW.json (content)
⚔️ src/environments/environment.prod.ts (content)
⚔️ src/environments/environment.ts (content)
⚔️ src/typings.d.ts (content)

These conflicts must be resolved before merging into dev.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: restricting negative values for Office Phone, MTN, and Airtel fields in datatable forms, which matches the core objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch WEB-705-restrict-negative-values-for-office-phone-mtn-and-airtel-fields-in-datatable-forms
  • Post resolved changes as copyable diffs in a comment

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 and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/app/core/utils/datatables.ts (1)

58-71: ⚠️ Potential issue | 🟡 Minor

Guard min: 0 to apply only to numeric column types.

The min validator is applied to all column types regardless of their type attribute. For STRING and TEXT fields (where type is 'text'), the HTML min attribute is ignored by the browser, and Angular's Validators.min() may produce unexpected behavior when attempting to validate string values numerically.

Apply the proposed fix to ensure min is set only for INTEGER and DECIMAL columns:

Proposed fix
-          if (isMinSavingsAmount || isPriceOneShare || isRestrictedField) {
+          if ((isMinSavingsAmount || isPriceOneShare || isRestrictedField) &&
+              (column.columnDisplayType === 'INTEGER' || column.columnDisplayType === 'DECIMAL')) {
             inputOptions.min = 0;
           }
🧹 Nitpick comments (1)
src/app/core/utils/datatables.ts (1)

50-56: Redundant entries in the restricted-field list.

After the .replace(/[_\s]+/g, '') normalization on Line 56, 'office_phone' and 'office phone' both collapse to 'officephone'—which is already in the list. Those two entries are dead weight.

♻️ Suggested simplification
       const isRestrictedField = [
         'mtn',
         'airtel',
         'officephone',
-        'office_phone',
-        'office phone'
       ].some((name) => colName.includes(name.replace(/[_\s]+/g, '')));
+      ].some((name) => colName.includes(name));

Since both colName and the literal names are already fully normalized (lowercased, no separators), you can drop the redundant entries and the inner .replace() call.

Copy link
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

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

Lgtm

@IOhacker IOhacker merged commit fc30847 into openMF:dev Feb 14, 2026
5 checks passed
@JaySoni1
Copy link
Contributor Author

@IOhacker Thank You for the review

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.

2 participants