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(core): Display template inherited items (mptv2) as read only #7102

Merged
merged 2 commits into from
Jun 11, 2019

Conversation

louisjimenez
Copy link
Contributor

@louisjimenez louisjimenez commented Jun 10, 2019

MPTv1 saved inherited items (notifications, triggers, parameters) directly to the pipeline config which allowed them to be displayed while configuring in deck. MPTv2 pipelines saved in spin do not save them to the pipeline config and instead rely on them being inherited during the plan phase. This PR renders items inherited from the plan and marks them as read only since they cannot be edited or deleted directly through the pipeline config.

spinnaker/spinnaker#4451

Inherited Trigger
image

Inherited Notification
image

Inherited Parameter
image

<div className="checkbox">
<label>
<input type="checkbox" checked={required} onChange={handleRequiredChange} />
<fieldset disabled={inherited} className={classNames({ 'templated-pipeline-item': inherited })}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this large diff comes from indenting pre-existing elements in the added <fieldset> container and from making the delete button conditional to inheritance (line 154)


const configWithInheritedValues = {
...config,
...(inheritTemplateParameters && parameterConfig ? { parameterConfig } : {}),
...(inheritTemplateNotifications && notifications ? { notifications } : {}),
...(inheritTemplateTriggers && triggers ? { triggers } : {}),
...(inheritTemplateExpectedArtifacts && expectedArtifacts ? { expectedArtifacts } : {}),
...(expectedArtifacts ? { expectedArtifacts } : {}),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As of this PR, inheritance of expectedArtifacts changed from opt in to always inherited. Currently only parameters, triggers, and notifications can be marked for exclusion.

<option value="">Select...</option>
</select>
{{description}}
<fieldset ng-disabled="trigger.inherited" ng-class="{'templated-pipeline-item': trigger.inherited}">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to Parameter.tsx changes, much of this is shifting elements and Prettier. Inheritance related changes are on lines 3, 22-32, 55

Copy link
Contributor

@maggieneterval maggieneterval left a comment

Choose a reason for hiding this comment

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

Thanks for the helpful comments, your PRs are always a breeze to review and it also helps keep me in the loop about MPTv2 updates 😀

@louisjimenez louisjimenez merged commit e5d6115 into spinnaker:master Jun 11, 2019
@louisjimenez
Copy link
Contributor Author

@spinnakerbot cherry-pick 1.14

@spinnakerbot
Copy link
Contributor

Cherry pick successful: #7110

alanmquach added a commit to alanmquach/deck that referenced this pull request Jun 18, 2019
939d608 fix(pipeline): fix invisible parameter when default is not in options (spinnaker#7125)
5c4facb fix(core/pipeline): stop searching stage context, being greedy about parentExecutions (spinnaker#7127)
8fe74bc fix(core): do not inject default execution window values on render (spinnaker#7122)
ee6fbe0 fix(core/pipeline): use correct visibility default for stage durations (spinnaker#7121)
c88234c refactor(stages): Fixed alias matching, added fallback and unit tests (spinnaker#7080)
c358801 refactor(core): Reactify ExecutionWindows component (spinnaker#7113)
744123d fix(artifact): use artifact icons in server group link (spinnaker#7118)
b8aebd7 fix(forms): Fixed SpelText not firing onChange upon autocomplete (spinnaker#7114)
4e2f749 refactor(core): reactify overrideFailure component (spinnaker#7107)
4f07aeb fix(core): Make template table list scrollable (spinnaker#7111)
e5d6115 fix(core): Display template inherited items (mptv2) as read only (spinnaker#7102)
anotherchrisberry pushed a commit that referenced this pull request Jun 18, 2019
939d608 fix(pipeline): fix invisible parameter when default is not in options (#7125)
5c4facb fix(core/pipeline): stop searching stage context, being greedy about parentExecutions (#7127)
8fe74bc fix(core): do not inject default execution window values on render (#7122)
ee6fbe0 fix(core/pipeline): use correct visibility default for stage durations (#7121)
c88234c refactor(stages): Fixed alias matching, added fallback and unit tests (#7080)
c358801 refactor(core): Reactify ExecutionWindows component (#7113)
744123d fix(artifact): use artifact icons in server group link (#7118)
b8aebd7 fix(forms): Fixed SpelText not firing onChange upon autocomplete (#7114)
4e2f749 refactor(core): reactify overrideFailure component (#7107)
4f07aeb fix(core): Make template table list scrollable (#7111)
e5d6115 fix(core): Display template inherited items (mptv2) as read only (#7102)
@louisjimenez louisjimenez deleted the louis/mark-inherited branch July 2, 2019 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants