-
Notifications
You must be signed in to change notification settings - Fork 18
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: can create new plans with test plan unchecked #370
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #370 +/- ##
==========================================
- Coverage 88.07% 88.07% -0.01%
==========================================
Files 161 161
Lines 3364 3362 -2
Branches 828 827 -1
==========================================
- Hits 2963 2961 -2
Misses 397 397
Partials 4 4 ☔ View full report in Codecov by Sentry. |
// `internalOnly` backs the "Test plan" checkbox. A true value means the plan will not be customer facing. | ||
internalOnly: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line fixes the bug.
let submittedFormInternalOnly; | ||
if (isEditMode) { | ||
submittedFormInternalOnly = formData.internalOnly; | ||
} | ||
const [value, setValue] = useState(submittedFormInternalOnly || false); | ||
// formData.internalOnly is always defined, whether it's hydrated from an API call (isEditMode = true) or not | ||
// (isEditMode = false) in which case ProvisioningContextProvider supplies a default. | ||
const [value, setValue] = useState(formData.internalOnly || false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we can assume that formData.internalOnly is always defined, we can kill the if-condition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏽 Great job
Somehow we let a regression slip through manual QA testing and unit testing which prevented non-test plans from being created. The provisioning tool would POST to create a subsidy, but failed to supply a
default_internal_only
request parameter, which is required. This change adds a default value for formData.internalOnly on context initialization to prevent the field from ever being empty.ENT-8107
testing done