Skip to content

Conversation

@ugouhar
Copy link
Contributor

@ugouhar ugouhar commented Jan 18, 2026

Describe your changes:

Fixed a bug where values containing commas, newlines, or in tags/domains columns were wrapped in quotes but inner quotes were not escaped, producing malformed CSV output

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

  • Fixed CSV export bug:
    • Added quote escaping using replaceAll(/"/g, '""') in CSV.utils.tsx:227 per RFC 4180 standard
  • Test coverage:
    • Added 4 test cases covering quotes with commas, newlines, tags column, and domains column

This will update automatically on new commits.


@ugouhar ugouhar added safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch labels Jan 18, 2026
@ugouhar ugouhar marked this pull request as ready for review January 18, 2026 17:01
@ugouhar ugouhar requested a review from a team as a code owner January 18, 2026 17:01
@gitar-bot
Copy link

gitar-bot bot commented Jan 18, 2026

🔍 CI failure analysis for 301c3b4: Multiple Playwright E2E test suites failed across two CI jobs with CSV import validation errors showing incorrect row counts - tests expect specific numbers of passed/failed rows but receive different values, indicating systemic CSV import processing issues unrelated to this PR's CSV export fix.

Issue

Playwright E2E tests failed in two separate CI jobs with a total of 10 test failures across multiple test suites:

Job 1: playwright-ci-postgresql (1, 6) - 3 failures:

  • DataAssetRulesDisabled.spec.ts bulk edit tests

Job 2: playwright-ci-postgresql (2, 6) - 7 failures:

  • BulkEditEntity.spec.ts tests (3 failures)
  • BulkImport.spec.ts tests (4 failures)
  • Additional timeout/browser close errors in other test suites

Root Cause

All failures share a common pattern in CSV import validation at importUtils.ts:528:

expect(passedRow).toBe(status.passed);

Observed mismatches:

Test Type Expected Passed Received Passed Issue
Database service bulk edit 2 1 1 row incorrectly failing
Database bulk edit 2 1 1 row incorrectly failing
Database Schema bulk edit 2 1 1 row incorrectly failing
Bulk import (various) 7 2 5 rows incorrectly failing
Bulk import processed 13 10 3 rows not processed
Delete selection 9 8 1 row not processed

Details

This failure is COMPLETELY UNRELATED to the current PR's changes.

The PR modifies CSV export logic (adding quote escaping in CSV.utils.tsx:227 for values with commas/newlines), while these failures occur in CSV import/bulk edit functionality during E2E test validation.

Key observations:

  1. Consistent failures - All tests failed on both initial run and retry, indicating this is NOT flakiness
  2. Multiple test suites affected - DataAssetRulesDisabled, BulkEditEntity, and BulkImport specs all show similar validation errors
  3. Pattern across both CI jobs - Same type of failure (expected vs actual row counts) in different test shards
  4. Additional symptoms:
    • Timeout errors in Database service bulk import (300s exceeded)
    • Page/context closed errors in Schema import tests (240s exceeded)
    • Browser crash errors in CustomizeDetailPage and other suites

Affected test files:

  • playwright/e2e/Features/DataAssetRulesDisabled.spec.ts
  • playwright/e2e/Features/BulkEditEntity.spec.ts
  • playwright/e2e/Features/BulkImport.spec.ts
  • playwright/utils/importUtils.ts:528 (validation helper)

Root cause hypothesis:

The bulk CSV import processing logic is failing to successfully process the expected number of rows. One or more rows that should pass validation are instead being marked as failed or not processed. This suggests:

  1. Recent backend changes to CSV import validation logic
  2. Changes to bulk edit/import row processing in the data asset rules system
  3. Database schema or entity validation rule changes affecting import success rates
  4. Pre-existing bug in main branch that these tests are now catching
Code Review ✅ Approved

Clean bug fix that properly escapes quotes in CSV export per RFC 4180. Well-tested with comprehensive coverage of edge cases.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@github-actions
Copy link
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
65.99% (53814/81549) 44.03% (27012/61349) 47.85% (8464/17688)

@sonarqubecloud
Copy link

@karanh37 karanh37 closed this Jan 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants