fixing new elo cards #75

lucianosousa · 2017-02-03T19:12:44Z

No description provided.

coveralls · 2017-02-03T19:18:26Z

Coverage remained the same at 98.101% when pulling fd556b3 on lucianosousa:new-elo-cards into 486329e on Fivell:master.

coveralls · 2017-02-06T22:53:47Z

Coverage decreased (-0.3%) to 97.819% when pulling 43b2193 on lucianosousa:new-elo-cards into 486329e on Fivell:master.

Fivell · 2017-02-07T00:36:46Z

@lucianosousa ,what are you trying to do ?

lucianosousa · 2017-02-07T12:24:40Z

@Fivell updating the rules for elo bins
they just changed a lot of stuffs about it
also, Discover card had a precedence on yml file that skips elo validations, that's why I move them to the bottom
this line basically conflict:
https://github.com/Fivell/credit_card_validations/pull/75/files#diff-f566cc47f10794c35671b2ed1173e266R360

Elo card now have a lot of bins started with this value.
What I did with the range was also Elo suggestion. They now have a range of bin values
I'm still working on it here today. Hope finish this issue soon.

Any suggestion about the code?

Fivell · 2017-02-07T12:31:46Z

@lucianosousa , you're trying to describe regexp in yaml file for bin, like

- '6500(3[5-9]|4[0-9]|5[0-1])'

I think it can not be supported out of the box, regexp is builded by prefixes here 486329e

Fivell · 2017-02-07T12:36:15Z

@lucianosousa , also please take a look here https://github.com/Fivell/credit_card_validations/blob/master/lib/credit_card_validations/factory.rb#L23

This means that this gem should be able to generate valid elo number. Also think it expects no regexp in yaml file

Fivell · 2017-02-07T12:40:34Z

'6500(3[5-9]|4[0-9]|5[0-1])' this one can be described as

lucianosousa · 2017-02-07T12:41:51Z

@Fivell take a look on my last commit. I already updated these things, just put bin code on yml again

lucianosousa · 2017-02-07T12:42:20Z

43b2193

Fivell · 2017-02-07T12:42:28Z

looks like
650034, 650030 are DISCOVER now and all other 65003* should be detected as elo

Fivell · 2017-02-07T12:43:09Z

@lucianosousa , it will not work for generating elo numbers

lucianosousa · 2017-02-07T12:47:17Z

So, this is the file they gave us:

