Skip to content

Conversation

Copy link

vercel bot commented Nov 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
planet-webapp-multi-tenancy-setup ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 12, 2024 6:51am
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
planet-webapp ⬜️ Ignored (Inspect) Dec 12, 2024 6:51am
planet-webapp-temp ⬜️ Ignored (Inspect) Dec 12, 2024 6:51am

Copy link
Contributor

coderabbitai bot commented Nov 5, 2024

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

npm warn config production Use --omit=dev instead.
npm error code ERESOLVE
npm error ERESOLVE could not resolve
npm error
npm error While resolving: eslint-config-airbnb@18.2.1
npm error Found: eslint@8.57.1
npm error node_modules/eslint
npm error dev eslint@"^8.26.0" from the root project
npm error peer eslint@"6 || 7 || 8" from @emotion/eslint-plugin@11.12.0
npm error node_modules/@emotion/eslint-plugin
npm error dev @emotion/eslint-plugin@"^11.12.0" from the root project
npm error 15 more (@eslint-community/eslint-utils, ...)
npm error
npm error Could not resolve dependency:
npm error peer eslint@"^5.16.0 || ^6.8.0 || ^7.2.0" from eslint-config-airbnb@18.2.1
npm error node_modules/eslint-config-airbnb
npm error dev eslint-config-airbnb@"^18.2.0" from the root project
npm error
npm error Conflicting peer dependency: eslint@7.32.0
npm error node_modules/eslint
npm error peer eslint@"^5.16.0 || ^6.8.0 || ^7.2.0" from eslint-config-airbnb@18.2.1
npm error node_modules/eslint-config-airbnb
npm error dev eslint-config-airbnb@"^18.2.0" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution.
npm error
npm error
npm error For a full report see:
npm error /root/.npm/_logs/2024-12-12T06_47_10_765Z-eresolve-report.txt
npm error A complete log of this run can be found in: /root/.npm/_logs/2024-12-12T06_47_10_765Z-debug-0.log

Walkthrough

The changes in this pull request encompass multiple files, primarily focusing on enhancing type safety through type imports and modifications to address management components. Key updates include reorganizing type imports, implementing optional chaining in functions, and introducing new components for address management, such as adding, editing, and deleting addresses. Additionally, several components related to address input and display have been created, ensuring a structured and user-friendly interface for managing user addresses.

Changes

File Change Summary
pages/sites/[slug]/[locale]/profile/edit.tsx Reorganized type imports, marked imports as types, updated getStaticProps to include profile in filenames, and modified getStaticPaths to use optional chaining for subDomainPaths.
public/assets/images/icons/KebabMenuIcon.tsx Introduced a new KebabMenuIcon functional component that renders an SVG graphic of three vertically aligned rectangles.
src/features/common/InputTypes/AutoCompleteCountry.tsx Reorganized type imports, removed unnecessary imports, and updated countries.sort to cast code to Lowercase<CountryCode> for consistency in translation.
src/features/common/types/profile.d.ts Converted certain imports to type-only imports and introduced a new type export AddressAction derived from ADDRESS_ACTIONS.
src/features/user/Profile/ForestProgress/ForestProgress.module.scss Added transform: translate(-50%, -50%); to .targetModalMainContainer for centering the modal.
src/features/user/Settings/EditProfile/EditProfileForm.tsx Removed address handling and country selection features, including associated state variables and methods, simplifying the form to focus on user profile information.
src/features/user/Settings/EditProfile/index.tsx Updated return type of EditProfile function for specificity and added <AddressManagement /> component to JSX structure.
src/utils/addressManagement.ts Introduced new constants and functions for address management, including getFormattedAddress, suggestAddress, and fetchAddressDetails, along with validation patterns and geocoding setup.
src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx Introduced a new component for adding addresses, managing form state, and handling API requests for address addition.
src/features/user/Settings/EditProfile/AddressManagement/AddressManagement.module.scss Created a new SCSS module for address management styles, defining mixins and various classes for layout and presentation.
src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx Introduced a new component for managing the deletion of user addresses, including API request handling and modal management.
src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx Introduced a new component for editing user addresses, managing form state and API requests for address updates.
src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx Introduced a new component for unsetting billing addresses, managing API requests and modal state.
src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx Introduced a new component for updating address types, managing user interactions and API requests.
src/features/user/Settings/EditProfile/AddressManagement/index.tsx Introduced a new component for managing user addresses, including fetching from API and managing address actions.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx Introduced a new component for managing address actions within a popover menu, including user interactions and rendering conditions.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressDetails.tsx Introduced a new component for displaying address details, utilizing translations for address types.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressForm.tsx Introduced a new component for managing address input within a form, integrating validation and suggestion functionalities.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormButtons.tsx Introduced a new component for rendering form buttons, handling submission and cancellation actions.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormLayout.tsx Introduced a new layout component for address forms, managing structure and styling.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressInput.tsx Introduced a new component for address input management, integrating with react-hook-form for validation and suggestions.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressList.tsx Introduced a new component for displaying a list of addresses, managing state and rendering individual address components.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/FormattedAddressBlock.tsx Introduced a new component for displaying formatted address information, utilizing translations and memoization for performance.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/SingleAddress.tsx Introduced a new component for managing and displaying a single address, integrating address details and action menu.

