Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Display Related Tables for Nodes without Relations #797

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

junkisai
Copy link
Member

@junkisai junkisai commented Feb 28, 2025

Issue

Why is this change needed?

There was an issue where, when selecting a table without any relations, the RelatedTables area remained in a loading state and was not displayed. Additionally, pressing the "Open in Main area" button resulted in nothing being shown.

What would you like reviewers to focus on?

To resolve these issues, we modified the logic that takes into account the parent-child relationships of nodes, so that everything now displays as intended.

Testing Verification

Before After
2025-02-14.18.19.15.mov
2025-02-14.18.31.50.mov

What was done

🤖 Generated by PR Agent at c799f2b

  • Fixed infinite loading for tables without relationships.
  • Added logic to handle non-related table group nodes visibility.
  • Refactored node visibility logic into reusable utility functions.
  • Updated tests to validate changes in table relationship extraction.

Detailed Changes

Relevant files
Enhancement
5 files
TableDetail.tsx
Refactored node visibility handling in TableDetail component
+6/-15   
useInitialAutoLayout.ts
Integrated new node visibility utilities in auto-layout hook
+6/-4     
hasNonRelatedChildNodes.ts
Added utility to check for non-related child nodes             
+6/-0     
index.ts
Exported new utility functions for node handling                 
+2/-0     
updateNodesHiddenState.ts
Added utility to update node hidden states                             
+21/-0   
Tests
1 files
extractDBStructureForTable.test.ts
Updated tests for table relationship extraction logic       
+1/-1     
Bug fix
1 files
extractDBStructureForTable.ts
Fixed handling of tables without relationships                     
+9/-0     
Configuration changes
1 files
constants.ts
Defined constant for non-related table group node ID         
+2/-0     
Cleanup
1 files
convertDBStructureToNodes.ts
Removed redundant constant declaration for non-related table group
node
+4/-3     
Documentation
2 files
late-swans-beam.md
Added changeset for handling tables without relationships
+5/-0     
violet-dolphins-reflect.md
Added changeset for non-related group node visibility fix
+5/-0     

Additional Notes


Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link

    changeset-bot bot commented Feb 28, 2025

    🦋 Changeset detected

    Latest commit: c799f2b

    The changes in this PR will be included in the next version bump.

    This PR includes changesets to release 2 packages
    Name Type
    @liam-hq/erd-core Patch
    @liam-hq/cli Patch

    Not sure what this means? Click here to learn what changesets are.

    Click here if you're a maintainer who wants to add another changeset to this PR

    Copy link

    vercel bot commented Feb 28, 2025

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

    Name Status Preview Comments Updated (UTC)
    liam-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 28, 2025 6:44am
    liam-erd-sample ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 28, 2025 6:44am
    3 Skipped Deployments
    Name Status Preview Comments Updated (UTC)
    test-liam-docs ⬜️ Ignored (Inspect) Feb 28, 2025 6:44am
    test-liam-erd-sample ⬜️ Ignored (Inspect) Feb 28, 2025 6:44am
    test-liam-erd-web ⬜️ Ignored (Inspect) Feb 28, 2025 6:44am

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    347 - Partially compliant

    Compliant requirements:

    • Fix infinite loading state for tables without relationships
    • Fix "Open in Main area" button functionality for tables without relationships

    Non-compliant requirements:

    • Display empty state for tables without relationships

    Requires further human verification:

    • Verify that the loading state is properly resolved in the UI
    • Verify that the "Open in Main area" button works correctly in the browser
    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Logic Change

    The change in behavior for tables without relationships now returns the table itself in the structure. Verify this doesn't cause side effects in other components expecting empty tables.

    if (relatedRelationshipsArray.length === 0) {
      return {
        tables: {
          [table.name]: table,
        },
        relationships: {},
      }
    }
    Edge Case

    New logic for handling node visibility might affect existing node display behavior. Ensure it works correctly with different combinations of hidden nodes and group nodes.

    return nodes.map((node) => ({
      ...node,
      hidden:
        hiddenNodeIds.includes(node.id) ||
        (shouldHideGroupNodeId && node.id === NON_RELATED_TABLE_GROUP_NODE_ID),
    }))

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add input validation safeguards

    Add input validation to ensure nodes and hiddenNodeIds arrays are not
    null/undefined before processing them to prevent runtime errors.

    frontend/packages/erd-core/src/features/erd/components/ERDContent/utils/updateNodesHiddenState.ts [10-15]

     export function updateNodesHiddenState({
       nodes,
       hiddenNodeIds,
       shouldHideGroupNodeId,
     }: Params): Node[] {
    +  if (!nodes?.length || !Array.isArray(hiddenNodeIds)) {
    +    return [];
    +  }
       return nodes.map((node) => ({
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion adds defensive programming by validating input parameters, which could prevent runtime errors. While useful, the impact is moderate as TypeScript's type system already provides some safety.

    Low
    Validate table object before access

    Add validation to ensure table parameter exists and has a valid name property
    before accessing it to prevent potential null reference exceptions.

    frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/extractDBStructureForTable.ts [20-27]

    +if (!table?.name) {
    +  return { tables: {}, relationships: {} }
    +}
     if (relatedRelationshipsArray.length === 0) {
       return {
         tables: {
           [table.name]: table,
         },
         relationships: {},
       }
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion adds a null check for the table parameter, which could prevent runtime errors. However, given the TypeScript typing and the context of how this function is used, this validation might be redundant.

    Low
    • More

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

    Successfully merging this pull request may close these issues.

    Related tables section for table without relationships are forever loading.
    1 participant