-
Notifications
You must be signed in to change notification settings - Fork 13
Add /device/session_age, /device/session_id and /email/first_seen #18
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
Conversation
| RFC 3339 date-time. | ||
| :type: str | None | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the response models. There are not models for the request. Instead we do validation on a dict:
minfraud-api-python/minfraud/validation.py
Line 230 in 9b15e69
| Required('device'): { |
This code should be removed.
tests/test_models.py
Outdated
| self.assertEqual(id, device.id) | ||
| self.assertEqual(last_seen, device.last_seen) | ||
| self.assertEqual(session_age, device.session_age) | ||
| self.assertEqual(session_id, device.session_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to be replaced with tests in test_validation.py. Also, add the new fields to https://github.com/maxmind/minfraud-api-python/tree/9b15e69971bd27b9a5dc4282df78de5147ab0504/tests/data.
bfcb950 to
78687a7
Compare
HISTORY.rst
Outdated
|
|
||
| * Added the following new values to the ``/payment/processor`` validation: | ||
| ``ebs``, ``hipay``, and ``lemon_way``. | ||
| * Added the following new values to the ``/device`` validation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is phrased a bit weird. Maybe we should at least say "input" values or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in d36a0bc
HISTORY.rst
Outdated
| ``ebs``, ``hipay``, and ``lemon_way``. | ||
| * Added the following new values to the ``/device`` validation: | ||
| ``session_age`` and ``session_id``. | ||
| * Added the following new values to the ``/email`` validation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems wrong. We added the attribute to the Email response model or, alternatively, we added support for the /email/first_seen output. We didn't do anything related to validation for this output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in d36a0bc
minfraud/validation.py
Outdated
| 'accept_language': _unicode_or_printable_ascii, | ||
| Required('ip_address'): _ip_address, | ||
| 'user_agent': _unicode_or_printable_ascii | ||
| 'session_age': float, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we make this Range(min=0)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 6e2bf16
tests/test_models.py
Outdated
| id = 'b643d445-18b2-4b9d-bad4-c9c4366e402a' | ||
| last_seen = '2016-06-08T14:16:38Z' | ||
| device = Device({'confidence': 99, 'id': id, 'last_seen': last_seen}) | ||
| device = Device({'confidence': 99, 'id': id, 'last_seen': last_seen }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please run the most recent version of yapf to tidy the code.
tests/test_validation.py
Outdated
|
|
||
| def test_session_age(self): | ||
| self.check_transaction({'device': {'ip_address': '4.4.4.4', 'session_age': 3600.5}}) | ||
| for invalid in (3600, 0, -1): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are 3600 and 0 invalid? Let's fix that so they aren't and maybe add a test for a non-numeric value too, e.g., 'foo'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess those weren't valid floats. Fixed in 6e2bf16
|
Oh, could we also add these to the example in the README.rst? |
|
Added device inputs to |
a7e1fbf to
bab7dd3
Compare
No description provided.