(For Jintae or Gary) Add session level decision. #55
Conversation
|
Mohammed, can you also update Note: https://siftscience.com/developers/docs/python/decisions-api/decisions-list |
sift/client.py
Outdated
| user_id: id of user | ||
| session_id: id of session | ||
| properties: | ||
| decision_id: decision to apply to user |
There was a problem hiding this comment.
I guess decision_id here is "decision to apply to session" (and that the same comment in apply_order_decision should be changed accordingly.)
For example, in the unit test, decision_id used was session_looks_bad_ato.
| @@ -522,6 +528,42 @@ def test_apply_decision_to_order_ok(self): | |||
| assert(response.http_status_code == 200) | |||
| assert(response.body['entity']['type'] == 'order') | |||
|
|
|||
| def test_apply_decision_to_session_ok(self): | |||
There was a problem hiding this comment.
Just a quick question: no need to test failed cases?
There was a problem hiding this comment.
Seems that test_apply_decision_to_session does test with no session_id. I think it's enough to change the name of test_apply_decision_to_session.
tests/test_client.py
Outdated
| '{' \ | ||
| '"data": [' \ | ||
| '{' \ | ||
| '"id": "block_user",' \ |
There was a problem hiding this comment.
well, it's a very minor nit, but id, name, and description do not match the entity type, though it isn't really related to what the test case is supposed to test.
tests/test_client.py
Outdated
| '"name" : "Block user",' \ | ||
| '"description": "user has a different billing and shipping addresses",' \ | ||
| '"entity_type": "session",' \ | ||
| '"abuse_type": "legacy",' \ |
There was a problem hiding this comment.
Also, we only have session decision associated with account_takeover today. Its logic may change in the future, and it isn't really related to the test case, either, so you may leave it as is now. It just doesn't look real.
There was a problem hiding this comment.
agree that it would be nice to change this to account_takeover to reflect a correct integration pattern
|
please also update CHANGES.md and version.py |
|
LGTM |
Purpose:
Technical overview:
Testing Plan
Deployment