BINS RANGE ELO(V.20151028)				
(#)	PRODUCT	ACCEPTANCE   START RANGE FINAL RANGE
26	CRÉDITO	NACIONAL	431274	431274
27	CRÉDITO	NACIONAL	506717	506717
28	CRÉDITO	NACIONAL	506718	506718
29	CRÉDITO	NACIONAL	506733	506733
30	CRÉDITO	NACIONAL	506739	506739
31	CRÉDITO	NACIONAL	506741	506741
32	CRÉDITO	NACIONAL	506742	506742
33	CRÉDITO	NACIONAL	506743	506743
34	CRÉDITO	NACIONAL	506774	506774
35	CRÉDITO	NACIONAL	506776	506776
36	CRÉDITO	NACIONAL	506778	506778
37	CRÉDITO	NACIONAL	509004	509004
38	CRÉDITO	NACIONAL	509005	509005
39	CRÉDITO	NACIONAL	509006	509006
40	CRÉDITO	NACIONAL	509007	509007
41	CRÉDITO	NACIONAL	509008	509008
42	CRÉDITO	NACIONAL	509009	509009
43	CRÉDITO	NACIONAL	509013	509013
44	CRÉDITO	NACIONAL	509020	509020
45	CRÉDITO	NACIONAL	509021	509021
46	CRÉDITO	NACIONAL	509022	509022
47	CRÉDITO	NACIONAL	509023	509023
48	CRÉDITO	NACIONAL	509024	509024
49	CRÉDITO	NACIONAL	509025	509025
50	CRÉDITO	NACIONAL	509026	509026
51	CRÉDITO	NACIONAL	509027	509027
52	CRÉDITO	NACIONAL	509028	509028
53	CRÉDITO	NACIONAL	509029	509029
54	CRÉDITO	NACIONAL	509031	509031
55	CRÉDITO	NACIONAL	509033	509033
56	CRÉDITO	NACIONAL	509035	509035
57	CRÉDITO	NACIONAL	509036	509036
58	CRÉDITO	NACIONAL	509037	509037
59	CRÉDITO	NACIONAL	509038	509038
60	CRÉDITO	NACIONAL	509039	509039
61	CRÉDITO	NACIONAL	509040	509040
62	CRÉDITO	NACIONAL	509041	509041
63	CRÉDITO	NACIONAL	509042	509042
64	CRÉDITO	NACIONAL	509043	509043
65	CRÉDITO	NACIONAL	509044	509044
66	CRÉDITO	NACIONAL	509045	509045
67	CRÉDITO	NACIONAL	509046	509046
68	CRÉDITO	NACIONAL	509047	509047
69	CRÉDITO	NACIONAL	509048	509048
70	CRÉDITO	NACIONAL	509049	509049
71	CRÉDITO	NACIONAL	509050	509050
72	CRÉDITO	NACIONAL	509051	509051
73	CRÉDITO	NACIONAL	509052	509052
74	CRÉDITO	NACIONAL	509053	509053
75	CRÉDITO	NACIONAL	509064	509064
76	CRÉDITO	NACIONAL	509077	509077
77	CRÉDITO	NACIONAL	509078	509078
78	CRÉDITO	NACIONAL	509079	509079
79	CRÉDITO	NACIONAL	509080	509080
80	CRÉDITO	NACIONAL	636297	636297
94	MÚLTIPLO	NACIONAL	401178	401178
95	MÚLTIPLO	NACIONAL	401179	401179
96	MÚLTIPLO	NACIONAL	438935	438935
97	MÚLTIPLO	NACIONAL	457393	457393
98	MÚLTIPLO	NACIONAL	457631	457631
99	MÚLTIPLO	NACIONAL	457632	457632
100	MÚLTIPLO	NACIONAL	504175	504175
101	MÚLTIPLO	NACIONAL	506720	506720
104	MÚLTIPLO	NACIONAL	506721	506721
105	MÚLTIPLO	NACIONAL	506724	506724
106	MÚLTIPLO	NACIONAL	506725	506725
107	MÚLTIPLO	NACIONAL	506726	506726
108	MÚLTIPLO	NACIONAL	506727	506727
109	MÚLTIPLO	NACIONAL	506728	506728
110	MÚLTIPLO	NACIONAL	506729	506729
111	MÚLTIPLO	NACIONAL	506730	506730
112	MÚLTIPLO	NACIONAL	506731	506731
113	MÚLTIPLO	NACIONAL	506732	506732
114	MÚLTIPLO	NACIONAL	506740	506740
115	MÚLTIPLO	NACIONAL	506744	506744
116	MÚLTIPLO	NACIONAL	506745	506745
117	MÚLTIPLO	NACIONAL	506746	506746
118	MÚLTIPLO	NACIONAL	506747	506747
119	MÚLTIPLO	NACIONAL	506748	506748
120	MÚLTIPLO	NACIONAL	506753	506753
121	MÚLTIPLO	NACIONAL	506775	506775
122	MÚLTIPLO	NACIONAL	506777	506777
123	MÚLTIPLO	NACIONAL	509000	509000
124	MÚLTIPLO	NACIONAL	509001	509001
125	MÚLTIPLO	NACIONAL	509002	509002
126	MÚLTIPLO	NACIONAL	509066	509066
127	MÚLTIPLO	NACIONAL	509067	509067
128	MÚLTIPLO	NACIONAL	509068	509068
129	MÚLTIPLO	NACIONAL	509069	509069
130	MÚLTIPLO	NACIONAL	509072	509072
131	MÚLTIPLO	NACIONAL	509074	509074
132	MÚLTIPLO	NACIONAL	509076	509076
133	MÚLTIPLO	NACIONAL	509081	509081
134	MÚLTIPLO	NACIONAL	509082	509082
135	MÚLTIPLO	NACIONAL	509083	509083
136	MÚLTIPLO	NACIONAL	509085	509810
137	MÚLTIPLO	NACIONAL	636368	636368
185	CRÉDITO	NACIONAL E INTERNACIONAL	650901	650920
188	MÚLTIPLO	NACIONAL E INTERNACIONAL	650485	650538
189	MÚLTIPLO	NACIONAL E INTERNACIONAL	650541	650598
190	MÚLTIPLO	NACIONAL E INTERNACIONAL	650700	650718
191	MÚLTIPLO	NACIONAL E INTERNACIONAL	650720	650727
192	MÚLTIPLO	NACIONAL E INTERNACIONAL	651652	651679
193	MÚLTIPLO	NACIONAL E INTERNACIONAL	655000	655019
194	MÚLTIPLO	NACIONAL E INTERNACIONAL	655021	655058

Fivell · 2017-02-07T12:48:35Z

@lucianosousa , see my comment above with example #75 (comment)

lucianosousa · 2017-02-07T12:48:45Z

65003 is not an elo bin number anymore, isn't?
that's why I removed few old elo cards

Fivell · 2017-02-07T12:56:34Z

@lucianosousa , I have no information that 65003 bin was moved to another brand

Fivell · 2017-02-07T12:58:18Z

@lucianosousa , however there is a discussion https://gist.github.com/erikhenrique/5931368

Fivell · 2017-02-07T13:01:06Z

https://gist.github.com/erikhenrique/5931368#gistcomment-1663782

lucianosousa · 2017-02-07T13:43:51Z

thanks for the links. I'll take a look on it here

lucianosousa · 2017-02-07T14:51:53Z

@Fivell in my list, there are more than 900 bins in ranges, and I really think my list is the most recent one.
on these links you sent me, there are 200 in few ranges, 80 in one, 50 in other, 50 in another, etc.

do you think it's a good idea put all these values at brands.yml?
do you have a better idea about my range list approach?

Fivell · 2017-02-07T15:14:02Z

@lucianosousa , this is now only way to make work both validation and generation numbers.
in other case I don't know how to fix this.

lucianosousa · 2017-02-07T15:19:34Z

Number generation/validation is already working. the problem with generate numbers is that it will not create a number with the starts in the range, I got it and i'll try improve it

Fivell · 2017-02-07T15:21:55Z

@lucianosousa , please think about placing all bins to brands.yaml
I do belive there should be only one place to contain all bins information

lucianosousa · 2017-02-07T15:32:19Z

@Fivell even if there are plus than 1000 bins?

Fivell · 2017-02-07T15:34:47Z

@lucianosousa , yes, I think it is bad idea to place bins in both yaml file and ruby code.

coveralls · 2017-02-07T17:38:56Z

Coverage remained the same at 98.101% when pulling d75d51e on lucianosousa:new-elo-cards into 486329e on Fivell:master.

coveralls · 2017-02-07T17:44:20Z

Changes Unknown when pulling 67b44f9 on lucianosousa:new-elo-cards into ** on Fivell:master**.

coveralls · 2017-02-07T17:56:22Z

Changes Unknown when pulling 67b44f9 on lucianosousa:new-elo-cards into ** on Fivell:master**.

lucianosousa · 2017-02-07T17:59:22Z

@Fivell done.
hope it works :)

