Tenant config upload #1416
Tenant config upload #1416
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes encompass multiple files within the micro-ui project, introducing new styles to enhance the visual presentation of various UI components, particularly in the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (3)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/ConfigUploaderComponent.js (1)
36-60: LGTM with a minor suggestion!The
selectFilefunction and the use ofuseEffectto trigger the file upload are implemented correctly. The component UI is rendered using the necessary components and props.Consider removing the commented-out lines of code (lines 41-43 and lines 53-55) as they seem to be unnecessary.
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/applicationMgmt/SetupMaster.js (1)
194-194: Consider removing the commented-out line.Commenting out the
iconproperty is fine if the icon is no longer needed. However, consider removing the commented-out line entirely to keep the code clean and maintainable.micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1)
163-163: Remove unused 'onFormValueChange' prop from 'FormComposerV2'.The
onFormValueChangeprop is currently assigned an empty function. If it's not needed for this component, consider removing it to simplify the code.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (11)
micro-ui/web/core/inter-package.jsonis excluded by!**/*.jsonmicro-ui/web/core/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/example/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/packages/modules/core/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/packages/modules/hrms/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/packages/modules/pgr/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/packages/modules/sandbox/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/packages/modules/utilities/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/packages/react-components/package.jsonis excluded by!**/*.jsonmicro-ui/web/package.jsonis excluded by!**/*.json
Files selected for processing (8)
- micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (7 hunks)
- micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js (4 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/Module.js (2 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/applicationMgmt/SetupMaster.js (3 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/index.js (2 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/ConfigUploaderComponent.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/LogoUploaderComponent.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1 hunks)
Additional context used
Path-based instructions (7)
micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/modules/sandbox/src/Module.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/applicationMgmt/SetupMaster.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/index.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/ConfigUploaderComponent.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/LogoUploaderComponent.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1)
Pattern
**/*.js: check
Additional comments not posted (38)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/Module.js (2)
8-9: LGTM!The import statements for the new components are syntactically correct and follow the proper naming conventions.
38-39: Verify the integration and test the new components.The new components,
ConfigUploaderComponentandLogoUploaderComponent, are correctly added to thecomponentsToRegisterobject. This will allow them to be used within theSandboxModuleor its related pages/components.Please ensure that:
- The components are properly integrated into the relevant parts of the module.
- Adequate testing is performed to verify the functionality and handle any potential edge cases or error scenarios.
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/ConfigUploaderComponent.js (4)
1-5: LGTM!The imports are relevant and necessary for the component. The code segment looks good.
6-14: LGTM!The component setup and state management are properly implemented. The code segment looks good.
16-27: LGTM!The file upload functionality is implemented correctly, and the error handling mechanism is in place. The code segment looks good.
29-33: LGTM!The use of
useEffectis appropriate, and theonSelectfunction is called with the correct arguments. The code segment looks good.micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/LogoUploaderComponent.js (5)
1-5: LGTM!The imports are correct and necessary for the component.
6-14: LGTM!The component is defined correctly, and the state variables are initialized correctly. The
useTranslationhook is used correctly to access the translation function.
17-28: LGTM!The
handleUploadFilefunction is defined correctly. It uses thetry-catchblock to handle errors during the upload and sets the state variables correctly.
30-45: LGTM!The
useEffecthooks are defined correctly. TheonSelectprop is called correctly with thefileStoreIdandtype. ThehandleUploadFilefunction is called correctly when thefilestate variable changes. TheselectFilefunction is defined correctly.
46-63: LGTM!The JSX is defined correctly. The
LabelFieldPaircomponent is used correctly with theCardLabelandUploadercomponents. TheUploadercomponent is configured correctly to call theselectFilefunction. The component is exported correctly as the default export.micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/index.js (2)
14-14: LGTM!The import statement for
TenantConfigUploadis syntactically correct and follows the expected relative path convention.
58-58: LGTM!The new
PrivateRoutecorrectly defines a route for tenant configuration uploads at the path${path}/tenant-management/config/upload. TheTenantConfigUploadcomponent is appropriately rendered for this route.This change aligns with the PR objective of implementing functionality related to the uploading of tenant configuration.
micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js (8)
69-71: LGTM!The code segment correctly filters the documents array to extract the
fileStoreIdof documents with type "logoUrl". The resultinglogoArraywill be used to fetch the actual logo URLs.
73-75: LGTM!Similar to the
logoArray, the code segment correctly filters the documents array to extract thefileStoreIdof documents with type "bannerUrl". The resultingbannerArraywill be used to fetch the actual banner URLs.
77-78: LGTM!The code segment uses the
Digit.UploadServices.Filefetchmethod to fetch the actual logo and banner URLs using thefileStoreIdvalues fromlogoArrayandbannerArray. The fetched URLs are stored inlogoUrlandbannerUrlvariables for further use.
85-85: LGTM!The code segment correctly prioritizes the fetched
logoUrlover the URL from the tenant configuration document. If the fetched URL is not available, it falls back to the original method of retrieving the URL from the document with type "logoUrl". This ensures that the most current logo URL is used when available.
90-90: LGTM!Similar to the
logoUrl, the code segment correctly prioritizes the fetchedbannerUrlover the URL from the tenant configuration document. If the fetched URL is not available, it falls back to the original method of retrieving the URL from the document with type "bannerUrl". This ensures that the most current banner URL is used when available.
93-93: LGTM!The code segment correctly filters and sorts the
MdmsRes?.tenant?.citymodulearray to include only the active and enabled modules in the returned object. The filtering is based on theactiveproperty and the presence of the module code in theenabledModulesarray. The sorting is based on theorderproperty of each module.
108-108: This code segment is a duplicate of the changes made at line 93. The same review comment applies here.
120-122: Verify the commented-out code and its impact.The code segment correctly maps the
MdmsRes?.tenant?.tenantsarray to create theinitData.tenantsarray with transformed tenant objects. However, the commented-out code suggests that there was an intention to filter the tenants based on themoduleTenantsarray.Please verify if the filtering is still required and if commenting it out has any impact on the functionality. If the filtering is necessary, consider uncommenting the code and testing the functionality thoroughly.
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (13)
112-117: LGTM!The margin-top addition to the submit-bar class looks good. It should help improve the layout and spacing.
Line range hint
178-183: LGTM!The margin-top and text-align additions to the sandbox-resend-otp class look good. They should help improve the layout and text alignment.
Line range hint
220-228: LGTM!The height and width adjustments for the bannerLogo within the signupCardClassName class look good. They should help set the logo to the desired size.
Line range hint
229-237: LGTM!The width and height adjustments for the bannerLogo within the card-sandbox class look good. They should help set the logo to the desired size.
246-262: LGTM!The various style adjustments within the card-sandbox class look good:
- Removing the margin-top for sandbox-success-signup should fix its positioning.
- Adding margin-top to digit-field and sandbox-url-footer should improve the spacing.
- Setting the width of digit-button-primary to 100% should make the button fill its container.
263-266: LGTM!The background-color adjustment for the submit-bar class looks good. It should help set the desired visual appearance for the submit bar.
267-272: LGTM!The color adjustment for the cardHeader-sandbox class within the card-sandbox class looks good. It should help set the desired text color for the card header.
Line range hint
273-284: LGTM!The styles for the sandbox-url-wrapper class and its child elements look good:
- Setting the display to flex and width to 100% for sandbox-url-wrapper should lay out its children horizontally and make it fill its container.
- Adjusting the width and padding of the copyButton should size and position the button as desired next to the URL input.
297-310: LGTM!The styles for elements within the sandboxSetupMasterInfo class look good:
- Setting the display to flex, gap, and align-items for the headerFlex class should lay out its children horizontally with spacing and center them vertically.
- Adjusting the font-size, font-weight, and color for the subHeader class should style the subheader text as desired.
311-316: LGTM!The font-size, color, and margin-left adjustments for the setupMaster-subheading class look good. They should help style and position the setup master subheading as desired.
Line range hint
317-326: LGTM!Hiding the digit-popup-close button within the masterHandlerPopup class looks good. It should remove the close button from the master handler popup as desired.
327-341: LGTM!The adjustments within the digit-topbar-ulb and masterHandlerPopUpFooter classes look good:
- Setting the width and height for the state class should size the state element as desired within the topbar.
- Removing the margin-left, setting width to 100%, and justify-content to center for the digit-popup-footer-buttons should lay out the popup footer buttons to fill the width and center them.
400-410: LGTM!The width adjustment for the digit-button-label within the digit-uploader-wrap class looks good. Setting it to fit-content should size the button label to fit its text content.
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/applicationMgmt/SetupMaster.js (3)
107-107: LGTM!The change in the
variationproperty from"secondary"to"primary"is appropriate for emphasizing the button as a primary action.
183-183: LGTM!The change in the
onClickhandler to redirect the user to the employee homepage usinghistory.pushis appropriate for the intended navigation behavior.
193-193: LGTM!The change in the
variationproperty from"secondary"to"primary"is appropriate for emphasizing the button as a primary action.micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1)
117-130: Verify the correct usage of the mutation function.Ensure that the
mutation.mutatefunction is being used correctly with the expected parameters. Theurl,body, andconfigpassed to it should align with the API's requirements.Run the following script to check the mutation function's usage in the codebase:
Verification successful
The
mutation.mutatefunction is being used correctly in TenantConfigUpload.js.The usage of
mutation.mutatein the file aligns with the general pattern observed across the codebase. The function is called with an object containingurl,body, andconfigproperties, which is consistent with other implementations and appears to meet the API's requirements.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for all usages of 'mutation.mutate' to verify parameter correctness. # Search for 'mutation.mutate' usage in all JavaScript files. rg --type js 'mutation\.mutate\(' -A 5Length of output: 23812
...ro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
Show resolved
Hide resolved
| auditDetails: { | ||
| createdBy: null, | ||
| lastModifiedBy: null, | ||
| createdTime: null, | ||
| lastModifiedTime: null, | ||
| }, |
There was a problem hiding this comment.
Consider providing meaningful values for 'auditDetails'.
In the auditDetails fields of both the documents and tenantConfig objects, all properties are set to null. If these details are required by the backend or for auditing purposes, consider populating them with actual values or omitting them if not necessary.
Also applies to: 89-94
...ro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1)
162-185: LGTM: Component rendering is well-structured.The component's rendering logic is clear and appropriate. The use of translation functions (
t) for internationalization is a good practice.Consider using a constant for toast duration.
For better maintainability, consider extracting the toast duration (5000ms) into a named constant at the top of the file.
You could add this at the beginning of the file:
const TOAST_DURATION_MS = 5000;Then use it in the
closeToastfunction:const closeToast = () => { setTimeout(() => { setShowToast(false); }, TOAST_DURATION_MS); };
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js (4 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/LogoUploaderComponent.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/LogoUploaderComponent.js
Additional context used
Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1)
Pattern
**/*.js: check
Biome
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
[error] 112-112: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Additional comments not posted (5)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (5)
1-8: LGTM: Imports and initial setup are appropriate.The necessary React hooks, components, and custom services are imported correctly. The
fieldStyleobject is defined for later use in the component.
31-66: LGTM: Form configuration is well-structured.The form configuration is well-defined, using custom uploader components for different types of uploads (banner and logo). This structure allows for flexibility and clear separation of concerns.
71-76: Ensure proper handling of undefined data in 'onSubmit' function.As mentioned in a previous review, there's a possibility that
data[key]might beundefined. To prevent potential errors, consider adding a check to ensure thatdata[key]exists before accessing its properties.
156-160: Unused 'closeToast' function can be removed or integrated.As mentioned in a previous review, the
closeToastfunction is defined but not used within the component. If the intent was to auto-dismiss the toast after 5 seconds, consider integrating this function by calling it whenshowToastis set totrue. Otherwise, it can be removed to clean up the code.
1-185: Overall assessment: Well-implemented component with minor improvement opportunities.The
TenantConfigUploadcomponent is well-structured and implements the necessary functionality for uploading tenant configurations. It demonstrates good practices such as using React hooks, custom components, and internationalization.Key strengths:
- Clear separation of concerns in the form configuration
- Proper use of React hooks for state management and side effects
- Error handling and user feedback through toast notifications
Areas for potential improvement:
- Parameterizing the API URL for better maintainability
- Refactoring the
updateFileStoreIdsfunction for clarity and immutability- Addressing the unused
closeToastfunction- Minor optimizations like using constants for magic numbers and avoiding the
deleteoperatorOverall, the code is of good quality and with these minor improvements, it will be even more robust and maintainable.
Tools
Biome
[error] 112-112: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
...ro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
Show resolved
Hide resolved
| const updateFileStoreIds=(inputData, requestData) =>{ | ||
| // Iterate over the input data to update fileStoreId in requestData | ||
| inputData.forEach(input => { | ||
| requestData[0].documents.forEach(doc => { | ||
| if (doc.type.toLowerCase() === input.type.toLowerCase()) { | ||
| doc.fileStoreId = input.fileStoreId; | ||
| } | ||
| }); | ||
| }); | ||
| requestData["tenantConfig"] = requestData["0"]; | ||
| delete requestData["0"]; | ||
| return requestData; | ||
| } |
There was a problem hiding this comment.
Refactor updateFileStoreIds for clarity and immutability.
The updateFileStoreIds function modifies the input data directly, which can lead to unexpected side effects. Consider refactoring it to create a new object instead of modifying the existing one.
Here's a suggested refactoring:
const updateFileStoreIds = (inputData, requestData) => {
const updatedDocuments = requestData[0].documents.map(doc => {
const matchingInput = inputData.find(input => input.type.toLowerCase() === doc.type.toLowerCase());
return matchingInput ? { ...doc, fileStoreId: matchingInput.fileStoreId } : doc;
});
return {
tenantConfig: {
...requestData[0],
documents: updatedDocuments
}
};
};This approach creates a new object, maintaining immutability and improving clarity.
Tools
Biome
[error] 112-112: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
...ro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (8 hunks)
- micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
Additional context used
Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js (1)
Pattern
**/*.js: check
Biome
micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js
[error] 133-134: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '<<'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 134-134: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 134-134: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '((module) => module?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found ')?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '((module) =>'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '(module?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '))?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '(('.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found ','.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found ') =>'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '-'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found ') || [],'.
Expected a JSX attribute here.
(parse)
[error] 137-137: Unexpected token. Did you mean
{'}'}or}?(parse)
Additional comments not posted (1)
micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js (1)
126-131: Approve updates to stateInfo objectThe changes to the
stateInfoobject, particularly forlogoUrlandbannerUrl, are well-implemented. The new logic correctly prioritizes the fetched URLs while maintaining fallback options. This ensures that the most up-to-date URLs are used when available, improving the overall user experience.
micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js
Outdated
Show resolved
Hide resolved
| <<<<<<< HEAD | ||
| modules: MdmsRes?.tenant?.citymodule?.filter((module) => module?.active)?.filter((module) => enabledModules?.includes(module?.code))?.sort((x, y) => x?.order - y?.order) || [], | ||
| uiHomePage: uiHomePage | ||
| }; | ||
| }; | ||
| const initData = tenantConfigFetch ? await fetchTenantConfig() : { | ||
| languages: stateInfo.hasLocalisation ? stateInfo.languages : [{ label: "ENGLISH", value: Digit.Utils.getDefaultLanguage() }], | ||
| stateInfo: { | ||
| code: stateInfo.code, | ||
| name: stateInfo.name, | ||
| logoUrl: stateInfo.logoUrl, | ||
| statelogo: stateInfo.statelogo, | ||
| logoUrlWhite: stateInfo.logoUrlWhite, | ||
| bannerUrl: stateInfo.bannerUrl, | ||
| }, | ||
| localizationModules: stateInfo.localizationModules, | ||
| modules: MdmsRes?.tenant?.citymodule?.filter((module) => module?.active)?.filter((module) => enabledModules?.includes(module?.code))?.sort((x, y) => x?.order - y?.order) || [], | ||
| uiHomePage: uiHomePage | ||
| }; | ||
| ======= |
There was a problem hiding this comment.
Resolve merge conflict in modules property
There's a merge conflict in the implementation of the modules property. Both versions achieve the same result, but they differ in formatting and readability.
Please resolve this merge conflict. I recommend using the develop version as it's more readable:
modules:
MdmsRes?.tenant?.citymodule
?.filter((module) => module?.active)
?.filter((module) => enabledModules?.includes(module?.code))
?.sort((x, y) => x?.order - y?.order) || [],This version is more readable and maintainable, especially if the filtering or sorting logic needs to be modified in the future.
Tools
Biome
[error] 134-134: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 134-134: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '((module) => module?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found ')?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '((module) =>'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '(module?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '))?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '(('.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found ','.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found ') =>'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '-'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found '?.'.
Expected a JSX attribute here.
(parse)
[error] 135-135: Expected a JSX attribute but instead found ') || [],'.
Expected a JSX attribute here.
(parse)
[error] 137-137: Unexpected token. Did you mean
{'}'}or}?(parse)
| <<<<<<< HEAD | ||
| initData.tenants = MdmsRes?.tenant?.tenants | ||
| .map((tenant) => ({ i18nKey: `TENANT_TENANTS_${tenant.code.replace(".", "_").toUpperCase()}`, ...tenant })); | ||
| ======= | ||
| initData.tenants = MdmsRes?.tenant?.tenants.map((tenant) => ({ | ||
| i18nKey: `TENANT_TENANTS_${tenant.code.replace(".", "_").toUpperCase()}`, | ||
| ...tenant, | ||
| })); | ||
| >>>>>>> develop |
There was a problem hiding this comment.
Resolve merge conflict in tenants property
There's a merge conflict in the implementation of the tenants property. Both versions achieve the same result, but they differ in formatting and readability.
Please resolve this merge conflict. I recommend using the develop version as it's more readable:
initData.tenants = MdmsRes?.tenant?.tenants.map((tenant) => ({
i18nKey: `TENANT_TENANTS_${tenant.code.replace(".", "_").toUpperCase()}`,
...tenant,
}));This version is more readable and maintainable, especially if the mapping logic needs to be modified in the future.
| <<<<<<< HEAD | ||
| if (typeof moduleCode !== "string") moduleCode.forEach(code => { moduleCodes.push(modulePrefix ? `${modulePrefix}-${code.toLowerCase()}` : `${code.toLowerCase()}`) }); | ||
| ======= | ||
| if (typeof moduleCode !== "string") | ||
| moduleCode.forEach((code) => { | ||
| moduleCodes.push(modulePrefix ? `${modulePrefix}-${code.toLowerCase()}` : `${code.toLowerCase()}`); | ||
| }); | ||
| >>>>>>> develop |
There was a problem hiding this comment.
Resolve merge conflict in moduleCodes logic
There's a merge conflict in the implementation of the moduleCodes logic. Both versions achieve the same result, but they differ in formatting and readability.
Please resolve this merge conflict. I recommend using the develop version as it's more readable:
if (typeof moduleCode !== "string")
moduleCode.forEach((code) => {
moduleCodes.push(modulePrefix ? `${modulePrefix}-${code.toLowerCase()}` : `${code.toLowerCase()}`);
});This version is more readable and maintainable, especially if the logic for formatting module codes needs to be modified in the future.
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (1)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1)
119-119: Fix typo in comment.There's a minor typo in the comment. "elsewher" should be "elsewhere".
Correct the comment as follows:
// Assuming the mutation for API call is defined elsewhere
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (9 hunks)
- micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js (2 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/ConfigUploaderComponent.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/LogoUploaderComponent.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
- micro-ui/web/micro-ui-internals/packages/libraries/src/services/molecules/Store/service.js
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/ConfigUploaderComponent.js
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/LogoUploaderComponent.js
Additional context used
Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1)
Pattern
**/*.js: check
Biome
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
[error] 111-111: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Additional comments not posted (3)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (3)
1-18: Imports and initial setup look good.The necessary React hooks and components are correctly imported, and the state variables are properly initialized using the
useStatehook. This follows React best practices for managing component state and notifications.
22-28: Consider parameterizing the API URL.The API URL is currently hardcoded. For better maintainability and flexibility, consider parameterizing it or storing it in a configuration file, as suggested in a previous review comment.
31-66: Form configuration is well-structured.The configuration for the form, including the two uploader components (ConfigUploaderComponent and LogoUploaderComponent), is well-organized and clearly defined. Each uploader is properly configured with specific properties and populators.
| const updateFileStoreIds=(inputData, requestData) =>{ | ||
| // Iterate over the input data to update fileStoreId in requestData | ||
| inputData.forEach(input => { | ||
| requestData[0].documents.forEach(doc => { | ||
| if (doc.type === input.type) { | ||
| doc.fileStoreId = input.fileStoreId; | ||
| } | ||
| }); | ||
| }); | ||
| requestData["tenantConfig"] = requestData["0"]; | ||
| delete requestData["0"]; | ||
| return requestData; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor updateFileStoreIds for immutability and performance.
The updateFileStoreIds function currently mutates the input object directly and uses the delete operator, which can lead to unexpected side effects and potential performance issues. Consider refactoring this function to create a new object instead of modifying the existing one.
Here's a suggested refactoring:
const updateFileStoreIds = (inputData, requestData) => {
const updatedDocuments = requestData[0].documents.map(doc => {
const matchingInput = inputData.find(input => input.type.toLowerCase() === doc.type.toLowerCase());
return matchingInput ? { ...doc, fileStoreId: matchingInput.fileStoreId } : doc;
});
return {
tenantConfig: {
...requestData[0],
documents: updatedDocuments
}
};
};This approach creates a new object, maintaining immutability and avoiding the use of the delete operator.
Tools
Biome
[error] 111-111: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js (1)
Pattern
**/*.js: check
Biome
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
[error] 111-111: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
| import React, { useState, useEffect } from "react"; | ||
| import { useTranslation } from "react-i18next"; | ||
| import { useHistory } from "react-router-dom"; | ||
| import { FormComposerV2, Header, Toast } from "@egovernments/digit-ui-react-components"; | ||
| import { TenantConfigSearch } from "../../../../../../libraries/src/services/elements/TenantConfigService"; | ||
|
|
||
|
|
||
| const fieldStyle = { marginRight: 0 }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using path aliases for better maintainability.
The import path for TenantConfigSearch is quite long and nested. Consider using path aliases to improve readability and maintainability. This can be set up in your project's configuration (e.g., webpack, tsconfig.json for TypeScript).
Example:
import { TenantConfigSearch } from "@services/elements/TenantConfigService";
...ro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
Show resolved
Hide resolved
...ro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
Show resolved
Hide resolved
...ro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
Show resolved
Hide resolved
...ro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
Show resolved
Hide resolved
...ro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
Show resolved
Hide resolved
...ro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/TenantConfigUpload.js
Show resolved
Hide resolved
tenant config upload