Allow editing plan hooks and mappings only for specific plan's statuses #1394
Conversation
|
cc: @RichardHoch |
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1394 +/- ##
==========================================
- Coverage 36.81% 36.21% -0.60%
==========================================
Files 158 159 +1
Lines 2548 2579 +31
Branches 599 614 +15
==========================================
- Hits 938 934 -4
- Misses 1428 1450 +22
- Partials 182 195 +13 ☔ View full report in Codecov by Sentry. |
| @@ -1,6 +1,6 @@ | |||
| import { V1beta1Plan } from '@kubev2v/types'; | |||
|
|
|||
| export const hasPlanEditable = (plan: V1beta1Plan) => { | |||
| export const hasPlanIncludedCompleteRunningVMs = (plan: V1beta1Plan) => { | |||
There was a problem hiding this comment.
nitpick - naming suggestions: hasASuccessfullyMigratedVM, hasACompleteRunningVM, or hasSomeCompleteRunningVMs
The function is operating on the plan argument, so I don't think plan needs to be part of the function name.
| @@ -74,7 +74,7 @@ | |||
| "Clear all filters": "Clear all filters", | |||
| "Click the pencil for setting provider web UI link": "Click the pencil for setting provider web UI link", | |||
| "Click the update credentials button to save your changes, button is disabled until a change is detected.": "Click the update credentials button to save your changes, button is disabled until a change is detected.", | |||
| "Click the update hooks button to save your changes, button is disabled until a change is detected.": "Click the update hooks button to save your changes, button is disabled until a change is detected.", | |||
| "Click the update hooks button to save your changes, button is disabled until a change is detected or when the plan status does not enable editing.": "Click the update hooks button to save your changes, button is disabled until a change is detected or when the plan status does not enable editing.", | |||
There was a problem hiding this comment.
Do you think it would make sense to display different messages based on the reason the button is disabled instead of explaining both sets of criteria to them since we know when it's disabled due to a status. Ex: Plans in <status> status are not editable or something like that.
sgratch
force-pushed
the
plan-hooks-and-mapping-editing-based-on-plan-status
branch
2 times, most recently
from
ef03be7 to
73b5c6a
Compare
Reference: https://issues.redhat.com/browse/MTV-1713 Enable editing of plan hooks (under Hooks tab) or plan mappings (under Mappings tab) fields only for the following plan statuses: 'Unknown', 'Canceled', 'Error', 'Failed', 'vmError', 'Warning', 'Ready ' Disable the edit option for those Hooks, Mappings tabs fields for all other plan statuses. Signed-off-by: Sharon Gratch <[email protected]>
sgratch
force-pushed
the
plan-hooks-and-mapping-editing-based-on-plan-status
branch
from
73b5c6a to
052d676
Compare
|
|
lgtm. Do we need to wait on the docs team to review the copy? |
If doc team doesn't comment till merging the PR then I prefer not to block by waiting and ping them later on once they document this part or have time. |
Reference: https://issues.redhat.com/browse/MTV-1713
Enable editing of plan hooks (under Hooks tab) or plan mappings (under Mappings tab) fields only for the following plan statuses: '
Unknown', 'Canceled', 'Error', 'Failed', 'vmError', 'Warning', 'Ready'Disable the edit option for those Hooks, Mappings tabs fields for all other plan statuses.
Screenshots
Before - an example for an archived plan
After - an example for an archived plan