diff --git a/src/sentry/static/sentry/app/options.jsx b/src/sentry/static/sentry/app/options.jsx index 7c0309de8ee207..f6c6253b0cce09 100644 --- a/src/sentry/static/sentry/app/options.jsx +++ b/src/sentry/static/sentry/app/options.jsx @@ -171,6 +171,11 @@ export function getOption(option) { return definitionsMap[option]; } +export function getOptionDefault(option) { + const meta = getOption(option); + return meta.defaultValue ? meta.defaultValue() : undefined; +} + function optionsForSection(section) { return definitions.filter(option => option.key.split('.')[0] === section.key); } @@ -183,7 +188,7 @@ export function getOptionField(option, field) { {...meta} name={option} key={option} - defaultValue={meta.defaultValue ? meta.defaultValue() : undefined} + defaultValue={getOptionDefault(option)} required={meta.required && !meta.allowEmpty} 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..c636367b7398f1 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} from 'app/options'; export default class InstallWizard extends AsyncView { static propTypes = { @@ -64,18 +64,21 @@ 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(optionName); 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' && + // 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; 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); }); });