Address PR feedback

server/mscalendar/settings.go

@@ -19,7 +19,7 @@ func (c *mscalendar) ClearSettingsPosts(userID string) {
 	c.SettingsPanel.Clear(userID)
 }
 
-func NewSettingsPanel(bot bot.Bot, panelStore settingspanel.PanelStore, settingStore settingspanel.SettingStore, settingsHandler, pluginURL string, getTimezone func(userID string) string) settingspanel.Panel {
+func NewSettingsPanel(bot bot.Bot, panelStore settingspanel.PanelStore, settingStore settingspanel.SettingStore, settingsHandler, pluginURL string, getTimezone func(userID string) (string, error)) settingspanel.Panel {
 	settings := []settingspanel.Setting{}
 	settings = append(settings, settingspanel.NewBoolSetting(
 		store.UpdateStatusSettingID,

server/mscalendar/settings_daily_summary.go

@@ -19,13 +19,13 @@ type dailySummarySetting struct {
 	optionsM    []string
 	optionsAPM  []string
 	store       settingspanel.SettingStore
-	getTimezone func(userID string) string
+	getTimezone func(userID string) (string, error)
 }
 
-func NewDailySummarySetting(inStore settingspanel.SettingStore, getTimezone func(userID string) string) settingspanel.Setting {
+func NewDailySummarySetting(inStore settingspanel.SettingStore, getTimezone func(userID string) (string, error)) settingspanel.Setting {
 	os := &dailySummarySetting{
 		title:       "Daily Summary",
-		description: "When do you want to receive the daily summary.\n If you update this setting, it will automatically update to your the timezone currently set on your calendar.",
+		description: "When do you want to receive the daily summary?\n If you update this setting, it will automatically update to your the timezone currently set on your calendar.",
 		id:          store.DailySummarySettingID,
 		dependsOn:   "",
 		store:       inStore,
@@ -47,6 +47,10 @@ func NewDailySummarySetting(inStore settingspanel.SettingStore, getTimezone func
 }
 
 func (s *dailySummarySetting) Set(userID string, value interface{}) error {
+	_, ok := value.(string)
+	if !ok {
+		return errors.New("trying to set Daily Summary Setting without a string value")
+	}
 	err := s.store.SetSetting(userID, s.id, value)
 	if err != nil {
 		return err
@@ -118,7 +122,14 @@ func (s *dailySummarySetting) GetSlackAttachments(userID, settingHandler string,
 			currentTextValue = fmt.Sprintf("%s (%s) (%s)", dsum.PostTime, dsum.Timezone, enableText)
 		}
 
-		timezone := s.getTimezone(userID)
+		timezone, err := s.getTimezone(userID)
+		if err != nil {
+			if dsum != nil {
+				timezone = dsum.Timezone
+			} else {
+				timezone = "UTC"
+			}
+		}
 		fullTime = fullTime + " " + timezone
 
 		currentValueMessage = fmt.Sprintf("Current value: %s", currentTextValue)
@@ -164,26 +175,24 @@ func (s *dailySummarySetting) GetSlackAttachments(userID, settingHandler string,
 
 		actions = []*model.PostAction{&actionOptionsH, &actionOptionsM, &actionOptionsAPM}
 
-		if currentTextValue != "Not set" {
-			buttonText := "Enable"
-			enable := "true"
-			if currentEnable {
-				buttonText = "Disable"
-				enable = "false"
-			}
-			actionToggle := model.PostAction{
-				Name: buttonText,
-				Integration: &model.PostActionIntegration{
-					URL: settingHandler,
-					Context: map[string]interface{}{
-						settingspanel.ContextIDKey:          s.id,
-						settingspanel.ContextButtonValueKey: enable + " " + timezone,
-					},
+		buttonText := "Enable"
+		enable := "true"
+		if currentEnable {
+			buttonText = "Disable"
+			enable = "false"
+		}
+		actionToggle := model.PostAction{
+			Name: buttonText,
+			Integration: &model.PostActionIntegration{
+				URL: settingHandler,
+				Context: map[string]interface{}{
+					settingspanel.ContextIDKey:          s.id,
+					settingspanel.ContextButtonValueKey: enable + " " + timezone,
 				},
-			}
-
-			actions = append(actions, &actionToggle)
+			},
 		}
+
+		actions = append(actions, &actionToggle)
 	}
 
 	text := fmt.Sprintf("%s\n%s", s.description, currentValueMessage)

server/plugin/plugin.go

@@ -138,9 +138,8 @@ func (p *Plugin) OnConfigurationChange() (err error) {
 			e.Dependencies.Store,
 			"/settings",
 			pluginURL,
-			func(userID string) string {
-				timezone, _ := mscalendar.New(e.Env, userID).GetTimezone(mscalendar.NewUser(userID))
-				return timezone
+			func(userID string) (string, error) {
+				return mscalendar.New(e.Env, userID).GetTimezone(mscalendar.NewUser(userID))
 			},
 		)
 

server/store/setting_store.go

@@ -3,8 +3,6 @@ package store
 import (
 	"fmt"
 	"strings"
-
-	"github.com/mattermost/mattermost-plugin-mscalendar/server/utils/kvstore"
 )
 
 const (
@@ -58,27 +56,12 @@ func (s *pluginStore) GetSetting(userID, settingID string) (interface{}, error)
 	case GetConfirmationSettingID:
 		return user.Settings.GetConfirmation, nil
 	case DailySummarySettingID:
-		return s.getDailySummarySettingForUser(userID)
+		return s.LoadDailySummaryUserSettings(userID)
 	default:
 		return nil, fmt.Errorf("setting %s not found", settingID)
 	}
 }
 
-func (s *pluginStore) getDailySummarySettingForUser(userID string) (*DailySummaryUserSettings, error) {
-	dsumIndex, err := s.LoadDailySummaryIndex()
-	if err != nil {
-		return nil, err
-	}
-
-	for _, dsum := range dsumIndex {
-		if dsum.MattermostUserID == userID {
-			return dsum, nil
-		}
-	}
-
-	return nil, nil
-}
-
 func (s *pluginStore) updateDailySummarySettingForUser(userID string, value interface{}) error {
 	dsum, err := s.LoadDailySummaryUserSettings(userID)
 	if err != nil {
@@ -122,21 +105,20 @@ func (s *pluginStore) updateDailySummarySettingForUser(userID string, value inte
 }
 
 func (s *pluginStore) SetPanelPostID(userID string, postID string) error {
-	err := kvstore.StoreJSON(s.settingsPanelKV, userID, postID)
+	err := s.settingsPanelKV.Store(userID, []byte(postID))
 	if err != nil {
 		return err
 	}
 	return nil
 }
 
 func (s *pluginStore) GetPanelPostID(userID string) (string, error) {
-	var postID string
-	err := kvstore.LoadJSON(s.settingsPanelKV, userID, &postID)
+	postID, err := s.settingsPanelKV.Load(userID)
 	if err != nil {
 		return "", err
 	}
 
-	return postID, nil
+	return string(postID), nil
 }
 
 func (s *pluginStore) DeletePanelPostID(userID string) error {

server/utils/settingspanel/handler.go

@@ -58,7 +58,7 @@ func (sh *handler) handleAction(w http.ResponseWriter, r *http.Request) {
 	sh.panel.Set(mattermostUserID, idString, value)
 
 	response := model.PostActionIntegrationResponse{}
-	post, err := sh.panel.GetUpdatePost(mattermostUserID)
+	post, err := sh.panel.ToPost(mattermostUserID)
 	if err == nil {
 		response.Update = post
 	}

server/utils/settingspanel/read_only_setting.go

@@ -36,7 +36,7 @@ func (s *readOnlySetting) Get(userID string) (interface{}, error) {
 	}
 	stringValue, ok := value.(string)
 	if !ok {
-		return "", errors.New("current value is not a bool")
+		return "", errors.New("current value is not a string")
 	}
 
 	return stringValue, nil

server/utils/settingspanel/settings.go

@@ -21,7 +21,7 @@ type Setting interface {
 type Panel interface {
 	Set(userID, settingID string, value interface{}) error
 	Print(userID string)
-	GetUpdatePost(userID string) (*model.Post, error)
+	ToPost(userID string) (*model.Post, error)
 	Clear(userID string) error
 	URL() string
 	GetSettingIDs() []string
@@ -103,7 +103,6 @@ func (p *panel) Print(userID string) {
 			continue
 		}
 		sas = append(sas, sa)
-
 	}
 	postID, err := p.poster.DMWithAttachments(userID, sas...)
 	if err != nil {
@@ -117,7 +116,7 @@ func (p *panel) Print(userID string) {
 	}
 }
 
-func (p *panel) GetUpdatePost(userID string) (*model.Post, error) {
+func (p *panel) ToPost(userID string) (*model.Post, error) {
 	post := &model.Post{}
 
 	sas := []*model.SlackAttachment{}