Skip to content

fix(filtercell): filter cell bug fix for cell and frame id#1333

Merged
johbaxter merged 8 commits intodevfrom
filter-data-new-app-bug-fix
Jun 20, 2025
Merged

fix(filtercell): filter cell bug fix for cell and frame id#1333
johbaxter merged 8 commits intodevfrom
filter-data-new-app-bug-fix

Conversation

@bannaarisamy-shanmugham-kanini
Copy link
Copy Markdown
Contributor

Description

Filter data cell is not loading the proper records, when the frame is selected

Changes Made

When a frame is selected, the respective targetcell is not correctly selected. Made a fix for finding the correct targetcell respective to frame selected.

How to Test

Notes

@bannaarisamy-shanmugham-kanini bannaarisamy-shanmugham-kanini requested a review from a team as a code owner June 20, 2025 14:22
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

fix(filtercell): filter cell bug fix for cell and frame id


User description

Description

Filter data cell is not loading the proper records, when the frame is selected

Changes Made

When a frame is selected, the respective targetcell is not correctly selected. Made a fix for finding the correct targetcell respective to frame selected.

How to Test

Notes


PR Type

Bug fix


Description

  • Improve targetCell finding logic using frameVariableName

  • Add fallback lookup when direct targetCell missing

  • Reset cellId per query iteration

  • Include selectedFrame in effect dependency


Changes walkthrough 📝

Relevant files
Bug fix
FilterDataCell.tsx
Enhance targetCell lookup and effect deps                               

libs/renderer/src/components/cell-defaults/filter-data-cell/FilterDataCell.tsx

  • Introduce cellId and parse frameVariableName for targetCell
  • Add fallback branch when query.cells[targetCell.id] missing
  • Reset cellId after each query iteration
  • Extend useEffect deps to include selectedFrame
  • Insert console.log in getOptionLabel for debugging
  • +19/-2   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /review

    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /improve

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Inefficient Iteration

    The nested forEach loops in the computed block prevent early exit and may hurt performance. Consider using for…of loops or array find methods to break out once the matching cell is found.

    let cellId: number | null = null;
    Object.values(state.queries).forEach((query) => {
        Object.entries(query.cells).forEach(([key, value], cellIndex)=> {
            let parsedId = (value['parameters']?.['frameVariableName'])|| null;
            if (cellId || parsedId === null) return;
    
    
            let target = (parsedId as String)?.match(/\d+/);
            const targetID = target ? target[0] : null;
            if(targetID && Number(targetID) === Number(cell.parameters.targetCell.id) && cellId === null) {
                cellId = parseInt(key, 10);
            }
        });
    
        if (query.cells[cell.parameters.targetCell.id]) {
            c = query.cells[
                cell.parameters.targetCell.id
            ] as CellState<QueryImportCellDef>;
        }
        if(!query.cells[cell.parameters.targetCell.id] && cellId){
            c = query.cells[cellId] as CellState<QueryImportCellDef>;
        }
        cellId=null;
    forEach Return Misuse

    Using return inside a forEach callback only skips the current iteration rather than breaking the loop. This may lead to incorrect cellId assignment. Refactor to use breakable loops or separate functions.

    Object.entries(query.cells).forEach(([key, value], cellIndex)=> {
        let parsedId = (value['parameters']?.['frameVariableName'])|| null;
        if (cellId || parsedId === null) return;
    
    
        let target = (parsedId as String)?.match(/\d+/);
        const targetID = target ? target[0] : null;
        if(targetID && Number(targetID) === Number(cell.parameters.targetCell.id) && cellId === null) {
            cellId = parseInt(key, 10);
        }
    });
    Debug Logging

    The console.log call in getOptionLabel will flood the console in production. Remove it or replace with a controlled debug logger.

    value={selectedFrame}
    options={framelist}
    getOptionLabel={(option) => {                                    console.log(option, 'option in dropdown');
        return option;

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Simplify nested loops

    Replace nested forEach calls with for…of loops and break out as soon as a match is
    found to simplify control flow and avoid unnecessary iterations. This also ensures
    that once the correct cellId is identified, the loops terminate immediately.

    libs/renderer/src/components/cell-defaults/filter-data-cell/FilterDataCell.tsx [130-138]

    -Object.entries(query.cells).forEach(([key, value]) => {
    -    let parsedId = (value['parameters']?.['frameVariableName'])|| null;
    -    if (cellId || parsedId === null) return;
    +for (const query of Object.values(state.queries)) {
    +    for (const [key, value] of Object.entries(query.cells)) {
    +        const frameVar = value.parameters?.frameVariableName;
    +        if (!frameVar) continue;
    +        const match = frameVar.match(/\d+/);
    +        if (match && Number(match[0]) === cell.parameters.targetCell.id) {
    +            cellId = Number(key);
    +            break;
    +        }
    +    }
    +    if (cellId != null) {
    +        c = query.cells[cell.parameters.targetCell.id]
    +            || query.cells[cellId] as CellState<QueryImportCellDef>;
    +        break;
    +    }
    +}
     
    -    let target = (parsedId as String)?.match(/\d+/);
    -    const targetID = target ? target[0] : null;
    -    if(targetID && Number(targetID) === Number(cell.parameters.targetCell.id) && cellId === null) {
    -        cellId = parseInt(key, 10);
    -    }
    -});
    -
    Suggestion importance[1-10]: 7

    __

    Why: The for…of loops with breaks simplify control flow and avoid unnecessary iterations compared to nested forEach, improving performance and readability.

    Medium
    General
    Remove debug logging

    Remove the inline console.log from the getOptionLabel callback to avoid polluting
    the console and impacting rendering performance. If needed, move logging outside of
    the render path or guard it with a debug flag.

    libs/renderer/src/components/cell-defaults/filter-data-cell/FilterDataCell.tsx [1020-1022]

    -getOptionLabel={(option) => {                                    console.log(option, 'option in dropdown');
    -    return option;
    -}}
    +getOptionLabel={(option) => option}
    Suggestion importance[1-10]: 5

    __

    Why: Removing the inline console.log avoids polluting the console during rendering and improves performance.

    Low
    Use primitive string type

    Cast parsedId to the primitive string type instead of the String object to ensure
    correct typing and prevent boxing overhead. This aligns with TypeScript best
    practices for string operations.

    libs/renderer/src/components/cell-defaults/filter-data-cell/FilterDataCell.tsx [135]

    -let target = (parsedId as String)?.match(/\d+/);
    +const target = (parsedId as string)?.match(/\d+/);
    Suggestion importance[1-10]: 4

    __

    Why: Casting to the primitive string type instead of the String object aligns with TypeScript best practices and avoids boxing overhead.

    Low

    @johbaxter johbaxter merged commit 4ae1d1c into dev Jun 20, 2025
    3 checks passed
    @johbaxter johbaxter deleted the filter-data-new-app-bug-fix branch June 20, 2025 17:13
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

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

    Labels

    None yet

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants