Skip to content
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

Conversion Bot Message Reaction To Python #1481

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Mohammed-MSFT
Copy link
Contributor

No description provided.

@Pawank-MSFT Pawank-MSFT requested a review from Copilot January 3, 2025 10:09
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 19 out of 33 changed files in this pull request and generated 3 comments.

Files not reviewed (14)
  • samples/bot-message-reaction/python/.gitignore: Language not supported
  • samples/bot-message-reaction/python/.vscode/extensions.json: Language not supported
  • samples/bot-message-reaction/python/.vscode/launch.json: Language not supported
  • samples/bot-message-reaction/python/.vscode/settings.json: Language not supported
  • samples/bot-message-reaction/python/.vscode/tasks.json: Language not supported
  • samples/bot-message-reaction/python/appManifest/manifest.json: Language not supported
  • samples/bot-message-reaction/python/assets/sample.json: Language not supported
  • samples/bot-message-reaction/python/env/.env.local: Language not supported
  • samples/bot-message-reaction/python/infra/azure.bicep: Language not supported
  • samples/bot-message-reaction/python/infra/azure.parameters.json: Language not supported
  • samples/bot-message-reaction/python/requirements.txt: Language not supported
  • samples/bot-message-reaction/python/config.py: Evaluated as low risk
  • samples/bot-message-reaction/python/teamsapp.local.yml: Evaluated as low risk
  • samples/bot-message-reaction/python/teamsapp.yml: Evaluated as low risk
Comments suppressed due to low confidence (5)

samples/bot-message-reaction/python/bots/messageReactionBot.py:47

  • [nitpick] The error message 'Activity {turn_context.activity.reply_to_id} not found in the log.' could be more descriptive. Consider changing it to 'The activity with ID {turn_context.activity.reply_to_id} was not found in the activity log.'
f"Activity {turn_context.activity.reply_to_id} not found in the log."

samples/bot-message-reaction/python/bots/messageReactionBot.py:69

  • [nitpick] The error message 'Activity {turn_context.activity.reply_to_id} not found in the log.' could be more descriptive. Consider changing it to 'The activity with ID {turn_context.activity.reply_to_id} was not found in the activity log.'
f"Activity {turn_context.activity.reply_to_id} not found in the log."

samples/bot-message-reaction/python/activity_log.py:13

  • [nitpick] The error message could be clearer. Consider changing it to 'The activity_id parameter is required for the append method in ActivityLog.'
raise TypeError('activity_id is required for ActivityLog.append')

samples/bot-message-reaction/python/activity_log.py:17

  • [nitpick] The error message could be clearer. Consider changing it to 'The activity parameter is required for the append method in ActivityLog.'
raise TypeError('activity is required for ActivityLog.append')

samples/bot-message-reaction/python/activity_log.py:29

  • [nitpick] The error message could be clearer. Consider changing it to 'The activity_id parameter is required for the find method in ActivityLog.'
raise TypeError('activity_id is required for ActivityLog.find')

- Python
extensions:
contentType: samples
createdDate: "12-12-2024 17:00:25"
Copy link
Preview

Copilot AI Jan 3, 2025

Choose a reason for hiding this comment

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

The date format should be corrected to "2024-12-12T17:00:25Z".

Suggested change
createdDate: "12-12-2024 17:00:25"
createdDate: "2024-12-12T17:00:25Z"

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Contributor Author

Choose a reason for hiding this comment

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

please check other samples readme we were following the same like this
12-12-2024 17:00:25

![Message Reaction](Images/MessageReactions.gif)

## Try it yourself - experience the App in your Microsoft Teams client
Please find below demo manifest which is deployed on Microsoft Azure and you can try it yourself by uploading the app package (.zip file link below) to your teams and/or as a personal app. (Sideloading must be enabled for your tenant, [see steps here](https://docs.microsoft.com/microsoftteams/platform/concepts/build-and-test/prepare-your-o365-tenant#enable-custom-teams-apps-and-turn-on-custom-app-uploading)).
Copy link
Preview

Copilot AI Jan 3, 2025

Choose a reason for hiding this comment

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

The phrase 'to your teams' should be corrected to 'to your Teams'.

Suggested change
Please find below demo manifest which is deployed on Microsoft Azure and you can try it yourself by uploading the app package (.zip file link below) to your teams and/or as a personal app. (Sideloading must be enabled for your tenant, [see steps here](https://docs.microsoft.com/microsoftteams/platform/concepts/build-and-test/prepare-your-o365-tenant#enable-custom-teams-apps-and-turn-on-custom-app-uploading)).
Please find below demo manifest which is deployed on Microsoft Azure and you can try it yourself by uploading the app package (.zip file link below) to your Teams and/or as a personal app. (Sideloading must be enabled for your tenant, [see steps here](https://docs.microsoft.com/microsoftteams/platform/concepts/build-and-test/prepare-your-o365-tenant#enable-custom-teams-apps-and-turn-on-custom-app-uploading)).

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


In the new Azure Bot resource in the Portal,
- Ensure that you've [enabled the Teams Channel](https://learn.microsoft.com/azure/bot-service/channel-connect-teams?view=azure-bot-service-4.0)
- In Settings/Configuration/Messaging endpoint, enter the current `https` URL you were given by running the tunneling application. Append with the path `/api/messages`
Copy link
Preview

Copilot AI Jan 3, 2025

Choose a reason for hiding this comment

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

The phrase 'Append with the path' should be corrected to 'Append it with the path'.

Suggested change
- In Settings/Configuration/Messaging endpoint, enter the current `https` URL you were given by running the tunneling application. Append with the path `/api/messages`
- In Settings/Configuration/Messaging endpoint, enter the current `https` URL you were given by running the tunneling application. Append it with the path `/api/messages`

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Collaborator

@Pawank-MSFT Pawank-MSFT left a comment

Choose a reason for hiding this comment

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

please fix the PR comments

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove "Python" from app name. please take same app name which we have took for c# or nodejs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

"outline": "outline.png"
},
"name": {
"short": "BotMessageReaction Python",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change app name refer nodejs and c#

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 19 out of 33 changed files in this pull request and generated 2 comments.

Files not reviewed (14)
  • samples/bot-message-reaction/python/.gitignore: Language not supported
  • samples/bot-message-reaction/python/.vscode/extensions.json: Language not supported
  • samples/bot-message-reaction/python/.vscode/launch.json: Language not supported
  • samples/bot-message-reaction/python/.vscode/settings.json: Language not supported
  • samples/bot-message-reaction/python/.vscode/tasks.json: Language not supported
  • samples/bot-message-reaction/python/appManifest/manifest.json: Language not supported
  • samples/bot-message-reaction/python/assets/sample.json: Language not supported
  • samples/bot-message-reaction/python/env/.env.local: Language not supported
  • samples/bot-message-reaction/python/infra/azure.bicep: Language not supported
  • samples/bot-message-reaction/python/infra/azure.parameters.json: Language not supported
  • samples/bot-message-reaction/python/requirements.txt: Language not supported
  • samples/bot-message-reaction/python/teamsapp.yml: Evaluated as low risk
  • samples/bot-message-reaction/python/bots/init.py: Evaluated as low risk
  • samples/bot-message-reaction/python/config.py: Evaluated as low risk
Comments suppressed due to low confidence (6)

samples/bot-message-reaction/python/activity_log.py:13

  • The error message could be more descriptive. Consider changing it to 'activity_id is required for append method in ActivityLog'.
raise TypeError('activity_id is required for ActivityLog.append')

samples/bot-message-reaction/python/activity_log.py:17

  • The error message could be more descriptive. Consider changing it to 'activity is required for append method in ActivityLog'.
raise TypeError('activity is required for ActivityLog.append')

samples/bot-message-reaction/python/activity_log.py:29

  • The error message could be more descriptive. Consider changing it to 'activity_id is required for find method in ActivityLog'.
raise TypeError('activity_id is required for ActivityLog.find')

samples/bot-message-reaction/python/activity_log.py:20

  • [nitpick] The variable name 'obj' is ambiguous. It should be renamed to 'activity_data'.
obj = {activity_id: {"activity": activity}}

samples/bot-message-reaction/python/app.py:19

  • [nitpick] The import statements should be alphabetically ordered for better readability and maintainability.
from bots import MessageReactionBot

samples/bot-message-reaction/python/app.py:62

  • [nitpick] The APP_ID generation logic could be simplified by using a ternary operator.
APP_ID = SETTINGS.app_id if SETTINGS.app_id else uuid.uuid4()

- Python
extensions:
contentType: samples
createdDate: "12-12-2024 17:00:25"
Copy link
Preview

Copilot AI Jan 8, 2025

Choose a reason for hiding this comment

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

The date format in the createdDate field is incorrect. It should be in the format "YYYY-MM-DD".

Suggested change
createdDate: "12-12-2024 17:00:25"
createdDate: "2024-12-12"

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

## Try it yourself - experience the App in your Microsoft Teams client
Please find below demo manifest which is deployed on Microsoft Azure and you can try it yourself by uploading the app package (.zip file link below) to your Teams and/or as a personal app. (Sideloading must be enabled for your tenant, [see steps here](https://docs.microsoft.com/microsoftteams/platform/concepts/build-and-test/prepare-your-o365-tenant#enable-custom-teams-apps-and-turn-on-custom-app-uploading)).

**Teams Message Reactions Bot:** [Manifest](/samples/bot-message-reaction/csharp/demo-manifest/bot-message-reaction.zip)
Copy link
Preview

Copilot AI Jan 8, 2025

Choose a reason for hiding this comment

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

The link to the manifest file points to the C# version instead of the Python version. It should be corrected to avoid confusion.

Suggested change
**Teams Message Reactions Bot:** [Manifest](/samples/bot-message-reaction/csharp/demo-manifest/bot-message-reaction.zip)
**Teams Message Reactions Bot:** [Manifest](/samples/bot-message-reaction/python/demo-manifest/bot-message-reaction.zip)

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Contributor Author

@Mohammed-MSFT Mohammed-MSFT Jan 8, 2025

Choose a reason for hiding this comment

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

We have demo manifest in chsarp so we added in nodejs same like this and also following same in python.

Choose a reason for hiding this comment

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

Copilot reviewed 19 out of 33 changed files in this pull request and generated no comments.

Files not reviewed (14)
  • samples/bot-message-reaction/python/.gitignore: Language not supported
  • samples/bot-message-reaction/python/.vscode/extensions.json: Language not supported
  • samples/bot-message-reaction/python/.vscode/launch.json: Language not supported
  • samples/bot-message-reaction/python/.vscode/settings.json: Language not supported
  • samples/bot-message-reaction/python/.vscode/tasks.json: Language not supported
  • samples/bot-message-reaction/python/appManifest/manifest.json: Language not supported
  • samples/bot-message-reaction/python/assets/sample.json: Language not supported
  • samples/bot-message-reaction/python/env/.env.local: Language not supported
  • samples/bot-message-reaction/python/infra/azure.bicep: Language not supported
  • samples/bot-message-reaction/python/infra/azure.parameters.json: Language not supported
  • samples/bot-message-reaction/python/requirements.txt: Language not supported
  • samples/bot-message-reaction/python/bots/init.py: Evaluated as low risk
  • samples/bot-message-reaction/python/app.py: Evaluated as low risk
  • samples/bot-message-reaction/python/config.py: Evaluated as low risk
Comments suppressed due to low confidence (2)

samples/bot-message-reaction/python/activity_log.py:10

  • Ensure that the append method is covered by tests, including cases where activity_id or activity is None.
async def append(self, activity_id, activity):

samples/bot-message-reaction/python/activity_log.py:26

  • Ensure that the find method is covered by tests, including cases where activity_id is None and where the activity is not found.
async def find(self, activity_id):
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