Possibly related PRs

  • Address mgmt/separate translation #2323: Modifications to the same edit.tsx file, focusing on type imports and optional chaining, closely aligning with the changes made in this PR regarding type imports and the getStaticProps function.

🐇 Hopping through the code with glee,
New addresses to manage, oh what a spree!
With forms and buttons, all in a row,
Our users will love the smooth flow!
From edits to deletes, we’ve got it all,
A rabbit’s delight, we’ll never fall! 🐰✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

…date hover effect styling for kabab menu button
- Extract duplicated address input logic into a shared component
- Add TypeScript interfaces for improved type safety
- Implement flexible validation through props
- Maintain compatibility with react-hook-form and Material-UI
- Reduce code duplication in address form implementations.
- add address type and action constant definitions
- Implement address editing functionality with error handling
- add TypeScript type definitions for address operations
- add delete functionality for address entries
- move isUploadingData state to parent component
- refactor child components to use shared loading state
@mariahosfeld
Copy link
Contributor

If "Billing Address" has been set, the option to remove this label needs to be included in the options shown in the included image. Text: "Remove Billing Address"
image

@mohitb35
Copy link
Collaborator Author

If "Billing Address" has been set, the option to remove this label needs to be included in the options shown in the included image. Text: "Remove Billing Address" image

#2330 resolves this

…emove-billing-address-option

Address mgmt/remove billing address option
@mohitb35 mohitb35 requested a review from sunilsabatp December 11, 2024 11:44
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Outside diff range and nitpick comments (5)
src/features/user/Settings/EditProfile/AddressManagment/UnsetBillingAddress.tsx (1)

39-61: Add error message translation and improve error handling

The error handling could be improved by:

  1. Using translated error messages
  2. Adding specific error handling for network issues
  3. Adding retry logic for transient failures

Consider implementing the following improvements:

  } catch (error) {
-   setErrors(handleError(error as APIError));
+   const apiError = error as APIError;
+   setErrors([
+     {
+       message: tAddressManagement('errors.unsetBillingAddress') || apiError.message,
+       code: apiError.code
+     }
+   ]);
+   // Optionally add retry logic here
  } finally {
src/features/user/Settings/EditProfile/AddressManagment/index.tsx (2)

52-65: Add error retry mechanism for address fetching

The updateUserAddresses function should include a retry mechanism for transient failures.

  const updateUserAddresses = useCallback(async () => {
    if (!user || !token || !contextLoaded) return;
+   const MAX_RETRIES = 3;
+   let retries = 0;
+   
+   const fetchWithRetry = async (): Promise<Address[] | null> => {
+     try {
+       return await getAuthenticatedRequest<Address[]>(
+         tenantConfig.id,
+         '/app/addresses',
+         token,
+         logoutUser
+       );
+     } catch (error) {
+       if (retries < MAX_RETRIES && isRetryableError(error)) {
+         retries++;
+         await new Promise(resolve => setTimeout(resolve, 1000 * retries));
+         return fetchWithRetry();
+       }
+       throw error;
+     }
+   };

    try {
-     const res = await getAuthenticatedRequest<Address[]>(
-       tenantConfig.id,
-       '/app/addresses',
-       token,
-       logoutUser
-     );
+     const res = await fetchWithRetry();
      if (res) setUserAddresses(res);
    } catch (error) {
      setErrors(handleError(error as APIError));
    }
  }, [user, token, contextLoaded, tenantConfig.id, logoutUser]);

80-149: Consider extracting modal content mapping to a separate component

The renderModalContent logic is complex and could be extracted to improve maintainability.

Consider creating a separate AddressActionModal component to handle the modal content rendering based on the action type. This would:

  1. Reduce the complexity of the main component
  2. Make the code more maintainable
  3. Improve testability
  4. Make it easier to add new action types in the future

Example structure:

interface AddressActionModalProps {
  action: AddressAction;
  props: {
    setIsModalOpen: SetState<boolean>;
    setAddressAction: SetState<AddressAction | null>;
    selectedAddressForAction?: Address | null;
    updateUserAddresses?: () => Promise<void>;
    // ... other props
  };
}

const AddressActionModal: React.FC<AddressActionModalProps> = ...
src/utils/addressManagement.ts (2)

53-58: Consider handling the 'other' address type in findAddressByType function

The findAddressByType function currently handles only 'primary' and 'mailing' address types. Since ADDRESS_TYPE includes 'other', consider updating the function to handle the 'other' type or clarify if this exclusion is intentional.

Modify the function to accept all address types:

 export const findAddressByType = (
   addresses: Address[],
-  addressType: 'primary' | 'mailing'
+  addressType: keyof typeof ADDRESS_TYPE
 ) => {
   return addresses.find((address) => address.type === addressType);
 };

87-98: Use appropriate logging methods for error handling

Currently, errors are logged using console.log(error);. It's recommended to use console.error(error); for logging errors, which aids in distinguishing error messages from general logs.

Apply this diff to update the error logging:

 // In suggestAddress function
 } catch (error) {
-  console.log(error);
+  console.error(error);
   return [];
 }

 // In fetchAddressDetails function
 } catch (error) {
-  console.log(error);
+  console.error(error);
   return null;
 }

Also applies to: 122-138

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4b28506 and 9c9ac78.

