Add test range param to NHS numbers

neanias commented 7 months ago

The NHS sets aside a range of numbers for test purposes (999 000 0000 to 999 999 9999). By default, this generator creates valid NHS numbers that may be in use by actual people in the UK health services. This param will switch to using the test range which aren't in use anywhere.

Motivation / Background

This Pull Request has been created because the NHS sets aside a range of numbers for test purposes (999 000 0000 to 999 999 9999). By default, this generator creates valid NHS numbers that may be in use by actual people in the UK health services. This param will switch to using the test range which aren't in use anywhere.

Additional information

NHS Digital documentation about "synthetic" data

Checklist

Before submitting the PR make sure the following are checked:

[x] This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
[x] Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
[x] Tests are added or updated if you fix a bug, refactor something, or add a feature.
[x] Tests and Rubocop are passing before submitting your proposed changes.

stefannibrasil commented 6 months ago

thank you @neanias this is good to know. As @thdaraujo mentioned, we did something similar for Faker URL and email addresses. I'm sure there are others but there's so many it's hard to spot all opportunities that need to be changed.

We can add a callout for this change when this PR gets released so folks know ahead of bumping the version. Therefore, I am okay with changing it to only generate test numbers by default and add the callout in the release notes.

Since we're working on this file, could you also change check_digit to be a private method?

neanias commented 6 months ago

@stefannibrasil Ok, happy to just move it to be enabled by default! If we're changing the check_digit method to be private, should I also remove the related tests? The code paths will be exercised by the british_number method, I suppose.

stefannibrasil commented 6 months ago

@stefannibrasil Ok, happy to just move it to be enabled by default! If we're changing the check_digit method to be private, should I also remove the related tests? The code paths will be exercised by the british_number method, I suppose.

Yes, that's the approach I follow too. No need to test private methods 👍

neanias commented 6 months ago

Glad we got it through in the end! 🚀

neanias commented 5 months ago

Hello! Just wondering about this going in the recent changelog. It wasn't mentioned that it's potentially a breaking change for people using the generator and expecting the old range. Is it worth adding it later or is it just done for now?

stefannibrasil commented 4 months ago

HI @neanias thanks for the reminder! I've cut a release with a note about this: https://github.com/faker-ruby/faker/releases/tag/v3.4.2

faker-ruby / faker