added the Workbench module patches which was done as part of Mukta ro… #1967
Conversation
📝 WalkthroughWalkthroughThe pull request introduces several enhancements across multiple components in the micro-ui workbench module. The changes focus on improving form handling, data management, and user interface interactions. Key modifications include updating the Changes
Sequence DiagramsequenceDiagram
participant User
participant DigitJSONForm
participant CustomSelectWidget
participant APIService
User->>DigitJSONForm: Submits form
DigitJSONForm->>DigitJSONForm: Validate form data
alt Validation fails
DigitJSONForm->>User: Display error toast
else Validation passes
DigitJSONForm->>APIService: Submit form data
APIService->>DigitJSONForm: Return response
end
User->>CustomSelectWidget: Select option
CustomSelectWidget->>APIService: Fetch option details
APIService->>CustomSelectWidget: Return option details
CustomSelectWidget->>User: Update selected option
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: 20
🧹 Outside diff range comments (4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSSearchv2.js (2)
Line range hint
112-131: Optimize dropdown options sorting logic and add safety checksThe current implementation has redundant conditional checks and potential runtime safety issues.
Consider refactoring to:
- dropDownOptions = - dropDownOptions?.length > 0 && - Digit?.Customizations?.["commonUiConfig"]?.["SearchMDMSv2Config"]?.[schemaCodeToValidate]?.sortValidDatesFirst(dropDownOptions) - ? Digit?.Customizations?.["commonUiConfig"]?.["SearchMDMSv2Config"]?.[schemaCodeToValidate]?.sortValidDatesFirst(dropDownOptions) - : dropDownOptions; + const sortFn = Digit?.Customizations?.["commonUiConfig"]?.["SearchMDMSv2Config"]?.[schemaCodeToValidate]?.sortValidDatesFirst; + dropDownOptions = dropDownOptions?.length > 0 && typeof sortFn === 'function' ? sortFn(dropDownOptions) : dropDownOptions;
Line range hint
200-204: Improve conditional readability and function namingThe current condition chain is hard to read and the function name has grammatical issues.
Consider refactoring to:
- updatedConfig && Digit.Utils.didEmployeeHasAtleastOneRole(updatedConfig?.actionRoles) && Digit.Utils.didEmployeeisAllowed(master,modulee) + const hasRequiredRole = Digit.Utils.didEmployeeHasAtleastOneRole(updatedConfig?.actionRoles); + const isAllowedAccess = Digit.Utils.didEmployeeisAllowed(master, modulee); + updatedConfig && hasRequiredRole && isAllowedAccessAlso, consider renaming the function to
isEmployeeAllowedfor better grammar.🧰 Tools
🪛 Biome (1.9.4)
[error] 192-192: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 198-198: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js (1)
Line range hint
91-108: Simplify session storage managementThe session storage clearing logic is complex and scattered. Consider consolidating this into a custom hook for better maintainability.
const useSessionStorageCleanup = (prefixes, dependencies) => { useEffect(() => { const clearSessionStorageWithPrefix = (prefix) => { Object.keys(sessionStorage) .filter(key => key.startsWith(`Digit.${prefix}`)) .forEach(key => sessionStorage.removeItem(key)); }; const currentUrl = window.location.href; const cleanupRules = { 'MDMS_add': !currentUrl.includes('mdms-add-v2') && !currentUrl.includes('mdms-add-v4') && !currentUrl.includes('mdms-view'), 'MDMS_view': !currentUrl.includes('mdms-view'), 'MDMS_edit': !currentUrl.includes('mdms-edit') }; prefixes.forEach(prefix => { if (cleanupRules[prefix]) { clearSessionStorageWithPrefix(prefix); } }); }, dependencies); };micro-ui/web/micro-ui-internals/packages/modules/workbench/src/utils/index.js (1)
Line range hint
194-279: Add unit tests for date handling functionsThe date handling functions are critical components that should be thoroughly tested. Consider adding unit tests to cover:
- Different timezone scenarios
- Edge cases for start/end of day
- Various key parameter values
- Date format conversions
Would you like me to help create a test suite for these date handling functions?
🧰 Tools
🪛 Biome (1.9.4)
[error] 200-200: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 201-201: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 206-206: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 196-196: Reassigning a function parameter is confusing.
The parameter is declared here:
Use a local variable instead.
(lint/style/noParameterAssign)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (10)
micro-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/dss/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/packages/modules/workbench/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/packages/react-components/package.jsonis excluded by!**/*.jsonmicro-ui/web/package.jsonis excluded by!**/*.jsonmicro-ui/web/sandbox/package.jsonis excluded by!**/*.jsonmicro-ui/web/workbench/package.jsonis excluded by!**/*.json
📒 Files selected for processing (13)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js(4 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/MultiSelect.js(5 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js(4 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/searchMDMSConfigPopup.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/hooks/inbox.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/hooks/index.js(2 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js(2 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js(4 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSSearchv2.js(4 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js(4 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js(4 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/utils/index.js(4 hunks)micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/BreadCrumb.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/searchMDMSConfigPopup.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/BreadCrumb.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/hooks/index.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/workbench/src/pages/employee/MDMSEdit.js (1)
Pattern **/*.js: check
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/hooks/inbox.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/MultiSelect.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/utils/index.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/components/DigitJSONForm.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/hooks/index.js
[error] 2-2: Declarations inside of a import declaration may not have duplicates
a second declaration of isSchemaCodeInMDMSAction is not allowed
isSchemaCodeInMDMSAction is first declared here
(parse)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSSearchv2.js
[error] 162-162: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
[error] 192-192: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 198-198: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/hooks/inbox.js
[error] 5-5: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 83-83: This let declares a variable that is only assigned once.
'updatedPath' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js
[error] 24-24: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 44-44: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/MultiSelect.js
[error] 157-157: This let declares a variable that is only assigned once.
'newFormattedOptions2' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/utils/index.js
[error] 200-200: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 201-201: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 206-206: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 196-196: Reassigning a function parameter is confusing.
The parameter is declared here:
Use a local variable instead.
(lint/style/noParameterAssign)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
[error] 68-68: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
[error] 77-77: This let declares a variable that is only assigned once.
'propsToSendButtons' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 137-137: This let declares a variable that is only assigned once.
'propsToSend' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js
[error] 843-843: This let declares a variable that is only assigned once.
'hostname' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 845-845: This let declares a variable that is only assigned once.
'roleActions' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (13)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSSearchv2.js (1)
161-162: LGTM! Good handling of EPOC date format
The conditional additionalCustomization based on format type is a good approach for handling EPOC dates.
🧰 Tools
🪛 Biome (1.9.4)
[error] 162-162: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/BreadCrumb.js (1)
30-32: 🧹 Nitpick (assertive)
Consider using React Router's navigation for better UX
Full page reloads via window.location.href can disrupt the user experience. Consider using React Router's navigation methods for smoother transitions.
- crumb?.externalPath ? (
- <Link onClick={() => handleRedirect(crumb?.path)}>{crumb.content} </Link>) :
- (<Link to={{ pathname:crumb.path, state: {count : crumb?.count }, search: crumb?.query }}>{crumb.content}</Link>)
+ crumb?.externalPath ? (
+ <Link to={crumb.path} target={crumb.openInNewTab ? "_blank" : "_self"}>{crumb.content}</Link>
+ ) : (
+ <Link to={{ pathname:crumb.path, state: {count : crumb?.count }, search: crumb?.query }}>{crumb.content}</Link>
+ )Likely invalid or redundant comment.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js (1)
83-83: Consider implications of useRef for initialContextPath
Using useRef for initialContextPath means the value won't trigger re-renders if window.contextPath changes. Verify if this is the intended behavior.
Also applies to: 125-125
✅ Verification successful
Based on the search results, I can see that window.contextPath is a critical configuration value used extensively throughout the application for routing and navigation. The value is:
- Initially set at the application level from global configs:
window.contextPath = window?.globalConfigs?.getConfig("CONTEXT_PATH");- In some cases, it's dynamically updated with tenant information:
window.contextPath = window?.globalConfigs?.getConfig("CONTEXT_PATH") + `${tenant ? `/${tenant}` : ""}` || "digit-ui";Using useRef for initialContextPath is actually the correct approach here because:
- The initial context path needs to be preserved as a reference point, regardless of any subsequent changes to
window.contextPath - The component doesn't need to re-render when this reference value changes since it's used as an initial snapshot
- This pattern helps compare the initial route context with any subsequent changes
Using useRef is the correct implementation here
The current implementation correctly captures the initial context path value that can be used as a reference point throughout the component lifecycle, without triggering unnecessary re-renders.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if contextPath is used consistently across the codebase
rg "window\.contextPath" -A 2 -B 2
Length of output: 39359
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js (2)
35-35: Ensure Proper Handling of URL Generation
The url is constructed using Digit.Utils.workbench.getMDMSActionURL(moduleName, masterName, "update"). Please verify that moduleName and masterName are always defined and valid strings to prevent potential runtime errors due to undefined or null values.
124-124: Validate Structure of transformedData Before Mutation
When passing transformedData in the mutation's body, ensure that it matches the expected schema required by the backend API. An incorrect data structure may result in failed updates or unexpected behavior.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/searchMDMSConfigPopup.js (1)
21-21: LGTM: Custom hook integration looks good.
The addition of customHook property to use useMDMSPopupSearch is a clean way to integrate search functionality.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js (2)
96-104: Good addition of validation logic.
The validation implementation is well-structured and handles error cases appropriately by:
- Validating form data before submission
- Showing translated error messages
- Preventing form submission on validation failure
120-120: LGTM: Improved error message formatting.
The error message now includes both the generic error message and the specific error code, making it more informative for users.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (3)
221-221: LGTM: Improved tooltip readability.
Setting max-width for tooltips prevents them from becoming too wide and ensures better readability.
282-282: LGTM: Better form disabled state management.
Centralizing the form disabled state logic into a single variable improves code maintainability and readability.
Also applies to: 317-317
336-336: LGTM: Enhanced menu visibility check.
The additional check for viewActions length prevents rendering an empty menu container when there are no actions.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/utils/index.js (2)
Line range hint 233-242: LGTM: Changes correctly propagate the key parameter
The changes to formatData appropriately pass the key parameter to the formatDates function.
🧰 Tools
🪛 Biome (1.9.4)
[error] 239-239: Duplicate case label.
The first similar label is here:
(lint/suspicious/noDuplicateCase)
[error] 238-238: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
257-257: Verify the impact of key parameter changes
The changes correctly propagate the key parameter. However, let's verify all usages to ensure consistent behavior.
Also applies to: 279-279
✅ Verification successful
Key parameter changes are correctly implemented
The verification shows that the key parameter is properly implemented and serves a specific purpose:
- The
formatDatafunction correctly passes the key parameter toformatDates - The
formatDatesfunction uses the key parameter to handle special cases for "validTo" and "effectiveTo" fields, setting appropriate time values (23:59:59.999 for end dates, 00:00:00.000 for others) - The changes are localized to the workbench module and don't require modifications elsewhere
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all calls to formatData to verify key parameter usage
# Search for formatData calls
echo "Searching for formatData calls..."
rg -A 2 "formatData\("
# Search for date-related fields in schema files
echo "Searching for date fields in schema files..."
rg -l '"type":\s*"(date|datetime|EPOC)"' --type json
Length of output: 944
Script:
#!/bin/bash
# Let's find the formatData function definition and its usages in a more comprehensive way
# Find the formatData function definition
echo "Searching for formatData function definition..."
ast-grep --pattern 'const formatData = $_'
# Search for any other files that might import and use formatData
echo "Searching for files importing formatData..."
rg "import.*formatData"
# Look for the schema structure to understand the formatType usage
echo "Searching for schema structure..."
rg -A 5 "formatType.*:"
Length of output: 2625
Script:
#!/bin/bash
# Let's examine the formatDates function since it's using the key parameter
# Find the formatDates function definition
echo "Searching for formatDates function definition..."
ast-grep --pattern 'const formatDates = $_'
# Look for any date-related schema configurations
echo "Searching for EPOC type configurations in the codebase..."
rg -B 2 -A 2 '"formatType":\s*"EPOC"'
# Search for any test files that might validate this functionality
echo "Searching for related test files..."
fd -e test.js -e spec.js -e test.jsx -e spec.jsx
Length of output: 2943
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSSearchv2.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/BreadCrumb.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/hooks/index.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/hooks/index.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/utils/index.js
Show resolved
Hide resolved
| @@ -279,6 +279,8 @@ const DigitJSONForm = ({ | |||
| }; | |||
| const person = { t: t }; | |||
|
|
|||
| const formDisabled = screenType === "view" ? true : disabled; | |||
There was a problem hiding this comment.
this is already done as part of view screen wrapper right @Swathi-eGov
| @@ -49,14 +49,15 @@ const CustomSelectWidget = (props) => { | |||
| /* | |||
| logic added to fetch data of schemas in each component itself | |||
| */ | |||
| const limit = schemaCode === "WORKS-SOR.SOR" ? 1000 : 100; | |||
There was a problem hiding this comment.
even with 1000 data it didnt solve the issue right ?
| const [limitedOptions, setLimitedOptions] = useState([]); | ||
| const [selectedDetails, setSelectedDetails] = useState(null); | ||
| const [showDetails, setShowDetails] = useState(null); | ||
| const [isSelect, setIsSelect] = useState(false); | ||
| const [showModal, setShowModal] = useState(false); | ||
| const [isSeeAll, setIsSeeAll] = useState(false); | ||
|
|
||
| const fetchDetailsForSelectedOption = async (value) => { | ||
| const response = await fetch(`/${Digit.Hooks.workbench.getMDMSContextPath()}/v2/_search`, { |
There was a problem hiding this comment.
use custom service instead of fetch
| @@ -190,7 +224,8 @@ const CustomSelectWidget = (props) => { | |||
| isSearchable={true} | |||
| isMulti={multiple} | |||
| styles={customStyles} | |||
| components={isSeeAll ? { MenuList: SelectMenuButton, Option: OptionWithInfo } : { Option: OptionWithInfo }} | |||
| // components={isSeeAll ? { MenuList: SelectMenuButton, Option: OptionWithInfo } : { Option: OptionWithInfo }} | |||
| components={schema?.schemaCode ? { MenuList: SelectMenuButton, Option: OptionWithInfo } : { Option: OptionWithInfo }} | |||
There was a problem hiding this comment.
can u check what was that seeall and from where it is calculated
| const crumbs = [ | ||
| { | ||
| path: `/${window?.contextPath}/employee`, | ||
| path: getHomePath(), |
There was a problem hiding this comment.
revert this change
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range comments (1)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js (1)
Line range hint
89-102: Extract session management logic into a utility functionThe session storage clearing logic could be more maintainable if extracted into a separate utility function.
+ const clearSessionStorageForURLs = (currentUrl) => { + const urlMappings = { + 'MDMS_add': ['mdms-add-v2', 'mdms-add-v4', 'mdms-view'], + 'MDMS_view': ['mdms-view'], + 'MDMS_edit': ['mdms-edit'] + }; + + Object.entries(urlMappings).forEach(([prefix, urls]) => { + if (!urls.some(url => currentUrl.includes(url))) { + clearSessionStorageWithPrefix(prefix); + } + }); + }; + useEffect(() => { - // Function to clear session storage for keys with specific prefixes const clearSessionStorageWithPrefix = (prefix) => { Object.keys(sessionStorage).forEach((key) => { if (key.startsWith(`Digit.${prefix}`)) { sessionStorage.removeItem(key); } }); }; const currentUrl = window.location.href; - if (!currentUrl.includes("mdms-add-v2") && !currentUrl.includes("mdms-add-v4") && !currentUrl.includes("mdms-view")) { - clearSessionStorageWithPrefix("MDMS_add"); - } - if (!currentUrl.includes("mdms-view")) { - clearSessionStorageWithPrefix("MDMS_view"); - } - if (!currentUrl.includes("mdms-edit")) { - clearSessionStorageWithPrefix("MDMS_edit"); - } + clearSessionStorageForURLs(currentUrl); }, [window.location.href]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/MultiSelect.js(4 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/MultiSelect.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/index.js
[error] 24-24: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/MultiSelect.js
[error] 154-154: This let declares a variable that is only assigned once.
'newFormattedOptions2' 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/index.js (4)
1-1: LGTM: Import statement update
The addition of useRef to the React imports is appropriate and follows the established import grouping pattern.
Line range hint 36-38: Simplify conditional expressions
The ternary expressions returning boolean literals can be simplified.
- show: from ? false : pathVar.includes("mdms-") ? true : false,
+ show: !from && pathVar.includes("mdms-"),30-30: 🧹 Nitpick (assertive)
Use template literals for path construction
Replace string concatenation with template literals for better readability.
- path:`/${window.contextPath}/employee`,
+ path: `/${window.contextPath}/employee`,Likely invalid or redundant comment.
Line range hint 89-111: Verify session storage clearing impact
The session storage clearing logic might affect other parts of the application. Please verify that clearing these specific session storage keys doesn't impact other features or cause unintended side effects.
✅ Verification successful
Session storage clearing is safely scoped to MDMS module
Based on the verification results:
- The MDMS session storage keys are only used within the workbench module (
packages/modules/workbench/) - The
clearSessionFormDataandclearSessionFormDataViewfunctions are module-specific implementations - The session storage clearing logic is properly scoped to MDMS-related routes and doesn't overlap with other features
The session storage clearing implementation is well-contained and won't affect other parts of the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other usage of these session storage keys in the codebase
rg -l "Digit\.(MDMS_add|MDMS_view|MDMS_edit)"
Length of output: 47
Script:
#!/bin/bash
# Search for any usage of MDMS session storage keys without the Digit prefix
rg -l "MDMS_(add|view|edit)"
Length of output: 117
Script:
#!/bin/bash
# Search for sessionStorage operations with these prefixes
rg "sessionStorage\.(get|set|remove)Item.*MDMS_(add|view|edit)" -A 2
Length of output: 70
Script:
#!/bin/bash
# Look for clearSessionStorage related functions
ast-grep --pattern 'function clearSessionStorage$_($$$) { $$$ }'
Length of output: 66
Script:
#!/bin/bash
# Search for any form data clearing functions
rg "clearSessionFormData|clearSessionFormDataView"
Length of output: 3397
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/MultiSelect.js (3)
97-112: Add error handling in fetchDetailsForSelectedOption
The function fetchDetailsForSelectedOption does not handle potential errors from the async operation. This can lead to unhandled promise rejections and application crashes.
154-154: Use const instead of let for newFormattedOptions2
The variable newFormattedOptions2 is not reassigned after its initial assignment. Using const ensures it cannot be accidentally reassigned.
🧰 Tools
🪛 Biome (1.9.4)
[error] 154-154: This let declares a variable that is only assigned once.
'newFormattedOptions2' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
158-160: 🛠️ Refactor suggestion
Avoid mutating state directly within useEffect
Adding items to newFormattedOptions2 within the useEffect hook might lead to unexpected side effects. Consider using state updater functions or ensuring immutability.
Apply this diff to modify the code:
- newFormattedOptions2.push({ value, label: formattedLabel });
+ const updatedOptions = [...newFormattedOptions2, { value, label: formattedLabel }];
+ setFormattedOptions2(updatedOptions);Likely invalid or redundant comment.
| @@ -49,14 +49,15 @@ const CustomSelectWidget = (props) => { | |||
| /* | |||
| logic added to fetch data of schemas in each component itself | |||
| */ | |||
| const limit = schemaCode === "WORKS-SOR.SOR" ? 1000 : 100; | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider defining limit values as constants
To enhance code readability and maintainability, consider defining the limit values (1000 and 100) as constants or configuration variables.
| @@ -85,12 +86,31 @@ | |||
| () => optionsList.map((e) => ({ label: t(Digit.Utils.locale.getTransformedLocale(`${schemaCode}_${e?.label}`)), value: e.value })), | |||
| [optionsList, schemaCode, data] | |||
| ); | |||
| const [formattedOptions2, setFormattedOptions2] = useState([]); | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve variable naming for better clarity
The variable formattedOptions2 may not clearly convey its purpose. Consider using a more descriptive name to enhance code readability.
| @@ -111,7 +131,7 @@ | |||
| </div> | |||
| ) | |||
| } | |||
| const selectedOption = formattedOptions?.filter((obj) => (multiple ? value?.includes(obj.value) : obj.value == value)); | |||
| const selectedOption = formattedOptions2?.filter((obj) => (multiple ? value?.includes(obj.value) : obj.value == value)); | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use strict equality checks for reliable comparisons
Consider using === instead of == when comparing values in selectedOption to prevent unexpected type coercion.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (9)
micro-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/workbench/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/packages/react-components/package.jsonis excluded by!**/*.jsonmicro-ui/web/package.jsonis excluded by!**/*.jsonmicro-ui/web/sandbox/package.jsonis excluded by!**/*.jsonmicro-ui/web/workbench/package.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js(4 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
🔇 Additional comments (3)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (3)
221-221: Consider verifying the translation availability for tooltips.
You are using t(\TIP_${titleCode}`)` for tooltip text. Ensure that the corresponding key exists in the translation files to avoid displaying missing translations or fallback text.
282-283: Revisit form disable logic for "view" screen type
As previously noted in a past comment (see line 282 prior discussion), disabling the entire form for a "view" screen may already be handled by the view screen wrapper logic. Confirm whether you still need to explicitly set the disabled state here.
336-336: Conditionally rendering the action menu is a good practice.
Displaying the action menu only when viewActions?.length > 0 prevents empty or meaningless UI elements. This change looks good.
| @@ -312,7 +314,7 @@ const DigitJSONForm = ({ | |||
| transformErrors={transformErrors.bind(person)} | |||
| uiSchema={{ ...uiSchema, ...inputUiSchema }} | |||
| onError={onError} | |||
| disabled={disabled} | |||
| disabled={formDisabled} | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure consistent read-only styles when the form is disabled.
When the form is disabled, certain elements (e.g., Buttons, custom widgets) may still appear active. Consider applying a uniform read-only or disabled styling for consistency and clarity in the UI.
…llout.
Choose the appropriate template for your PR:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor