Skip to content

Added Select all to the Data Import Flow #1161

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

Merged
merged 6 commits into from
May 27, 2025
Merged

Conversation

KirthikaKumar-K
Copy link
Contributor

Description

Changes Made

How to Test

  1. Steps to reproduce/test the behavior
  2. Expected outcomes

Notes

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…ectall

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@KirthikaKumar-K KirthikaKumar-K requested a review from johbaxter May 16, 2025 09:27
@KirthikaKumar-K KirthikaKumar-K requested a review from a team as a code owner May 16, 2025 09:27
@KirthikaKumar-K KirthikaKumar-K linked an issue May 16, 2025 that may be closed by this pull request
2 tasks
Copy link

@CodiumAI-Agent /describe

@CodiumAI-Agent
Copy link

Title

Added Select all to the Data Import Flow


User description

Description

Changes Made

How to Test

  1. Steps to reproduce/test the behavior
  2. Expected outcomes

Notes


PR Type

Enhancement


Description

  • Add "Select All" checkbox for table columns

  • Toggle all columns selection with handler logic

  • Maintain alias counts on bulk select/unselect

  • Prepopulate select-all state in edit mode


Changes walkthrough 📝

Relevant files
Enhancement
DataImportFormModal.tsx
Implement Select All feature in DataImportFormModal           

libs/renderer/src/components/shared/DataImportFormModal.tsx

  • Added Checkbox import replacing icon button
  • Introduced isAllSelected state and handler
  • Implemented addAllTableColumnsHandler for bulk select
  • Updated form prepopulate and alias count logic
  • +104/-28

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

    @CodiumAI-Agent /review

    Copy link

    @CodiumAI-Agent /improve

    @CodiumAI-Agent
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Missing Joins

    In updatePixelRef, pixelJoins is initialized but never populated, so join information will not be included when updating the pixel.

    const pixelTables: Set<string> = new Set();
    const pixelColumnNames: string[] = [];
    const pixelColumnAliases: string[] = [];
    const pixelJoins: string[] = [];             
    watchedTables?.forEach((tableObject) => {
    Alias Count

    The removal branch in updateAliasCountObj sets the old alias count to zero and deletes it instead of decrementing by one, which may prematurely remove alias entries.

    } else {
        newAliasesCountObj[oldAlias] = 0;
    }
    
    if (newAliasesCountObj[oldAlias] < 1) {
        delete newAliasesCountObj[oldAlias];
    }
    Checkbox Indeterminate

    The indeterminate calculation for the "Select All" checkbox uses dataImportwatch inline and may not reflect the latest form state or could incur performance overhead. Verify correctness and efficiency.

    <Checkbox
        checked={
            isAllSelected
        } // Checked if all rows are selected
        indeterminate={
            checkedColumnsCount >
                0 &&
            checkedColumnsCount <
                dataImportwatch(
                    "tables",
                )[
                    tableIndex
                ]
                    .columns
                    .length
        } 
        onChange={() =>
            addAllTableColumnsHandler(
                tableIndex,

    @CodiumAI-Agent
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Impact
    General
    Track selection per table

    Replace the single boolean isAllSelected with a per-table selection map to avoid
    conflicts when multiple tables are present. This ensures each table's "select all"
    state is tracked independently.

    libs/renderer/src/components/shared/DataImportFormModal.tsx [381]

    -const [isAllSelected, setIsAllSelected] = useState<boolean>(false);
    +const [isAllSelectedMap, setIsAllSelectedMap] = useState<Record<number, boolean>>({});
    +// Usage example when toggling:
    +const toggleSelectAll = (tableIndex: number, value: boolean) => {
    +  setIsAllSelectedMap(prev => ({ ...prev, [tableIndex]: value }));
    +};
    Suggestion importance[1-10]: 8

    __

    Why: The use of a single isAllSelected boolean will cause state conflicts across multiple tables; tracking per table prevents this critical bug.

    Medium
    Reset counters per table

    Reset the column counters inside each table iteration instead of accumulating across
    all tables. This ensures the "all selected" check is per-table and not global.

    libs/renderer/src/components/shared/DataImportFormModal.tsx [1219-1228]

    -let totalColumnsToCheck = 0;
    -let totalCheckedColumns = 0;
    -// ... inside forEach over watchedTables ...
    -totalColumnsToCheck += watchedTableColumns.length;
    +watchedTables?.forEach((tableObj, tableIdx) => {
    +  const watchedTableColumns = tableObj.columns;
    +  const totalColumnsToCheck = watchedTableColumns.length;
    +  let totalCheckedColumns = 0;
    +  watchedTableColumns.forEach(col => {
    +    if (col.checked) totalCheckedColumns++;
    +  });
    +  setIsAllSelected(totalCheckedColumns === totalColumnsToCheck && totalColumnsToCheck > 0);
    +  // ...
    +});
    Suggestion importance[1-10]: 7

    __

    Why: Declaring counters outside the loop accumulates counts across all tables, so resetting them inside each iteration is necessary for accurate per-table "all selected" checks.

    Medium
    Possible issue
    Ensure toggle state updated

    Add an else branch to reset the "select all" state when not all columns are
    selected. Without it, unchecking a column will leave the checkbox checked
    incorrectly.

    libs/renderer/src/components/shared/DataImportFormModal.tsx [1190-1192]

     if (selectedCount === totalColumns) {
         setIsAllSelected(true);
    +} else {
    +    setIsAllSelected(false);
     }
    Suggestion importance[1-10]: 5

    __

    Why: Adding an else branch corrects the missing reset of isAllSelected when not all columns are selected, preventing stale checkbox state.

    Low

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    …ectall
    …ectall
    @johbaxter johbaxter merged commit 6b017a0 into dev May 27, 2025
    3 checks passed
    @johbaxter johbaxter deleted the 240_dataimportflow_selectall branch May 27, 2025 16:23
    Copy link

    @CodiumAI-Agent /update_changelog

    @CodiumAI-Agent
    Copy link

    Changelog updates: 🔄

    2025-05-27 *

    Added

    • Bulk "Select All"/deselect all checkbox in Data Import Flow for table columns
    • Automatic alias count updates when toggling column selection

    to commit the new content to the CHANGELOG.md file, please type:
    '/update_changelog --pr_update_changelog.push_changelog_changes=true'

    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.

    Add a Select All option to the Data Import flow
    3 participants