Fix/1521/excel export ignore hidden columns

[EthanJappie]

Excel export should ignore hidden (isVisible == false) columns when exporting data tables

Summary by CodeRabbit

• Bug Fixes
  • Excel exports now respect column visibility settings and only include visible columns in exported files.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The exportToExcel function in the data table backend repository now filters columns to include only those marked as visible before mapping them to Excel column definitions. A fallback mechanism that added an 'id' column when no columns were present has been removed.

Changes

Cohort / File(s)	Change Summary
Excel Export Visibility Filter shesha-reactjs/src/providers/dataTable/repository/backendRepository.tsx	Modified exportToExcel to filter columns by isVisible property and removed fallback logic that inserted an 'id' column for empty column sets

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Verify the isVisible filter correctly identifies columns intended for export
• Confirm removal of the 'id' fallback doesn't cause issues when exporting datasets with no visible columns
• Check that the change aligns with expected export behavior and doesn't introduce edge case regressions

Poem

🐰 A filter of truth, so neat and precise,
Only the visible, not once but thrice,
No fallback crutch for columns that hide,
Excel exports now with focused pride! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: filtering hidden columns from Excel exports, which directly matches the changeset and PR objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[IvanIlyichev]

Hi @EthanJappie. Your changes don't address issue #1521. Make sure it works as expected

[EthanJappie]

Hi @EthanJappie. Your changes don't address issue #1521. Make sure it works as expected

Exclude Id column

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b39ed4 and d011522.

📒 Files selected for processing (1)

• shesha-reactjs/src/providers/dataTable/repository/backendRepository.tsx (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type instead for values with unknown types, forcing explicit type checking
Prefer type guards over type casting for type checking
Avoid monolithic types; use discriminated unions with a discriminator property instead
Leverage TypeScript to its full potential as a type system, not merely as a linter

Files:

• shesha-reactjs/src/providers/dataTable/repository/backendRepository.tsx

🧠 Learnings (1)

📚 Learning: 2025-06-12T16:55:57.638Z

Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.

Applied to files:

• shesha-reactjs/src/providers/dataTable/repository/backendRepository.tsx

🔇 Additional comments (1)

shesha-reactjs/src/providers/dataTable/repository/backendRepository.tsx (1)

258-260: Remove the undefined isVisible handling concern—the property is a required boolean.

The second concern in this review is incorrect. The ITableDataColumn interface extends ITableColumn, which defines isVisible: boolean as a required property (not optional). TypeScript guarantees that isVisible is always a boolean value and cannot be undefined or null, so the filter correctly excludes only columns where isVisible is false.

The code properly leverages TypeScript's type system and follows the coding guidelines—no type guards are needed for isVisible since its type is guaranteed by the interface.

The first concern about empty columns is valid: if all columns have isVisible === false, the excelColumns array will be empty. However, this is a behavioral question rather than a code defect. Confirm whether the backend /ExportToExcel endpoint gracefully handles empty column arrays or if a user-facing message is needed when no visible columns exist.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Use const instead of let for immutable variable.

The excelColumns variable is never reassigned, so it should be declared with const to prevent accidental mutations and improve code clarity.

Apply this diff:

-    let excelColumns = payload.columns
+    const excelColumns = payload.columns
         .filter(c => c.isVisible)
         .map<IExcelColumn>(c => ({ propertyName: c.propertyName, label: c.caption }));

📝 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.

Suggested change

	let excelColumns = payload.columns
	const excelColumns = payload.columns
	.filter(c => c.isVisible)
	.map<IExcelColumn>(c => ({ propertyName: c.propertyName, label: c.caption }));

🤖 Prompt for AI Agents

In shesha-reactjs/src/providers/dataTable/repository/backendRepository.tsx
around line 258, the variable declaration uses "let excelColumns =
payload.columns" but excelColumns is never reassigned; change the declaration to
"const excelColumns = payload.columns" to make it immutable and prevent
accidental reassignments, leaving the rest of the code unchanged.