mattermost-plugin-playbooks icon indicating copy to clipboard operation
mattermost-plugin-playbooks copied to clipboard

Published 20 hours ago • verified publisher icon mattermost

[MM-42886] Introduce weekly digest

Open batebobo opened this issue 2 years ago • 6 comments

Summary

Introduces a weekly digest, similar to the daily one. Extracted functions that decide whether a daily/weekly digest should be sent. Currently users would receive weekly digest if the daily digest is enabled.

What's left:

  • [x] Introduce a enable/disable weekly digest flag
  • [x] The above flag should default to the current daily digest setting ~- [ ] Add a feature flag, as this is a user-facing feature and includes refactoring~ ~- [ ] Add more tests for corner cases around sending daily/weekly digests~

Ticket Link

Weekly Digest

Checklist

  • [x] Telemetry updated
  • [x] Gated by experimental feature flag
  • [x] Unit tests updated

batebobo avatar Oct 09 '22 14:10 batebobo

Hello @batebobo,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

mattermod avatar Oct 09 '22 14:10 mattermod

Hey @batebobo! I'd love to have @crspeller take a look at this review -- should we wait until it leaves draft status?

lieut-data avatar Oct 18 '22 21:10 lieut-data

Hi @lieut-data! Yesterday I re-requested a review from @mkraft and GitHub automatically removed @crspeller :( I'd also like one more person to review the task!

batebobo avatar Oct 19 '22 11:10 batebobo

No worries! That's weird behaviour on GitHub's part -- fixed :)

lieut-data avatar Oct 19 '22 12:10 lieut-data

Thanks for re-requesting @Phrynobatrachus! I didn't mean to remove you, but rather to re-request review from @crspeller :(

batebobo avatar Oct 26 '22 13:10 batebobo

No worries, it's a Github bug I've seen a few times recently.

Phrynobatrachus avatar Oct 26 '22 13:10 Phrynobatrachus

Hey @Phrynobatrachus! Thanks for the comment and the screenshots! It seems that the timestamp is from a while ago (~30 years), so it definitely doesn't make sense to receive a daily digest. I'll take a look and let you know

batebobo avatar Nov 01 '22 08:11 batebobo

Hey again @Phrynobatrachus! There was still code left to change with regard to the difference between daily digest, weekly digest and /playbook todo. I pushsed the necessary changes and tested it with the following cases:

  • Didn't send any digest because of no data to show
  • /playbook todo still showed everything (even with 0s all around)
  • Added 5 playbooks and 1 run with tasks assigned to me. Weekly digest was sent (all sections, even the ones with 0s)
  • Daily digest was sent (not showing the 0 overdue tasks)
  • /playbook todo still works with all the data

Let me know if anything else comes up :)

batebobo avatar Nov 01 '22 09:11 batebobo

Apologies for the delay, once we sort out

https://github.com/mattermost/mattermost-plugin-playbooks/pull/1509#discussion_r1016979222

this should be good, everything else appears to be working after the updates.

Phrynobatrachus avatar Nov 08 '22 18:11 Phrynobatrachus

Hi @Phrynobatrachus! Thanks for the feedback! I'll check the case with the daily/weekly flags later today and let you know :)

batebobo avatar Nov 15 '22 10:11 batebobo

It looks like another pull request won the migration race for the name 0.61.

mkraft avatar Nov 21 '22 19:11 mkraft