fix(web): warning occurs when re-referring to an item referenced earlier #1264
Conversation
WalkthroughThe changes involve updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
✅ Deploy Preview for reearth-cms ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (14)
web/src/components/molecules/Content/Form/fields/ComplexFieldComponents/ReferenceField.tsx (3)
23-27: Approve changes with a minor suggestion.The updated
onCheckItemReferenceprop signature is an improvement. It provides more specific parameter names (itemIdinstead ofvalue) and adds an optionalgroupIdparameter, which aligns with the PR objective of improving item reference handling.Consider adding JSDoc comments to explain the purpose of each parameter, especially the new
groupId. This would enhance code readability and maintainability. For example:/** * Checks if an item can be referenced. * @param itemId - The ID of the item to be referenced. * @param correspondingFieldId - The ID of the corresponding field. * @param groupId - Optional. The ID of the group, if applicable. * @returns A promise that resolves to a boolean indicating if the item can be referenced. */ onCheckItemReference?: ( itemId: string, correspondingFieldId: string, groupId?: string ) => Promise<boolean>;
62-75: Approve changes with a suggestion for consistency.The modifications to the props passed to
ReferenceFormItemare appropriate and align well with the updatedonCheckItemReferencesignature. The changes improve the component's flexibility and provide more specific data to the child component.For consistency, consider reordering the props alphabetically. This can improve readability and make it easier to locate specific props. Here's a suggested order:
<ReferenceFormItem correspondingField={field.typeProperty?.correspondingField} disabled={disabled} fieldId={field.id} formItemsData={formItemsData} itemGroupId={itemGroupId} key={field.id} linkItemModalPage={linkItemModalPage} linkItemModalPageSize={linkItemModalPageSize} linkItemModalTitle={linkItemModalTitle} linkItemModalTotalCount={linkItemModalTotalCount} linkedItemsModalList={linkedItemsModalList} loading={loading} modelId={field.typeProperty?.modelId} onCheckItemReference={onCheckItemReference} onLinkItemTableChange={onLinkItemTableChange} onLinkItemTableReload={onLinkItemTableReload} onReferenceModelUpdate={onReferenceModelUpdate} onSearchTerm={onSearchTerm} titleFieldId={field.typeProperty?.schema?.titleFieldId} />This reordering doesn't change the functionality but can make the code more maintainable.
Line range hint
1-83: Summary: Improved item reference handling with consistent changes.The modifications in this file successfully address the PR objective of improving item reference handling. The changes to both the
ReferenceFieldPropstype and theReferenceFieldcomponent are consistent and well-implemented.Key improvements:
- Updated
onCheckItemReferencesignature for more precise item referencing.- Enhanced
ReferenceFormItemprops for better flexibility and data passing.These changes should effectively resolve the warning issue when re-referring to an item referenced earlier, as mentioned in the PR objectives.
To further improve the overall architecture:
- Consider creating a custom hook (e.g.,
useReferenceField) to encapsulate the logic for handling reference fields. This could simplify theReferenceFieldcomponent and make it easier to reuse the logic in other parts of the application if needed.- If not already done, ensure that unit tests are updated or added to cover the new functionality, especially the behavior of
onCheckItemReferencewith the newgroupIdparameter.web/src/components/molecules/Content/Form/ReferenceFormItem/index.tsx (2)
19-22: LGTM: Props update enhances component flexibilityThe changes to the Props type definition, including the rename of
correspondingFieldIdtofieldIdand the addition of new optional props, align well with the PR objectives. These modifications provide more context and flexibility to the component.Consider adding JSDoc comments for the new props to improve documentation:
/** The ID of the field */ fieldId: string; /** Optional group ID for the item */ itemGroupId?: string; /** Optional array of form item data */ formItemsData?: FormItem[]; /** Optional corresponding field information */ correspondingField?: CorrespondingField;
33-37: LGTM: Enhanced onCheckItemReference callback signatureThe updated
onCheckItemReferencecallback signature provides more context for the reference check, aligning with the PR objectives. The addition ofitemIdas the first parameter and the optionalgroupIdallows for more specific and flexible reference checks.Consider adding a JSDoc comment to explain the purpose and parameters of this callback:
/** * Callback to check if an item can be referenced * @param itemId - The ID of the item to be referenced * @param correspondingFieldId - The ID of the corresponding field * @param groupId - Optional group ID for more specific checks * @returns A promise that resolves to a boolean indicating if the reference is valid */ onCheckItemReference?: ( itemId: string, correspondingFieldId: string, groupId?: string ) => Promise<boolean>;web/src/components/molecules/Content/Form/fields/ComplexFieldComponents/GroupField.tsx (1)
51-55: LGTM! Consider adding JSDoc comments for clarity.The updated
onCheckItemReferencefunction signature improves specificity and aligns with the PR objectives. The addition of the optionalgroupIdparameter allows for more contextual reference checking, which should help address the warning issue mentioned in the PR summary.Consider adding JSDoc comments to clarify the purpose of each parameter, especially the new
groupId. This would enhance code readability and maintainability. For example:/** * Checks if an item can be referenced. * @param itemId - The ID of the item to be referenced. * @param correspondingFieldId - The ID of the corresponding field. * @param groupId - Optional. The ID of the group if the item is within a group context. * @returns A promise that resolves to a boolean indicating if the item can be referenced. */ onCheckItemReference?: ( itemId: string, correspondingFieldId: string, groupId?: string ) => Promise<boolean>;web/src/components/molecules/Common/MultiValueField/MultiValueGroup/index.tsx (1)
62-66: LGTM! Consider minor improvements for clarity.The updated
onCheckItemReferenceprop signature is a good improvement, making it more flexible and descriptive. The changes are backwards-compatible due to the optionalgroupIdparameter.Suggestions for further enhancement:
- Consider renaming the function to be more specific, e.g.,
onValidateItemReference.- Add JSDoc comments to explain the purpose of each parameter, especially the new
groupId.Example:
/** * Validates an item reference. * @param itemId - The ID of the item being referenced. * @param correspondingFieldId - The ID of the field containing the reference. * @param groupId - Optional. The ID of the group if the reference is within a group. * @returns A promise that resolves to a boolean indicating if the reference is valid. */ onValidateItemReference?: ( itemId: string, correspondingFieldId: string, groupId?: string ) => Promise<boolean>;web/src/components/molecules/Content/Details/index.tsx (1)
102-106: Approved: Function signature update improves clarity and functionality.The changes to the
onCheckItemReferencefunction signature are well-considered:
- Renaming
valuetoitemIdimproves clarity.- Adding the optional
groupIdparameter allows for more granular reference checks, which aligns with the PR objective of addressing issues with re-referencing items.Consider adding a brief comment above the function to explain its purpose and the significance of the
groupIdparameter. This would help other developers understand when and how to use this optional parameter.web/src/components/organisms/Project/Content/ContentDetails/hooks.ts (1)
570-582: Improved item reference checking logic, but consider edge casesThe changes to
handleCheckItemReferenceeffectively address the issue of unnecessary warnings when re-referring to an item. The new logic checks against initial values before making an API call, which should improve performance and user experience.However, consider the following improvements:
- Add a null check for
initialFormValues[groupId]to handle cases where the group might not exist:const initialValue = groupId - ? initialFormValues[groupId][correspondingFieldId] + ? initialFormValues[groupId]?.[correspondingFieldId] : initialFormValues[correspondingFieldId];
- Improve error handling for the API call:
const res = await checkIfItemIsReferenced({ variables: { itemId, correspondingFieldId }, }); -return res.data?.isItemReferenced ?? false; +return res.data?.isItemReferenced || false;
- Consider adding a comment explaining the purpose of the initial value check, as it might not be immediately clear to other developers.
web/src/components/molecules/Content/LinkItemModal/index.tsx (5)
Line range hint
75-75: Rename 'hoveredAssetId' to 'hoveredItemId' for consistency.The state variable is named
hoveredAssetId, but the setter function issetHoveredItemId. RenaminghoveredAssetIdtohoveredItemIdwill improve readability and maintain consistent naming conventions.Apply this diff to rename the variable:
- const [hoveredAssetId, setHoveredItemId] = useState<string>(); + const [hoveredItemId, setHoveredItemId] = useState<string>();Also, update all occurrences of
hoveredAssetIdtohoveredItemIdthroughout the component:- (item.id === linkedItem && hoveredAssetId !== item.id) || - (item.id !== linkedItem && hoveredAssetId === item.id); + (item.id === linkedItem && hoveredItemId !== item.id) || + (item.id !== linkedItem && hoveredItemId === item.id);Update the dependency array in the
useMemohook:- [t, linkedItem, hoveredAssetId, handleClick], + [t, linkedItem, hoveredItemId, handleClick],
93-100: Handle empty values more explicitly when unlinking an item.When unlinking an item,
handleChangeis called with an empty string. Depending on howonChangehandles empty strings, this might lead to unintended behavior. Consider passingnullto represent the absence of a linked item more clearly.Apply this diff to adjust the value passed when unlinking:
if (!link) { - handleChange(""); + handleChange(null); return; }Ensure that the
onChangeprop is updated to acceptstring | null:onChange?: (value: string) => void; +// Update to: +onChange?: (value: string | null) => void;
Line range hint
102-115: Add error handling for 'onCheckItemReference' to prevent unhandled promise rejections.The
onCheckItemReferencefunction is awaited without a try-catch block. If it throws an error, the application might crash or behave unexpectedly. Wrap the asynchronous call in a try-catch block to handle potential errors gracefully.Apply this diff to include error handling:
+ try { const isReferenced = await onCheckItemReference(item.id, fieldId, itemGroupId); if (isReferenced) { confirm({ title: t("This item has already been referenced"), content: t( "Do you want to reference this item again? The previous reference will be canceled automatically.", ), icon: <Icon icon="exclamationCircle" />, onOk() { handleChange(item.id); }, }); } else { handleChange(item.id); } + } catch (error) { + // Handle error (e.g., display a notification to the user) + console.error("Failed to check item reference:", error); + }
Line range hint
104-110: Improve the wording in the confirmation dialog for better user understanding.The current confirmation message may be unclear. Rephrasing it can enhance user experience by providing a clearer explanation of the action.
Apply this diff to update the dialog content:
confirm({ - title: t("This item has been referenced"), + title: t("This item has already been referenced"), content: t( - "Are you going to refer to it? The previous reference will be canceled automatically", + "Do you want to reference this item again? The previous reference will be canceled automatically.", ), icon: <Icon icon="exclamationCircle" />, onOk() { handleChange(item.id); }, });
118-118: Remove unnecessary dependency 'confirm' from 'handleClick' useCallback hook.The
confirmfunction is sourced from theModalcomponent and does not change between renders. Removing it from the dependency array can prevent unnecessary re-renders and aligns with best practices.Apply this diff to update the dependency array:
- [confirm, correspondingField, fieldId, handleChange, itemGroupId, onCheckItemReference, t], + [correspondingField, fieldId, handleChange, itemGroupId, onCheckItemReference, t],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- web/src/components/molecules/Common/Form/GroupItem/index.tsx (1 hunks)
- web/src/components/molecules/Common/MultiValueField/MultiValueGroup/index.tsx (1 hunks)
- web/src/components/molecules/Content/Details/index.tsx (1 hunks)
- web/src/components/molecules/Content/Form/ReferenceFormItem/index.tsx (5 hunks)
- web/src/components/molecules/Content/Form/fields/ComplexFieldComponents/GroupField.tsx (1 hunks)
- web/src/components/molecules/Content/Form/fields/ComplexFieldComponents/ReferenceField.tsx (2 hunks)
- web/src/components/molecules/Content/Form/index.tsx (1 hunks)
- web/src/components/molecules/Content/LinkItemModal/index.tsx (5 hunks)
- web/src/components/organisms/Project/Content/ContentDetails/hooks.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (12)
web/src/components/molecules/Content/Form/ReferenceFormItem/index.tsx (4)
9-9: LGTM: New import for CorrespondingField typeThe addition of the
CorrespondingFieldtype import is consistent with the newcorrespondingFieldprop in the component. This change enhances type safety and improves code readability.
46-48: LGTM: Updated props destructuringThe destructuring of the new props
fieldId,itemGroupId, andcorrespondingFieldis consistent with the changes made to the Props type definition. This ensures that these new props are available for use within the component, enhancing its functionality as intended.
Line range hint
1-156: Summary: Improved ReferenceFormItem component with enhanced flexibilityThe changes made to the ReferenceFormItem component align well with the PR objectives to fix the warning when re-referring to an item. The modifications include:
- Updated prop definitions with more specific naming and additional optional props.
- Enhanced
onCheckItemReferencecallback signature for more precise reference checking.- Consistent usage of new props throughout the component and in the LinkItemModal.
These changes should provide better control over the item referencing process and resolve the reported issue. The code quality is good, with only minor suggestions for documentation improvements.
To ensure the changes are effective:
- Verify that the LinkItemModal component correctly utilizes the new props.
- Test the component with various scenarios, including re-referring to previously referenced items.
- Consider adding unit tests to cover the new functionality and prevent regressions.
123-125: LGTM: Updated LinkItemModal propsThe addition of
fieldId,itemGroupId, andcorrespondingFieldprops to the LinkItemModal component is consistent with the earlier changes and aligns with the PR objectives. This allows for more specific handling of item references within the modal.To ensure that these changes are properly utilized, please verify that the LinkItemModal component is updated to handle these new props correctly. Run the following script to check the LinkItemModal component implementation:
web/src/components/molecules/Content/Form/fields/ComplexFieldComponents/GroupField.tsx (1)
Line range hint
1-180: Overall implementation looks good!The changes to the
onCheckItemReferencefunction signature are consistently applied throughout theGroupFieldcomponent. BothMultiValueGroupandGroupItemreceive the updated prop, ensuring that the new functionality is available regardless of whether the field is multiple or single-valued.The rest of the component structure remains unchanged, which helps maintain existing functionality while introducing the new feature. This approach minimizes the risk of unintended side effects.
web/src/components/molecules/Common/MultiValueField/MultiValueGroup/index.tsx (1)
62-66: Verify the impact on component usage.The changes to the
onCheckItemReferenceprop are localized to the type definition. To ensure these changes are properly integrated:
- Verify that all usages of the
MultiValueGroupcomponent have been updated to pass the correct parameters toonCheckItemReference.- Check if the implementation of
onCheckItemReferencein parent components has been updated to handle the newgroupIdparameter.- Update unit tests for the
MultiValueGroupcomponent and its parent components to cover the new function signature.To help with this verification, you can run the following script:
This script will help identify areas that might need updates due to the changes in the
onCheckItemReferenceprop.web/src/components/molecules/Common/Form/GroupItem/index.tsx (2)
Line range hint
1-268: Review child components for updated usage.While the
GroupItemcomponent's implementation remains unchanged, it's important to verify that child components, especiallyReferenceField, are correctly using the updatedonCheckItemReferencefunction signature.To check the implementation in child components:
#!/bin/bash # Search for the ReferenceField component definition rg --type-add 'typescript:*.{ts,tsx}' --type typescript 'const ReferenceField.*=.*\{' # Search for usage of onCheckItemReference within ReferenceField rg --type-add 'typescript:*.{ts,tsx}' --type typescript 'onCheckItemReference.*\(' -A 5 -B 5
64-68: LGTM! Verify implementation and usage.The updated
onCheckItemReferencefunction signature looks good and aligns with the PR objective. The addition ofitemIdand optionalgroupIdparameters should allow for more precise and flexible item reference checking.To ensure this change is properly implemented and used:
- Verify that the
onCheckItemReferenceprop is correctly passed down to this component with the new signature.- Check if the function is called with the correct parameters within this component.
- Review other components that may use this prop to ensure they've been updated accordingly.
You can use the following script to check for usage of
onCheckItemReference:✅ Verification successful
Verification Successful: All
onCheckItemReferenceusages are updated correctly.The
onCheckItemReferencefunction signature changes have been consistently applied across the codebase. All call sites provide the required parameters (itemId,correspondingFieldId, and optionallygroupId).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of onCheckItemReference in TypeScript and TypeScript React files rg --type-add 'typescript:*.{ts,tsx}' --type typescript 'onCheckItemReference'Length of output: 11775
Script:
#!/bin/bash # Search for calls to onCheckItemReference in TypeScript and TypeScript React files rg --type-add 'typescript:*.{ts,tsx}' --type typescript 'onCheckItemReference\s*\(' -A 3 -B 3Length of output: 730
web/src/components/organisms/Project/Content/ContentDetails/hooks.ts (2)
570-582: Summary: Improved item reference checking with minor suggestionsThe changes to
handleCheckItemReferenceeffectively address the issue of unnecessary warnings when re-referring to an item. The new logic checks against initial values before making an API call, which aligns with the PR objectives and should improve performance and user experience.Key improvements:
- Added check against initial values to prevent unnecessary API calls.
- Improved function signature for better clarity and flexibility.
Suggestions for further improvement:
- Add null checks for grouped fields.
- Enhance error handling for the API call.
- Consider adding explanatory comments for the initial value check.
- Verify the correct population of
initialFormValuesfor all field types, especially grouped fields.Overall, the changes are approved with minor suggestions for improvement.
570-582: Verify initialFormValues population for all field typesThe changes to
handleCheckItemReferencenow rely oninitialFormValuesbeing correctly populated. Please ensure that the useEffect hook that populatesinitialFormValues(around line 449) correctly handles all field types, especially grouped fields. This is crucial for the new reference checking logic to work as intended.To verify the correct population of
initialFormValues, run the following script:web/src/components/molecules/Content/Form/index.tsx (1)
120-124: To effectively locate all usages ofonCheckItemReference, please run the updated script below usingast-grep:#!/bin/bash # Description: Find all occurrences of onCheckItemReference to verify correct usage using ast-grep # Search for onCheckItemReference usage in TypeScript and TSX files echo "Searching for onCheckItemReference usage with ast-grep:" ast-grep --lang typescript --pattern 'onCheckItemReference($_, $_, $_?)' .If
ast-grepis not available or does not yield results, you can use the following script withrgand glob patterns:#!/bin/bash # Description: Find all occurrences of onCheckItemReference to verify correct usage using ripgrep with glob patterns # Search for onCheckItemReference usage in .ts and .tsx files echo "Searching for onCheckItemReference usage with ripgrep using glob patterns:" rg 'onCheckItemReference\s*\(' --glob '*.ts' --glob '*.tsx' -A 3web/src/components/molecules/Content/LinkItemModal/index.tsx (1)
82-88:⚠️ Potential issueAdd dependency 'visible' to 'handleChange' useCallback hook.
The
handleChangefunction callsonLinkItemModalCancel, which might depend onvisible. To ensure the latest value is used, addvisibleto the dependency array.Apply this diff to update the dependency array:
const handleChange = useCallback( (value: string) => { onChange?.(value); onLinkItemModalCancel(); }, - [onChange, onLinkItemModalCancel], + [onChange, onLinkItemModalCancel, visible], );Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
web/src/components/molecules/Content/Form/ReferenceFormItem/index.tsx (1)
46-48: Consider maintaining consistent naming between props and function parameters.The prop name
fieldIdis used here while theonCheckItemReferencefunction usescorrespondingFieldId. Consider using consistent naming to improve code maintainability.- fieldId, + correspondingFieldId, itemGroupId, correspondingField,web/src/components/organisms/Project/Content/ContentDetails/hooks.ts (1)
570-582: LGTM! Consider adding type safety and null checks.The changes effectively prevent unnecessary backend queries when re-referring to previously referenced items. However, consider these improvements:
- Add type safety for the nested object access
- Add null checks when accessing grouped values
async (itemId: string, correspondingFieldId: string, groupId?: string) => { - const initialValue = groupId - ? initialFormValues[groupId][correspondingFieldId] - : initialFormValues[correspondingFieldId]; + const initialValue = groupId + ? initialFormValues[groupId]?.[correspondingFieldId] + : initialFormValues[correspondingFieldId]; if (initialValue === itemId) { return false; } const res = await checkIfItemIsReferenced({ variables: { itemId, correspondingFieldId }, }); return res.data?.isItemReferenced ?? false; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- web/src/components/molecules/Content/Details/index.tsx (1 hunks)
- web/src/components/molecules/Content/Form/ReferenceFormItem/index.tsx (5 hunks)
- web/src/components/molecules/Content/Form/index.tsx (1 hunks)
- web/src/components/molecules/Content/LinkItemModal/index.tsx (5 hunks)
- web/src/components/organisms/Project/Content/ContentDetails/hooks.ts (1 hunks)
🔇 Additional comments (10)
web/src/components/molecules/Content/Form/ReferenceFormItem/index.tsx (2)
123-125: LGTM! Props are properly forwarded to LinkItemModal.The changes correctly pass the necessary context to the LinkItemModal component, enabling proper reference handling with group awareness.
9-9: LGTM! Type changes enhance reference context handling.The updated Props type and onCheckItemReference signature now properly handle group context, which should help prevent false warnings when re-referring to items.
Let's verify the consistency of these changes across the codebase:
Also applies to: 19-22, 33-37
✅ Verification successful
Let me try a different approach to verify the usage of
onCheckItemReferencein the codebase.This should help us understand:
- How onCheckItemReference is used across the codebase
- Where CorrespondingField type is imported and potentially used with onCheckItemReference
Based on the ripgrep results, I can now generate a final response as we have sufficient information about the usage of
onCheckItemReferenceacross the codebase.The type changes are consistent across the codebase
The search results confirm that all implementations and usages of
onCheckItemReferencefollow the updated signature with the optionalgroupIdparameter:
- All type definitions consistently show
(itemId: string, correspondingFieldId: string, groupId?: string)- The function is properly passed down through component hierarchies
- The implementation in
LinkItemModalshows actual usage withgroupIdparameterKey files verified:
- web/src/components/molecules/Content/Form/ReferenceFormItem/index.tsx (the file under review)
- web/src/components/molecules/Content/Details/index.tsx
- web/src/components/molecules/Content/LinkItemModal/index.tsx
- web/src/components/molecules/Common/Form/GroupItem/index.tsx
- web/src/components/molecules/Content/Form/fields/ComplexFieldComponents/ReferenceField.tsx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all implementations of onCheckItemReference follow the new signature # Search for onCheckItemReference implementations ast-grep --pattern 'onCheckItemReference = async ($params) => { $$$ }'Length of output: 74
Script:
#!/bin/bash # Search for onCheckItemReference usage patterns rg "onCheckItemReference" -A 3 # Also search for any files importing CorrespondingField rg "import.*CorrespondingField.*from" --type typescriptLength of output: 11209
web/src/components/molecules/Content/LinkItemModal/index.tsx (5)
14-14: LGTM: Import added for new type.The import is correctly placed and used for the new prop type.
25-27: LGTM: Props type updates align with PR objectives.The changes improve type safety and flexibility:
- Renamed field for better clarity
- Added optional props for enhanced functionality
- Updated function signature to support the new reference checking logic
Also applies to: 39-43
49-51: LGTM: Props destructuring matches type changes.The destructuring is correctly updated to match the Props type changes.
81-87: LGTM: Good refactoring to reduce code duplication.The new handleChange function effectively consolidates the onChange and modal cancellation logic, improving maintainability.
Line range hint
91-117: Consider adding error handling for the reference check.While the logic flow is improved, consider adding error handling for the
onCheckItemReferencecall to handle potential failures gracefully.Let's verify the changes:
Suggested improvement:
- const isReferenced = await onCheckItemReference(item.id, fieldId, itemGroupId); + try { + const isReferenced = await onCheckItemReference(item.id, fieldId, itemGroupId); + if (isReferenced) { + confirm({ + title: t("This item has been referenced"), + content: t( + "Are you going to refer to it? The previous reference will be canceled automatically", + ), + icon: <Icon icon="exclamationCircle" />, + onOk() { + handleChange(item.id); + }, + }); + } else { + handleChange(item.id); + } + } catch (error) { + Modal.error({ + title: t("Failed to check item reference"), + content: t("Please try again later"), + }); + }web/src/components/molecules/Content/Details/index.tsx (2)
103-107: LGTM! The updated signature enhances reference handling.The addition of the optional
groupIdparameter toonCheckItemReferenceis a well-structured change that:
- Maintains backward compatibility through the optional parameter
- Improves type safety with explicit TypeScript types
- Aligns with the PR objective by providing context-aware reference checking
103-107: Verify consistent implementation across the codebase.Let's ensure the updated signature is consistently implemented across all related components.
✅ Verification successful
Let me try a different approach to search for the implementations and usages.
Let me check the actual implementation of these components to see if they properly handle the new
groupIdparameter.
Let me try one final search to check the actual implementations with more context and different patterns.
Based on the search results, I can now provide a final response:
The
groupIdparameter is consistently implemented across the codebaseThe verification shows that:
- The type definition in all components (
LinkItemModal,Form,ReferenceField) properly includes the optionalgroupIdparameter- The actual implementation in
LinkItemModalcorrectly passes theitemGroupIdparameter when callingonCheckItemReference- The function signature maintains backward compatibility by making
groupIdoptional, which ensures existing callers continue to work🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent implementation of onCheckItemReference # Expected results: All implementations should accept itemId, correspondingFieldId, and optional groupId # Search for onCheckItemReference implementations echo "Searching for onCheckItemReference implementations..." rg -t typescript -t tsx "onCheckItemReference.*=.*\(" --no-heading # Search for calls to onCheckItemReference echo -e "\nSearching for onCheckItemReference calls..." rg -t typescript -t tsx "onCheckItemReference\(" --no-headingLength of output: 422
Script:
#!/bin/bash # Search for onCheckItemReference implementations and calls using ripgrep # Using just .ts and .tsx extensions directly echo "Searching for onCheckItemReference implementations..." rg "onCheckItemReference.*=.*\(" -g "*.ts" -g "*.tsx" -A 2 echo -e "\nSearching for onCheckItemReference calls..." rg "onCheckItemReference\(" -g "*.ts" -g "*.tsx" -A 1 # Also search for the type definition echo -e "\nSearching for type definitions..." rg "onCheckItemReference.*:.*" -g "*.ts" -g "*.tsx" -A 2Length of output: 3043
Script:
#!/bin/bash # Get more context around the implementations and calls echo "Checking LinkItemModal implementation..." rg -A 5 "onCheckItemReference\(.*\)" web/src/components/molecules/Content/LinkItemModal/index.tsx echo -e "\nChecking Form implementations..." rg -A 5 "onCheckItemReference\(.*\)" web/src/components/molecules/Content/Form/index.tsx echo -e "\nChecking ReferenceField implementation..." rg -A 5 "onCheckItemReference\(.*\)" web/src/components/molecules/Content/Form/fields/ComplexFieldComponents/ReferenceField.tsx echo -e "\nChecking GroupField implementation..." rg -A 5 "onCheckItemReference\(.*\)" web/src/components/molecules/Content/Form/fields/ComplexFieldComponents/GroupField.tsx echo -e "\nChecking ReferenceFormItem implementation..." rg -A 5 "onCheckItemReference\(.*\)" web/src/components/molecules/Content/Form/ReferenceFormItem/index.tsx echo -e "\nChecking GroupItem implementation..." rg -A 5 "onCheckItemReference\(.*\)" web/src/components/molecules/Common/Form/GroupItem/index.tsx echo -e "\nChecking MultiValueGroup implementation..." rg -A 5 "onCheckItemReference\(.*\)" web/src/components/molecules/Common/MultiValueField/MultiValueGroup/index.tsxLength of output: 1733
Script:
#!/bin/bash # Search for method implementations and prop passing with more context echo "Checking LinkItemModal with more context..." rg -B 5 -A 10 "onCheckItemReference.*=.*\(|onCheckItemReference:.*\(|onCheckItemReference\?:.*\(|onCheckItemReference.*\(" web/src/components/molecules/Content/LinkItemModal/index.tsx echo -e "\nChecking Form component with more context..." rg -B 5 -A 10 "onCheckItemReference.*=.*\(|onCheckItemReference:.*\(|onCheckItemReference\?:.*\(|onCheckItemReference.*\(" web/src/components/molecules/Content/Form/index.tsx echo -e "\nChecking ReferenceField with more context..." rg -B 5 -A 10 "onCheckItemReference.*=.*\(|onCheckItemReference:.*\(|onCheckItemReference\?:.*\(|onCheckItemReference.*\(" web/src/components/molecules/Content/Form/fields/ComplexFieldComponents/ReferenceField.tsx echo -e "\nChecking prop passing patterns..." rg -B 2 -A 2 "onCheckItemReference={.*}" -g "*.tsx"Length of output: 3048
web/src/components/molecules/Content/Form/index.tsx (1)
121-125: LGTM! Function signature improvement enhances reference checking.The updated
onCheckItemReferencesignature:
- Uses more descriptive parameter names (
value→itemId)- Adds optional
groupIdparameter for group-specific reference checks- Aligns with the PR's objective of fixing re-referencing warnings
Let's verify that all callers have been updated to use the new signature:
✅ Verification successful
Let me analyze the search results to verify the function signature changes:
Function type definitions found in:
web/src/components/molecules/Content/Details/index.tsxweb/src/components/molecules/Content/LinkItemModal/index.tsxweb/src/components/molecules/Content/Form/index.tsx
All show the updated signature withitemId,correspondingFieldId, and optionalgroupId.Function calls found in:
web/src/components/molecules/Content/LinkItemModal/index.tsx: Uses all three parameters(item.id, fieldId, itemGroupId)- Other components pass the function as a prop
Let's verify the implementation of
handleCheckItemReferencein the ContentDetails component since it's the top-level implementation:
Let me try a different search pattern to find the implementation:
Based on the comprehensive verification results, I can now provide the final response:
✅ Function signature changes are properly implemented across the codebase
The verification confirms that:
- All type definitions consistently use the new signature with
itemId,correspondingFieldId, and optionalgroupId- The main implementation in
ContentDetails/hooks.tscorrectly handles all three parameters- All component usages properly pass through the function as a prop
- The actual function call in
LinkItemModaluses all three parameters correctly🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all usages of onCheckItemReference to ensure they're updated # Expected: All callers should use the new 3-parameter signature # Search for function calls rg -A 3 "onCheckItemReference\(" # Search for function type definitions rg -A 3 "onCheckItemReference\s*[:=]"Length of output: 5483
Script:
#!/bin/bash # Find the implementation of handleCheckItemReference ast-grep --pattern 'handleCheckItemReference = async ($$$) => { $$$ }' rg -A 5 'handleCheckItemReference\s*=\s*async'Length of output: 121
Script:
#!/bin/bash # Find the implementation of handleCheckItemReference with different patterns rg -A 10 "const handleCheckItemReference" rg -A 10 "handleCheckItemReference.*=.*async" # Also search for any function declaration ast-grep --pattern 'async function handleCheckItemReference($$$) { $$$ }'Length of output: 1379
| onCheckItemReference?: ( | ||
| itemId: string, | ||
| correspondingFieldId: string, | ||
| groupId?: string, | ||
| ) => Promise<boolean>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for reference checking.
The onCheckItemReference function might benefit from explicit error handling to gracefully handle network failures or invalid states.
onCheckItemReference?: (
itemId: string,
correspondingFieldId: string,
groupId?: string,
- ) => Promise<boolean>;
+ ) => Promise<{ success: boolean; error?: string }>;📝 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.
| onCheckItemReference?: ( | |
| itemId: string, | |
| correspondingFieldId: string, | |
| groupId?: string, | |
| ) => Promise<boolean>; | |
| onCheckItemReference?: ( | |
| itemId: string, | |
| correspondingFieldId: string, | |
| groupId?: string, | |
| ) => Promise<{ success: boolean; error?: string }>; |
Overview
This PR fixes the bug that the warning occurs when re-referring to an item referenced earlier.
When editing an item with a two-way reference value, the warning occurs if you try to dereference the item and then reference the same item again.
Memo
I fixed the logic to check if the item is referenced to reduce unnecessary queries.
Summary by CodeRabbit
New Features
ReferenceFormItemandLinkItemModalfor improved data management.Bug Fixes