Skip to content

Bug #1388 Solution #1393

Open
bmyoungquist wants to merge 3 commits into
sysadminsmedia:mainfrom
bmyoungquist:1388-improve-datatable-export
Open

Bug #1388 Solution #1393
bmyoungquist wants to merge 3 commits into
sysadminsmedia:mainfrom
bmyoungquist:1388-improve-datatable-export

Conversation

@bmyoungquist
Copy link
Copy Markdown

@bmyoungquist bmyoungquist commented Mar 23, 2026

What type of PR is this?

  • bug

What this PR does / why we need it:

Fixes bug where location is exported incorrectly when "Download Table as CSV" is selected

Which issue(s) this PR fixes:

Fixes #1388

Special notes for your reviewer:

I decided to put everything in ~/lib/utils because the code is generic enough to be useful elsewhere. I was also thinking about adding the ability to show tags in the datatable in a later PR, hence why I also added the formatArrayAsString function.

Testing

Manually tested by exporting table as CSV when an item that belongs to a child location is tested. Verified that the location was the string representation of the location instead of "[object Object]"

Summary by CodeRabbit

  • New Features

    • CSV exports now convert arrays and nested values into readable comma-separated strings and prefer name-like labels when available.
  • Bug Fixes

    • CSV fields are consistently quoted and internal quotes escaped for reliable imports.
    • Improved protection against CSV formula-injection by neutralizing dangerous leading characters in exported fields.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 312eca0a-54be-4184-bf76-d84e4480b738

📥 Commits

Reviewing files that changed from the base of the PR and between fd1e2c5 and 8ca4476.

📒 Files selected for processing (1)
  • frontend/lib/utils.ts
✅ Files skipped from review due to trivial changes (1)
  • frontend/lib/utils.ts

Walkthrough

Replaces a local CSV-escaping helper in the table dropdown with shared utilities: adds functions to extract object names, flatten arrays, and format values as CSV-safe fields (including formula-injection mitigation and quote escaping), and updates the component to use the new formatter for headers and cell values.

Changes

Cohort / File(s) Summary
CSV export component
frontend/components/Item/View/table/data-table-dropdown.vue
Removed local escapeCsvField; now imports and uses formatValueAsCsvField for CSV headers and cell values during CSV generation.
CSV formatting utilities
frontend/lib/utils.ts
Added exports: getNameOrValue, formatArrayAsString, and formatValueAsCsvField to derive names from objects, flatten arrays to comma-separated strings, escape internal quotes, wrap fields in double quotes, and mitigate formula-injection.
Type declarations (whitespace)
frontend/global.d.ts
Whitespace/indentation normalization for NuxtApp.$otelEnabled / ComponentCustomProperties.$otelEnabled; no signature or type changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Security Recommendations

  • Verify formula-injection mitigation is consistent (strings starting with =, +, -, @ are prefixed appropriately) and matches downstream consumers (spreadsheet apps) expectations.
  • Confirm internal double quotes are doubled (""") and fields are wrapped in double quotes per RFC 4180.
  • Test handling of nested objects and arrays to avoid "[object Object]" outputs; ensure getNameOrValue returns meaningful strings for object cases.
  • Add safeguards for null/undefined and circular references in formatArrayAsString to prevent crashes or infinite recursion.

Poem

CSVs once hid names in gloom,
utils arrived to lift the plume.
Quotes escaped and values shown,
nested lists and objects known —
export blooms, no longer doom. 🎉

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Bug #1388 Solution' is vague and generic, referring only to the issue number without describing the actual change being made. Use a more descriptive title that captures the core change, such as 'Fix CSV export of location fields by centralizing field formatting' or 'Prevent [object Object] in CSV exports by extracting formatting utilities'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description includes all required sections: type (bug), what it does with clear explanation, linked issue (#1388), special notes explaining design decisions, and testing details.
Linked Issues check ✅ Passed The PR successfully addresses issue #1388 by fixing the '[object Object]' CSV export problem through centralized field formatting that properly handles location objects.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the location CSV export bug; the utility functions added to lib/utils are appropriate generalizations that enable the fix and support future features.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@coderabbitai coderabbitai Bot requested a review from tankerkiller125 March 23, 2026 22:30
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/lib/utils.ts`:
- Around line 85-87: The current guard only matches formula chars at string
start and misses cases with leading whitespace (e.g. " =1+1"), so update the
regex that tests the variable str in the CSV sanitization logic to detect
formula characters even when preceded by whitespace — replace the test
/^[=+\-@]/ with one that allows any leading whitespace (e.g. /^\s*[=+\-@]/) and
keep the existing behavior of prefixing the string with a single quote when a
match occurs; refer to the variable str and the regex test in the CSV
export/sanitization function to locate and update the check.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9a329c62-6a63-47d7-a441-9ea4b681d009

📥 Commits

Reviewing files that changed from the base of the PR and between 144920c and 13a0b0e.

📒 Files selected for processing (2)
  • frontend/components/Item/View/table/data-table-dropdown.vue
  • frontend/lib/utils.ts

Comment thread frontend/lib/utils.ts
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.

CSV export of selected table rows produces wrong location text, if one of the entries has nested location

1 participant