From e32a1b1cd02f8f9f47321ca9eb99fc0588220424 Mon Sep 17 00:00:00 2001
From: Burak Yigit Kaya <ben@byk.im>
Date: Tue, 18 Jun 2019 18:22:03 +0300
Subject: [PATCH 1/5] fix(installWizard): Fix client default values not set

Fixes #12813.
---
 src/sentry/static/sentry/app/options.jsx        | 14 ++++++++++++--
 .../static/sentry/app/views/installWizard.jsx   | 17 +++++++++--------
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/sentry/static/sentry/app/options.jsx b/src/sentry/static/sentry/app/options.jsx
index 7c0309de8ee207..d164c295b45cd9 100644
--- a/src/sentry/static/sentry/app/options.jsx
+++ b/src/sentry/static/sentry/app/options.jsx
@@ -171,6 +171,16 @@ export function getOption(option) {
   return definitionsMap[option];
 }
 
+export function getOptionDefault(option) {
+  const meta = getOption(option);
+  return meta.defaultValue ? meta.defaultValue() : undefined;
+}
+
+export function isOptionRequired(option) {
+  const meta = getOption(option);
+  return meta.required && !meta.allowEmpty;
+}
+
 function optionsForSection(section) {
   return definitions.filter(option => option.key.split('.')[0] === section.key);
 }
@@ -183,8 +193,8 @@ export function getOptionField(option, field) {
       {...meta}
       name={option}
       key={option}
-      defaultValue={meta.defaultValue ? meta.defaultValue() : undefined}
-      required={meta.required && !meta.allowEmpty}
+      defaultValue={getOptionDefault(option)}
+      required={isOptionRequired(option)}
       disabledReason={meta.disabledReason && disabledReasons[meta.disabledReason]}
     />
   );
diff --git a/src/sentry/static/sentry/app/views/installWizard.jsx b/src/sentry/static/sentry/app/views/installWizard.jsx
index a6586d3c1341e5..7f7a0a9b2b07b1 100644
--- a/src/sentry/static/sentry/app/views/installWizard.jsx
+++ b/src/sentry/static/sentry/app/views/installWizard.jsx
@@ -6,7 +6,7 @@ import AsyncView from 'app/views/asyncView';
 import {t} from 'app/locale';
 import ConfigStore from 'app/stores/configStore';
 import {ApiForm} from 'app/components/forms';
-import {getOptionField, getForm} from 'app/options';
+import {getOptionDefault, getOptionField, getForm, isOptionRequired} from 'app/options';
 
 export default class InstallWizard extends AsyncView {
   static propTypes = {
@@ -64,16 +64,17 @@ export default class InstallWizard extends AsyncView {
       if (option.field.disabled) {
         return;
       }
-      // XXX(dcramer): we need the user to explicitly choose beacon.anonymous
-      // vs using an implied default so effectively this is binding
-      // all values to their server-defaults (as client-side defaults dont really work)
+
       // TODO(dcramer): we need to rethink this logic as doing multiple "is this value actually set"
       // is problematic
+      // all values to their server-defaults (as client-side defaults dont really work)
+      const displayValue = option.value || getOptionDefault(option);
       if (
-        option.value !== undefined &&
-        option.value !== '' &&
-        option.value !== null &&
-        (option.field.isSet || optionName != 'beacon.anonymous')
+        // XXX(dcramer): we need the user to explicitly choose beacon.anonymous
+        // vs using an implied default so effectively this is binding
+        optionName != 'beacon.anonymous' &&
+        option.field.isSet &&
+        displayValue !== undefined
       ) {
         data[optionName] = option.value;
       }

From adeaa04a8cc1ae0fd33da1be9491182078c0d9d0 Mon Sep 17 00:00:00 2001
From: Burak Yigit Kaya <ben@byk.im>
Date: Tue, 18 Jun 2019 19:52:56 +0300
Subject: [PATCH 2/5] Use correct values

---
 src/sentry/static/sentry/app/options.jsx             | 7 +------
 src/sentry/static/sentry/app/views/installWizard.jsx | 4 ++--
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/src/sentry/static/sentry/app/options.jsx b/src/sentry/static/sentry/app/options.jsx
index d164c295b45cd9..6ac046bfd2d4c9 100644
--- a/src/sentry/static/sentry/app/options.jsx
+++ b/src/sentry/static/sentry/app/options.jsx
@@ -176,11 +176,6 @@ export function getOptionDefault(option) {
   return meta.defaultValue ? meta.defaultValue() : undefined;
 }
 
-export function isOptionRequired(option) {
-  const meta = getOption(option);
-  return meta.required && !meta.allowEmpty;
-}
-
 function optionsForSection(section) {
   return definitions.filter(option => option.key.split('.')[0] === section.key);
 }
@@ -194,7 +189,7 @@ export function getOptionField(option, field) {
       name={option}
       key={option}
       defaultValue={getOptionDefault(option)}
-      required={isOptionRequired(option)}
+      required={meta.required && !meta.allowEmpt}
       disabledReason={meta.disabledReason && disabledReasons[meta.disabledReason]}
     />
   );
diff --git a/src/sentry/static/sentry/app/views/installWizard.jsx b/src/sentry/static/sentry/app/views/installWizard.jsx
index 7f7a0a9b2b07b1..d9af6e0a58d696 100644
--- a/src/sentry/static/sentry/app/views/installWizard.jsx
+++ b/src/sentry/static/sentry/app/views/installWizard.jsx
@@ -6,7 +6,7 @@ import AsyncView from 'app/views/asyncView';
 import {t} from 'app/locale';
 import ConfigStore from 'app/stores/configStore';
 import {ApiForm} from 'app/components/forms';
-import {getOptionDefault, getOptionField, getForm, isOptionRequired} from 'app/options';
+import {getOptionDefault, getOptionField, getForm} from 'app/options';
 
 export default class InstallWizard extends AsyncView {
   static propTypes = {
@@ -68,7 +68,7 @@ export default class InstallWizard extends AsyncView {
       // TODO(dcramer): we need to rethink this logic as doing multiple "is this value actually set"
       // is problematic
       // all values to their server-defaults (as client-side defaults dont really work)
-      const displayValue = option.value || getOptionDefault(option);
+      const displayValue = option.value || getOptionDefault(optionName);
       if (
         // XXX(dcramer): we need the user to explicitly choose beacon.anonymous
         // vs using an implied default so effectively this is binding

From f933460e8f1a711bffb21c9c6feff7a3483ad345 Mon Sep 17 00:00:00 2001
From: Burak Yigit Kaya <ben@byk.im>
Date: Tue, 18 Jun 2019 20:31:10 +0300
Subject: [PATCH 3/5] fix logic

---
 src/sentry/static/sentry/app/views/installWizard.jsx | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/sentry/static/sentry/app/views/installWizard.jsx b/src/sentry/static/sentry/app/views/installWizard.jsx
index d9af6e0a58d696..c636367b7398f1 100644
--- a/src/sentry/static/sentry/app/views/installWizard.jsx
+++ b/src/sentry/static/sentry/app/views/installWizard.jsx
@@ -73,10 +73,12 @@ export default class InstallWizard extends AsyncView {
         // XXX(dcramer): we need the user to explicitly choose beacon.anonymous
         // vs using an implied default so effectively this is binding
         optionName != 'beacon.anonymous' &&
-        option.field.isSet &&
+        // XXX(byk): if we don't have a set value but have a default value filled
+        // instead, from the client, set it on the data so it is sent to the server
+        !option.field.isSet &&
         displayValue !== undefined
       ) {
-        data[optionName] = option.value;
+        data[optionName] = displayValue;
       }
     });
     return data;

From fe237a7d96b48f87c257ee9e6607c9264ac53b5f Mon Sep 17 00:00:00 2001
From: Burak Yigit Kaya <ben@byk.im>
Date: Tue, 18 Jun 2019 23:36:45 +0300
Subject: [PATCH 4/5] fix tests

---
 .nvmrc                                        |  2 +-
 .../__snapshots__/installWizard.spec.jsx.snap | 20 +++++++++----------
 .../__snapshots__/adminSettings.spec.jsx.snap |  2 +-
 tests/js/spec/views/installWizard.spec.jsx    |  6 +++---
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/.nvmrc b/.nvmrc
index 5816526f95d5a0..894aa0bc688135 100644
--- a/.nvmrc
+++ b/.nvmrc
@@ -1 +1 @@
-8.15.1
+8.16.0
diff --git a/tests/js/spec/views/__snapshots__/installWizard.spec.jsx.snap b/tests/js/spec/views/__snapshots__/installWizard.spec.jsx.snap
index b559c57613d2dc..ef0bc29bafeba6 100644
--- a/tests/js/spec/views/__snapshots__/installWizard.spec.jsx.snap
+++ b/tests/js/spec/views/__snapshots__/installWizard.spec.jsx.snap
@@ -247,10 +247,10 @@ exports[`InstallWizard renders 1`] = `
                     key="mail.username"
                     label="SMTP Username"
                     name="mail.username"
-                    required={false}
+                    required={true}
                   >
                     <div
-                      className="control-group"
+                      className="control-group required"
                     >
                       <div
                         className="controls"
@@ -267,7 +267,7 @@ exports[`InstallWizard renders 1`] = `
                           id="id-mail.username"
                           name="mail.username"
                           onChange={[Function]}
-                          required={false}
+                          required={true}
                           type="text"
                           value=""
                         />
@@ -285,10 +285,10 @@ exports[`InstallWizard renders 1`] = `
                     key="mail.password"
                     label="SMTP Password"
                     name="mail.password"
-                    required={false}
+                    required={true}
                   >
                     <div
-                      className="control-group"
+                      className="control-group required"
                     >
                       <div
                         className="controls"
@@ -305,7 +305,7 @@ exports[`InstallWizard renders 1`] = `
                           id="id-mail.password"
                           name="mail.password"
                           onChange={[Function]}
-                          required={false}
+                          required={true}
                           type="text"
                           value=""
                         />
@@ -324,7 +324,7 @@ exports[`InstallWizard renders 1`] = `
                     key="mail.use-tls"
                     label="Use TLS?"
                     name="mail.use-tls"
-                    required={false}
+                    required={true}
                   >
                     <div
                       className="control-group checkbox"
@@ -367,7 +367,7 @@ exports[`InstallWizard renders 1`] = `
                     key="auth.allow-registration"
                     label="Allow Registration"
                     name="auth.allow-registration"
-                    required={false}
+                    required={true}
                   >
                     <div
                       className="control-group checkbox"
@@ -431,12 +431,12 @@ exports[`InstallWizard renders 1`] = `
                     label="Usage Statistics"
                     name="beacon.anonymous"
                     noLabel="Send my contact information along with usage statistics"
-                    required={false}
+                    required={true}
                     yesFirst={false}
                     yesLabel="Please keep my usage information anonymous"
                   >
                     <div
-                      className="control-group"
+                      className="control-group required"
                     >
                       <div
                         className="controls"
diff --git a/tests/js/spec/views/admin/__snapshots__/adminSettings.spec.jsx.snap b/tests/js/spec/views/admin/__snapshots__/adminSettings.spec.jsx.snap
index 4413a6c1c2b02e..b99a8d7d85cea0 100644
--- a/tests/js/spec/views/admin/__snapshots__/adminSettings.spec.jsx.snap
+++ b/tests/js/spec/views/admin/__snapshots__/adminSettings.spec.jsx.snap
@@ -53,7 +53,7 @@ exports[`AdminSettings render() renders 1`] = `
         label="Root URL"
         name="system.url-prefix"
         placeholder="https://sentry.example.com"
-        required={false}
+        required={true}
       />
       <EmailField
         allowEmpty={false}
diff --git a/tests/js/spec/views/installWizard.spec.jsx b/tests/js/spec/views/installWizard.spec.jsx
index 07e3ed38d6ad86..f3dcff7e1c3357 100644
--- a/tests/js/spec/views/installWizard.spec.jsx
+++ b/tests/js/spec/views/installWizard.spec.jsx
@@ -80,14 +80,14 @@ describe('InstallWizard', function() {
 
     expect(
       wrapper.find('input[name="beacon.anonymous"][value="false"]').prop('checked')
-    ).toBe(true);
+    ).toBe(false);
 
     expect(
       wrapper.find('input[name="beacon.anonymous"][value="true"]').prop('checked')
     ).toBe(false);
   });
 
-  it('has "Please keep my usage anonymous" when beacon.anonymous is true', function() {
+  it('has "Please keep my usage anonymous" when beacon.anonymous is false', function() {
     MockApiClient.addMockResponse({
       url: '/internal/options/?query=is:required',
       body: TestStubs.InstallWizard({
@@ -112,6 +112,6 @@ describe('InstallWizard', function() {
 
     expect(
       wrapper.find('input[name="beacon.anonymous"][value="true"]').prop('checked')
-    ).toBe(true);
+    ).toBe(false);
   });
 });

From 95595e68080b0f658db879e1dc933b908709dd7d Mon Sep 17 00:00:00 2001
From: Burak Yigit Kaya <ben@byk.im>
Date: Tue, 18 Jun 2019 23:53:37 +0300
Subject: [PATCH 5/5] fix blunders

---
 .nvmrc                                        |  2 +-
 src/sentry/static/sentry/app/options.jsx      |  2 +-
 .../__snapshots__/installWizard.spec.jsx.snap | 20 +++++++++----------
 .../__snapshots__/adminSettings.spec.jsx.snap |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/.nvmrc b/.nvmrc
index 894aa0bc688135..5816526f95d5a0 100644
--- a/.nvmrc
+++ b/.nvmrc
@@ -1 +1 @@
-8.16.0
+8.15.1
diff --git a/src/sentry/static/sentry/app/options.jsx b/src/sentry/static/sentry/app/options.jsx
index 6ac046bfd2d4c9..f6c6253b0cce09 100644
--- a/src/sentry/static/sentry/app/options.jsx
+++ b/src/sentry/static/sentry/app/options.jsx
@@ -189,7 +189,7 @@ export function getOptionField(option, field) {
       name={option}
       key={option}
       defaultValue={getOptionDefault(option)}
-      required={meta.required && !meta.allowEmpt}
+      required={meta.required && !meta.allowEmpty}
       disabledReason={meta.disabledReason && disabledReasons[meta.disabledReason]}
     />
   );
diff --git a/tests/js/spec/views/__snapshots__/installWizard.spec.jsx.snap b/tests/js/spec/views/__snapshots__/installWizard.spec.jsx.snap
index ef0bc29bafeba6..b559c57613d2dc 100644
--- a/tests/js/spec/views/__snapshots__/installWizard.spec.jsx.snap
+++ b/tests/js/spec/views/__snapshots__/installWizard.spec.jsx.snap
@@ -247,10 +247,10 @@ exports[`InstallWizard renders 1`] = `
                     key="mail.username"
                     label="SMTP Username"
                     name="mail.username"
-                    required={true}
+                    required={false}
                   >
                     <div
-                      className="control-group required"
+                      className="control-group"
                     >
                       <div
                         className="controls"
@@ -267,7 +267,7 @@ exports[`InstallWizard renders 1`] = `
                           id="id-mail.username"
                           name="mail.username"
                           onChange={[Function]}
-                          required={true}
+                          required={false}
                           type="text"
                           value=""
                         />
@@ -285,10 +285,10 @@ exports[`InstallWizard renders 1`] = `
                     key="mail.password"
                     label="SMTP Password"
                     name="mail.password"
-                    required={true}
+                    required={false}
                   >
                     <div
-                      className="control-group required"
+                      className="control-group"
                     >
                       <div
                         className="controls"
@@ -305,7 +305,7 @@ exports[`InstallWizard renders 1`] = `
                           id="id-mail.password"
                           name="mail.password"
                           onChange={[Function]}
-                          required={true}
+                          required={false}
                           type="text"
                           value=""
                         />
@@ -324,7 +324,7 @@ exports[`InstallWizard renders 1`] = `
                     key="mail.use-tls"
                     label="Use TLS?"
                     name="mail.use-tls"
-                    required={true}
+                    required={false}
                   >
                     <div
                       className="control-group checkbox"
@@ -367,7 +367,7 @@ exports[`InstallWizard renders 1`] = `
                     key="auth.allow-registration"
                     label="Allow Registration"
                     name="auth.allow-registration"
-                    required={true}
+                    required={false}
                   >
                     <div
                       className="control-group checkbox"
@@ -431,12 +431,12 @@ exports[`InstallWizard renders 1`] = `
                     label="Usage Statistics"
                     name="beacon.anonymous"
                     noLabel="Send my contact information along with usage statistics"
-                    required={true}
+                    required={false}
                     yesFirst={false}
                     yesLabel="Please keep my usage information anonymous"
                   >
                     <div
-                      className="control-group required"
+                      className="control-group"
                     >
                       <div
                         className="controls"
diff --git a/tests/js/spec/views/admin/__snapshots__/adminSettings.spec.jsx.snap b/tests/js/spec/views/admin/__snapshots__/adminSettings.spec.jsx.snap
index b99a8d7d85cea0..4413a6c1c2b02e 100644
--- a/tests/js/spec/views/admin/__snapshots__/adminSettings.spec.jsx.snap
+++ b/tests/js/spec/views/admin/__snapshots__/adminSettings.spec.jsx.snap
@@ -53,7 +53,7 @@ exports[`AdminSettings render() renders 1`] = `
         label="Root URL"
         name="system.url-prefix"
         placeholder="https://sentry.example.com"
-        required={true}
+        required={false}
       />
       <EmailField
         allowEmpty={false}