Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add creditCardIssuer #888

Merged
merged 9 commits into from Apr 30, 2022
Merged

feat: add creditCardIssuer #888

merged 9 commits into from Apr 30, 2022

Conversation

beninsydney
Copy link
Contributor

Adds method for randomly selecting a credit card provider from the keys of faker.definitions.finance.credit_card

> faker.finance.creditCardProvider()
'visa'
> faker.finance.creditCardProvider()
'diners_club'

For pull request #882

@beninsydney beninsydney requested a review from a team as a code owner April 29, 2022 04:03
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

Please add a test for this.

expect(Object.keys(.....credit_card)).toContain(actual)

src/finance.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT added c: feature Request for new feature needs test More tests are needed p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug labels Apr 29, 2022
@ST-DDT ST-DDT added this to the v6.3 - Next Minor milestone Apr 29, 2022
@ST-DDT ST-DDT linked an issue Apr 29, 2022 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #888 (6306222) into main (36cd461) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #888   +/-   ##
=======================================
  Coverage   99.41%   99.41%           
=======================================
  Files        1959     1959           
  Lines      210905   210917   +12     
  Branches      906      907    +1     
=======================================
+ Hits       209675   209687   +12     
  Misses       1172     1172           
  Partials       58       58           
Impacted Files Coverage Δ
src/definitions/finance.ts 100.00% <100.00%> (ø)
src/finance.ts 99.31% <100.00%> (+0.01%) ⬆️

@ST-DDT ST-DDT removed the needs test More tests are needed label Apr 29, 2022
@ST-DDT
Copy link
Member

ST-DDT commented Apr 29, 2022

Docs Preview (Click to expand)

grafik

ST-DDT
ST-DDT previously approved these changes Apr 29, 2022
@ST-DDT ST-DDT requested review from a team April 29, 2022 07:26
@Shinigami92
Copy link
Member

Should we focus and merge #503 first?

src/finance.ts Outdated Show resolved Hide resolved
test/finance.spec.ts Outdated Show resolved Hide resolved
test/finance.spec.ts Outdated Show resolved Hide resolved
@beninsydney
Copy link
Contributor Author

beninsydney commented Apr 29, 2022

I've got this ready locally but I've noticed 'provider' is also used in src/finance.ts in creditCardNumber() and in src/definitions/finance.ts in a description block, do you want me to go ahead and update the nomenclature in those files too for consistency?

  /**
   * Generates a random credit card number.
   *
   * @param provider The name of the provider (case insensitive) or the format used to generate one.
   *
   * @example
   * faker.finance.creditCardNumber() // '4427163488668'
   * faker.finance.creditCardNumber('visa') // '4882664999003'
   * faker.finance.creditCardNumber('63[7-9]#-####-####-###L') // '6375-3265-4676-6644'
   */
  creditCardNumber(provider = ''): string {
    let format: string;
    const localeFormat = this.faker.definitions.finance.credit_card;
    const normalizedProvider = provider.toLowerCase();
    if (normalizedProvider in localeFormat) {
      format = this.faker.random.arrayElement(localeFormat[normalizedProvider]);
    } else if (provider.match(/#/)) {
      // The user chose an optional scheme
      format = provider;
    } else {
      // Choose a random provider
      // Credit cards are in an object structure
      const formats = this.faker.random.objectElement(localeFormat, 'value'); // There could be multiple formats
      format = this.faker.random.arrayElement(formats);
    }
    format = format.replace(/\//g, '');
    return this.faker.helpers.replaceCreditCardSymbols(format);
  }

@Shinigami92 Shinigami92 changed the title feat: add creditCardProvider (#882) feat: add creditCardIssuer (#882) Apr 29, 2022
@Shinigami92
Copy link
Member

test/finance.spec.ts line 397

Shinigami92
Shinigami92 previously approved these changes Apr 29, 2022
ST-DDT
ST-DDT previously approved these changes Apr 29, 2022
@ST-DDT ST-DDT requested review from a team April 29, 2022 12:58
pkuczynski
pkuczynski previously approved these changes Apr 29, 2022
test/finance.spec.ts Outdated Show resolved Hide resolved
ST-DDT
ST-DDT previously approved these changes Apr 29, 2022
@ST-DDT ST-DDT requested review from a team April 29, 2022 14:28
Shinigami92
Shinigami92 previously approved these changes Apr 29, 2022
@Shinigami92 Shinigami92 dismissed stale reviews from ST-DDT and themself via 6306222 April 30, 2022 09:00
@Shinigami92 Shinigami92 requested a review from ST-DDT April 30, 2022 09:00
@Shinigami92 Shinigami92 changed the title feat: add creditCardIssuer (#882) feat: add creditCardIssuer Apr 30, 2022
@Shinigami92 Shinigami92 enabled auto-merge (squash) April 30, 2022 09:01
@Shinigami92 Shinigami92 merged commit 58b4f10 into faker-js:main Apr 30, 2022
@xDivisionByZerox xDivisionByZerox added the m: finance Something is referring to the finance module label Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature m: finance Something is referring to the finance module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Credit/debit cards
5 participants