Sn368 localizationfixes #2057
Sn368 localizationfixes #2057
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces comprehensive updates across multiple files in the micro-ui workbench modules, focusing on enhancing UI customization, form handling, and styling. The changes span CSS styling in the workbench interface, modifications to UI configuration files, improvements in JSON form components, and refinements to MDMS (Master Data Management System) search and view functionalities. The updates aim to improve code structure, add localization support, and streamline data handling across various employee-facing components. Changes
Possibly related PRs
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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: 23
🔭 Outside diff range comments (7)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js (4)
Line range hint
9-76: Consider reducing code duplication in updatePayload function.
The function repeats a near-identical pattern of collecting workflow data (comments, documents, assignees, etc.) for each business service. Extracting common steps into a helper could simplify maintenance and reduce errors.
Line range hint
98-132: Evaluate getBusinessService logic for potential fallback.
Similar to the above recommendation, if moduleCode includes a substring that doesn't match known patterns (e.g., “works.purchase”), the function returns the entire object rather than a single service. This may cause confusion in the calling code.
Line range hint
164-245: Optimize repeated code in AttendanceInboxConfig preProcess.
The function has explicit clones, deletes, and transformations for multiple fields. Consider modularizing these transformations into smaller utility functions for readability and testability.
Line range hint
512-812: Assess potential side effects of combineData overwriting content.
The combineData method in SearchLocalisationConfig modifies defaultData messages by pulling values from data messages. Confirm that updates are always in sync with the desired workflow and do not inadvertently mask or lose critical information.micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/configs/UICustomizations.js (2)
Line range hint
16-35: Confirm necessity of commented-out code in SearchPucarConfig preProcess.
The entire block is commented out, potentially reducing functionality. If the code is no longer needed, remove it to clean up the codebase. If it’s temporarily disabled, add a descriptive comment explaining the reason and expected reactivation timeline.
Line range hint
147-199: Add error checks for missing module or master details in moduleMasterConfig.
If additionalDetails.moduleName is absent or doesn’t map to a documented module, the select function could return undefined. Ensure the system gracefully handles missing data.micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (1)
Line range hint
9-225: Consider architectural improvements for better maintainabilityWhile the component functions correctly, consider the following architectural improvements:
- Extract localization logic into a custom hook
- Create a separate configuration file for API endpoints
- Implement proper error boundary handling
Would you like assistance in implementing any of these architectural improvements?
🧰 Tools
🪛 Biome (1.9.4)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 150-150: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 189-189: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 211-219: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 220-220: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 174-174: This let declares a variable that is only assigned once.
'messageMap' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 190-190: This let declares a variable that is only assigned once.
'updatedData' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
micro-ui/web/core/inter-package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/package.jsonis excluded by!**/*.jsonmicro-ui/web/sandbox/inter-package.jsonis excluded by!**/*.jsonmicro-ui/web/workbench/inter-package.jsonis excluded by!**/*.json
📒 Files selected for processing (8)
micro-ui/web/micro-ui-internals/packages/css/src/digitv2/pages/employee/workbench.scss(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/configs/UICustomizations.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js(10 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js(5 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSSearchv2.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSSearchv2.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/configs/UICustomizations.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js
[error] 128-128: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
[error] 14-14: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 35-35: This let declares a variable that is only assigned once.
'actionItems' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 150-150: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 189-189: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 211-219: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 220-220: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 174-174: This let declares a variable that is only assigned once.
'messageMap' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 190-190: This let declares a variable that is only assigned once.
'updatedData' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
[error] 156-158: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 166-168: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 224-224: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 319-319: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 336-336: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 364-364: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 370-370: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 389-389: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 455-462: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 285-285: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js
[error] 135-135: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
🔇 Additional comments (20)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js (2)
Line range hint 9-76: Validate businessService inputs or handle unknown services gracefully.
Currently, there is no fallback/else case if the businessService key doesn't match any in businessServiceMap. Consider adding a default condition to prevent silent failures or undefined returns.
Line range hint 307-404: Check that new isActive logic addresses all combination endpoints.
Under SearchMDMSConfig, the conditional block for isActive uses “all” to remove the attribute altogether. Confirm that the backend handles missing isActive vs. isActive=false properly.
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/configs/UICustomizations.js (1)
Line range hint 48-70: Validate partial logic in tenantSearchConfig.
The code sets only a subset of query parameters (name, code). If additional search fields (e.g., date ranges) become relevant, consider broadening coverage or clarifying that these are the only supported filters.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js (4)
1-2: No issues found in these import changes.
21-28: Validate loader logic before navigation.
Setting “renderLoader” immediately and navigating with “history.push” may cause a flicker or lost loader state, depending on how quickly the new route loads. Confirm this meets your intended user experience.
100-110: Check performance impact of deep cloning.
Deep cloning the entire “data” object may be expensive if “data” is large. Make sure it’s necessary to create a full copy.
128-180: Exception handling and success flow are consistent.
Overall, the logic for updating MDMS and upserting localization messages looks good. Make sure to thoroughly test partial failures (e.g., localization failing but MDMS succeeding) if such scenarios are valid for your system.
🧰 Tools
🪛 Biome (1.9.4)
[error] 135-135: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js (6)
67-74: Ensure data consistency in MDMS add body object.
Currently, the "isActive" field is hardcoded to true. Confirm whether you need a dynamic value based on user input or business rules. Otherwise, this seems acceptable.
76-76: Validate the fallback URL.
The fallback URL utilizes Digit.Utils.workbench.getMDMSActionURL. Verify consistency if an API override is provided.
99-109: Localization upsert mutation looks good.
No issues identified. Good practice keeping it disabled until triggered.
Line range hint 180-202: Error handling appears adequate.
The toast message is displayed, and the spinner is disabled. Be sure to confirm that error codes from the backend are readable in your localization.
🧰 Tools
🪛 Biome (1.9.4)
[error] 128-128: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
230-230: Schema assignment approach is correct.
Reassigning with spread syntax ensures reactivity. No issues here.
266-266: Valid reference to custom UI schema.
Implementation looks consistent with other usage patterns in the codebase.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (6)
25-26: AdditionalPropertiesContext usage is fine.
Exporting a custom hook (useAdditionalProperties) is good for reusability. No concerns here.
46-48: Removal of transformErrors is acceptable.
Reducing complexity is beneficial, but ensure that nothing depends on the removed feature.
58-58: ArrayFieldItemTemplate naming clarity.
The logic is straightforward. The naming is consistent, which helps with readability.
129-132: Property references in formContext are well-structured.
Good use of destructuring to maintain clarity.
130-137: Ensure localizable fields are fully validated.
Check whether the code properly handles empty or invalid form inputs (e.g., an empty string producing a blank code).
364-364: Optional chaining suggestion.
These suggestions likely come from static analysis. Carefully see if performance or clarity is improved by using this approach (e.g., someObj?.someMethod?).
Also applies to: 370-370
🧰 Tools
🪛 Biome (1.9.4)
[error] 364-364: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
micro-ui/web/micro-ui-internals/packages/css/src/digitv2/pages/employee/workbench.scss (1)
1054-1089: CSS styles for .code-details & related classes look well-structured.
These additions are consistent with the existing pattern. Ensure that color tokens match the design system if theming evolves.
| @@ -810,4 +810,4 @@ export const UICustomizations = { | |||
|
|
|||
| } | |||
| } | |||
| }; | |||
| }; | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding unit tests and error handling for new additions.
It appears a substantial amount of logic was added. Having comprehensive unit tests will ensure that any data transformations (or absent edge cases) are caught early.
| @@ -269,4 +269,4 @@ export const UICustomizations = { | |||
| } | |||
| }, | |||
| }, | |||
| }; | |||
| }; | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve code clarity with well-structured comments and consistent naming.
A single final bracket at line 272 is not enough to convey the boundaries of major sections within the file. Consider adding headings or in-line documentation to guide future maintainers.
| import { useTranslation } from "react-i18next"; | ||
| import { useHistory } from "react-router-dom"; | ||
| const MDMSEdit = ({...props}) => { | ||
| const history = useHistory() | ||
| import _ from "lodash"; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure lodash usage is necessary.
If the only lodash functionality used is “cloneDeep,” consider using a smaller library like “clone-deep” or a native technique to minimize bundle size.
| const closeToast = () => { | ||
| setTimeout(() => setShowToast(null), 5000); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Confirm user experience with delayed toast closure.
Using setTimeout to close the toast might result in stale state if the user navigates away. Consider using a cleanup mechanism or a library that provides managed toast lifecycles.
| // Localization Search | ||
| const localizationModule = `DIGIT_MDMS_${data?.schemaCode}`.toUpperCase(); | ||
| const locale = "en_IN"; | ||
|
|
||
| const reqCriteriaUpdate = { | ||
| url: Digit.Utils.workbench.getMDMSActionURL(moduleName,masterName,"update"), | ||
| const localizationReqCriteria = { | ||
| url: `/localization/messages/v1/_search?locale=${locale}&tenantId=${stateId}&module=${localizationModule}`, | ||
| params: {}, | ||
| body: { | ||
|
|
||
| }, | ||
| body: { RequestInfo: { authToken: Digit.UserService.getUser()?.access_token || "" } }, | ||
| config: { | ||
| enabled: true, | ||
| enabled: !!data, | ||
| select: (respData) => { | ||
| const messageMap = {}; | ||
| if (Array.isArray(respData?.messages)) { | ||
| respData.messages.forEach((msg) => { | ||
| messageMap[msg.code] = msg.message; | ||
| }); | ||
| } | ||
| return messageMap; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Centralize localization logic for maintainability.
The newly introduced localization request criteria and message processing look good. However, consider abstracting these into a reusable hook or utility for consistent usage across the application.
| <Card className="workbench-create-form"> | ||
| <Header className="digit-form-composer-sub-header"> | ||
| {typeof Digit?.Utils?.workbench?.getMDMSLabel === "function" | ||
| ? t(Digit.Utils.workbench.getMDMSLabel(`SCHEMA_` + schema?.code)) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer template literals.
Consistent with recommended modern JavaScript style.
🧰 Tools
🪛 Biome (1.9.4)
[error] 389-389: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
| <Toast | ||
| label={t(showToast)} | ||
| error={showErrorToast} | ||
| onClose={() => { | ||
| setShowToast(null); | ||
| }} | ||
| isDleteBtn={true} | ||
| ></Toast> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use self-closing tag for empty JSX elements.
Shortens code and is more idiomatic in React.
🧰 Tools
🪛 Biome (1.9.4)
[error] 455-462: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
| if (schema?.code && MdmsRes && formData) { | ||
| const localisableFields = | ||
| MdmsRes?.find((item) => item.schemaCode === schema.code)?.localisation?.localisableFields || []; | ||
| let newAdditionalProps = {}; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use const instead of let.
The variable newAdditionalProps is never reassigned.
- let newAdditionalProps = {};
+ const newAdditionalProps = {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let newAdditionalProps = {}; | |
| const newAdditionalProps = {}; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 285-285: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
| !label?.includes(Digit.Utils.locale.getTransformedLocale(moduleName)) && | ||
| !label?.includes(Digit.Utils.locale.getTransformedLocale(masterName)) | ||
| ) { | ||
| titleCode = Digit.Utils.locale.getTransformedLocale(`${moduleName}.${moduleName}_${label?.slice(0, -2)}`); | ||
| additionalCode = label?.slice(-2); | ||
| } | ||
|
|
||
| return ( | ||
| <span> | ||
| <div className={classNames} style={style}> | ||
| <label htmlFor={id} className="control-label" id={"label_" + id}> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use template literals over string concatenation.
This is a minor style improvement for readability.
- label={t("WBH_LOC_BULK_UPLOAD_XLS") + " some text"}
+ label={`${t("WBH_LOC_BULK_UPLOAD_XLS")} some text`}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 224-224: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
| @@ -21,8 +21,7 @@ const toDropdownObj = (master = "", mod = "") => { | |||
| const MDMSSearchv2 = () => { | |||
| let Config = _.clone(Configg) | |||
| const { t } = useTranslation(); | |||
| const history = useHistory(); | |||
|
|
|||
| const history = useHistory(); | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
General logic for “history” usage is fine.
The destructuring of query parameters into modulee & master may be slightly confusing—consider more descriptive naming for maintainability.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js (3)
6-6: 🧹 Nitpick (assertive)Consider using a lighter alternative to lodash.
Since only
cloneDeepis used from lodash, consider using a smaller alternative likeclone-deepor the structured clone API to reduce bundle size.-import _ from "lodash"; +import cloneDeep from "clone-deep"; // or use the native API: // const clonedData = structuredClone(data);
17-19: 🛠️ Refactor suggestionAdd cleanup for setTimeout.
The current implementation could lead to memory leaks if the component unmounts while the timeout is active.
const closeToast = () => { - setTimeout(() => setShowToast(null), 5000); + const timeoutId = setTimeout(() => setShowToast(null), 5000); + return () => clearTimeout(timeoutId); };
134-146: 🛠️ Refactor suggestionReplace hasOwnProperty with Object.hasOwn.
Using Object.prototype methods directly on objects can be problematic if the object has been tampered with.
-if (additionalProperties.hasOwnProperty(fieldName)) { +if (Object.hasOwn(additionalProperties, fieldName)) {🧰 Tools
🪛 Biome (1.9.4)
[error] 135-135: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js
[error] 135-135: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
[error] 14-14: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 35-35: This let declares a variable that is only assigned once.
'actionItems' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 173-173: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 195-203: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 204-204: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 158-158: This let declares a variable that is only assigned once.
'messageMap' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 174-174: This let declares a variable that is only assigned once.
'updatedData' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (7)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js (3)
30-74: LGTM! Well-structured data fetching logic.
The code demonstrates good practices:
- Proper dependency handling with enabled config
- Clear separation of concerns between MDMS and schema fetching
- Efficient data transformation
149-160: LGTM! Robust error handling for localization.
The code properly handles localization errors and provides user feedback through toast messages.
182-194: LGTM! Clean and efficient rendering logic.
The code demonstrates good practices:
- Comprehensive loading state handling
- Clean component structure with proper prop passing
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (4)
33-47: Simplify action items logic and use const
The action items logic can be simplified, and actionItems should use const as it's never reassigned.
🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: This let declares a variable that is only assigned once.
'actionItems' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
129-182: Localization implementation looks good!
The implementation of localization support is well-structured and includes:
- Proper field detection from schema
- Robust code generation for localization keys
- Efficient data transformation with fallbacks
🧰 Tools
🪛 Biome (1.9.4)
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 173-173: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 158-158: This let declares a variable that is only assigned once.
'messageMap' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 174-174: This let declares a variable that is only assigned once.
'updatedData' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
13-15: 🛠️ Refactor suggestion
Use const for variables that aren't reassigned
The destructured variables are never reassigned, so they should use const. Also, consider destructuring tenantId after getting the current tenant ID for better clarity.
- let { moduleName, masterName, tenantId, uniqueIdentifier } = Digit.Hooks.useQueryParams();
- let { from, screen, action } = Digit.Hooks.useQueryParams()
- tenantId = Digit.ULBService.getCurrentTenantId();
+ const { moduleName, masterName, uniqueIdentifier } = Digit.Hooks.useQueryParams();
+ const { from, screen, action } = Digit.Hooks.useQueryParams();
+ const tenantId = Digit.ULBService.getCurrentTenantId();Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 14-14: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
129-133: 🛠️ Refactor suggestion
Use const and optional chaining for localization setup
The localization setup can be improved with proper variable declarations and optional chaining.
- let localisableFields = [];
- if (MdmsRes && Array.isArray(MdmsRes)) {
- const schemaDef = MdmsRes.find(item => item.schemaCode === `${moduleName}.${masterName}`);
- localisableFields = schemaDef?.localisation?.localisableFields || [];
- }
+ const localisableFields = Array.isArray(MdmsRes)
+ ? MdmsRes.find(item => item?.schemaCode === `${moduleName}.${masterName}`)?.localisation?.localisableFields ?? []
+ : [];Likely invalid or redundant comment.
| // Replace values with localized messages | ||
| let finalData = data; | ||
| if (data?.data && localizationMap) { | ||
| const updatedData = _.cloneDeep(data); | ||
| Object.keys(updatedData.data).forEach((field) => { | ||
| const localizationKey = `${data.schemaCode}_${field}_${updatedData.data[field]}`.toUpperCase(); | ||
| if (localizationMap[localizationKey]) { | ||
| updatedData.data[field] = localizationMap[localizationKey]; | ||
| } | ||
| }); | ||
| finalData = updatedData; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider memoizing the localized data transformation.
The data transformation is performed on every render. Consider using useMemo to cache the results.
-let finalData = data;
-if (data?.data && localizationMap) {
- const updatedData = _.cloneDeep(data);
- Object.keys(updatedData.data).forEach((field) => {
- const localizationKey = `${data.schemaCode}_${field}_${updatedData.data[field]}`.toUpperCase();
- if (localizationMap[localizationKey]) {
- updatedData.data[field] = localizationMap[localizationKey];
- }
- });
- finalData = updatedData;
-}
+const finalData = useMemo(() => {
+ if (!data?.data || !localizationMap) return data;
+ const updatedData = _.cloneDeep(data);
+ Object.keys(updatedData.data).forEach((field) => {
+ const localizationKey = `${data.schemaCode}_${field}_${updatedData.data[field]}`.toUpperCase();
+ if (localizationMap[localizationKey]) {
+ updatedData.data[field] = localizationMap[localizationKey];
+ }
+ });
+ return updatedData;
+}, [data, localizationMap]);Committable suggestion skipped: line range outside the PR's diff.
| //action===DISABLE || ENABLE call update api and show toast respectively | ||
| else{ | ||
| //call update mutation | ||
| history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? "&" + additionalParamString : ""}`) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use template literals for URL construction
Replace string concatenation with template literals for better readability.
- history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? "&" + additionalParamString : ""}`)
+ history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? `&${additionalParamString}` : ""}`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? "&" + additionalParamString : ""}`) | |
| history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? `&${additionalParamString}` : ""}`) |
🧰 Tools
🪛 Biome (1.9.4)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
| history.push(`../utilities/audit-log?id=${finalData?.id}&tenantId=${tenantId}`) | ||
| }} | ||
| ></Button> | ||
| {showToast && <Toast label={t(showToast.label)} error={showToast?.isError} onClose={() => setShowToast(null)}></Toast>} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance Toast error handling
Consider adding a fallback message when the translation key is missing.
- {showToast && <Toast label={t(showToast.label)} error={showToast?.isError} onClose={() => setShowToast(null)}></Toast>}
+ {showToast && (
+ <Toast
+ label={t(showToast.label, { defaultValue: 'Operation completed' })}
+ error={showToast?.isError}
+ onClose={() => setShowToast(null)}
+ />
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {showToast && <Toast label={t(showToast.label)} error={showToast?.isError} onClose={() => setShowToast(null)}></Toast>} | |
| {showToast && ( | |
| <Toast | |
| label={t(showToast.label, { defaultValue: 'Operation completed' })} | |
| error={showToast?.isError} | |
| onClose={() => setShowToast(null)} | |
| /> | |
| )} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 204-204: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
| let messageMap = {}; | ||
| if (Array.isArray(respData?.messages)) { | ||
| respData.messages.forEach(msg => { | ||
| messageMap[msg.code] = msg.message; | ||
| }); | ||
| } | ||
| return messageMap; | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optimize message map creation
The message map creation can be simplified using reduce and proper variable declaration.
- let messageMap = {};
- if (Array.isArray(respData?.messages)) {
- respData.messages.forEach(msg => {
- messageMap[msg.code] = msg.message;
- });
- }
- return messageMap;
+ return Array.isArray(respData?.messages)
+ ? respData.messages.reduce((acc, msg) => ({ ...acc, [msg.code]: msg.message }), {})
+ : {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let messageMap = {}; | |
| if (Array.isArray(respData?.messages)) { | |
| respData.messages.forEach(msg => { | |
| messageMap[msg.code] = msg.message; | |
| }); | |
| } | |
| return messageMap; | |
| }, | |
| return Array.isArray(respData?.messages) | |
| ? respData.messages.reduce((acc, msg) => ({ ...acc, [msg.code]: msg.message }), {}) | |
| : {}; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 158-158: This let declares a variable that is only assigned once.
'messageMap' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js(10 hunks)micro-ui/web/public/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
[error] 156-158: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 166-168: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 224-224: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 318-318: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 332-332: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 360-360: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 366-366: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 385-385: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 451-458: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 285-285: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (10)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (9)
25-26: Context logic is straightforward.
Defining both the context and the custom hook side-by-side helps keep usage concise and flexible.
46-48: Removed transformErrors and Digit references.
The removal seems intentional to simplify error management. No immediate issues found.
156-158: Associate labels with inputs for accessibility.
This is consistent with a past comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 156-158: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
166-168: Same accessibility concern for labels.
Please associate these labels with their respective input for improved a11y compliance.
🧰 Tools
🪛 Biome (1.9.4)
[error] 166-168: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
285-285: Use const instead of let.
This was previously noted. Switching to const clarifies intent since the variable is never reassigned.
🧰 Tools
🪛 Biome (1.9.4)
[error] 285-285: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
318-318: Replace hasOwnProperty with Object.hasOwn().
Accessing Object.prototype methods directly can pose safety issues.
🧰 Tools
🪛 Biome (1.9.4)
[error] 318-318: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
332-332: Same hasOwnProperty vs. Object.hasOwn() concern.
Replacing it prevents potential prototype pollution.
🧰 Tools
🪛 Biome (1.9.4)
[error] 332-332: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
385-385: Prefer template literals.
String concatenation is noted in a prior comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 385-385: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
451-458: Use self-closing tag for empty JSX elements.
This was flagged in a past review.
🧰 Tools
🪛 Biome (1.9.4)
[error] 451-458: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
micro-ui/web/public/index.html (1)
10-10: Verify that the new CSS version is backward-compatible.
Before updating the version in production, confirm that there are no breaking changes or deprecated styles.
| <span className="array-children">{children}</span> | ||
| {props.hasRemove && ( | ||
| <div className="array-remove-button-wrapper"> | ||
| <Button | ||
| label={t("WBH_DELETE_ACTION")} | ||
| variation="secondary" | ||
| className="array-remove-button" | ||
| icon={<SVG.Delete width={"28"} height={"28"} />} | ||
| onButtonClick={onDropIndexClick(index)} | ||
| type="button" | ||
| isDisabled={disabled} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Suggestion: Confirm removal with the user.
When removing array items, consider prompting the user with a confirmation dialog to avoid accidental removals.
| const { children, className, index, onDropIndexClick, schema, disabled } = props; | ||
| const isArrayOfObjects = schema?.type == "object"; | ||
| const isArrayOfObjects = schema?.type === "object"; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Unused variable (isArrayOfObjects).
The variable is defined but never used. If it’s not needed, consider removing it for cleanliness.
- const isArrayOfObjects = schema?.type === "object";
Choose the appropriate template for your PR:
Feature PR
Feature Request
JIRA ID
https://digit-discuss.atlassian.net/browse/SN-368
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores