fix: summary tab is not read-only for kubectl edit command

Fixes #4809

packages/core/src/index.ts

@@ -146,6 +146,7 @@ export {
   Mode as MultiModalMode,
   MultiModalResponse
 } from './models/mmr/types'
+export { Editable, EditableSpec } from './models/editable'
 export {
   Breadcrumb,
   NavResponse,

packages/core/src/models/editable.ts

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2020 IBM Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { ToolbarText } from '../webapp/views/toolbar-text'
+
+export type Editable = { spec: EditableSpec }
+
+export interface EditableSpec {
+  readOnly: boolean
+  clearable: boolean
+  save: {
+    label: string
+    onSave(data: string): Save | Promise<Save>
+  }
+  revert: {
+    label?: string
+    onRevert(): string | Promise<string>
+  }
+}
+
+type Save = void | { noToolbarUpdate?: boolean; toolbarText?: ToolbarText }

packages/core/src/models/mmr/content-types.ts

@@ -22,6 +22,7 @@ import { Entity, MetadataBearing } from '../entity'
 import { isHTML } from '../../util/types'
 import { ModeOrButton, Button } from './types'
 import { ToolbarText } from '../../webapp/views/toolbar-text'
+import { Editable } from '../editable'
 
 /**
  * A `ScalarResource` is Any kind of resource that is directly
@@ -90,7 +91,8 @@ interface WithOptionalContentType<ContentType = SupportedStringContent> {
  *
  */
 export type StringContent<ContentType = SupportedStringContent> = ScalarContent<string> &
-  WithOptionalContentType<ContentType>
+  WithOptionalContentType<ContentType> &
+  Partial<Editable>
 
 export function isStringWithOptionalContentType<T extends MetadataBearing>(
   entity: Entity | Content<T> | MetadataBearing | ModeOrButton<T>

plugins/plugin-client-common/src/components/Content/Editor/index.tsx

@@ -32,26 +32,11 @@ import '../../../../web/scss/components/Editor/Editor.scss'
 
 const strings = i18n('plugin-client-common', 'editor')
 
-interface WithOptions {
-  spec: {
-    readOnly?: boolean
-    clearable?: boolean
-    save?: {
-      label: string
-      onSave: (data: string) => Promise<void | { noToolbarUpdate?: boolean; toolbarText: ToolbarText }>
-    }
-    revert?: {
-      label: string
-      onRevert: () => Promise<string>
-    }
-  }
-}
-
 type Props = MonacoOptions &
   ToolbarProps & {
     repl: REPL
     content: StringContent
-    response: File | (MultiModalResponse & Partial<WithOptions>)
+    response: File | MultiModalResponse
   }
 
 interface State {
@@ -138,7 +123,7 @@ export default class Editor extends React.PureComponent<Props, State> {
   private static isClearable(props: Props) {
     return (
       (isFile(props.response) && !props.readOnly) ||
-      (!isFile(props.response) && props.response.spec && props.response.spec.clearable !== false)
+      (!isFile(props.response) && props.content.spec && props.content.spec.clearable !== false)
     )
   }
 
@@ -158,11 +143,11 @@ export default class Editor extends React.PureComponent<Props, State> {
           }
         })
         buttons.push(save)
-      } else if (props.response.spec && props.response.spec.save) {
-        const { onSave } = props.response.spec.save
+      } else if (props.content.spec && props.content.spec.save) {
+        const { onSave } = props.content.spec.save
         buttons.push({
           mode: 'Save',
-          label: props.response.spec.save.label || strings('saveLocalFile'),
+          label: props.content.spec.save.label || strings('saveLocalFile'),
           kind: 'view' as const,
           command: async () => {
             try {
@@ -203,11 +188,11 @@ export default class Editor extends React.PureComponent<Props, State> {
           }
         })
         buttons.push(revert)
-      } else if (props.response.spec && props.response.spec.revert) {
-        const { onRevert } = props.response.spec.revert
+      } else if (props.content.spec && props.content.spec.revert) {
+        const { onRevert } = props.content.spec.revert
         buttons.push({
           mode: 'Revert',
-          label: props.response.spec.revert.label || strings('revert'),
+          label: props.content.spec.revert.label || strings('revert'),
           kind: 'view' as const,
           command: async () => {
             try {
@@ -259,7 +244,7 @@ export default class Editor extends React.PureComponent<Props, State> {
         value: props.content.content,
         readOnly:
           !isFile(props.response) &&
-          (!props.response.spec || props.response.spec.readOnly !== false) &&
+          (!props.content.spec || props.content.spec.readOnly !== false) &&
           (props.readOnly || !isFile(props.response) || false),
         language:
           props.content.contentType === 'text/plain'

plugins/plugin-client-common/src/components/Content/Eval.tsx

@@ -26,6 +26,7 @@ import {
   isReactProvider,
   isScalarContent,
   ScalarContent,
+  EditableSpec,
   isStringWithOptionalContentType
 } from '@kui-shell/core'
 
@@ -45,6 +46,7 @@ interface EvalState {
   react: ReactProvider
   content: ScalarResource
   contentType: SupportedStringContent
+  spec?: EditableSpec
 }
 
 /**
@@ -69,6 +71,7 @@ export default class Eval extends React.PureComponent<EvalProps, EvalState> {
       isLoading: true,
       command: props.command,
       react: undefined,
+      spec: undefined,
       content: undefined,
       contentType: props.contentType
     }
@@ -98,7 +101,12 @@ export default class Eval extends React.PureComponent<EvalProps, EvalState> {
           if (isReactProvider(content)) {
             this.setState({ isLoading: false, react: content })
           } else if (isStringWithOptionalContentType(content)) {
-            this.setState({ isLoading: false, content: content.content, contentType: content.contentType })
+            this.setState({
+              isLoading: false,
+              content: content.content,
+              contentType: content.contentType,
+              spec: content.spec
+            })
           } else if (isScalarContent(content)) {
             done(content.content)
           } else {
@@ -116,6 +124,7 @@ export default class Eval extends React.PureComponent<EvalProps, EvalState> {
     const mode = this.state.react
       ? this.state.react
       : {
+          spec: this.state.spec,
           content: this.state.content,
           contentType: this.state.contentType
         }

plugins/plugin-kubectl/src/controller/kubectl/edit.ts

@@ -15,7 +15,15 @@
  */
 
 import { v4 as uuid } from 'uuid'
-import { Arguments, MultiModalResponse, Registrar, ExecOptions, i18n } from '@kui-shell/core'
+import {
+  Arguments,
+  MultiModalResponse,
+  Registrar,
+  ExecOptions,
+  i18n,
+  EditableSpec,
+  isStringWithOptionalContentType
+} from '@kui-shell/core'
 
 import flags from './flags'
 import { doExecWithStdout } from './exec'
@@ -24,34 +32,23 @@ import { viewTransformer as getView } from './get'
 import { isUsage, doHelp } from '../../lib/util/help'
 import { KubeOptions, getNamespace } from './options'
 import { KubeResource, isKubeResource, KubeItems, isKubeItems } from '../../lib/model/resource'
+import { label as yamlModeLabel, mode as yamlMode, order as yamlModeOrder } from '../../lib/view/modes/yaml'
 
 const strings = i18n('plugin-kubectl')
 const strings2 = i18n('plugin-client-common', 'editor')
 
-interface EditableSpec {
-  readOnly: boolean
-  clearable: boolean
-  save: {
-    label: string
-    onSave(data: string): Promise<void | { noToolbarUpdate: boolean }>
-  }
-  revert: {
-    onRevert(): string | Promise<string>
-  }
-}
-
-interface Editable extends MultiModalResponse<KubeResource> {
-  spec: EditableSpec
-}
+export function isEditable(resource: KubeResource) {
+  const editable = resource as MultiModalResponse<KubeResource>
+  const editableMode = editable.modes.find(mode => isStringWithOptionalContentType(mode) && mode.spec)
 
-export function isEditable(resource: KubeResource): resource is Editable {
-  const editable = resource as Editable
   return (
-    typeof editable.spec === 'object' &&
-    typeof editable.spec.readOnly === 'boolean' &&
-    typeof editable.spec.clearable === 'boolean' &&
-    typeof editable.spec.save === 'object' &&
-    typeof editable.spec.revert === 'object'
+    editableMode &&
+    isStringWithOptionalContentType(editableMode) &&
+    typeof editableMode.spec === 'object' &&
+    typeof editableMode.spec.readOnly === 'boolean' &&
+    typeof editableMode.spec.clearable === 'boolean' &&
+    typeof editableMode.spec.save === 'object' &&
+    typeof editableMode.spec.revert === 'object'
   )
 }
 
@@ -131,6 +128,7 @@ export function editSpec(
 }
 
 function editMode(
+  spec: EditableSpec,
   resource: KubeResource,
   mode = 'edit',
   label = strings2('Edit'),
@@ -142,6 +140,7 @@ function editMode(
     label,
     order,
     priority,
+    spec,
     contentType: 'yaml',
     content: resource.kuiRawData
   }
@@ -179,7 +178,7 @@ export async function doEdit(cmd: string, args: Arguments<KubeOptions>) {
         namespace
       },
       spec,
-      modes: [editMode(resource)]
+      modes: [editMode(spec, resource)]
     }
     return response
   } else {
@@ -199,20 +198,24 @@ function isEditAfterApply(options: ExecOptions): options is EditAfterApply {
   return opts && opts.data && opts.data.partOfApply !== undefined
 }
 
-export async function editable(cmd: string, args: Arguments<KubeOptions>, response: KubeResource): Promise<Editable> {
+export async function editable(
+  cmd: string,
+  args: Arguments<KubeOptions>,
+  response: KubeResource
+): Promise<MultiModalResponse> {
   const spec = editSpec(cmd, response.metadata.namespace, args, response)
 
   const baseView = await getView(args, response)
+  const baseEditToolbar = {
+    type: 'info',
+    text: strings2('isUpToDate')
+  }
 
   const view = Object.assign(baseView, {
-    spec: Object.assign(response.spec || {}, spec),
+    modes: [editMode(spec, response, yamlMode, yamlModeLabel, yamlModeOrder - 1)], // overwrite the pre-registered yaml tab
     toolbarText: !isEditAfterApply(args.execOptions)
-      ? baseView.toolbarText
-      : {
-          type: baseView.toolbarText.type,
-          text: baseView.toolbarText.text,
-          alerts: [{ type: 'success', title: strings('Successfully Applied') }]
-        }
+      ? response.toolbarText
+      : Object.assign(baseEditToolbar, { alerts: [{ type: 'success', title: strings('Successfully Applied') }] })
   })
 
   return view
@@ -225,7 +228,7 @@ function showingMode(defaultMode: string, resource: MultiModalResponse) {
 /** KubeResource -> MultiModalResponse view transformer */
 export async function viewTransformer(cmd: string, args: Arguments<KubeOptions>, response: KubeResource | KubeItems) {
   if (!isKubeItems(response) && isKubeResource(response)) {
-    return showingMode('raw', await editable(cmd, args, response))
+    return showingMode(yamlMode, await editable(cmd, args, response))
   }
 }
 

plugins/plugin-kubectl/src/test/k8s1/edit.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import * as assert from 'assert'
 import { Common, CLI, ReplExpect, SidecarExpect, Selectors, Keys, Util } from '@kui-shell/test'
 import {
   waitForGreen,
@@ -169,6 +170,9 @@ commands.forEach(command => {
           console.error('1')
           await new Promise(resolve => setTimeout(resolve, 5000))
 
+          // edit button should not exist
+          await this.app.client.waitForVisible(Selectors.SIDECAR_MODE_BUTTON('edit-button'), 5000, true)
+
           // should still be showing pod {name}, but now with the yaml tab selected
           console.error('2')
           await SidecarExpect.showing(name, undefined, undefined, ns)
@@ -191,14 +195,35 @@ commands.forEach(command => {
       validationError(true) // do unsupported edits in the current tab, validate the error alert, and then undo the changes
       modify(name, 'clickfoo3', 'clickbar3') // after error, should re-modify the resource in the current tab successfully
 
-      it('should switch to summary tab and see no alerts', async () => {
+      it('should switch to summary tab, expect no alerts and not editable', async () => {
         try {
           await this.app.client.waitForVisible(Selectors.SIDECAR_MODE_BUTTON('summary'))
           await this.app.client.click(Selectors.SIDECAR_MODE_BUTTON('summary'))
           await this.app.client.waitForVisible(Selectors.SIDECAR_MODE_BUTTON_SELECTED('summary'))
 
           // toolbar alert should not exist
           await this.app.client.waitForExist(Selectors.SIDECAR_ALERT('success'), CLI.waitTimeout, true)
+
+          // edit button should not exist
+          await this.app.client.waitForVisible(Selectors.SIDECAR_MODE_BUTTON('edit-button'), 5000, true)
+
+          // try editing the summary mode
+          const actualText = await Util.getValueFromMonaco(this.app)
+          const labelsLineIdx = actualText.split(/\n/).indexOf('Name:')
+
+          // +2 here because nth-child is indexed from 1, and we want the line after that
+          const lineSelector = `.view-lines > .view-line:nth-child(${labelsLineIdx + 2}) .mtk5:last-child`
+          await this.app.client.click(lineSelector)
+
+          await new Promise(resolve => setTimeout(resolve, 2000))
+          await this.app.client.keys('x') // random key
+          await new Promise(resolve => setTimeout(resolve, 2000))
+
+          // should have same text
+          const actualText2 = await Util.getValueFromMonaco(this.app)
+          assert.ok(actualText === actualText2)
+
+          await this.app.client.waitForVisible(Selectors.SIDECAR_MODE_BUTTON('Save'), 10000, true) // should not have apply button
         } catch (err) {
           await Common.oops(this, true)(err)
         }