Skip to content

(For Mohammed) Add get session decisions#59

Merged
mlegore merged 4 commits intomasterfrom
get_session_decisions
Jun 4, 2018
Merged

(For Mohammed) Add get session decisions#59
mlegore merged 4 commits intomasterfrom
get_session_decisions

Conversation

@mlegore
Copy link
Copy Markdown
Contributor

@mlegore mlegore commented Jun 1, 2018

Purpose:

  • Adds support for get session level decisions.

Technical overview:

Testing Plan

  • Expanded and added unit tests and run them locally.

Deployment

  • Publish gems
  • Github release

@mlegore mlegore requested review from garylee1 and mjouahri June 1, 2018 18:38
@mjouahri
Copy link
Copy Markdown
Contributor

mjouahri commented Jun 4, 2018

Can you fill up the description section please. Taking a look at the code now.

Comment thread HISTORY Outdated
@@ -1,3 +1,6 @@
=== 3.1.0 2018-06-04
- Adds support for sessions decisions to [Decisions API](https://siftscience.com/developers/docs/curl/decisions-api)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe "get session decisions" is better wording. Depends are we returning all the decisions associated with a session or just the latest ?
Also do we have support for batch get sessions or just get session ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's just get session, not batched

Comment thread lib/sift/client.rb Outdated
# A Hash of optional parameters for this request --
#
# :account_id::
# Overrides the API key for this call.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you update this to account id

Copy link
Copy Markdown
Contributor

@mjouahri mjouahri left a comment

Choose a reason for hiding this comment

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

LGTM! Please update the description before you merge.

@mlegore mlegore merged commit 463d7ed into master Jun 4, 2018
@garylee1 garylee1 deleted the get_session_decisions branch May 15, 2019 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants