"Source model format" and "source model format version" should be edi…

…table (#3369)

* working version

* tests - not working version

* tests - not working version2

* tests - not working version3

* added test-ids

* removed un needed

* removed un neede

* updated the intercept

* new Component

* new Component584619

* final working version

* lintt

* final

frontend/src/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDetails.ts

@@ -24,6 +24,14 @@ class ModelVersionDetails {
     return cy.findByTestId('model-version-description');
   }
 
+  findSourceModelFormat(subComponent: 'group' | 'edit' | 'save' | 'cancel') {
+    return cy.findByTestId(`source-model-format-${subComponent}`);
+  }
+
+  findSourceModelVersion(subComponent: 'group' | 'edit' | 'save' | 'cancel') {
+    return cy.findByTestId(`source-model-version-${subComponent}`);
+  }
+
   findMoreLabelsButton() {
     return cy.findByTestId('label-group').find('button');
   }

frontend/src/__tests__/cypress/cypress/support/commands/odh.ts

@@ -642,6 +642,13 @@ declare global {
         ((
           type: 'POST /api/modelRegistryRoleBindings',
           response: OdhResponse<RoleBindingKind>,
+        ) => Cypress.Chainable<null>) &
+        ((
+          type: 'PATCH /api/service/modelregistry/:serviceName/api/model_registry/:apiVersion/model_artifacts/:artifactId',
+          options: {
+            path: { serviceName: string; apiVersion: string; artifactId: string };
+          },
+          response: OdhResponse<ModelArtifact>,
         ) => Cypress.Chainable<null>);
     }
   }

frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDetails.cy.ts

@@ -13,6 +13,7 @@ import {
   mockProjectK8sResource,
   mockDscStatus,
 } from '~/__mocks__';
+import { mockModelArtifact } from '~/__mocks__/mockModelArtifact';
 
 import {
   InferenceServiceModel,
@@ -383,4 +384,63 @@ describe('Model version details', () => {
       modelServingGlobal.getModelRow('Test Inference Service').should('exist');
     });
   });
+
+  describe('Model Version Details', () => {
+    beforeEach(() => {
+      initIntercepts();
+      modelVersionDetails.visit();
+    });
+
+    it('should update source model format', () => {
+      cy.interceptOdh(
+        'PATCH /api/service/modelregistry/:serviceName/api/model_registry/:apiVersion/model_artifacts/:artifactId',
+        {
+          path: {
+            serviceName: 'modelregistry-sample',
+            apiVersion: MODEL_REGISTRY_API_VERSION,
+            artifactId: '1',
+          },
+        },
+        mockModelArtifact({}),
+      ).as('updateModelFormat');
+
+      modelVersionDetails.findSourceModelFormat('edit').click();
+      modelVersionDetails
+        .findSourceModelFormat('group')
+        .find('input')
+        .clear()
+        .type('UpdatedFormat');
+      modelVersionDetails.findSourceModelFormat('save').click();
+
+      cy.wait('@updateModelFormat').then((interception) => {
+        expect(interception.request.body).to.deep.equal({
+          modelFormatName: 'UpdatedFormat',
+        });
+      });
+    });
+
+    it('should update source model version', () => {
+      cy.interceptOdh(
+        'PATCH /api/service/modelregistry/:serviceName/api/model_registry/:apiVersion/model_artifacts/:artifactId',
+        {
+          path: {
+            serviceName: 'modelregistry-sample',
+            apiVersion: MODEL_REGISTRY_API_VERSION,
+            artifactId: '1',
+          },
+        },
+        mockModelArtifact({}),
+      ).as('updateModelVersion');
+
+      modelVersionDetails.findSourceModelVersion('edit').click();
+      modelVersionDetails.findSourceModelVersion('group').find('input').clear().type('2.0.0');
+      modelVersionDetails.findSourceModelVersion('save').click();
+
+      cy.wait('@updateModelVersion').then((interception) => {
+        expect(interception.request.body).to.deep.equal({
+          modelFormatVersion: '2.0.0',
+        });
+      });
+    });
+  });
 });

frontend/src/components/DashboardDescriptionListGroup.scss

@@ -1,4 +1,4 @@
 .odh-custom-description-list-term-with-action > span {
   /* Workaround for missing functionality in PF DescriptionList, see https://github.com/patternfly/patternfly/issues/6583 */
   width: 100%;
-}
+}

frontend/src/components/DashboardDescriptionListGroup.tsx

