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

[Notifier] Add notifier for Microsoft Teams #39007

Merged
merged 1 commit into from
Apr 8, 2021

Conversation

idetox
Copy link
Contributor

@idetox idetox commented Nov 5, 2020

Q A
Branch? 5.x
Bug fix? no
New feature? yes
License MIT
Doc PR symfony/symfony-docs#15193
Recipe PR symfony/recipes#929

Add notifier bridge for Teams using Incoming Webhook implementation : see Incoming Webhook using curl

EDIT by @OskarStark:
I removed the options logic in the first step and will create a follow up PR adding them back to the bridge ❗

@idetox idetox force-pushed the notifier-bridge-teams branch 2 times, most recently from dd889cb to 4a59f17 Compare November 5, 2020 19:05
@jderusse jderusse added this to the 5.x milestone Nov 5, 2020
@idetox idetox force-pushed the notifier-bridge-teams branch 2 times, most recently from 204679d to 6764453 Compare November 6, 2020 08:16
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

Can you add @experimental in 5.3 in all classes?

@OskarStark
Copy link
Contributor

What do you think of renaming the bridge to MicrosoftTeams or MsTeams?

@OskarStark
Copy link
Contributor

Hi @idetox 👋

Are you interested in finishing this PR or am I allowed to take over?

Cheers Oskar

@idetox
Copy link
Contributor Author

idetox commented Jan 6, 2021

Hey sry @OskarStark , been busy with the end of the year at work & family, should have more time soon (i should be able to do it nxt week), if the changes need to be done fast you can take over, let me know !

@OskarStark
Copy link
Contributor

OskarStark commented Jan 6, 2021

Hi 👋 thank you for the feedback, next week would be super 👌🏻👍🏻

@OskarStark OskarStark changed the title [Notifier] Add notifier for Teams [Notifier] Add notifier for Microsoft Teams Jan 15, 2021
@OskarStark
Copy link
Contributor

As we are lacking tests for the many new classes I would like to introduce the bridge without custom options and rework the options part in another follow up PR.

In this case we have a bridge which can send chat messages to MicrosoftTeams, which is more than we are supporting right now.

WDYT @Nyholm @jderusse ?

@fabpot
Copy link
Member

fabpot commented Apr 7, 2021

Adding options in another PR works for me, that's what we did in the past for another provider (I don't remember which one though).

@OskarStark OskarStark force-pushed the notifier-bridge-teams branch 3 times, most recently from 40b6563 to 5ff55ef Compare April 7, 2021 07:19
@Nyholm
Copy link
Member

Nyholm commented Apr 7, 2021

Dont forget the .gitignore =)

Wink wink #40700

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

This looks way more simple now.

Let me know when you are ready for a final review.

@OskarStark
Copy link
Contributor

Dont forget the .gitignore =)

Added

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Thank you.
Im happy with this PR now.

Just make sure the CI is green.

@OskarStark OskarStark force-pushed the notifier-bridge-teams branch 3 times, most recently from 5b2c154 to 4e8bf02 Compare April 7, 2021 08:14
@OskarStark
Copy link
Contributor

All green now 👍

@Nyholm
Copy link
Member

Nyholm commented Apr 8, 2021

Could you do a quick rebase before merge?

@Nyholm
Copy link
Member

Nyholm commented Apr 8, 2021

Thank you Oskar and @idetox

@Nyholm Nyholm merged commit 8bc0a8f into symfony:5.x Apr 8, 2021
OskarStark added a commit to symfony/symfony-docs that referenced this pull request Apr 8, 2021
This PR was merged into the 5.3-dev branch.

Discussion
----------

[Notifier] Add MicrosoftTeams

Docs for symfony/symfony#39007

Best viewed with: https://github.com/symfony/symfony-docs/pull/15193/files?diff=unified&w=1

Commits
-------

9e1e0a9 [Notifier] Add MicrosoftTeams
@fabpot fabpot mentioned this pull request Apr 18, 2021
fabpot added a commit that referenced this pull request Jun 23, 2021
…karStark)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[Notifier] Add options to Microsoft Teams notifier

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Follows #39007
| License       | MIT
| Doc PR        | symfony/symfony-docs#15288

### After rework:
<img width="374" alt="CleanShot 2021-04-15 at 09 40 45@2x" src="https://user-images.githubusercontent.com/995707/114832421-c04c9400-9dce-11eb-8135-77ee1fb21314.png">

Commits
-------

d039ce7 [Notifier] Add options to Microsoft Teams notifier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants