Skip to content

Add on_teams_team_renamed "overload" #1418

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 22, 2020
Merged

Add on_teams_team_renamed "overload" #1418

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63fd173
add on_teams_team_renamed "overload"
Oct 22, 2020
9f286e6
Merge branch 'main' into teamsOverload
axelsrz Oct 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion libraries/botbuilder-core/botbuilder/core/teams/teams_activity_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ async def on_conversation_update_activity(self, turn_context: TurnContext):
channel_data.channel, channel_data.team, turn_context
)
if channel_data.event_type == "teamRenamed":
return await self.on_teams_team_renamed_activity(
return await self.on_teams_team_renamed(
Copy link
Contributor Author

@mdrichardson mdrichardson Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This was actually a little trickier to implement than just using a simple overload because of this line. One of on_teams_team_renamed_activity() and on_teams_team_renamed() has to call the other (line 615). I tried to consider how this impacts backwards-compat (below), but please look it over and think it through, letting me know if I missed anything.

Regarding backwards-compat, there's really two ways to do this and I'm open to changing this

1. (current PR) Have on_conversation_update_activity() call on_teams_team_renamed()

The flow works like this:

  1. Activity comes in to on_conversation_update_activity()
  2. on_conversation_update_activity() calls on_teams_team_renamed()
  3. on_teams_team_renamed() calls on_teams_team_renamed_activity(), if it hasn't been overridden

Works for developers who:

  • Have already overridden on_conversation_update_activity()
  • Have already overridden on_teams_team_renamed_activity()
  • Have already overridden both on_conversation_update_activity() and on_teams_team_renamed_activity()
  • Are writing a new bot and simply want to override on_teams_team_renamed()

Does NOT work for developers who:

  • Have created a method on_teams_team_renamed() AND overridden on_teams_team_renamed_activity(), since on_conversation_update_activity() will no longer call on_teams_team_renamed_activity()
    • This is pretty unlikely since creating on_teams_team_renamed() would have accomplished nothing.

1. (alternative PR) Have on_conversation_update_activity() call on_teams_team_renamed_**activity**()

The difference is basically that there are no changes made to on_conversation_update_activity() and on_teams_team_renamed_activity() calls on_teams_team_renamed()

The flow would work like this:

  1. Activity comes in to on_conversation_update_activity()
  2. on_conversation_update_activity() calls on_teams_team_renamed_**activity**()
  3. on_teams_team_renamed_activity() calls on_teams_team_renamed(), if it hasn't been overridden

Works for developers who:

  • Have already overridden on_conversation_update_activity()
  • Have already overridden on_teams_team_renamed_activity()
  • Have already overridden both on_conversation_update_activity() and on_teams_team_renamed_activity()
  • Are writing a new bot and simply want to override on_teams_team_renamed()

Does NOT work for developers who:

  • Works for all since if the developer has created on_teams_team_renamed(), they're already going to use it how they please.

Summary

I went with the first option because it keeps on_conversation_update_activity() aligned through all the methods so that if future developers override it, it makes more "sense". However, option #1 also has potential backwards-compat issues, albeit very minimal. Open to changing things.

channel_data.team, turn_context
)
if channel_data.event_type == "teamRestored":
Expand Down Expand Up @@ -600,10 +600,27 @@ async def on_teams_team_hard_deleted( # pylint: disable=unused-argument
"""
return

async def on_teams_team_renamed( # pylint: disable=unused-argument
self, team_info: TeamInfo, turn_context: TurnContext
):
"""
Invoked when a Team Renamed event activity is received from the connector.
Team Renamed correspond to the user renaming an existing team.

:param team_info: The team info object representing the team.
:param turn_context: A context object for this turn.

:returns: A task that represents the work queued to execute.
"""
return await self.on_teams_team_renamed_activity(team_info, turn_context)

async def on_teams_team_renamed_activity( # pylint: disable=unused-argument
self, team_info: TeamInfo, turn_context: TurnContext
):
"""
DEPRECATED. Please use on_teams_team_renamed(). This method will remain in place throughout
v4 so as not to break existing bots.

Invoked when a Team Renamed event activity is received from the connector.
Team Renamed correspond to the user renaming an existing team.

Expand Down