GH-88 Add settings command to toggle daily reminders #90

openmohan · 2020-07-03T12:36:58Z

Summary

The TODO bot reminds the user with the list of issue daily if it is not empty. With this feature added the reminder preference can be set, so the user can turn off or on the daily reminder functionality. By default, the value will be set as true.

Ticket Link

https://github.com/openmohan/mattermost-plugin-todo/tree/GH-88

####Usage
/todo settings summary on or /todo settings summary off

codecov-commenter · 2020-07-03T12:39:53Z

Codecov Report

Merging #90 into master will increase coverage by 3.81%.
The diff coverage is 49.15%.

@@            Coverage Diff            @@
##           master     #90      +/-   ##
=========================================
+ Coverage    0.00%   3.81%   +3.81%     
=========================================
  Files           8       8              
  Lines         861     918      +57     
=========================================
+ Hits            0      35      +35     
- Misses        861     883      +22

Impacted Files	Coverage Δ
server/plugin.go	`0.00% <0.00%> (ø)`
server/store.go	`3.75% <38.09%> (+3.75%)`	⬆️
server/command.go	`12.27% <56.75%> (+12.27%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffa4359...f1a2e26. Read the comment docs.

hanzei

Two small requests. The rest looks good. 👍

hanzei · 2020-07-03T13:03:37Z

server/command.go

+		if args[1] == "on" {
+			err = p.saveReminderPreference(extra.UserId, true)
+		} else if args[1] == "off" {
+			err = p.saveReminderPreference(extra.UserId, false)
+		}


Using a switch here would make the code cleaner and would make it easy to add a default path.

Changed to switch model.

server/store.go

hanzei · 2020-07-03T13:08:23Z

server/command.go

@@ -276,6 +308,9 @@ func getAutocompleteData() *model.AutocompleteData {
 	send.AddTextArgument("Todo message", "[message]", "")
 	todo.AddCommand(send)

+	settings := model.NewAutocompleteData("settings", "summary [on] [off]", "Sets the user preference on daily reminder")


Maybe out of scope of the PR: It would be great if there are autocomplete options for on and off

Sure, I can cover in this PR.

server/command.go

server/store.go

reflog

looks good in general, please add a test

server/command.go

openmohan · 2020-07-06T10:32:22Z

Squash commits at end.

hanzei

Awesome 👍

DHaussermann · 2020-07-07T15:33:51Z

@hanzei or @aaronrothschild The 3.0 release is about ready now. I can test this today if you confirm we want it in the release.

hanzei · 2020-07-07T15:35:10Z

It would be quite awesome to get this into the release.

larkox · 2020-07-15T10:56:35Z

@asaadmahmood @aaronrothschild What do you think about along the daily reminder show a message to tell how to stop the daily reminder? Something like:
If you want to stop receiving daily reminders type /todo settings reminder off
@DHaussermann This request is a bit last minute, and can be added in a different PR, so if you are already done testing this, I will move this to a different PR.

DHaussermann · 2020-07-15T21:06:58Z

@larkox my personal opinion on this is that since the feature will be included in 3.0, the message should go along with.

Regarding testing progress I'm starting now by figuring out how I can trigger the reminder on demand. But I don't see this addition as invalidating much testing so I am not opposed to including it now.

DHaussermann · 2020-07-15T21:41:06Z

Not sure how to test this without actually waiting a full day to test both states of the flag. @larkox I need a way to make the app think I got the ping 25hrs ago. Should I be able to do this my modifying the reminder_<mmuserid> key value in the KV store? This did not work for me. I'm not sure this is some kind of timestamp 🙃 Thoughts?
@openmohan there is a test no longer working on this branch.

Running golangci-lint
golangci-lint run ./...
server/command_test.go:28:30: `successfull` is a misspelling of `successful` (misspell)
			name:    "Setting summary successfull",
			                          ^
make: *** [golangci-lint] Error 1

Can you please see if you can repro and resolve this?

larkox · 2020-07-16T11:31:15Z

@DHaussermann If I understand correctly the underlying logic, you should be able to trigger the daily summary by:

setting reminder_ to 0
refreshing the page
clicking anywhere

If you were not to refresh the page, you may have to wait for 1 hour before the daily reminder get's checked again.

DHaussermann · 2020-07-16T14:48:36Z

Thanks @larkox I will try this again shortly.

After team discussion, this will be moved to next release so we can ship this now. Adjusting milestone.

openmohan · 2020-07-16T16:56:00Z

Not sure how to test this without actually waiting a full day to test both states of the flag. @larkox I need a way to make the app think I got the ping 25hrs ago. Should I be able to do this my modifying the reminder_<mmuserid> key value in the KV store? This did not work for me. I'm not sure this is some kind of timestamp 🙃 Thoughts?

@openmohan there is a test no longer working on this branch.
Running golangci-lint
golangci-lint run ./...
server/command_test.go:28:30: `successfull` is a misspelling of `successful` (misspell)
			name:    "Setting summary successfull",
			                          ^
make: *** [golangci-lint] Error 1
Can you please see if you can repro and resolve this?

Sure I will check it out.

DHaussermann · 2020-07-29T20:32:23Z

/update-branch

DHaussermann

Tested and passed

When setting is True user continues to receive reminder DMs as ususal
When False the reminder DMs are not received regardless of how long ago last reminder was sent
Build issue mentioned above has been resolved
This is working as expected.
LGTM!
Huge thanks @openmohan for this enhancement! This makes for a very a very nice improvement.

Added release test to cover this feature.

Thanks @larkox for help with testing. (I think the vlue I was setting was just too large. 0 worked fine :) )

openmohan requested a review from larkox as a code owner July 3, 2020 12:36

hanzei requested changes Jul 3, 2020

View reviewed changes

hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Jul 3, 2020

openmohan force-pushed the GH-88 branch from bfebaf5 to a4b730f Compare July 3, 2020 13:46

openmohan changed the title ~~GH-88 Add settings command to toggle daily reminders~~ [WIP]GH-88 Add settings command to toggle daily reminders Jul 3, 2020

mattermost-communityGH-88 Add settings command to toggle daily reminders

24e1c44

openmohan force-pushed the GH-88 branch from a4b730f to 24e1c44 Compare July 3, 2020 13:50

openmohan changed the title ~~[WIP]GH-88 Add settings command to toggle daily reminders~~ GH-88 Add settings command to toggle daily reminders Jul 3, 2020

hanzei requested changes Jul 6, 2020

View reviewed changes

server/command.go Outdated Show resolved Hide resolved

server/store.go Outdated Show resolved Hide resolved

server/store.go Outdated Show resolved Hide resolved

reflog suggested changes Jul 6, 2020

View reviewed changes

server/command.go Show resolved Hide resolved

openmohan requested a review from hanzei July 6, 2020 10:32

openmohan force-pushed the GH-88 branch from 074e3ec to abfba1a Compare July 6, 2020 10:34

jwilander requested a review from reflog July 6, 2020 14:05

reflog approved these changes Jul 6, 2020

View reviewed changes

hanzei approved these changes Jul 7, 2020

View reviewed changes

hanzei requested a review from DHaussermann July 7, 2020 10:14

hanzei removed the 2: Dev Review Requires review by a core committer label Jul 7, 2020

hanzei added this to the v0.3.0 milestone Jul 7, 2020

larkox approved these changes Jul 8, 2020

View reviewed changes

larkox mentioned this pull request Jul 13, 2020

Release v0.3.0 #75

Merged

DHaussermann modified the milestones: v0.3.0, v0.4.0 Jul 16, 2020

settings summary command testcases

a9b0c2a

openmohan force-pushed the GH-88 branch from abfba1a to a9b0c2a Compare July 21, 2020 10:34

larkox linked an issue Jul 28, 2020 that may be closed by this pull request

Support daily summary notification on/off setting #88

Closed

Merge branch 'master' into mattermost-communityGH-88

f1a2e26

DHaussermann approved these changes Jul 29, 2020

View reviewed changes

DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Jul 29, 2020

larkox merged commit bb127b1 into mattermost-community:master Jul 30, 2020

larkox mentioned this pull request Sep 7, 2020

Bump version to 0.4.0 #117

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-88 Add settings command to toggle daily reminders #90

GH-88 Add settings command to toggle daily reminders #90

openmohan commented Jul 3, 2020

codecov-commenter commented Jul 3, 2020 •

edited

Loading

hanzei left a comment

hanzei Jul 3, 2020

openmohan Jul 3, 2020

hanzei Jul 3, 2020

openmohan Jul 3, 2020

reflog left a comment

openmohan commented Jul 6, 2020 •

edited by reflog

Loading

hanzei left a comment

DHaussermann commented Jul 7, 2020

hanzei commented Jul 7, 2020

larkox commented Jul 15, 2020

DHaussermann commented Jul 15, 2020

DHaussermann commented Jul 15, 2020

larkox commented Jul 16, 2020

DHaussermann commented Jul 16, 2020

openmohan commented Jul 16, 2020

DHaussermann commented Jul 29, 2020

DHaussermann left a comment

GH-88 Add settings command to toggle daily reminders #90

GH-88 Add settings command to toggle daily reminders #90

Conversation

openmohan commented Jul 3, 2020

Summary

Ticket Link

codecov-commenter commented Jul 3, 2020 • edited Loading

Codecov Report

hanzei left a comment

Choose a reason for hiding this comment

hanzei Jul 3, 2020

Choose a reason for hiding this comment

openmohan Jul 3, 2020

Choose a reason for hiding this comment

hanzei Jul 3, 2020

Choose a reason for hiding this comment

openmohan Jul 3, 2020

Choose a reason for hiding this comment

reflog left a comment

Choose a reason for hiding this comment

openmohan commented Jul 6, 2020 • edited by reflog Loading

hanzei left a comment

Choose a reason for hiding this comment

DHaussermann commented Jul 7, 2020

hanzei commented Jul 7, 2020

larkox commented Jul 15, 2020

DHaussermann commented Jul 15, 2020

DHaussermann commented Jul 15, 2020

larkox commented Jul 16, 2020

DHaussermann commented Jul 16, 2020

openmohan commented Jul 16, 2020

DHaussermann commented Jul 29, 2020

DHaussermann left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jul 3, 2020 •

edited

Loading

openmohan commented Jul 6, 2020 •

edited by reflog

Loading