Fix JS error on dis/connect (#540)

* Fix JS error on dis/connect * Naming * Send websocket event when client config changes Fixes #536 * Adress feedback * Default instead of overwrite Co-authored-by: Mattermod <[email protected]>
mattermost · Feb 25, 2022 · 05062d3 · 05062d3
1 parent 99372e2
commit 05062d3
Show file tree

Hide file tree

Showing 12 changed files with 81 additions and 44 deletions.
diff --git a/server/plugin/api.go b/server/plugin/api.go
@@ -484,6 +484,7 @@ func (p *Plugin) completeConnectUserToGitHub(c *Context, w http.ResponseWriter,
 			"github_client_id":    config.GitHubOAuthClientID,
 			"enterprise_base_url": config.EnterpriseBaseURL,
 			"organization":        config.GitHubOrg,
+			"configuration":       config.ClientConfiguration(),
 		},
 		&model.WebsocketBroadcast{UserId: state.UserID},
 	)
@@ -555,20 +556,20 @@ func (p *Plugin) getConnected(c *Context, w http.ResponseWriter, r *http.Request
 	config := p.getConfiguration()
 
 	type ConnectedResponse struct {
-		Connected         bool               `json:"connected"`
-		GitHubUsername    string             `json:"github_username"`
-		GitHubClientID    string             `json:"github_client_id"`
-		EnterpriseBaseURL string             `json:"enterprise_base_url,omitempty"`
-		Organization      string             `json:"organization"`
-		UserSettings      *UserSettings      `json:"user_settings"`
-		PluginSettings    ClientSafeSettings `json:"plugin_settings"`
+		Connected           bool                   `json:"connected"`
+		GitHubUsername      string                 `json:"github_username"`
+		GitHubClientID      string                 `json:"github_client_id"`
+		EnterpriseBaseURL   string                 `json:"enterprise_base_url,omitempty"`
+		Organization        string                 `json:"organization"`
+		UserSettings        *UserSettings          `json:"user_settings"`
+		ClientConfiguration map[string]interface{} `json:"configuration"`
 	}
 
 	resp := &ConnectedResponse{
-		Connected:         false,
-		EnterpriseBaseURL: config.EnterpriseBaseURL,
-		Organization:      config.GitHubOrg,
-		PluginSettings:    p.getPluginSettings(),
+		Connected:           false,
+		EnterpriseBaseURL:   config.EnterpriseBaseURL,
+		Organization:        config.GitHubOrg,
+		ClientConfiguration: p.getConfiguration().ClientConfiguration(),
 	}
 
 	if c.UserID == "" {
@@ -637,14 +638,6 @@ func (p *Plugin) getConnected(c *Context, w http.ResponseWriter, r *http.Request
 	p.writeJSON(w, resp)
 }
 
-func (p *Plugin) getPluginSettings() ClientSafeSettings {
-	pluginSettings := ClientSafeSettings{
-		LeftSidebarEnabled: p.getConfiguration().EnableLeftSidebar,
-	}
-
-	return pluginSettings
-}
-
 func (p *Plugin) getMentions(c *UserContext, w http.ResponseWriter, r *http.Request) {
 	config := p.getConfiguration()
 

diff --git a/server/plugin/configuration.go b/server/plugin/configuration.go
@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/mattermost/mattermost-plugin-api/experimental/telemetry"
+	"github.com/mattermost/mattermost-server/v6/model"
 	"github.com/pkg/errors"
 )
 
@@ -106,11 +107,17 @@ func (c *Configuration) IsOAuthConfigured() bool {
 		c.UsePreregisteredApplication
 }
 
-// IsSASS return if SASS GitHub at https://github.com is used
+// IsSASS return if SASS GitHub at https://github.com is used.
 func (c *Configuration) IsSASS() bool {
 	return c.EnterpriseBaseURL == "" && c.EnterpriseUploadURL == ""
 }
 
+func (c *Configuration) ClientConfiguration() map[string]interface{} {
+	return map[string]interface{}{
+		"left_sidebar_enabled": c.EnableLeftSidebar,
+	}
+}
+
 // Clone shallow copies the configuration. Your implementation may require a deep copy if
 // your configuration has reference types.
 func (c *Configuration) Clone() *Configuration {
@@ -192,6 +199,8 @@ func (p *Plugin) OnConfigurationChange() error {
 
 	configuration.sanitize()
 
+	p.sendWebsocketEventIfNeeded(p.getConfiguration(), configuration)
+
 	p.setConfiguration(configuration)
 
 	command, err := p.getCommand(configuration)
@@ -216,6 +225,19 @@ func (p *Plugin) OnConfigurationChange() error {
 	return nil
 }
 
+func (p *Plugin) sendWebsocketEventIfNeeded(oldConfig, newConfig *Configuration) {
+	// If the plugin just started, oldConfig is the zero value.
+	// Hence, an unnecessary websocket event is sent.
+	// Given that oldConfig is never nil, that case is hard to catch.
+	if !reflect.DeepEqual(oldConfig.ClientConfiguration(), newConfig.ClientConfiguration()) {
+		p.API.PublishWebSocketEvent(
+			WSEventConfigUpdate,
+			newConfig.ClientConfiguration(),
+			&model.WebsocketBroadcast{},
+		)
+	}
+}
+
 func generateSecret() (string, error) {
 	b := make([]byte, 256)
 	_, err := rand.Read(b)

diff --git a/server/plugin/plugin.go b/server/plugin/plugin.go
@@ -31,10 +31,14 @@ const (
 	githubUsernameKey    = "_githubusername"
 	githubPrivateRepoKey = "_githubprivate"
 
-	wsEventConnect     = "connect"
-	wsEventDisconnect  = "disconnect"
-	wsEventRefresh     = "refresh"
-	wsEventCreateIssue = "createIssue"
+	wsEventConnect    = "connect"
+	wsEventDisconnect = "disconnect"
+	// WSEventConfigUpdate is the WebSocket event to update the configurations on webapp.
+	WSEventConfigUpdate = "config_update"
+	wsEventRefresh      = "refresh"
+	wsEventCreateIssue  = "createIssue"
+
+	WSEventRefresh = "refresh"
 
 	settingButtonsTeam   = "team"
 	settingNotifications = "notifications"
@@ -393,10 +397,6 @@ type UserSettings struct {
 	Notifications         bool   `json:"notifications"`
 }
 
-type ClientSafeSettings struct {
-	LeftSidebarEnabled bool `json:"left_sidebar_enabled"`
-}
-
 func (p *Plugin) storeGitHubUserInfo(info *GitHubUserInfo) error {
 	config := p.getConfiguration()
 

diff --git a/webapp/src/action_types/index.js b/webapp/src/action_types/index.js
@@ -13,6 +13,7 @@ export default {
     RECEIVED_MENTIONS: pluginId + '_received_mentions',
     RECEIVED_UNREADS: pluginId + '_received_unreads',
     RECEIVED_CONNECTED: pluginId + '_received_connected',
+    RECEIVED_CONFIGURATION: pluginId + '_received_configuration',
     RECEIVED_GITHUB_USER: pluginId + '_received_github_user',
     RECEIVED_SHOW_RHS_ACTION: pluginId + '_received_rhs_action',
     UPDATE_RHS_STATE: pluginId + '_update_rhs_state',

diff --git a/webapp/src/actions/index.js b/webapp/src/actions/index.js
@@ -33,7 +33,7 @@ function checkAndHandleNotConnected(data) {
                     connected: false,
                     github_username: '',
                     github_client_id: '',
-                    settings: {},
+                    user_settings: {},
                 },
             });
             return false;

diff --git a/webapp/src/client/client.js b/webapp/src/client/client.js
@@ -79,10 +79,6 @@ export default class Client {
         return this.doGet(`${this.url}/pr?owner=${owner}&repo=${repo}&number=${prNumber}`);
     }
 
-    getSettings = async () => {
-        return this.doGet(`${this.url}/settings`);
-    }
-
     doGet = async (url, body, headers = {}) => {
         headers['X-Timezone-Offset'] = new Date().getTimezoneOffset();
 

diff --git a/webapp/src/components/sidebar_header/index.js b/webapp/src/components/sidebar_header/index.js
@@ -3,14 +3,14 @@
 
 import {connect} from 'react-redux';
 
-import {pluginSettings} from '../../selectors';
+import {configuration} from '../../selectors';
 
 import SidebarHeader from './sidebar_header.jsx';
 
 function mapStateToProps(state) {
     const members = state.entities.teams.myMembers || {};
 
-    const sidebarEnabled = pluginSettings(state).left_sidebar_enabled;
+    const sidebarEnabled = configuration(state).left_sidebar_enabled;
     const show = sidebarEnabled && Object.keys(members).length <= 1;
 
     return {

diff --git a/webapp/src/components/team_sidebar/index.js b/webapp/src/components/team_sidebar/index.js
@@ -3,14 +3,14 @@
 
 import {connect} from 'react-redux';
 
-import {pluginSettings} from '../../selectors';
+import {configuration} from '../../selectors';
 
 import TeamSidebar from './team_sidebar.jsx';
 
 function mapStateToProps(state) {
     const members = state.entities.teams.myMembers || {};
 
-    const sidebarEnabled = pluginSettings(state).left_sidebar_enabled;
+    const sidebarEnabled = configuration(state).left_sidebar_enabled;
     const show = sidebarEnabled && Object.keys(members).length > 1;
 
     return {

diff --git a/webapp/src/index.js b/webapp/src/index.js
@@ -13,7 +13,7 @@ import LinkTooltip from './components/link_tooltip';
 import Reducer from './reducers';
 import Client from './client';
 import {getConnected, setShowRHSAction} from './actions';
-import {handleConnect, handleDisconnect, handleOpenCreateIssueModal, handleReconnect, handleRefresh} from './websocket';
+import {handleConnect, handleDisconnect, handleConfigurationUpdate, handleOpenCreateIssueModal, handleReconnect, handleRefresh} from './websocket';
 import {getServerRoute} from './selectors';
 import {id as pluginId} from './manifest';
 
@@ -42,6 +42,7 @@ class PluginClass {
 
         registry.registerWebSocketEventHandler(`custom_${pluginId}_connect`, handleConnect(store));
         registry.registerWebSocketEventHandler(`custom_${pluginId}_disconnect`, handleDisconnect(store));
+        registry.registerWebSocketEventHandler(`custom_${pluginId}_config_update`, handleConfigurationUpdate(store));
         registry.registerWebSocketEventHandler(`custom_${pluginId}_refresh`, handleRefresh(store));
         registry.registerWebSocketEventHandler(`custom_${pluginId}_createIssue`, handleOpenCreateIssueModal(store));
         registry.registerReconnectHandler(handleReconnect(store));

diff --git a/webapp/src/reducers/index.js b/webapp/src/reducers/index.js
@@ -57,10 +57,12 @@ function userSettings(state = {sidebar_buttons: Constants.SETTING_BUTTONS_TEAM,
     }
 }
 
-function pluginSettings(state = {left_sidebar_enabled: true}, action) {
+function configuration(state = {left_sidebar_enabled: true}, action) {
     switch (action.type) {
     case ActionTypes.RECEIVED_CONNECTED:
-        return action.data.plugin_settings;
+        return action.data.configuration;
+    case ActionTypes.RECEIVED_CONFIGURATION:
+        return action.data;
     default:
         return state;
     }
@@ -234,7 +236,7 @@ export default combineReducers({
     organization,
     username,
     userSettings,
-    pluginSettings,
+    configuration,
     clientId,
     reviews,
     reviewsDetails,

diff --git a/webapp/src/selectors.js b/webapp/src/selectors.js
@@ -19,4 +19,4 @@ export const getServerRoute = (state) => {
     return basePath;
 };
 
-export const pluginSettings = (state) => getPluginState(state).pluginSettings;
+export const configuration = (state) => getPluginState(state).configuration;
diff --git a/webapp/src/websocket/index.js b/webapp/src/websocket/index.js
@@ -19,9 +19,17 @@ export function handleConnect(store) {
         if (!msg.data) {
             return;
         }
+
         store.dispatch({
             type: ActionTypes.RECEIVED_CONNECTED,
-            data: {...msg.data, settings: {sidebar_buttons: Constants.SETTING_BUTTONS_TEAM, daily_reminder: true}},
+            data: {
+                ...msg.data,
+                user_settings: {
+                    sidebar_buttons: Constants.SETTING_BUTTONS_TEAM,
+                    daily_reminder: true,
+                    ...msg.data.user_settings,
+                },
+            },
         });
     };
 }
@@ -34,12 +42,26 @@ export function handleDisconnect(store) {
                 connected: false,
                 github_username: '',
                 github_client_id: '',
-                settings: {},
+                user_settings: {},
+                configuration: {},
             },
         });
     };
 }
 
+export function handleConfigurationUpdate(store) {
+    return (msg) => {
+        if (!msg.data) {
+            return;
+        }
+
+        store.dispatch({
+            type: ActionTypes.RECEIVED_CONFIGURATION,
+            data: msg.data,
+        });
+    };
+}
+
 export function handleReconnect(store, reminder = false) {
     return async () => {
         const {data} = await getConnected(reminder)(store.dispatch, store.getState);