Skip to content

Conversation

tech-and-avinash
Copy link

Updated the regex for the mk-MK locale to match only valid Macedonian mobile phone numbers.
This excludes landlines (e.g., 02, 03x), toll-free/service numbers (e.g., 800), and other invalid formats.

Supports both national (07x) and international (+3897x) mobile formats.

Includes tests for valid and invalid examples.

@Copilot Copilot AI review requested due to automatic review settings August 4, 2025 10:34
Copilot

This comment was marked as outdated.

@rubiin
Copy link
Member

rubiin commented Aug 4, 2025

could you fix the failed tests as well

Copy link

codecov bot commented Aug 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (72573b3) to head (b1bc18a).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2579   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          114       114           
  Lines         2535      2535           
  Branches       641       641           
=========================================
  Hits          2535      2535           

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

@tech-and-avinash tech-and-avinash requested a review from rubiin August 4, 2025 11:25
rubiin
rubiin previously approved these changes Aug 4, 2025
Copilot

This comment was marked as outdated.

Copilot

This comment was marked as outdated.

Copilot

This comment was marked as outdated.

Copy link

@Copilot Copilot AI left a comment

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 fixes the Macedonian (mk-MK) mobile phone number validation to only accept valid mobile numbers, excluding landlines and service numbers. The regex is simplified to match only mobile prefixes (07x for national format and +3897x for international format).

  • Simplified the mk-MK regex pattern to match only mobile number prefixes
  • Updated test cases to reflect the new validation rules
  • Moved test case position in the test file for better organization

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/lib/isMobilePhone.js Updated regex pattern to restrict mk-MK validation to mobile numbers only
test/validators.test.js Replaced existing mk-MK test cases with new valid/invalid examples matching the updated validation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -161,7 +161,7 @@ const phones = {
'ar-YE': /^(((\+|00)9677|0?7)[0137]\d{7}|((\+|00)967|0)[1-7]\d{6})$/,
'ar-EH': /^(\+?212|0)[\s\-]?(5288|5289)[\s\-]?\d{5}$/,
'fa-AF': /^(\+93|0)?(2{1}[0-8]{1}|[3-5]{1}[0-4]{1})(\d{7})$/,
'mk-MK': /^(\+?389|0)?((?:2[2-9]\d{6}|(?:3[1-4]|4[2-8])\d{6}|500\d{5}|5[2-9]\d{6}|7[0-9][2-9]\d{5}|8[1-9]\d{6}|800\d{5}|8009\d{4}))$/,
'mk-MK': /^(\+3897[0-9]|07[0-9])[0-9]{6}$/,
Copy link
Preview

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

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

The regex pattern is too restrictive. It only matches numbers with exactly 8 digits after the prefix, but the test cases show '+38971234567' (9 digits after +389) and '071234567' (8 digits total including prefix). The pattern should be /^(+3897[0-9]\d{6}|07[0-9]\d{6})$/ to match the expected formats.

Suggested change
'mk-MK': /^(\+3897[0-9]|07[0-9])[0-9]{6}$/,
'mk-MK': /^(\+3897[0-9]\d{6}|07[0-9]\d{6})$/,

Copilot uses AI. Check for mistakes.

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