Skip to content

Deprecate CardScheme and Luhn validators in favor of a new Card validator #26436

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.

Sign up for GitHub

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

Jump to bottom
Closed
javiereguiluz opened this issue Mar 7, 2018 · 7 comments
Closed

Deprecate CardScheme and Luhn validators in favor of a new Card validator #26436

javiereguiluz opened this issue Mar 7, 2018 · 7 comments
Labels
Deprecation Feature Validator

Comments

@javiereguiluz
Copy link
Member

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? no
Symfony version 4.1

I don't use it often, but today I realized that our credit card validators could be a bit friendlier. In my opinion, they are overengineered (you need to use two different validators: scheme and luhn) and a bit pedantic (because they use the most technically correct term "luhn algorithm validator" to refer to something that everybody calls "credit card number validator").

I propose to deprecate CardScheme and Luhn validators and merge them into a new Card validator. The "card" term is more correct than "credit card" (and it's used by experts in the field like Stripe) because there are several types of cards, so CreditCard constraint would be confusing for "debit cards" for example.

Before

use Symfony\Component\Validator\Constraints\CardScheme;
use Symfony\Component\Validator\Constraints\Luhn;

class Transaction
{
    /**
     * @CardScheme(
     *     schemes={"VISA"},
     *     message="Only VISA cards are accepted."
     * )
     * @Luhn(message="The credit card number is wrong.")
     */
    protected $cardNumber;
}

After

use Symfony\Component\Validator\Constraints\Card;

class Transaction
{
    /**
     * @Card(
     *     schemes={"VISA"},
     *     schemeMessage="Only VISA cards are accepted.",
     *     message="The credit card number is wrong."
     * )
     */
    protected $cardNumber;
}

The @Card constraint would have these options:

  • schemes, same as CardScheme
  • schemeMessage, equivalent to message from CardScheme
  • message, same as Luhn
  • payload, same as CardScheme and Luhn

By the way, in case this proposal is accepted and we want to fix other things, Wikipedia says that our "schemes" option is called "issuers" (https://en.wikipedia.org/wiki/Payment_card_number) and Stripe calls it "brands" (https://stripe.com/docs/testing#cards).
@Pierstoval
Copy link
Contributor

Pierstoval commented Mar 7, 2018 •
edited
Loading

I totally agree. You just made me learn the reason why the Luhn algorithm is used, so I think the constraints can be merged 👍

@javiereguiluz
Copy link
Member Author

On Slack some people expressed some concerns:

  1. First, Luhn is used to validate more things that the credit card numbers: IMEIs, social security numbers, etc.
  2. Other propose to not deprecate the existing validators but instead: "Wouldn't it make more sense to keep Luhn and CardScheme as they are now and have a Card that uses both of them"
  3. Another user disagrees: "Can't see a need for a "new" validator which is basically a shortcut for two other validators. Not even saving typing time."

@runawaycoin
Copy link

Agree this would be good but the use case for this at all might be quite low as (eg for Stripe and should be others) card numbers should never be sent to the server rather be toekenized by the front end and Stripe.

@ostrolucky
Copy link
Contributor

I don't see separation of these as a problem. Problem you want to solve is just discoverability, since only few people know that Luhn is used for credit cards, but current name is more correct and you will find this validator by simple google search regardless. Therefore I'm not in favour deprecation.

@Gabb1995
Copy link

Gabb1995 commented Mar 7, 2018

Maybe the card scheme validator should have Luhn Validation inside of it? Does it make sense for that when validating a card scheme you can enter a wrong credit card number?

@fabpot
Copy link
Member

fabpot commented Mar 22, 2018

My 2cts:

  • I would not deprecate the Luhn validator (as already mentioned, it's not limited to credit cards, and hopefully, most people are using for something else than cards validation)
  • I would not deprecate CardScheme either
  • Now, for a new Card validator. Is it really needed? Do we really want to make it easier for people to accept cards without an integration with a third-party service?

@fabpot fabpot closed this as completed Mar 22, 2018
@fabpot fabpot reopened this Mar 22, 2018
@javiereguiluz
Copy link
Member Author

I didn't think about that, but reading Fabien's comments now makes a lot of sense to me: we shouldn't make it easier to validate credit card numbers. Real apps should use services like Stripe to process payments and not process, validate or do anything else with credit cards themselves. Let's close.

@stof stof mentioned this issue Sep 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Deprecation Feature Validator
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @javiereguiluz @ostrolucky @Pierstoval @Gabb1995 @runawaycoin