fix: provide fallback attachment text to fix notifications

mattermost / mattermost-plugin-mscalendar

Mattermost plugin for Microsoft Office365

Apache License 2.0

13 stars 21 forks source link

fix: provide fallback attachment text to fix notifications #353

Closed fmartingr closed 4 months ago

fmartingr commented 8 months ago

Summary

This pull request adds a fallback text to event attachment so Mattermost notifications for event reminders are useful.

Fixes #338

matthewbirtch commented 8 months ago

@fmartingr based on the screenshot, it's not clear to me how we're incorporating the title into the notification. Is 'Test' the title in this case? I'm also seeing a blank space there between the username and the time

fmartingr commented 8 months ago

@fmartingr based on the screenshot, it's not clear to me how we're incorporating the title into the notification. Is 'Test' the title in this case? I'm also seeing a blank space there between the username and the time

"Test" is the event title. The space I'm not sure where it comes from, I'm not adding it. Maybe has something to do with this being a fallback text for an attachment.

matthewbirtch commented 8 months ago

@fmartingr can we switch the order of the title and the time? That would align better with how it displays in the actual message.

fmartingr commented 8 months ago

@fmartingr can we switch the order of the title and the time? That would align better with how it displays in the actual message.

Sure, done. I also removed the parenthesis.

codecov-commenter commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 20.94%. Comparing base (d1fce7a) to head (63c05cd).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #353 +/- ## ======================================= Coverage 20.94% 20.94% ======================================= Files 67 67 Lines 3442 3442 ======================================= Hits 721 721 Misses 2628 2628 Partials 93 93 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

matthewbirtch commented 8 months ago

Sure, done. I also removed the parenthesis.

Would you mind sharing a screenshot? We should be good to go

fmartingr commented 8 months ago

Sure, done. I also removed the parenthesis.

Would you mind sharing a screenshot? We should be good to go

fmartingr commented 8 months ago

Approving. Thanks @fmartingr. Hopefully we can track down where the extra space is coming from, but I don't want to block on that.

I just received the notification for an upcoming meeting and the Apple Watch notification has no newline after the username. Just leaving the details around for when we tackle that issue.