-
Notifications
You must be signed in to change notification settings - Fork 13
Fix/optional activity id conversation resource #272
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: main
Are you sure you want to change the base?
Fix/optional activity id conversation resource #272
Conversation
The activity_id field in ConversationResource is now optional to match the actual API behavior. When creating a conversation without an initial activity, the Teams API does not include activityId in the response, which was causing validation errors. Changes: - Made activity_id Optional[str] with default None in ConversationResource - Updated mock transport to conditionally include activityId based on request body - Added test case for creating conversation without activity Fixes validation error when creating conversations without activities.
|
@microsoft-github-policy-service agree |
lilyydu
left a comment
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.
Thank you for catching and filing this! Just a comment about splitting the test cases :)
| assert response.service_url is not None | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_conversation_resource_optional_fields(self): |
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 split these into 4 different test cases
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.
sure, made the changes now :)
lilyydu
left a comment
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.
Thank you for fixing!
| assert response.activity_id is None | ||
| assert response.service_url is not None | ||
|
|
||
| async def test_conversation_resource_with_all_fields(self): |
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.
tests seem to be failing b/c these methods are missing @pytest.mark.asyncio
Description
Fixes a validation error in
ConversationResourcewhen creating conversations without an initial activity.Problem
The
activity_idandservice_urlfield was required in theConversationResourcemodel but seems they are not returned from the api and thus we keep getting error:Seems
teams.netalready has it adapted - hereSolution
activity_idandservice_urloptional (Optional[str] = None) inConversationResourceactivityIdbased on request contentTesting
test_create_conversation_without_activityto verify the fixtest_create_conversationstill validates the activity scenario