braintree / credit-card-type

A library for determining credit card type
MIT License
980 stars 154 forks source link

ChangeOrder doesn't set desired priorities #99

Closed ooade closed 5 years ago

ooade commented 5 years ago

General information

Issue description

I added a custom card (custom-card) with some validator closely related to Maestro, then I changed my custom-card order to position 0 but I still get maestro coming up as the card type.

I debugged using Validate.creditCardType(''), and I was able to verify that my custom-card was in position 0.

Thanks for the awesome library. Looking forward to getting some feedback.

crookedneighbor commented 5 years ago

Can you provide the exact code you're using to so we can better debug it?

ooade commented 5 years ago

This is it:

creditCardType.addCard({
  niceType: 'Alat',
  type: 'alat',
  patterns: [
    [506, 507],
    [650002, 650007]
  ],
  gaps: [4, 8, 12, 16],
  lengths: [16, 18, 19],
  code: {
    name: 'CVV',
    size: 3
  }
});

Validate.creditCardType.changeOrder('alat', 0);

I should be getting back alat, instead of Maestro.

Thanks!

crookedneighbor commented 5 years ago

I wrote this test case:

it('can change the priority for a custom card', function () {
  var result;

  creditCardType.addCard({
    niceType: 'Custom',
    type: 'custom',
    patterns: [
      // 506 overlaps with maestro and elo
      // 507 overlaps with maestro
      [506, 507],
      // 650 overlaps with discover, maestro and elo
      [650002, 650007]
    ],
    gaps: [4, 8, 12, 16],
    lengths: [16, 18, 19],
    code: {
      name: 'CVV',
      size: 3
    }
  });

  creditCardType.changeOrder('custom', 0);

  result = creditCardType('506');

  expect(result).to.have.a.lengthOf(3);
  expect(result[0].type).to.equal('custom');
  expect(result[1].type).to.equal('maestro');
  expect(result[2].type).to.equal('elo');

  result = creditCardType('507');

  expect(result).to.have.a.lengthOf(2);
  expect(result[0].type).to.equal('custom');
  expect(result[1].type).to.equal('maestro');

  result = creditCardType('650');

  expect(result).to.have.a.lengthOf(4);
  expect(result[0].type).to.equal('custom');
  expect(result[1].type).to.equal('discover');
  expect(result[2].type).to.equal('maestro');
  expect(result[3].type).to.equal('elo');

  result = creditCardType('650004');

  expect(result).to.have.a.lengthOf(1);
  expect(result[0].type).to.equal('custom');
});

And these assertions are exactly what I expect. The custom card (which has the same attributes as your alat card) always comes up first when matching a card type.

Are you expecting something else to happen? If there's a bit of documentation that is confusing and leading you to believe something else should happen, can you point it out so we can fix it?

ooade commented 5 years ago

It's an issue from your parent library apparently (https://github.com/braintree/card-validator).

This issue still persists https://github.com/braintree/card-validator/issues/64.

Calling creditCardType('506'); works fine, as opposed to calling valid.number(value), which returns null when it matches 2 or more cards. It works for me tho since I can do the former.

crookedneighbor commented 5 years ago

This is expected behavior for card validator.

It will only mark something as valid when it determines that there is only one possible card type. Since 506 can match either maestro or your custom card, that's the expected outcome. Sounds like you need to make your custom card more specific for what ranges it supports (and possibly update the maestro card pattern as well)

ooade commented 5 years ago

Thanks :)