also, I can merge all commits only in one if you want

Fivell · 2017-02-07T19:30:30Z

@lucianosousa , thanks for PR, good job! please squash commits to make it possible to review

Fivell · 2017-02-07T19:40:39Z

Gemfile.lock

@@ -0,0 +1,56 @@
+PATH


please remove this file

Fivell · 2017-02-07T19:41:06Z

.gitignore

@@ -0,0 +1 @@
+coverage/


remove this from PR please

Fivell · 2017-02-07T19:41:54Z

lib/data/brands.yaml

    - '506699'
-    - '5067'
+    - '506700'


@lucianosousa , also I want to ask to make one more improvement might be done

in such cases

' - 506700'
- '506701'
- '506702'
- '506703'
- '506704'
- '506705'
- '506706'
- '506707'
- '506708'
- '506709'
you can just put 1 string

' - 50670'
that will include all the range 50670(0..9)

coveralls · 2017-02-07T23:13:50Z

Changes Unknown when pulling e2e9c7b on lucianosousa:new-elo-cards into ** on Fivell:master**.

coveralls · 2017-02-07T23:13:50Z

Changes Unknown when pulling e2e9c7b on lucianosousa:new-elo-cards into ** on Fivell:master**.

coveralls · 2017-02-07T23:25:06Z

