-
Notifications
You must be signed in to change notification settings - Fork 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
Add further workflow run support - create, find, and all #47
Add further workflow run support - create, find, and all #47
Conversation
"created_at_gt": ["2023-11-17T16:39:04Z"], | ||
"created_at_lt": ["2008-02-29T02:56:37Z"], | ||
"sort": ['asc'] | ||
} |
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.
There are other tests simply compare the fully-formed url with query parameters, such as:
assert history[0].url == "https://api.eu.onfido.com/v3.6/applicants?include_deleted=True&per_page=5&page=2"
in test_applicants.py
> test_list_applicants
.
However, this is a slightly different case because there are more params, and there are url-escaped values such as commas in properties like tags
and status
:
tags (optional): a list of comma separated tags to filter the results.
(docs)
Comparing the parsed url query params as a dict might be more readable and maintainable than something like:
assert history[0].url == "https://api.eu.onfido.com/v3.6/workflow_runs?page=2&sort=asc&status=approved%2Cdeclined&tags=dummy_tag1%2Cdummy_tag2&created_at_gt=2023-11-17T16%3A39%3A04Z&created_at_lt=2008-02-29T02%3A56%3A37Z
6d2db52
to
e80eb95
Compare
onfido/resources/workflow_runs.py
Outdated
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.
based on the existing onfido/resources/applicants.py
@DavidMealha hi David, was wondering if you would be able to review my PR |
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.
It looks good to me, thanks for your contribution.
Hey @peggalex, the changes look good but CI is failing, can you please fix it |
hey @DavidMealha, I was using |
While there exists a
WorkflowRuns
resource for Onfido Studio support, it only has the methodevidence()
. Some other actions, such as create, list, and retrieve are not yet added to this SDK. This PR adds those actions in.I performed validation using the following code, although depending on your Onfido settings you may have different required applicant fields and other configuration differences. There are also new unit tests for the added methods.