Skip to content

Comments

nsc-events-fullstack_30_135_e2e-schema-fix-and-ci-opt#154

Open
NahomAlemu wants to merge 5 commits intomainfrom
bug-135-e2e-schema-compat-pipeline-optimization
Open

nsc-events-fullstack_30_135_e2e-schema-fix-and-ci-opt#154
NahomAlemu wants to merge 5 commits intomainfrom
bug-135-e2e-schema-compat-pipeline-optimization

Conversation

@NahomAlemu
Copy link
Contributor

@NahomAlemu NahomAlemu commented Feb 4, 2026

Summary & Changes 📃

  • Resolves:
    Bug Report: Fix E2E Tests for ERD Schema Refactor Compatibility #150
    Issue: Update dbFieldMapper Utility for New Schema Fields #152

  • Related To: nsc-events-fullstack_30_130_erd-schema-refactoring #142

  • Summary: - 🔨 What does this issue fix? Fixes E2E failures due to schema changes and optimizes the Playwright CI pipeline. In addition included changes to backend returns tags as Tag objects instead of eventTags as strings these changes ensure frontend compatibility.

  • 👀 What is the expected behavior? Successful E2E runs using corrected endpoints and a "Smart CI" that scales based on file changes.

  • 🗨️ Any relevant technical details? Transitioned from a "run-all" approach to a tiered strategy: Smoke Tests for UI changes vs. Full Sharded Suite for critical backend/logic changes.

  • Changes:

    • Endpoint Update:** Fixed delete path to /events/remove/${id} in api-client.ts.
    • DTO Alignment:** Updated eventTagstagNames in event-management.spec.ts.
    • CI Optimization:** Implemented path-based filtering and 2-way parallel sharding.
    • Manual Trigger:** Added workflow_dispatch to allow manual scope selection (Smoke/Critical/Full).
      Backend:
    • activity.service.ts: Added relations: ['tags', 'coverPhoto',
      'document'] to getActivityById() to include tag data in API
      responses
      Frontend Data Layer:
    • dbFieldMapper.ts: Added tags→eventTags transformation to
      convert Tag objects ([{id, name, slug}]) to string arrays
      (["Tech", "Social"])
      Frontend Components (Defensive Coding):
    • EditDialog.tsx: Added null-safe fallback for .join() on
      eventTags
    • HomeEventsCard.tsx: Added null-safe fallback for .map() on
      eventTags
      Unit Test Updates:
    • activity.service.spec.ts: Updated mock expectation to include
      new relations
    • dbFieldMapper.test.ts: Updated 6 tests to expect eventTags: []
      in normalized output
  • 📝 Efficiency: Reduced average CI runtime for non-critical PRs.

Screenshots / Visual Aids 🔎

📌 Required for: UI changes, layout updates, or bug fixes.

Expand ⬇️

How to Test 🧪

Steps to Reproduce:
0. If you have the old schema, please makesure to drop the schema and run the application again.
run this quary in pgAdmin:
DROP SCHEMA public CASCADE; CREATE SCHEMA public;

  1. Run full E2E suite npx playwright test
  2. Run Unit Tests: npm run test
  3. Manual Verification:
    • Create a new event with tags → Verify tags display on event
      detail page
    • Edit an existing event → Verify tags load correctly in the
      edit dialog
    • View event cards on home page → Verify tags render without
      console errors
      Expected Behavior:
  • All E2E tests and unit-test pass

Checklist ✅

  • I have tested this PR locally and it works as expected.
  • This PR resolves an issue (Resolves #135).
  • Reviewers, assignees(self), tags, and labels are correctly assigned.
  • Squash commits and enable auto-merge if approved.

@NahomAlemu NahomAlemu self-assigned this Feb 4, 2026
@github-actions
Copy link

github-actions bot commented Feb 4, 2026

✅ Full E2E Test Results

Running full test suite (critical files changed)

Status Count
✅ Passed 0
❌ Failed 0
📊 Total 0

View detailed report

@github-actions
Copy link

✅ Full E2E Test Results

Running full test suite (critical files changed)

Status Count
✅ Passed 47
❌ Failed 0
📊 Total 47

View detailed report

Copy link
Contributor

@beimnettes beimnettes left a comment

Choose a reason for hiding this comment

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

I checked the branch and all CI checks are passing.
The changes align with the related issues and look good.
No blockers from my side.

Copy link
Contributor

@TVW96 TVW96 left a comment

Choose a reason for hiding this comment

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

Can connect to database from client. Cannot connect from Postman.
Screenshot 2026-02-11 at 3 02 33 PM
Screenshot 2026-02-11 at 2 52 05 PM

E2E test's are outputting flaky test which vary depending on test refresh rate. There is also about a dozen tests that we are skipping. Can we fix these before pushing this PR?

Screenshot 2026-02-11 at 1 07 55 PM Screenshot 2026-02-11 at 1 11 50 PM Screenshot 2026-02-11 at 1 13 43 PM

Unit test are all green. 🪴

Screenshot 2026-02-11 at 1 08 35 PM

@NahomAlemu
Copy link
Contributor Author

NahomAlemu commented Feb 19, 2026

E2E test's are outputting flaky test which vary depending on test refresh rate. There is also about a dozen tests that we are skipping. Can we fix these before pushing this PR?

Thanks for the thorough review, Troy!

  1. The 500 error on /api/events is reproducible from both the UI and Postman as shown in the screenshot, the frontend also returns no events. It's not a Postman-specific issue the issue "not being able to connect to the database" might be in the set up but all events was not showing up, and it is fixed now.
Screenshot 2026-02-19 at 10 21 27 AM (2)
  1. Regarding flaky tests, these are pre-existing and not introduced by this PR. Retries are already configured in playwright.config.ts (retries: 2 on CI, retries: 1 locally), so Playwright is already handling transient failures per its recommended approach. The root cause of flakiness in E2E tests like ours is typically race conditions or timing variance across environments, which is a broader codebase concern. I'll open a separate issue to investigate and address the underlying causes (e.g. replacing any waitForTimeout() calls with proper auto-waiting assertions). Here's the relevant Playwright docs for reference: https://playwright.dev/docs/test-retries

  2. Of the 12 skipped tests shown, 5 are intentionally skipped in the initial Playwright tests branch. The rest are surfacing from the unit test file as a result. The 5 skipped, fall into two categories: the Material UI date picker in our UI is difficult for Playwright to interact with (the test is kept for future use), and a few redirect simulation tests that were extra and can be iterated on. I covered this in the Playwright demo, but I'll document it now since it wasn't written down.

Screenshot 2026-02-19 at 10 03 49 AM (2)

These are all valid observations, but none are regressions introduced by this PR. I'll create separate issues for tracking. Happy to move further discussion to the GitHub Discussions section for visibility.

@NahomAlemu NahomAlemu requested a review from TVW96 February 19, 2026 19:09
@github-actions
Copy link

✅ Full E2E Test Results

Running full test suite (critical files changed)

Status Count
✅ Passed 53
❌ Failed 0
📊 Total 53

View detailed report

@github-actions
Copy link

✅ Full E2E Test Results

Running full test suite (critical files changed)

Status Count
✅ Passed 47
❌ Failed 0
📊 Total 47

View detailed report

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.

3 participants