Changes Unknown when pulling 4a81140 on lucianosousa:new-elo-cards into ** on Fivell:master**.

lucianosousa · 2017-02-08T00:27:29Z

done @Fivell

Fivell · 2017-02-08T07:24:33Z

@lucianosousa , please make optimisations to bins that you want to add. I haven't check all the list, but each bins range that ends in 0..9 can be short-cutted, see my comments above.

lucianosousa · 2017-02-08T12:56:15Z

@Fivell yes, find some data that could be improved

coveralls · 2017-02-08T13:01:50Z

Changes Unknown when pulling eadba17 on lucianosousa:new-elo-cards into ** on Fivell:master**.

Fivell · 2017-02-08T13:05:51Z

@lucianosousa , ping me when ready to merge 💳

lucianosousa · 2017-02-08T13:13:25Z

@Fivell sorry, it's done

Fivell · 2017-02-08T13:14:37Z

@lucianosousa what about '509000'-'509009'

Fivell · 2017-02-08T13:15:12Z

506730-506739 = 50673 etc

lucianosousa · 2017-02-08T13:30:56Z

@Fivell updated. done

coveralls · 2017-02-08T13:38:02Z

Changes Unknown when pulling 04fa4b6 on lucianosousa:new-elo-cards into ** on Fivell:master**.

Fivell · 2017-02-08T13:41:44Z

@lucianosousa , thanks

lucianosousa · 2017-02-08T13:43:35Z

cheers, bro

Fivell · 2017-02-08T07:13:50Z

lib/data/brands.yaml

+    - '506717'
+    - '506718'
+    - '506720'
+    - '506721'


506720
....
506729

can be replaced with single line 50672

Fivell · 2017-02-08T07:14:23Z

lib/data/brands.yaml

+    - '506727'
+    - '506728'
+    - '506729'
+    - '506730'


506730-506739 = 50673

Fivell · 2017-02-08T07:17:39Z

lib/data/brands.yaml

+    - '509006'
+    - '509007'
+    - '509008'
+    - '509009'


'509000'

'509001'

'509002'

'509004'

'509005'

'509006'

'509007'

'509008'

'509009'

can be replaced with single 50900

Fivell · 2017-02-08T07:19:48Z

lib/data/brands.yaml

    - '509040'
+    - '509041'


- '509040' + - '509041' - '509042' - '509042' - '509043' - '509043' + - '509044' - '509045' - '509045' - '509046' - '509046' - '509047' - '509047' - '509048' - '509048' - '509049' - '509049'

50904

Fivell · 2017-02-08T07:20:30Z

lib/data/brands.yaml

+    - '650046'
+    - '650047'
+    - '650048'
+    - '650049'


650040 ... 650049 => 65004

Fivell · 2017-02-08T07:21:03Z

lib/data/brands.yaml

+    - '650966'
+    - '650967'
+    - '650968'
+    - '650969'


650960-650969 => 65096

Fivell · 2017-02-08T07:21:28Z

lib/data/brands.yaml

+    - '650956'
+    - '650957'
+    - '650958'
+    - '650959'


650950 - 650959 => 65095

Fivell · 2017-02-08T13:56:24Z

3.4.0 🚀

lucianosousa changed the title ~~fixing new elo cards~~ [wip] fixing new elo cards Feb 7, 2017

Fivell mentioned this pull request Feb 7, 2017

Add new rules to elo #61

Closed

lucianosousa force-pushed the new-elo-cards branch from 6c1370b to 67b44f9 Compare February 7, 2017 17:38

