-
Notifications
You must be signed in to change notification settings - Fork 17
Sdk 2242 digital identity share v2 #415
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
base: development
Are you sure you want to change the base?
Conversation
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.
To avoid the need for a JSON session config while creating Share v2 session, we need to implement additional builders for Share v2 - Policy, Extensions, Constraints, Wanted.
This is in line with Other SDKs that have added Digital Identity (Share v2) support. Please refer to them.
</main> | ||
|
||
<script src="https://www.yoti.com/share/client/"></script> | ||
<script src="https://www.public.stg1.dmz.yoti.com/share/client"></script> |
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 change back to the Prod URL.
</section> | ||
</main> | ||
<script src="https://www.yoti.com/share/client/"></script> | ||
<script src="https://connect.public.stg1.dmz.yoti.com/share/client/"></script> |
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 change back to the Prod URL.
|
||
|
||
class DynamicScenarioBuilder(object): | ||
print("Hello, World!") |
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.
Remove the print statement.
|
||
if profile_attributes: | ||
for field in profile_attributes: | ||
# print("field %s" % field) |
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.
Remove the commented print statement.
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 remove this key.
"subject_id": "some_subject_id_string" | ||
}, # Optional reference to a user ID | ||
"notification": { | ||
"url": "https://webhook.site/818dc66b-e18b-4767-92c5-47c7af21629c", |
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.
Might be better to change URL to https://example.com/webhook-url
.
domId: 'webshare-target', | ||
sdkId: '99525f08-afbb-4420-98b9-0717309f7fed', | ||
skinId: 'yoti', | ||
flow: 'INLINE_QR', |
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.
INLINE_QR
is a legacy value. Please change that to INSTANT
.
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.
Pull Request Overview
This PR implements a new Digital Identity Share v2 functionality for the Yoti Python SDK, adding support for creating and managing share sessions, QR codes, and receipts with encryption handling.
- Adds new digital identity client with methods for session and QR code management
- Implements receipt decryption and content parsing functionality
- Updates activity details to use new identifier properties
Reviewed Changes
Copilot reviewed 44 out of 66 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
yoti_python_sdk/digital_identity/ | New module containing client and result classes for share v2 functionality |
yoti_python_sdk/tests/digital_identity/ | Test files for the new digital identity functionality |
yoti_python_sdk/tests/test_client.py | Updated to use new user_identifier and receipt_identifier properties |
examples/digitalidentity/ | Complete Flask example application demonstrating the new functionality |
setup.py | Updated protobuf and coverage versions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
|
||
class DynamicScenarioBuilder(object): | ||
print("Hello, World!") |
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.
Debug print statement should be removed from production code as it will execute when the class is defined.
print("Hello, World!") |
Copilot uses AI. Check for mistakes.
|
||
if profile_attributes: | ||
for field in profile_attributes: | ||
# print("field %s" % field) |
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.
Commented-out debug print statement should be removed from production code.
# print("field %s" % field) |
Copilot uses AI. Check for mistakes.
assert len(result.id_document_text_data_checks) == 1 | ||
assert isinstance(result.id_document_text_data_checks[0], TextDataCheckResponse) |
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.
Duplicate assertion lines 105-106 should be removed as they are identical to lines 102-103.
assert len(result.id_document_text_data_checks) == 1 | |
assert isinstance(result.id_document_text_data_checks[0], TextDataCheckResponse) |
Copilot uses AI. Check for mistakes.
@property | ||
@deprecated | ||
def user_id(self): | ||
return self.__remember_me_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.
Duplicate property definition for 'user_id' will cause the second definition to override the first.
@property | |
@deprecated | |
def user_id(self): | |
return self.__remember_me_id |
Copilot uses AI. Check for mistakes.
@user_id.setter | ||
@deprecated | ||
def user_id(self, value): | ||
def user_identifier(self, value): |
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.
The setter is named 'user_identifier' but should be 'user_id' to match the property it's setting.
def user_identifier(self, value): | |
def user_id(self, value): |
Copilot uses AI. Check for mistakes.
<table> | ||
{% for key, value in attribute.value.items() %} | ||
<tr> | ||
<td>bb{{ key }}<br/> |
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.
The 'bb' prefix appears to be debugging text that should be removed from the template.
<td>bb{{ key }}<br/> | |
<td>{{ key }}<br/> |
Copilot uses AI. Check for mistakes.
No description provided.