Bugfix FXIOS-14575 [Autofill] Credit Card doesn't validate in its last (=expiration) month #31558
Conversation
PARAIPAN9
left a comment
PARAIPAN9
left a comment
There was a problem hiding this comment.
Looks Good. Let's see if Bitrise also agrees.
💪 Quality guardian1 tests files modified. You're a champion of test coverage! 🚀 🧹 Tidy commitJust 2 file(s) touched. Thanks for keeping it clean and review-friendly! 💬 Description craftsmanGreat PR description! Reviewers salute you 🫡 ✅ Per-file coverageAll changed files meet the threshold of 35.0%. Client.app: Coverage: 37.28
Generated by 🚫 Danger Swift against 1153e17 |
| return cardType.validNumberLength.contains(String(card).count) | ||
| } | ||
|
|
||
| func isExpirationValidFor(date: String) -> Bool { |
There was a problem hiding this comment.
I think we should see some tests added in CreditCardValidatorTests ? This logic should be tested
There was a problem hiding this comment.
There are some hard-coded ones — would you want me to add one that dynamically tries "this month", too? (Perhaps a few more — a month before this, this one, and the month after I can imagine…)
There was a problem hiding this comment.
Yeah a couple of use cases like you said would be perfect!
There was a problem hiding this comment.
Would 1153e17 be alright for a test, or it should be more smart, or be mocking some logic?
(I'm never comfortable around testing live values as a form of "moving target" inside tests;D)
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
janbrasna
force-pushed
the
fix/autofill-cc-exp
branch
from
ee3117e to
1153e17
Compare
|
🚀 PR merged to |
4 participants
📜 Tickets
Jira ticket FXIOS-14575
Github issue #31538
💡 Description
Adding or editing card ending in January should be possible for the whole of January — the card only expires when January ends.
(The first commit used a range to construct the calendar components to represent the validity up to the last second to serve the date comparison, but that turned out too verbose and excessive. The second commit refactored the logic to just number comparison for simplicity and better understandability — while not as cool it seems to actually fit the logic way better, even when boring looking.)
It also simplifies some conditions for comprehension, or removes superfluous ones.
🎥 Demos
📝 Checklist