@@ -23,6 +23,9 @@ type EditableProps = {
   onEditClick: () => void;
   onSaveEditsClick: () => void;
   onDiscardEditsClick: () => void;
+  editButtonTestId?: string;
+  saveButtonTestId?: string;
+  cancelButtonTestId?: string;
 };
 
 export type DashboardDescriptionListGroupProps = {
@@ -32,6 +35,7 @@ export type DashboardDescriptionListGroupProps = {
   isEmpty?: boolean;
   contentWhenEmpty?: React.ReactNode;
   children: React.ReactNode;
+  groupTestId?: string;
 } & (({ isEditable: true } & EditableProps) | ({ isEditable?: false } & Partial<EditableProps>));
 
 const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps> = (props) => {
@@ -49,9 +53,13 @@ const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps
     onSaveEditsClick,
     onDiscardEditsClick,
     children,
+    groupTestId,
+    editButtonTestId,
+    saveButtonTestId,
+    cancelButtonTestId,
   } = props;
   return (
-    <DescriptionListGroup>
+    <DescriptionListGroup data-testid={groupTestId}>
       {action || isEditable ? (
         <DescriptionListTerm className="odh-custom-description-list-term-with-action">
           <Split>
@@ -62,7 +70,7 @@ const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps
                   <ActionList isIconList>
                     <ActionListItem>
                       <Button
-                        data-testid={`save-edit-button-${title}`}
+                        data-testid={saveButtonTestId}
                         aria-label={`Save edits to ${title}`}
                         variant="link"
                         onClick={onSaveEditsClick}
@@ -73,7 +81,7 @@ const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps
                     </ActionListItem>
                     <ActionListItem>
                       <Button
-                        data-testid={`discard-edit-button-${title}`}
+                        data-testid={cancelButtonTestId}
                         aria-label={`Discard edits to ${title} `}
                         variant="plain"
                         onClick={onDiscardEditsClick}
@@ -85,7 +93,7 @@ const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps
                   </ActionList>
                 ) : (
                   <Button
-                    data-testid={`edit-button-${title}`}
+                    data-testid={editButtonTestId}
                     aria-label={`Edit ${title}`}
                     isInline
                     variant="link"

frontend/src/components/EditableTextDescriptionListGroup.tsx

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { ExpandableSection, TextArea } from '@patternfly/react-core';
+import { ExpandableSection, TextArea, TextInput } from '@patternfly/react-core';
 import DashboardDescriptionListGroup, {
   DashboardDescriptionListGroupProps,
 } from './DashboardDescriptionListGroup';
@@ -10,8 +10,9 @@ type EditableTextDescriptionListGroupProps = Pick<
 > & {
   value: string;
   saveEditedValue: (value: string) => Promise<void>;
-  testid?: string;
+  baseTestId?: string;
   isArchive?: boolean;
+  editableVariant: 'TextInput' | 'TextArea';
 };
 
 const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGroupProps> = ({
@@ -20,7 +21,8 @@ const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGrou
   value,
   isArchive,
   saveEditedValue,
-  testid,
+  baseTestId,
+  editableVariant,
 }) => {
   const [isEditing, setIsEditing] = React.useState(false);
   const [unsavedValue, setUnsavedValue] = React.useState(value);
@@ -34,17 +36,32 @@ const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGrou
       isEditable={!isArchive}
       isEditing={isEditing}
       isSavingEdits={isSavingEdits}
+      groupTestId={`${baseTestId}-group`}
+      editButtonTestId={`${baseTestId}-edit`}
+      saveButtonTestId={`${baseTestId}-save`}
+      cancelButtonTestId={`${baseTestId}-cancel`}
       contentWhenEditing={
-        <TextArea
-          autoFocus
-          data-testid={`edit-text-area-${title}`}
-          aria-label={`Text box for editing ${title}`}
-          value={unsavedValue}
-          onChange={(_event, v) => setUnsavedValue(v)}
-          isDisabled={isSavingEdits}
-          rows={24}
-          resizeOrientation="vertical"
-        />
+        editableVariant === 'TextInput' ? (
+          <TextInput
+            autoFocus
+            data-testid={`${baseTestId}-input`}
+            aria-label={`Text input for editing ${title}`}
+            value={unsavedValue}
+            onChange={(_event, v) => setUnsavedValue(v)}
+            isDisabled={isSavingEdits}
+          />
+        ) : (
+          <TextArea
+            autoFocus
+            data-testid={`${baseTestId}-input`}
+            aria-label={`Text box for editing ${title}`}
+            value={unsavedValue}
+            onChange={(_event, v) => setUnsavedValue(v)}
+            isDisabled={isSavingEdits}
+            rows={24}
+            resizeOrientation="vertical"
+          />
+        )
       }
       onEditClick={() => {
         setUnsavedValue(value);
@@ -65,7 +82,7 @@ const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGrou
       }}
     >
       <ExpandableSection
-        data-testid={testid}
+        data-testid={baseTestId}
         variant="truncate"
         truncateMaxLines={12}
         toggleText={isTextExpanded ? 'Show less' : 'Show more'}

frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx

@@ -24,9 +24,11 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
   isArchiveVersion,
   refresh,
 }) => {
-  const [modelArtifact] = useModelArtifactsByVersionId(mv.id);
+  // TODO handle loading state / error for artifacts here?
+  const [modelArtifacts, , , refreshModelArtifacts] = useModelArtifactsByVersionId(mv.id);
+  const modelArtifact = modelArtifacts.items.length ? modelArtifacts.items[0] : null;
   const { apiState } = React.useContext(ModelRegistryContext);
-  const storageFields = uriToObjectStorageFields(modelArtifact.items[0]?.uri || '');
+  const storageFields = uriToObjectStorageFields(modelArtifact?.uri || '');
 
   return (
     <Flex
@@ -37,7 +39,8 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
       <FlexItem flex={{ default: 'flex_1' }}>
         <DescriptionList isFillColumns>
           <EditableTextDescriptionListGroup
-            testid="model-version-description"
+            editableVariant="TextArea"
+            baseTestId="model-version-description"
             isArchive={isArchiveVersion}
             title="Description"
             contentWhenEmpty="No description"
@@ -99,7 +102,7 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
             <>
               <DashboardDescriptionListGroup
                 title="Endpoint"
-                isEmpty={modelArtifact.size === 0 || !storageFields.endpoint}
+                isEmpty={modelArtifacts.size === 0 || !storageFields.endpoint}
                 contentWhenEmpty="No endpoint"
               >
                 <InlineTruncatedClipboardCopy
@@ -109,7 +112,7 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
               </DashboardDescriptionListGroup>
               <DashboardDescriptionListGroup
                 title="Region"
-                isEmpty={modelArtifact.size === 0 || !storageFields.region}
+                isEmpty={modelArtifacts.size === 0 || !storageFields.region}
                 contentWhenEmpty="No region"
               >
                 <InlineTruncatedClipboardCopy
@@ -119,7 +122,7 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
               </DashboardDescriptionListGroup>
               <DashboardDescriptionListGroup
                 title="Bucket"
-                isEmpty={modelArtifact.size === 0 || !storageFields.bucket}
+                isEmpty={modelArtifacts.size === 0 || !storageFields.bucket}
                 contentWhenEmpty="No bucket"
               >
                 <InlineTruncatedClipboardCopy
@@ -129,7 +132,7 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
               </DashboardDescriptionListGroup>
               <DashboardDescriptionListGroup
                 title="Path"
-                isEmpty={modelArtifact.size === 0 || !storageFields.path}
+                isEmpty={modelArtifacts.size === 0 || !storageFields.path}
                 contentWhenEmpty="No path"
               >
                 <InlineTruncatedClipboardCopy
@@ -143,12 +146,12 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
             <>
               <DashboardDescriptionListGroup
                 title="URI"
-                isEmpty={modelArtifact.size === 0 || !modelArtifact.items[0].uri}
+                isEmpty={modelArtifacts.size === 0 || !modelArtifact?.uri}
                 contentWhenEmpty="No URI"
               >
                 <InlineTruncatedClipboardCopy
                   testId="storage-uri"
-                  textToCopy={modelArtifact.items[0]?.uri || ''}
+                  textToCopy={modelArtifact?.uri || ''}
                 />
               </DashboardDescriptionListGroup>
             </>
@@ -158,20 +161,36 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
           Source model format
         </Title>
         <DescriptionList isFillColumns>
-          <DashboardDescriptionListGroup
-            title="Name"
-            isEmpty={modelArtifact.size === 0 || !modelArtifact.items[0].modelFormatName}
-            contentWhenEmpty="No source model format"
-          >
-            {modelArtifact.items[0]?.modelFormatName}
-          </DashboardDescriptionListGroup>
-          <DashboardDescriptionListGroup
+          <EditableTextDescriptionListGroup
+            editableVariant="TextInput"
+            baseTestId="source-model-format"
+            isArchive={isArchiveVersion}
+            value={modelArtifact?.modelFormatName || ''}
+            saveEditedValue={(value) =>
+              apiState.api
+                .patchModelArtifact({}, { modelFormatName: value }, modelArtifact?.id || '')
+                .then(() => {
+                  refreshModelArtifacts();
+                })
+            }
+            title="Model Format"
+            contentWhenEmpty="No model format specified"
+          />
+          <EditableTextDescriptionListGroup
+            editableVariant="TextInput"
+            baseTestId="source-model-version"
+            value={modelArtifact?.modelFormatVersion || ''}
+            isArchive={isArchiveVersion}
+            saveEditedValue={(newVersion) =>
+              apiState.api
+                .patchModelArtifact({}, { modelFormatVersion: newVersion }, modelArtifact?.id || '')
+                .then(() => {
+                  refreshModelArtifacts();
+                })
+            }
             title="Version"
-            isEmpty={modelArtifact.size === 0 || !modelArtifact.items[0].modelFormatVersion}
             contentWhenEmpty="No source model format version"
-          >
-            {modelArtifact.items[0]?.modelFormatVersion}
-          </DashboardDescriptionListGroup>
+          />
         </DescriptionList>
         <DescriptionList isFillColumns style={{ marginTop: '2em' }}>
           <DashboardDescriptionListGroup

frontend/src/pages/modelRegistry/screens/ModelVersions/ModelDetailsView.tsx

@@ -30,6 +30,7 @@ const ModelDetailsView: React.FC<ModelDetailsViewProps> = ({
       <FlexItem flex={{ default: 'flex_1' }}>
         <DescriptionList isFillColumns>
           <EditableTextDescriptionListGroup
+            editableVariant="TextArea"
             title="Description"
             isArchive={isArchiveModel}
             contentWhenEmpty="No description"