rveachkc / pymsteams

Format messages and post to Microsoft Teams.
https://pypi.org/project/pymsteams/
Apache License 2.0
418 stars 77 forks source link

Created Adaptive card function and also mention user is available on this card #118

Closed YugoHino closed 2 years ago

YugoHino commented 2 years ago

Hi

I love this repository and our time is so saved. Thank you so much!! We wanted to mention use on Teams, but connector card doesn't allow us to mention users yet.

This is new feature which is available for the type of Adaptive card. Now Adaptive card allows us to mention users.

https://docs.microsoft.com/en-us/microsoftteams/platform/task-modules-and-cards/cards/cards-format?tabs=adaptive-md%2Cconnector-html#user-mention-in-incoming-webhook-with-adaptive-cards

I am new to OSS activity. Let me know if something wrong with pull request rules.

Thanks, Hino

ALERTua commented 2 years ago

I would be thankful for Python2-compatible code on this one: on Python 2 super() call on line 7 has to be with arguments on Python 2 F-sting on line 145 is not supported. Also, base.py:24 has one. these changes won't ruin Python 3 support. thank you very much

YugoHino commented 2 years ago

Thank you for your comment. I will fix it.

YugoHino commented 2 years ago

Fixed and checked those points. Could you check this again.

Thanks, Hino

ALERTua commented 2 years ago

I confirm your fix. Thank you!

rveachkc commented 2 years ago

Thanks for the PR. I will review this as time allows. Maybe this weekend?

While I can sympathize with the need to support Python 2, maintaining compatibility will create additional work. It reached end of life over 2 years ago on January 1, 2020. I don't want python2 compatibility to limit code style improvements or increase the amount of work required to improve this library. In fact, per #119, the master branch already requires python3.

I would be willing to maintain and review pull requests on a python2 branch, but I don't feel that python 2 compatibility should limit code improvements.

YugoHino commented 2 years ago

I agree with that. For testing python 2 compatibility on my environment, I needed to install python 2 additionally. Also as noted, #119 async section is necesarry to be commented out while testing because this is not supported for python 2 until it's fixed.

maximBelitski commented 2 years ago

Hi guys! Any progress on merging the PR ? Very desired functionality :)

dmonlineuk commented 2 years ago

+1 on this being merged soon please

rveachkc commented 2 years ago

This is going to take a bit of time for me to review the adaptive card api and prepare for release. I just haven't found the time to do it yet.

ghost commented 2 years ago

If you can change the size of adaptive card to Large isntead of medium, that would be handy, as some messages maybe big. When trying the setup, it works great, but due to medium size, only half sized card is dispalyed and extra text are not displayed.

blacksheep0815 commented 2 years ago

I would also appreciate to get those changes released. :-)

Question: Can multiple people be mentioned with the changes here or just 1?

YugoHino commented 2 years ago

Multiple people mention is available on this PR.

anuragladdha-ml commented 2 years ago

when are we planning to merge this and release?

ghost commented 2 years ago

@rveachkc if you can get it released fast , would be very helpful

iblub1 commented 2 years ago

Any plans to merge this pr soon? Would love to have mention functionality

jsa34 commented 2 years ago

Hello! Any possibility for this? If any help needed for the code review (I know it's of little help as you would probably like to review it yourself) let me know - this is a feature we would like to try and use at the moment :)

rveachkc commented 2 years ago

I've been thinking about this quite a bit, but I don't think this is a direction in which I want to go. The adaptive card api is quite expansive (https://adaptivecards.io/), but this is a very narrow implementation of it.

If adaptive cards are to be supported, are to be supported with this library, it will be in one of the two following ways:

So....I'm going to decline this request for now, but know that I'm thinking about it. Expect some movement here, but in a different direction.

YugoHino commented 2 years ago

Thank you for thinking about it! I just wanted to mention a user on the card even it's adaptive card because there is only the way to mention user on teams incomming webhook now. But I understand your point as commented. I will find other way to mention a user on teams incomming webhook such like using the library you commented. Thank you so much!