⛔ Files ignored due to path filters (1)
  • public/static/locales/en/editProfile.json is excluded by !**/*.json
📒 Files selected for processing (4)
  • src/features/user/Settings/EditProfile/AddressManagment/UnsetBillingAddress.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagment/index.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagment/microComponents/AddressActionMenu.tsx (1 hunks)
  • src/utils/addressManagement.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/features/user/Settings/EditProfile/AddressManagment/index.tsx

[error] 183-183: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

🔇 Additional comments (4)
src/utils/addressManagement.ts (4)

34-46: Approved: getFormattedAddress function is well-implemented

The getFormattedAddress function correctly formats address components, handling undefined and null values appropriately.


82-101: Approved: suggestAddress function logic is correct

The suggestAddress function effectively retrieves address suggestions and handles errors by returning an empty array when necessary.


114-138: Approved: fetchAddressDetails function retrieves address details accurately

The fetchAddressDetails function properly fetches detailed address information and gracefully handles cases when no candidates are found.


60-67: Verify initialization of geocoder without credentials

When initializing the geocoder, if process.env.ESRI_CLIENT_SECRET is not set, an empty options object {} is used. Ensure that the geocoder-arcgis library functions correctly without credentials and that the application handles missing or invalid credentials gracefully.

Run the following script to check the behavior when credentials are missing:

Copy link
Contributor

@mariahosfeld mariahosfeld left a comment

Choose a reason for hiding this comment

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

Functionality looks good.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 15

🧹 Outside diff range and nitpick comments (24)
src/features/user/Settings/EditProfile/AddressManagement/AddressManagement.module.scss (5)

9-17: Consider enhancing the modal-container mixin for better reusability and compatibility.

The modal-container mixin could be improved in several ways:

 @mixin modal-container {
   position: absolute;
   left: 50%;
   top: 50%;
-  transform: translate(-50%, -50%);
+  transform: translate(-50%, -50%);
+  -webkit-transform: translate(-50%, -50%);
   padding: 20px;
   border-radius: 16px;
-  background: rgba(255, 255, 255, 1);
+  background: #ffffff;
+  z-index: 1000; // Ensure modal appears above other content
 }

101-120: Consider responsive design for address actions menu.

The fixed min-width: 184px might cause overflow issues on smaller screens. Consider using a relative width or adding a mobile breakpoint.

 .addressActions {
-  min-width: 184px;
+  min-width: min(184px, 50vw);
   cursor: pointer;
   padding: 2px 12px;
   list-style: none;
 }

128-148: Improve responsive design approach for modal forms.

Instead of relying on breakpoint overrides, consider using a more fluid approach from the start:

 .addressFormLayout {
   @include flex-container(column);
   @include modal-container;
-  min-width: 565px;
+  width: min(565px, 95vw);
+  max-height: 90vh;
+  overflow-y: auto;
   gap: 40px;
 }

177-200: Improve organization of mobile styles.

Consider using CSS custom properties for repeated values and consolidating mobile styles:

+:root {
+  --mobile-container-width: 95%;
+  --mobile-gap: 8px;
+}

 @include xsPhoneView {
   .addressFormLayout {
     min-width: fit-content;
-    width: 95%;
+    width: var(--mobile-container-width);
   }

   .addressActionContainer {
-    width: 95%;
+    width: var(--mobile-container-width);
     min-width: fit-content;

     .buttonContainer {
       flex-direction: column;
-      gap: 8px;
+      gap: var(--mobile-gap);
     }
   }
 }

203-205: Enhance spinner positioning.

Consider adding alignment properties for better centering:

 .addressMgmtSpinner {
   @include flex-container(row, center);
+  align-items: center;
+  min-height: 100px; // Provide space for spinner
 }
src/features/user/Settings/EditProfile/AddressManagement/index.tsx (1)

154-155: Use userAddresses.length to determine if the address list should be rendered.

Currently, shouldRenderAddressList relies on user?.addresses, but since userAddresses is your state variable that may have been updated independently, it's more consistent to use userAddresses.length > 0.

Apply this diff to update the condition:

-const shouldRenderAddressList =
-  user?.addresses !== undefined && user.addresses.length > 0;
+const shouldRenderAddressList = userAddresses.length > 0;
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressForm.tsx (1)

79-79: Remove unnecessary geocoder dependency from useCallback hooks.

The geocoder variable is imported but not directly used within the handleSuggestAddress and handleGetAddress functions. Including it in the dependency array is unnecessary and could cause unwanted re-renders.

Apply this diff to update the dependency arrays:

// For handleSuggestAddress
- [geocoder, country]
+ [country]

// For handleGetAddress
- [geocoder, setValue]
+ [setValue]

Also applies to: 102-102

src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressDetails.tsx (1)

16-19: Ensure type is a valid class name before using it in className.

Using type directly in className={styles[type]} could pose risks if type contains unexpected values. To prevent potential issues, consider validating type against a list of expected values or mapping it to a specific class name.

Example modification:

+ const validTypes = ['primary', 'mailing', 'other'];
+ const addressTypeClass = validTypes.includes(type) ? styles[type] : '';

return (
  <div className={styles.addressDetails}>
    {type !== 'other' && (
-     <span className={styles[type]}>
+     <span className={addressTypeClass}>
        {tAddressManagement(`addressType.${type}`)}
      </span>
    )}
    <FormattedAddressBlock userAddress={userAddress} />
  </div>
);
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormButtons.tsx (1)

5-9: Consider adding loading and disabled states to Props interface

The form buttons could benefit from additional states to handle form submission:

  • Loading state for the submit button
  • Disabled state for both buttons during submission
 interface Props {
   text: string;
   handleSubmit: () => void;
   handleCancel: () => void;
+  isSubmitting?: boolean;
 }
src/features/user/Settings/EditProfile/AddressManagement/microComponents/FormattedAddressBlock.tsx (1)

23-23: Improve null/empty string handling for address2

The current null check for address2 doesn't handle empty strings. Consider using a more robust check.

-      {address2 !== null && <p>{address2}</p>}
+      {address2 && address2.trim() !== '' && <p>{address2}</p>}
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressList.tsx (1)

8-13: Consider adding loading state to Props interface

The component could benefit from a loading state to show a skeleton loader while fetching addresses.

 interface Props {
   addresses: Address[];
   setAddressAction: SetState<AddressAction | null>;
   setSelectedAddressForAction: SetState<Address | null>;
   setIsModalOpen: SetState<boolean>;
+  isLoading?: boolean;
 }
src/features/user/Settings/EditProfile/AddressManagement/microComponents/SingleAddress.tsx (1)

17-36: Consider memoizing the component for performance optimization

The component is well-structured but could benefit from memoization since it receives multiple callback functions as props.

-const SingleAddress = ({
+const SingleAddress = React.memo(({
   userAddress,
   addressCount,
   setIsModalOpen,
   setAddressAction,
   setSelectedAddressForAction,
-}: Props) => {
+}: Props) => {
   return (
     <div className={styles.singleAddressContainer}>
       <AddressDetails userAddress={userAddress} />
       <AddressActionsMenu
         addressCount={addressCount}
         setAddressAction={setAddressAction}
         setIsModalOpen={setIsModalOpen}
         setSelectedAddressForAction={setSelectedAddressForAction}
         userAddress={userAddress}
       />
     </div>
   );
-};
+});
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressInput.tsx (2)

6-9: Consider restricting the index signature type

The [key: string]: any index signature is too permissive and could lead to type safety issues.

interface AddressOption {
  text: string;
-  [key: string]: any;
+  [key: string]: string | number | boolean | undefined;
}

63-72: Consider debouncing the onInputChange callback

For better performance and to avoid excessive API calls, consider debouncing the input change handler.

+import { useCallback } from 'react';
+import debounce from 'lodash/debounce';

+  const debouncedInputChange = useCallback(
+    debounce((value: string) => {
+      onInputChange?.(value);
+    }, 300),
+    [onInputChange]
+  );

   onInputChange={(_, newValue) => {
     field.onChange(newValue);
-    onInputChange?.(newValue);
+    debouncedInputChange(newValue);
   }}
src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx (2)

67-86: Extract loading spinner to a reusable component

The loading spinner UI could be extracted to a reusable component since it might be used in other address management actions.

+const LoadingSpinner = () => (
+  <div className={styles.addressMgmtSpinner}>
+    <CircularProgress color="success" />
+  </div>
+);

-      {!isLoading ? (
+      {isLoading ? (
+        <LoadingSpinner />
+      ) : (
         <div className={styles.buttonContainer}>
           <WebappButton
             text={tCommon('cancel')}
             elementType="button"
             variant="secondary"
             onClick={handleCancel}
           />
           <WebappButton
             text={tAddressManagement('deleteAction.deleteButton')}
             elementType="button"
             variant="primary"
             onClick={deleteAddress}
           />
         </div>
-      ) : (
-        <div className={styles.addressMgmtSpinner}>
-          <CircularProgress color="success" />
-        </div>
       )}

16-21: Consider adding data-testid attributes for testing

To improve testability, consider adding data-testid attributes to key elements.

interface Props {
  setIsModalOpen: SetState<boolean>;
  addressId: string | undefined;
  updateUserAddresses: () => Promise<void>;
  setAddressAction: SetState<AddressAction | null>;
+ 'data-testid'?: string;
}
src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx (1)

76-95: Add loading state for initial context loading

The component should show a loading state while the context is being loaded to prevent potential undefined behavior.

- {!isLoading ? (
+ {!contextLoaded ? (
+   <div className={styles.addressMgmtSpinner}>
+     <CircularProgress color="success" />
+   </div>
+ ) : !isLoading ? (
src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx (1)

73-86: Remove unnecessary dependencies from useCallback

The dependencies array includes utilities that don't need to be dependencies as they don't change between renders.

  [
    contextLoaded,
    user,
    token,
    country,
    selectedAddressForAction?.type,
    selectedAddressForAction?.id,
    tenantConfig.id,
    logoutUser,
    updateUserAddresses,
-   handleError,
-   putAuthenticatedRequest,
  ]
src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx (2)

73-76: Remove redundant condition in addAddress function

The setUserAddresses prop is required according to the Props interface, making the condition check unnecessary.

-  if (res && setUserAddresses) {
+  if (res) {
     setUserAddresses((prevAddresses) => [...prevAddresses, res]);
-  }
+  }

84-96: Remove unnecessary dependencies from useCallback

Similar to EditAddress.tsx, the dependencies array includes utilities that don't need to be dependencies.

  [
    contextLoaded,
    user,
    token,
    country,
    logoutUser,
    setUserAddresses,
-   handleError,
-   setIsLoading,
-   setIsModalOpen,
-   postAuthenticatedRequest,
  ]
src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx (2)

69-81: Consider extracting translation keys to constants

The component uses multiple translation keys directly in the JSX. This could make maintenance harder and is prone to typos.

Consider extracting translation keys to constants:

const TRANSLATION_KEYS = {
  ADDRESS_TYPE: 'addressType',
  SET_ADDRESS_CONFIRMATION: 'updateAddressType.setAddressConfirmation',
  REPLACE_ADDRESS_WARNING: 'updateAddressType.replaceAddressWarning'
} as const;

87-102: Consider extracting button components

The button section contains duplicated button structure. Consider extracting these into a separate component for better maintainability.

Example refactor:

interface ActionButtonProps {
  text: string;
  variant: 'primary' | 'secondary';
  onClick: () => void;
}

const ActionButton: React.FC<ActionButtonProps> = ({ text, variant, onClick }) => (
  <WebappButton
    text={text}
    elementType="button"
    variant={variant}
    onClick={onClick}
  />
);
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx (2)

40-70: Consider moving action configuration to a separate file

The addressActionConfig array contains business logic that could be reused across components. Consider moving it to a separate configuration file.

Create a new file addressActionConfig.ts:

import { ADDRESS_TYPE, ADDRESS_ACTIONS } from './addressManagement';
import type { AddressActionItem } from './types';

export const getAddressActionConfig = (
  t: (key: string) => string,
  type: string,
  addressCount: number
): AddressActionItem[] => [
  {
    label: t(`actions.edit`),
    action: ADDRESS_ACTIONS.EDIT,
    shouldRender: true,
  },
  // ... rest of the config
];

92-100: Add tooltip for kebab menu button

The kebab menu button could benefit from a tooltip to improve user experience.

Consider adding a tooltip:

+import { Tooltip } from '@mui/material';

+<Tooltip title="Address Actions">
   <button
     onClick={openPopover}
     className={styles.kebabMenuButton}
     aria-label="action menu"
     aria-expanded={open}
     aria-haspopup="true"
   >
     <KebabMenuIcon />
   </button>
+</Tooltip>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9c9ac78 and e1b69ad.

📒 Files selected for processing (17)
  • src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/AddressManagement.module.scss (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/index.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressDetails.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressForm.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormButtons.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormLayout.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressInput.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressList.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/FormattedAddressBlock.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/SingleAddress.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/index.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/features/user/Settings/EditProfile/index.tsx
🔇 Additional comments (7)
src/features/user/Settings/EditProfile/AddressManagement/AddressManagement.module.scss (1)

92-99: Verify color contrast ratios for address type labels.

Since these labels use white text (#fff), ensure that $primaryDarkColor and $mediumBlueColor provide sufficient contrast ratio (at least 4.5:1) for WCAG compliance.

src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormLayout.tsx (1)

1-17: Clean and well-structured component.

The AddressFormLayout component is straightforward and effectively organizes the form layout. Good use of TypeScript for type safety.

src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormButtons.tsx (1)

11-31: LGTM! Clean and focused component implementation

The component is well-structured with clear separation of concerns and proper use of translations.

src/features/user/Settings/EditProfile/AddressManagement/microComponents/FormattedAddressBlock.tsx (1)

11-27: LGTM! Well-structured component with good performance optimization

The component demonstrates good practices:

  • Proper null checking for userAddress
  • Effective use of useMemo for computed values
  • Clean type definitions
src/features/user/Settings/EditProfile/AddressManagement/microComponents/SingleAddress.tsx (1)

9-15: LGTM! Props interface is well-defined with appropriate types.

The interface clearly defines the expected props with proper typing, making good use of the SetState type for state management functions.

src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressInput.tsx (1)

11-24: LGTM! Props interface is comprehensive and well-typed

Good use of optional properties and specific types for callbacks.

src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx (1)

47-49: ⚠️ Potential issue

Validate and sanitize default country value

The default country value from stored config should be validated against a list of allowed countries to prevent potential security issues.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (5)
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx (2)

40-70: Consider extracting address action configuration.

The addressActionConfig array could be moved to a separate configuration file to improve maintainability and reusability.

Consider moving this configuration to a separate file like addressActionConfig.ts:

export const getAddressActionConfig = (
  tAddressManagement: (key: string) => string,
  type: string,
  addressCount: number
): AddressActionItem[] => [
  {
    label: tAddressManagement(`actions.edit`),
    action: ADDRESS_ACTIONS.EDIT,
    shouldRender: true,
  },
  // ... rest of the config
];

80-85: Consider adding error handling for action clicks.

The handleActionClick function should handle potential errors that might occur during state updates.

 const handleActionClick = (action: AddressAction) => {
+  try {
     setSelectedAddressForAction(userAddress);
     setIsModalOpen(true);
     setAddressAction(action);
     setPopoverAnchor(null);
+  } catch (error) {
+    console.error('Error handling address action:', error);
+    // Consider showing a user-friendly error message
+  }
 };
src/features/user/Settings/EditProfile/AddressManagement/index.tsx (3)

34-42: Consider using reducer pattern for complex state management.

The component manages multiple related states (userAddresses, addressAction, selectedAddressForAction). Using useReducer would make the state transitions more predictable and maintainable.

type AddressState = {
  addresses: Address[];
  action: AddressAction | null;
  selectedAddress: Address | null;
  isModalOpen: boolean;
};

type AddressAction = 
  | { type: 'SET_ADDRESSES'; payload: Address[] }
  | { type: 'SET_ACTION'; payload: AddressAction | null }
  | { type: 'SELECT_ADDRESS'; payload: Address | null }
  | { type: 'TOGGLE_MODAL'; payload: boolean };

const addressReducer = (state: AddressState, action: AddressAction): AddressState => {
  switch (action.type) {
    case 'SET_ADDRESSES':
      return { ...state, addresses: action.payload };
    // ... other cases
  }
};

80-157: Extract modal content rendering logic.

The renderModalContent switch statement is quite long and could be simplified by extracting the rendering logic for each action type into separate components or functions.

const modalContentMap = {
  [ADDRESS_ACTIONS.ADD]: () => (
    <AddAddress
      setIsModalOpen={setIsModalOpen}
      setUserAddresses={setUserAddresses}
      setAddressAction={setAddressAction}
    />
  ),
  // ... other action handlers
};

const renderModalContent = useMemo(() => {
  if (addressAction === null) return <></>;
  const renderer = modalContentMap[addressAction];
  return renderer ? renderer() : <></>;
}, [/* dependencies */]);

190-192: Add loading state for modal content.

The modal should show a loading state while the content is being prepared or when async operations are in progress.

 <Modal open={isModalOpen} aria-labelledby="address-action-modal-title">
+  {isLoading ? (
+    <div className={styles.modalLoading}>
+      <CircularProgress />
+    </div>
+  ) : (
     {renderModalContent}
+  )}
 </Modal>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1b69ad and f2838b1.

⛔ Files ignored due to path filters (1)
  • public/static/locales/en/editProfile.json is excluded by !**/*.json
📒 Files selected for processing (8)
  • src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/index.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressForm.tsx (1 hunks)
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressList.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressList.tsx
  • src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx
  • src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx
  • src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressForm.tsx
  • src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx
🔇 Additional comments (1)
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx (1)

15-19: LGTM! Well-structured interface definition.

The AddressActionItem interface clearly defines the contract for action items with appropriate types.

Copy link
Contributor

@sunilsabatp sunilsabatp left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants