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.

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(zero_value_checks): checks can now have a value set to 0 #17933

Merged
merged 6 commits into from
May 4, 2020

Conversation

asalem1
Copy link
Contributor

@asalem1 asalem1 commented May 1, 2020

Closes #17729

Problem

  1. Checks initialized with the value 0 would not be triggered. When a user would edit their check, the value would return blank
  2. On smaller screens, the Configure A Check options would be off the screen:

Screen Shot 2020-05-01 at 13 30 13

Solution

Updated the threshold value to remove omitEmpty on thresholds since that was removing values set to zero.

As per this helpful resource, go will remove nil values if the flag omitEmpty is present.

It doesn't seem like there was any obvious reason for this to be present since a check should never be created without a value set for the threshold. The UI doesn't allow for this condition, and the API shouldn't allow for this condition.

I added an e2e test to ensure that this bug doesn't pop its head again, and I added an additional check to ensure that we can name & rename checks. I also updated the AlertBuilder.scss to fix weird styling issue

  • CHANGELOG.md updated with a link to the PR (not the Issue)
  • Rebased/mergeable

check

@asalem1 asalem1 requested review from alexpaxton, a team and drdelambre and removed request for a team May 1, 2020 21:04
}
&.alert-builder--conditions-card {
flex: 2 0 320px !important;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexpaxton would love your eyes on this

Copy link
Contributor

@drdelambre drdelambre left a comment

Choose a reason for hiding this comment

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

dope

@asalem1 asalem1 removed the request for review from alexpaxton May 1, 2020 22:01
asalem1 added 6 commits May 3, 2020 06:31
…pty` on thresholds since that was removing values set to zero

Wrote an e2e test to ensure no regressions for this bug
Added a check condition to make sure that naming checks functionality is stable
Updated the AlertBuilder.scss to fix weird styling issue
@asalem1 asalem1 force-pushed the fix/check-zero-bug branch from 72e5af6 to 1d75c06 Compare May 3, 2020 13:31
@asalem1 asalem1 merged commit 2eb70ee into master May 4, 2020
@asalem1 asalem1 deleted the fix/check-zero-bug branch May 4, 2020 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI: Check threshold == 0 impossible
2 participants