lucianosousa changed the title ~~[wip] fixing new elo cards~~ fixing new elo cards Feb 7, 2017

Fivell self-requested a review February 7, 2017 19:39

Fivell requested changes Feb 7, 2017

View reviewed changes

lucianosousa force-pushed the new-elo-cards branch from e2e9c7b to 4a81140 Compare February 7, 2017 23:10

lucianosousa force-pushed the new-elo-cards branch from 4a81140 to eadba17 Compare February 8, 2017 12:55

new elo card bins

04fa4b6

lucianosousa force-pushed the new-elo-cards branch from eadba17 to 04fa4b6 Compare February 8, 2017 13:30

Fivell merged commit 516260e into didww:master Feb 8, 2017

lucianosousa deleted the new-elo-cards branch February 8, 2017 13:43

Fivell self-requested a review February 8, 2017 13:45

Fivell reviewed Feb 8, 2017

View reviewed changes

Fivell self-requested a review February 8, 2017 13:47

		@@ -0,0 +1 @@
		coverage/

fixing new elo cards #75

fixing new elo cards #75

Conversation

lucianosousa commented Feb 3, 2017

coveralls commented Feb 3, 2017 • edited Loading

coveralls commented Feb 6, 2017 • edited Loading

Fivell commented Feb 7, 2017

lucianosousa commented Feb 7, 2017 • edited Loading

Fivell commented Feb 7, 2017

Fivell commented Feb 7, 2017

Fivell commented Feb 7, 2017

lucianosousa commented Feb 7, 2017

lucianosousa commented Feb 7, 2017

Fivell commented Feb 7, 2017

Fivell commented Feb 7, 2017

lucianosousa commented Feb 7, 2017

Fivell commented Feb 7, 2017

lucianosousa commented Feb 7, 2017

Fivell commented Feb 7, 2017

Fivell commented Feb 7, 2017

Fivell commented Feb 7, 2017

lucianosousa commented Feb 7, 2017

lucianosousa commented Feb 7, 2017

Fivell commented Feb 7, 2017

lucianosousa commented Feb 7, 2017 • edited Loading

Fivell commented Feb 7, 2017

lucianosousa commented Feb 7, 2017

Fivell commented Feb 7, 2017

coveralls commented Feb 7, 2017 • edited Loading

coveralls commented Feb 7, 2017 • edited Loading

coveralls commented Feb 7, 2017 • edited Loading

lucianosousa commented Feb 7, 2017

Fivell commented Feb 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Feb 7, 2017 • edited Loading

coveralls commented Feb 7, 2017 • edited Loading

coveralls commented Feb 7, 2017 • edited Loading

lucianosousa commented Feb 8, 2017

Fivell commented Feb 8, 2017

lucianosousa commented Feb 8, 2017

coveralls commented Feb 8, 2017 • edited Loading

Fivell commented Feb 8, 2017

lucianosousa commented Feb 8, 2017

Fivell commented Feb 8, 2017

Fivell commented Feb 8, 2017

lucianosousa commented Feb 8, 2017 • edited Loading

coveralls commented Feb 8, 2017 • edited Loading

Fivell commented Feb 8, 2017

lucianosousa commented Feb 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fivell commented Feb 8, 2017

coveralls commented Feb 3, 2017 •

edited

Loading

coveralls commented Feb 6, 2017 •

edited

Loading

lucianosousa commented Feb 7, 2017 •

edited

Loading

lucianosousa commented Feb 7, 2017 •

edited

Loading

coveralls commented Feb 7, 2017 •

edited

Loading

coveralls commented Feb 7, 2017 •

edited

Loading

coveralls commented Feb 7, 2017 •

edited

Loading

coveralls commented Feb 7, 2017 •

edited

Loading

coveralls commented Feb 7, 2017 •

edited

Loading

coveralls commented Feb 7, 2017 •

edited

Loading

coveralls commented Feb 8, 2017 •

edited

Loading

lucianosousa commented Feb 8, 2017 •

edited

Loading

coveralls commented Feb 8, 2017 •

edited

Loading