bchavez / Bogus

:card_index: A simple fake data generator for C#, F#, and VB.NET. Based on and ported from the famed faker.js.
Other
8.51k stars 483 forks source link

Invalid Iban generation for CR (Costa Rica) #538

Closed SimonShvets closed 2 months ago

SimonShvets commented 3 months ago

Bogus NuGet Package

35.4.0

.NET Version

.NET 6.0

Visual Studio Version

17.8.4

What operating system are you using?

Windows

What locale are you using with Bogus?

en

Problem Description

correct iban format for CR -> CR05 0152 0200 1026 2840 66 (example) Bogus library is generating -> CR37 0051 0009 0045 0014 1 (example)

LINQPad Example or Reproduction Steps

var iban = new Faker().Finance.Iban(false, "CR");

Expected Behavior

expected valid 22 symbols iban

Actual Behavior

invalid 21 symbols iban

Known Workarounds

No response

Could you help with a pull-request?

No

dangerman commented 3 months ago

I looked into this and it seems like it's a bug that was already fixed in faker.js a couple years ago: https://github.com/faker-js/faker/issues/643

In summary:

The Costa Rican IBAN format is missing an extra 0 (reserve number) before the banking code.
(For reference: https://bank-code.net/iban/structure/costa-rica-international-bank-account-number)

Locale data

Bogus IBAN generation appears to use the iban_formats from the locale data: https://github.com/bchavez/Bogus/blob/80e68d75614bfad422aea8e37291b3cc8a7eb5da/Source/Bogus/DataSets/Finance.cs#L375-L377

And according to the documentation, this comes from:

Proposed fix

We've already got iban_formats in the data_extend en locale, so we can just update it there and run gulp importLocales, right? https://github.com/bchavez/Bogus/blob/80e68d75614bfad422aea8e37291b3cc8a7eb5da/Source/Bogus/data_extend/en.locale.json#L471-L485

@bchavez is it alright if I raise a PR with:

bchavez commented 3 months ago

Hey, very great find and great investigation work @dangerman. 👍 Thank you for this.

If we already have a data_extend for iban formats, which it looks like we do; then yes, I would accept a PR/patch for this specific CR (Costa Rica) IBAN issue that updates data_extend:en/iban/cr.

You're absolutely correct on the proposed fix:

and running gulp importLocales should gather all the JSON; merge data_extend, and finally write the necessary BSON resource files for Bogus.dll.

Just a few more points:

The README.md section for Building From Source should have the latest docs on the build basics for the Bogus repo (including node js dependencies etc):

Roadmap wise, right now Bogus is basically around fakerjs v5 API + dataset parity. Longer term, we'll be catching up to v8 API + datasets; which is why some of these datasets might be outdated and as we catch up; hopefully a lot of these issues will be resolved automatically.

bchavez commented 2 months ago

Thank you @dangerman for the PR; excellent and super great work.

The CR IBAN fix is now available in Bogus v35.5.1:

Thanks again for the bug report and PR fix. 👍 🚀