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

Updated Discover to have acceptCoBranded flag for the range "622126-622925" #83

Merged
merged 5 commits into from
Oct 20, 2021

Conversation

umair-fayaz
Copy link
Contributor

Fixes #80

@umair-fayaz
Copy link
Contributor Author

@aloisdg All the tests pass in my local. Can you help me here?

@aloisdg
Copy link
Member

aloisdg commented Oct 19, 2021

It passed on Ubuntu but not MacOs

image

@aloisdg
Copy link
Member

aloisdg commented Oct 20, 2021

Alright, the code was not testing standard Discover card in co-branded case.

Here is the generator:

static member NextDiscover([<Optional; DefaultParameterValue(From16To19.Random)>] discoverLengthOption, [<Optional; DefaultParameterValue(true)>] acceptCoBranded: bool) =
        let length =
            match discoverLengthOption with
            | From16To19.Random -> Cardizer.NextInRange 16 19
            | _ -> int discoverLengthOption

        let roll = Cardizer.next (if acceptCoBranded then 4 else 3)
        let prefix =
            match roll with
            | 0 -> [ 6; 0; 1; 1 ]
            | 1 -> [ 6; 5 ]
            | 2 -> Cardizer.NextSeqInRange 644 649
            | _ -> Cardizer.NextSeqInRange 622126 622925
        
        Cardizer.GenerateCard prefix length

and here is the testcases:

[<Theory>]
[<InlineData(From16To19.Sixteen, 16)>]
[<InlineData(From16To19.Seventeen, 17)>]
[<InlineData(From16To19.Eighteen, 18)>]
[<InlineData(From16To19.Nineteen, 19)>]
let ``Should generate valid Discover`` length expectedLength =
   let cardNotCobranded = Cardizer.NextDiscover(length, false)
   let cardCobranded = Cardizer.NextDiscover(length, true)

   let isPrefixValid (card: string) =
       if card.StartsWith "65" || card.StartsWith "6011"
           then true
       else
           let head = card.Substring(0, 3) |> int
           (head >= 644 && head <= 649)

   let headCobranded = cardCobranded.Substring(0, 6) |> int
   let prefixCobranded = isPrefixValid cardCobranded || (headCobranded >=  622126 && headCobranded <= 622925)
   let prefixNotCobranded = isPrefixValid cardNotCobranded 
       
   prefixCobranded |> should be True
   prefixNotCobranded |> should be True
   cardNotCobranded |> should haveLength expectedLength
   cardNotCobranded |> luhn |> should be LuhnCheck
   cardCobranded |> should haveLength expectedLength
   cardCobranded |> luhn |> should be LuhnCheck

@aloisdg aloisdg added bug Something isn't working hacktoberfest hacktoberfest-accepted A pull request is considered approved once it has an overall approving review from maintainers, or h labels Oct 20, 2021
@umair-fayaz
Copy link
Contributor Author

Alright, the code was not testing standard Discover card in co-branded case.

Here is the generator:

static member NextDiscover([<Optional; DefaultParameterValue(From16To19.Random)>] discoverLengthOption, [<Optional; DefaultParameterValue(true)>] acceptCoBranded: bool) =
        let length =
            match discoverLengthOption with
            | From16To19.Random -> Cardizer.NextInRange 16 19
            | _ -> int discoverLengthOption

        let roll = Cardizer.next (if acceptCoBranded then 4 else 3)
        let prefix =
            match roll with
            | 0 -> [ 6; 0; 1; 1 ]
            | 1 -> [ 6; 5 ]
            | 2 -> Cardizer.NextSeqInRange 644 649
            | _ -> Cardizer.NextSeqInRange 622126 622925
        
        Cardizer.GenerateCard prefix length

and here is the testcases:

[<Theory>]
[<InlineData(From16To19.Sixteen, 16)>]
[<InlineData(From16To19.Seventeen, 17)>]
[<InlineData(From16To19.Eighteen, 18)>]
[<InlineData(From16To19.Nineteen, 19)>]
let ``Should generate valid Discover`` length expectedLength =
   let cardNotCobranded = Cardizer.NextDiscover(length, false)
   let cardCobranded = Cardizer.NextDiscover(length, true)

   let isPrefixValid (card: string) =
       if card.StartsWith "65" || card.StartsWith "6011"
           then true
       else
           let head = card.Substring(0, 3) |> int
           (head >= 644 && head <= 649)

   let headCobranded = cardCobranded.Substring(0, 6) |> int
   let prefixCobranded = isPrefixValid cardCobranded || (headCobranded >=  622126 && headCobranded <= 622925)
   let prefixNotCobranded = isPrefixValid cardNotCobranded 
       
   prefixCobranded |> should be True
   prefixNotCobranded |> should be True
   cardNotCobranded |> should haveLength expectedLength
   cardNotCobranded |> luhn |> should be LuhnCheck
   cardCobranded |> should haveLength expectedLength
   cardCobranded |> luhn |> should be LuhnCheck

Thanks :)

@aloisdg aloisdg mentioned this pull request Oct 20, 2021
Copy link
Member

@aloisdg aloisdg left a comment

Choose a reason for hiding this comment

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

Good to go

@aloisdg aloisdg merged commit 2d55b53 into d-edge:main Oct 20, 2021
@aloisdg aloisdg added this to the v1 milestone Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hacktoberfest hacktoberfest-accepted A pull request is considered approved once it has an overall approving review from maintainers, or h
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discover should have an acceptCoBranded flag for the range "622126-622925".
4 participants