upcoming: [M3-7696] - Edit Access key Drawer - Fix Save button is disabled

packages/api-v4/src/object-storage/objectStorageKeys.ts

@@ -1,4 +1,7 @@
-import { createObjectStorageKeysSchema } from '@linode/validation/lib/objectStorageKeys.schema';
+import {
+  createObjectStorageKeysSchema,
+  updateObjectStorageKeysSchema,
+} from '@linode/validation/lib/objectStorageKeys.schema';
 import { API_ROOT } from '../constants';
 import Request, {
   setData,
@@ -51,7 +54,7 @@ export const updateObjectStorageKey = (
   Request<ObjectStorageKey>(
     setMethod('PUT'),
     setURL(`${API_ROOT}/object-storage/keys/${encodeURIComponent(id)}`),
-    setData(data, createObjectStorageKeysSchema)
+    setData(data, updateObjectStorageKeysSchema)
   );
 
 /**

packages/manager/.changeset/pr-10118-upcoming-features-1706544516803.md

@@ -0,0 +1,5 @@
+---
+"@linode/manager": Upcoming Features
+---
+
+Edit Access key Drawer - Fix Save button is disabled. ([#10118](https://github.com/linode/manager/pull/10118))

packages/manager/src/features/ObjectStorage/AccessKeyLanding/AccessKeyDrawer.tsx

@@ -6,7 +6,7 @@ import {
   Scope,
 } from '@linode/api-v4/lib/object-storage';
 import { createObjectStorageKeysSchema } from '@linode/validation/lib/objectStorageKeys.schema';
-import { Formik } from 'formik';
+import { Formik, FormikProps } from 'formik';
 import * as React from 'react';
 
 import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
@@ -33,7 +33,10 @@ export interface AccessKeyDrawerProps {
   // If the mode is 'editing', we should have an ObjectStorageKey to edit
   objectStorageKey?: ObjectStorageKey;
   onClose: () => void;
-  onSubmit: (values: ObjectStorageKeyRequest, formikProps: any) => void;
+  onSubmit: (
+    values: ObjectStorageKeyRequest,
+    formikProps: FormikProps<ObjectStorageKeyRequest>
+  ) => void;
   open: boolean;
 }
 
@@ -120,7 +123,10 @@ export const AccessKeyDrawer = (props: AccessKeyDrawerProps) => {
     label: initialLabelValue,
   };
 
-  const handleSubmit = (values: ObjectStorageKeyRequest, formikProps: any) => {
+  const handleSubmit = (
+    values: ObjectStorageKeyRequest,
+    formikProps: FormikProps<ObjectStorageKeyRequest>
+  ) => {
     // If the user hasn't toggled the Limited Access button,
     // don't include any bucket_access information in the payload.
 

packages/manager/src/features/ObjectStorage/AccessKeyLanding/AccessKeyLanding.tsx

@@ -5,7 +5,7 @@ import {
   revokeObjectStorageKey,
   updateObjectStorageKey,
 } from '@linode/api-v4/lib/object-storage';
-import { FormikBag } from 'formik';
+import { FormikBag, FormikHelpers } from 'formik';
 import * as React from 'react';
 
 import { DocumentTitleSegment } from 'src/components/DocumentTitle';
@@ -96,7 +96,11 @@ export const AccessKeyLanding = (props: Props) => {
 
   const handleCreateKey = (
     values: ObjectStorageKeyRequest,
-    { setErrors, setStatus, setSubmitting }: FormikProps
+    {
+      setErrors,
+      setStatus,
+      setSubmitting,
+    }: FormikHelpers<ObjectStorageKeyRequest>
   ) => {
     // Clear out status (used for general errors)
     setStatus(null);
@@ -152,7 +156,11 @@ export const AccessKeyLanding = (props: Props) => {
 
   const handleEditKey = (
     values: ObjectStorageKeyRequest,
-    { setErrors, setStatus, setSubmitting }: FormikProps
+    {
+      setErrors,
+      setStatus,
+      setSubmitting,
+    }: FormikHelpers<ObjectStorageKeyRequest>
   ) => {
     // This shouldn't happen, but just in case.
     if (!keyToEdit) {
@@ -170,7 +178,7 @@ export const AccessKeyLanding = (props: Props) => {
 
     setSubmitting(true);
 
-    updateObjectStorageKey(keyToEdit.id, { label: values.label })
+    updateObjectStorageKey(keyToEdit.id, values)
       .then((_) => {
         setSubmitting(false);
 

packages/manager/src/features/ObjectStorage/AccessKeyLanding/AccessKeyTable/AccessKeyActionMenu.tsx

@@ -38,7 +38,7 @@ export const AccessKeyActionMenu = ({
       onClick: () => {
         openDrawer('editing', objectStorageKey);
       },
-      title: 'Edit Label',
+      title: isObjMultiClusterEnabled ? 'Edit' : 'Edit Label',
     },
     {
       onClick: () => {

packages/manager/src/features/ObjectStorage/AccessKeyLanding/OMC_AccessKeyDrawer.tsx

@@ -5,9 +5,10 @@ import {
   ObjectStorageKey,
   ObjectStorageKeyRequest,
   Scope,
+  UpdateObjectStorageKeyRequest,
 } from '@linode/api-v4/lib/object-storage';
 import { createObjectStorageKeysSchema } from '@linode/validation/lib/objectStorageKeys.schema';
-import { useFormik } from 'formik';
+import { useFormik, FormikProps } from 'formik';
 import React, { useEffect, useState } from 'react';
 
 import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
@@ -28,14 +29,20 @@ import { confirmObjectStorage } from '../utilities';
 import { AccessKeyRegions } from './AccessKeyRegions/AccessKeyRegions';
 import { LimitedAccessControls } from './LimitedAccessControls';
 import { MODE } from './types';
+import { generateUpdatePayload, hasLabelOrRegionsChanged } from './utils';
 
 export interface AccessKeyDrawerProps {
   isRestrictedUser: boolean;
   mode: MODE;
   // If the mode is 'editing', we should have an ObjectStorageKey to edit
   objectStorageKey?: ObjectStorageKey;
   onClose: () => void;
-  onSubmit: (values: ObjectStorageKeyRequest, formikProps: any) => void;
+  onSubmit: (
+    values: ObjectStorageKeyRequest | UpdateObjectStorageKeyRequest,
+    formikProps: FormikProps<
+      ObjectStorageKeyRequest | UpdateObjectStorageKeyRequest
+    >
+  ) => void;
   open: boolean;
 }
 
@@ -116,10 +123,11 @@ export const OMC_AccessKeyDrawer = (props: AccessKeyDrawerProps) => {
   // and so not included in Formik's types
   const [limitedAccessChecked, setLimitedAccessChecked] = useState(false);
 
-  const title = createMode ? 'Create Access Key' : 'Edit Access Key Label';
+  const title = createMode ? 'Create Access Key' : 'Edit Access Key';
 
   const initialLabelValue =
     !createMode && objectStorageKey ? objectStorageKey.label : '';
+
   const initialRegions =
     !createMode && objectStorageKey
       ? objectStorageKey.regions?.map((region) => region.id)
@@ -148,13 +156,25 @@ export const OMC_AccessKeyDrawer = (props: AccessKeyDrawerProps) => {
           }
         : { ...values, bucket_access: null };
 
-      onSubmit(payload, formik);
+      const updatePayload = generateUpdatePayload(values, initialValues);
+
+      if (mode !== 'creating' && Object.keys(updatePayload).length > 0) {
+        onSubmit(updatePayload, formik);
+      } else {
+        onSubmit(payload, formik);
+      }
     },
     validateOnBlur: true,
     validateOnChange: false,
     validationSchema: createObjectStorageKeysSchema,
   });
 
+  const isSaveDisabled =
+    isRestrictedUser ||
+    (mode !== 'creating' &&
+      objectStorageKey &&
+      !hasLabelOrRegionsChanged(formik.values, objectStorageKey));
+
   const beforeSubmit = () => {
     confirmObjectStorage<FormState>(
       accountSettings?.object_storage || 'active',
@@ -287,10 +307,7 @@ export const OMC_AccessKeyDrawer = (props: AccessKeyDrawerProps) => {
           <ActionsPanel
             primaryButtonProps={{
               'data-testid': 'submit',
-              disabled:
-                isRestrictedUser ||
-                (mode !== 'creating' &&
-                  formik.values.label === initialLabelValue),
+              disabled: isSaveDisabled,
               label: createMode ? 'Create Access Key' : 'Save Changes',
               loading: formik.isSubmitting,
               onClick: beforeSubmit,

packages/manager/src/features/ObjectStorage/AccessKeyLanding/utils.test.ts

@@ -0,0 +1,98 @@
+import { ObjectStorageKey } from '@linode/api-v4/lib/object-storage';
+
+import { FormState } from './OMC_AccessKeyDrawer';
+import { generateUpdatePayload, hasLabelOrRegionsChanged } from './utils';
+
+describe('generateUpdatePayload', () => {
+  const initialValues: FormState = {
+    bucket_access: [],
+    label: 'initialLabel',
+    regions: ['region1', 'region2'],
+  };
+
+  test('should return empty object if no changes', () => {
+    const updatedValues = { ...initialValues };
+    expect(generateUpdatePayload(updatedValues, initialValues)).toEqual({});
+  });
+
+  test('should return updated label if only label changed', () => {
+    const updatedValues = { ...initialValues, label: 'newLabel' };
+    expect(generateUpdatePayload(updatedValues, initialValues)).toEqual({
+      label: 'newLabel',
+    });
+  });
+
+  test('should return updated regions if only regions changed', () => {
+    const updatedValues = { ...initialValues, regions: ['region3', 'region4'] };
+    expect(generateUpdatePayload(updatedValues, initialValues)).toEqual({
+      regions: ['region3', 'region4'],
+    });
+  });
+
+  test('should return updated label and regions if both changed', () => {
+    const updatedValues = {
+      bucket_access: [],
+      label: 'newLabel',
+      regions: ['region3', 'region4'],
+    };
+    expect(generateUpdatePayload(updatedValues, initialValues)).toEqual({
+      label: 'newLabel',
+      regions: ['region3', 'region4'],
+    });
+  });
+});
+
+describe('hasLabelOrRegionsChanged', () => {
+  const updatedValues: FormState = {
+    bucket_access: [],
+    label: 'initialLabel',
+    regions: ['region3', 'region4'],
+  };
+  const initialValues: ObjectStorageKey = {
+    access_key: '',
+    bucket_access: null,
+    id: 0,
+    label: updatedValues.label,
+    limited: false,
+    regions: [
+      { id: 'region3', s3_endpoint: '' },
+      { id: 'region4', s3_endpoint: '' },
+    ],
+
+    secret_key: '',
+  };
+
+  test('returns false when both label and regions are unchanged', () => {
+    expect(hasLabelOrRegionsChanged(updatedValues, initialValues)).toBe(false);
+  });
+
+  test('returns true when only the label has changed', () => {
+    expect(
+      hasLabelOrRegionsChanged(
+        { ...updatedValues, label: 'newLabel' },
+        initialValues
+      )
+    ).toBe(true);
+  });
+
+  test('returns true when only the regions have changed', () => {
+    expect(
+      hasLabelOrRegionsChanged(
+        {
+          ...updatedValues,
+          regions: ['region5'],
+        },
+        initialValues
+      )
+    ).toBe(true);
+  });
+
+  test('returns true when both label and regions have changed', () => {
+    expect(
+      hasLabelOrRegionsChanged(
+        { ...updatedValues, label: 'newLabel', regions: ['region5'] },
+        initialValues
+      )
+    ).toBe(true);
+  });
+});

packages/manager/src/features/ObjectStorage/AccessKeyLanding/utils.ts

@@ -0,0 +1,73 @@
+import { ObjectStorageKey } from '@linode/api-v4/lib/object-storage';
+
+import { areArraysEqual } from 'src/utilities/areArraysEqual';
+import { sortByString } from 'src/utilities/sort-by';
+
+import { FormState } from './OMC_AccessKeyDrawer';
+
+type UpdatePayload =
+  | { label: FormState['label']; regions: FormState['regions'] }
+  | { label: FormState['label'] }
+  | { regions: FormState['regions'] }
+  | {};
+
+const sortRegionOptions = (a: string, b: string) => {
+  return sortByString(a, b, 'asc');
+};
+
+/**
+ * Generates an update payload for edit access key based on changes in form values.
+ *
+ * @param {FormState} updatedValues - The current state of the form.
+ * @param {FormState} initialValues - The initial state of the form for comparison.
+ * @returns An object containing the fields that have changed.
+ */
+export const generateUpdatePayload = (
+  updatedValues: FormState,
+  initialValues: FormState
+): UpdatePayload => {
+  let updatePayload = {};
+
+  const labelChanged = updatedValues.label !== initialValues.label;
+  const regionsChanged = !areArraysEqual(
+    [...updatedValues.regions].sort(sortRegionOptions),
+    [...initialValues.regions].sort(sortRegionOptions)
+  );
+
+  if (labelChanged && regionsChanged) {
+    updatePayload = {
+      label: updatedValues.label,
+      regions: updatedValues.regions,
+    };
+  } else if (labelChanged) {
+    updatePayload = { label: updatedValues.label };
+  } else if (regionsChanged) {
+    updatePayload = { regions: updatedValues.regions };
+  }
+
+  return updatePayload;
+};
+
+/**
+ * Determines if there have been any changes in the label or regions
+ * between the updated form values and the initial values.
+ *
+ * @param {FormState} updatedValues - The current state of the form.
+ * @param {ObjectStorageKey} initialValues - The initial values for comparison.
+ * @returns {boolean} True if there are changes in label or regions, false otherwise.
+ */
+export const hasLabelOrRegionsChanged = (
+  updatedValues: FormState,
+  initialValues: ObjectStorageKey
+): boolean => {
+  const regionsChanged = !areArraysEqual(
+    [...updatedValues.regions].sort(sortRegionOptions),
+    [...initialValues.regions?.map((region) => region.id)].sort(
+      sortRegionOptions
+    )
+  );
+
+  const labelChanged = updatedValues.label !== initialValues.label;
+
+  return labelChanged || regionsChanged;
+};