Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ensure install wizard passes in common scenarios #12778

Merged
merged 3 commits into from
Apr 19, 2019
Merged

fix: Ensure install wizard passes in common scenarios #12778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6af7cdf
fix: Treat empty/null values as undefined
dcramer Apr 14, 2019
2ff503b
fix: Dont bind disabled fields to initialData
dcramer Apr 14, 2019
5f6952d
fix: Dont trigger upgrade on non-required fields which are empty
dcramer Apr 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/sentry/static/sentry/app/views/installWizard.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,16 @@ export default class InstallWizard extends AsyncView {
const data = {};
Object.keys(options).forEach(optionName => {
const option = options[optionName];
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
if (
option.value !== undefined &&
option.value !== undefined && option.value !== "" && option.value !== null &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So false and 0 are the only valid falsey values?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm not sure - Im not a big fan of the way this works (the install wizard options, forms used here), but its very hard to come up with a "correct" approach that could be achieved in an afternoon.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically this behavior happened with mail.username and mail.password - those will no longer force the install wizard to prompt, it was just something I caught in parallel.

(option.field.isSet || optionName != 'beacon.anonymous')
) {
data[optionName] = option.value;
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/templatetags/sentry_react.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ def _needs_upgrade():

# Check all required options to see if they've been set
for key in options.filter(flag=options.FLAG_REQUIRED):
# ignore required flags which can be empty
if key.flags & options.FLAG_ALLOW_EMPTY:
continue
# Ignore mail.* keys if smtp is disabled
if smtp_disabled and key.name[:5] == 'mail.':
continue
Expand Down