Skip to content

Comments

Add CSV validation warnings and example CSV files#195

Draft
Pranav-0440 wants to merge 4 commits intoeclipse-editdor:masterfrom
Pranav-0440:fix/csv-validation-clean
Draft

Add CSV validation warnings and example CSV files#195
Pranav-0440 wants to merge 4 commits intoeclipse-editdor:masterfrom
Pranav-0440:fix/csv-validation-clean

Conversation

@Pranav-0440
Copy link
Contributor

Fixes - #134

Copilot AI review requested due to automatic review settings February 10, 2026 19:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a warning-collecting CSV parsing path and documents example CSV inputs to help users spot “wrong-but-parseable” CSV values (Issue #134).

Changes:

  • Extend parseCsv to return { data, warnings } and add basic validations for type and modbus:entity.
  • Surface CSV warnings in the CSV import flow (CreateTd) and add example valid.csv / invalid.csv files plus README.
  • Add/adjust tests for the new parse result shape and validation behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/utils/parser.ts Returns parsed CSV data plus validation warnings for select fields.
src/utils/parser.test.ts Updates tests for the new return shape and adds warning assertions.
src/components/App/CreateTd.tsx Displays CSV warnings after import (currently via error state/dialog).
src/components/Dialogs/ConvertTmDialog.tsx Large refactor unrelated to CSV import; currently introduces a TM→TD conversion regression and misleading copy.
test/csv-examples.test.ts Adds a “test” file for example CSVs, but currently contains no actual Vitest tests/assertions.
doc/examples/csv/valid.csv Adds a “known-good” CSV example.
doc/examples/csv/invalid.csv Adds an “intentionally invalid” CSV example to trigger warnings.
doc/examples/csv/README.md Documents the examples and warning expectations (currently inconsistent).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@netlify
Copy link

netlify bot commented Feb 13, 2026

Deploy Preview for editdor ready!

Name Link
🔨 Latest commit f955ce6
🔍 Latest deploy log https://app.netlify.com/projects/editdor/deploys/699da8e2c84e9100087cb628
😎 Deploy Preview https://deploy-preview-195--editdor.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@TejInaco TejInaco marked this pull request as draft February 19, 2026 16:30
@TejInaco TejInaco marked this pull request as draft February 19, 2026 16:30
@TejInaco TejInaco requested review from TejInaco February 19, 2026 16:30
@TejInaco TejInaco assigned Pranav-0440 and unassigned Pranav-0440 Feb 19, 2026
Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>
@Pranav-0440 Pranav-0440 force-pushed the fix/csv-validation-clean branch from 4b0e4d0 to 2447d9f Compare February 23, 2026 17:15
Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>
@Pranav-0440 Pranav-0440 marked this pull request as ready for review February 23, 2026 17:24
@Pranav-0440
Copy link
Contributor Author

I’ve cleaned up the PR to reduce unrelated diffs by resetting to origin/master and cherry-picking only the CSV validation commit. Please let me know if any further adjustments are needed.

@TejInaco TejInaco marked this pull request as draft February 24, 2026 11:14
@TejInaco
Copy link
Contributor

I’ve cleaned up the PR to reduce unrelated diffs by resetting to origin/master and cherry-picking only the CSV validation commit. Please let me know if any further adjustments are needed.

Yes, you did. But you didn't implemented the changes requested by the code reviews. Change to draft until changes are committed.

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>
Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>
@Pranav-0440
Copy link
Contributor Author

Thanks @TejInaco for marking it draft.
I’ve now implemented all requested review changes:

  • Wired hasHeaders correctly into Papa.parse
  • Removed duplicate transform option
  • Precomputed the lowercase Modbus entity list outside the loop
  • Added separate case-mismatch warning logic for modbus:entity
  • Fixed warning rendering in CreateTd
  • Removed unused warnings state from ConvertTmDialog
  • Converted csv-examples.test.ts into proper Vitest tests
  • Updated README to clarify case-insensitive validation with casing warnings

Marking the PR ready for review again. Please let me know if anything else needs adjustment.

@Pranav-0440 Pranav-0440 marked this pull request as ready for review February 24, 2026 13:38
@TejInaco
Copy link
Contributor

You still have pending reviews @Pranav-0440 to be done. Return to draft.

@TejInaco TejInaco marked this pull request as draft February 24, 2026 17:48
Copy link
Contributor Author

@Pranav-0440 Pranav-0440 left a comment

Choose a reason for hiding this comment

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

@TejInaco - All review threads have now been addressed and resolved.
Please let me know if anything further needs adjustment.

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.

2 participants