Merge pull request #150 from netlify/fix-email-default

Flip email provider flag to a disabled flag. Defaults to false/enabled
netlify · Feb 12, 2018 · 529ae13 · 529ae13
2 parents 002e517 + da87e6a
commit 529ae13
Show file tree

Hide file tree

Showing 8 changed files with 10 additions and 15 deletions.
diff --git a/api/admin_test.go b/api/admin_test.go
@@ -44,7 +44,7 @@ func TestAdmin(t *testing.T) {
 
 func (ts *AdminTestSuite) SetupTest() {
 	models.TruncateAll(ts.API.db)
-	ts.Config.External.Email.Enabled = true
+	ts.Config.External.Email.Disabled = false
 	ts.token = ts.makeSuperAdmin("[email protected]")
 }
 
@@ -444,7 +444,7 @@ func (ts *AdminTestSuite) TestAdminUserCreateWithDisabledEmailLogin() {
 
 	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", ts.token))
 
-	ts.Config.External.Email.Enabled = false
+	ts.Config.External.Email.Disabled = true
 
 	ts.API.handler.ServeHTTP(w, req)
 	require.Equal(ts.T(), http.StatusBadRequest, w.Code)

diff --git a/api/api.go b/api/api.go
@@ -207,11 +207,6 @@ func (a *API) getConfig(ctx context.Context) *conf.Configuration {
 	if err := mergo.MergeWithOverwrite(&extConfig, config.External); err != nil {
 		return nil
 	}
-	// mergo only assigns zero values on destination
-	// See https://github.com/imdario/mergo/issues/24
-	if !config.External.Email.Enabled {
-		extConfig.Email.Enabled = false
-	}
 	config.External = extConfig
 
 	smtpConfig := (*a.config).SMTP

diff --git a/api/api_test.go b/api/api_test.go
@@ -96,7 +96,7 @@ func TestEmailEnabledByDefault(t *testing.T) {
 	api, _, err := setupAPIForTest()
 	require.NoError(t, err)
 
-	require.True(t, api.config.External.Email.Enabled)
+	require.False(t, api.config.External.Email.Disabled)
 }
 
 const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"

diff --git a/api/instance_test.go b/api/instance_test.go
@@ -144,7 +144,7 @@ func (ts *InstanceTestSuite) TestUpdate_DisableEmail() {
 		BaseConfig: &conf.Configuration{
 			External: conf.ProviderConfiguration{
 				Email: conf.EmailProviderConfiguration{
-					Enabled: true,
+					Disabled: false,
 				},
 			},
 		},
@@ -156,7 +156,7 @@ func (ts *InstanceTestSuite) TestUpdate_DisableEmail() {
 		"config": &conf.Configuration{
 			External: conf.ProviderConfiguration{
 				Email: conf.EmailProviderConfiguration{
-					Enabled: false,
+					Disabled: true,
 				},
 			},
 		},
@@ -172,5 +172,5 @@ func (ts *InstanceTestSuite) TestUpdate_DisableEmail() {
 
 	i, err := models.GetInstanceByUUID(ts.API.db, testUUID)
 	require.NoError(ts.T(), err)
-	require.False(ts.T(), i.BaseConfig.External.Email.Enabled)
+	require.True(ts.T(), i.BaseConfig.External.Email.Disabled)
 }
diff --git a/api/middleware.go b/api/middleware.go
@@ -147,7 +147,7 @@ func (a *API) requireEmailProvider(w http.ResponseWriter, req *http.Request) (co
 	ctx := req.Context()
 	config := a.getConfig(ctx)
 
-	if !config.External.Email.Enabled {
+	if config.External.Email.Disabled {
 		return nil, badRequestError("Unsupported email provider")
 	}
 

diff --git a/api/settings.go b/api/settings.go
@@ -27,7 +27,7 @@ func (a *API) Settings(w http.ResponseWriter, r *http.Request) error {
 			GitLab:    config.External.Gitlab.Enabled,
 			Google:    config.External.Google.Enabled,
 			Facebook:  config.External.Facebook.Enabled,
-			Email:     config.External.Email.Enabled,
+			Email:     !config.External.Email.Disabled,
 		},
 		DisableSignup: config.DisableSignup,
 		Autoconfirm:   config.Mailer.Autoconfirm,

diff --git a/api/settings_test.go b/api/settings_test.go
@@ -37,7 +37,7 @@ func TestSettings_EmailDisabled(t *testing.T) {
 	api, config, instanceID, err := setupAPIForTestForInstance()
 	require.NoError(t, err)
 
-	config.External.Email.Enabled = false
+	config.External.Email.Disabled = true
 
 	// Setup request
 	req := httptest.NewRequest(http.MethodGet, "http://localhost/settings", nil)

diff --git a/conf/configuration.go b/conf/configuration.go
@@ -22,7 +22,7 @@ type OAuthProviderConfiguration struct {
 }
 
 type EmailProviderConfiguration struct {
-	Enabled bool `json:"enabled" default:"true"`
+	Disabled bool `json:"disabled"`
 }
 
 // DBConfiguration holds all the database related configuration.