Skip to content

Comments

nsc-events-fullstack_30_130_erd-schema-refactoring#142

Merged
NahomAlemu merged 8 commits intomainfrom
refactor-130-erd-schema-redesign
Feb 10, 2026
Merged

nsc-events-fullstack_30_130_erd-schema-refactoring#142
NahomAlemu merged 8 commits intomainfrom
refactor-130-erd-schema-redesign

Conversation

@NahomAlemu
Copy link
Contributor

@NahomAlemu NahomAlemu commented Jan 20, 2026

Summary & Changes 📃

Resolves: #130, #134, #135, #136, #137, #138, #139, #140, #141

Summary: Complete database schema redesign for production readiness with proper TypeORM
relations, normalized entities, and robust media management.

  • 🔨 What does this issue fix? Refactors the database from loose string references to proper foreign key relationships, normalizes tags into a separate entity, and adds a Media entity for tracked file uploads with S3 cleanup.
  • 👀 What is the expected behavior? Schema follows database best practices - proper FK constraints,
    normalized many-to-many relationships, cascading deletes, and no orphaned S3 files.
  • 🗨️ Any relevant technical details? Database Reset Required All existing tables must be dropped
    before running. Please Run this query in pgAdmin:
DROP SCHEMA public CASCADE;
CREATE SCHEMA public;
  • Changes:
    Phase 1: Define Foreign Key Relationships (Issue: Define Foreign Key Relationships in Entities #137)

    • ✅ Added @ManyToOne/@onetomany relations between EventRegistration, Activity, and User
    • ✅ Added createdByUser relation to Activity entity
    • ✅ Configured onDelete: CASCADE for registrations, SET NULL for creator references

    Phase 2: Remove attendees Property from Activity (Issue: Remove attendees Property from Activity Entity #135)

    • ✅ Removed attendees JSON field and attendanceCount from Activity
    • ✅ Removed deprecated addAttendee/removeAttendee methods and /events/attend/:id endpoint
    • ✅ Attendance now handled via EventRegistration entity

    Phase 3: Remove Redundant User Fields from EventRegistration (Issue: Remove Redundant User Fields from EventRegistration Entity #136)

    • ✅ Removed firstName, lastName, email from EventRegistration
    • ✅ User data accessed via user relation with eager: true loading
    • ✅ Kept college and yearOfStudy as optional registration-specific fields

    Phase 4: Fix Foreign Key Types (Issue: Fix Foreign Key Types in event_registrations #134)

    • ✅ EventRegistration now uses proper FK columns with @joincolumn
    • ✅ Added activityId and userId UUID columns with corresponding relations

    Phase 5: Normalize eventTags to Separate Entity (Issue: Normalize eventTags to Separate Entity #138)

    • ✅ Created Tag entity with id, name, slug, createdAt
    • ✅ Created TagModule, TagService, TagController with CRUD and findOrCreateMany
    • ✅ Added @manytomany relation via activity_tags join table
    • ✅ Removed deprecated eventTags string array field

    Phase 6: Create Media Entity (NEW)

    • ✅ Created Media entity to track uploaded files with S3 metadata
    • ✅ Created MediaModule, MediaService, MediaController
    • ✅ Added coverPhoto and document relations to Activity
    • ✅ Removed deprecated eventCoverPhoto and eventDocument string fields
    • ✅ Removed old S3Service (functionality moved to MediaService)

    Phase 7: Robust S3 Cleanup Mechanism (NEW)

    • ✅ Added upload rollback - S3 files deleted if database save fails
    • ✅ Added replaceMedia() for atomic cover image/document replacement
    • ✅ Added deleteActivity() cleanup - removes associated media files
    • ✅ Added orphaned media detection (findOrphanedMedia())
    • ✅ Added admin cleanup endpoints (GET/POST /media/admin/orphaned, /cleanup)

    🛠️ Breaking Changes

    • Database must be recreated - All existing tables should be dropped
    • DTOs now use tagNames: string[] instead of eventTags
    • DTOs now use coverPhotoId/documentId instead of URL strings
    • updateCoverImage() now requires userId parameter

    📝 New Endpoints

    Method Endpoint Description
    GET /tags List all tags
    POST /tags Create a tag
    GET /tags/:id Get tag by ID
    DELETE /tags/:id Delete a tag
    GET /media List all media
    POST /media/upload Upload file
    POST /media/upload/image Upload image
    POST /media/upload/document Upload document
    DELETE /media/:id Delete media
    GET /media/admin/orphaned List orphaned media (admin)
    POST /media/admin/cleanup Cleanup orphaned media (admin)

Visual Aids 🔎

Updated ERD

https://dbdiagram.io/d/NSC-Events-ERD-696e9c7ad6e030a02480f84a

Expand (current ERD) ⬇️ Screenshot 2026-01-17 at 11 05 41 AM

How to Test 🧪

  1. Steps to Reproduce:
    - Step 1: Drop all existing database tables (DROP SCHEMA public CASCADE; CREATE SCHEMA
    public;)
    - Step 2: Run npm run start:dev - TypeORM will create new schema
    - Step 3: Test CRUD operations on /events, /tags, /media endpoints
    - Step 4: Verify tag filtering works: GET /events?tags=club,study
    - Step 5: Test cover image upload and replacement
  2. Expected Behavior:
    - All endpoints work with new schema
    - Tag filtering uses JOIN instead of regex
    - Cover image replacement deletes old file from S3
    - Activity deletion cleans up associated media
  3. Known Issue:
    • Please don't be alarmed with these issues they will be tracked in separate PR.
    • With this code change events created from UI wont display cover photo
    • Users can't see all created events
    • The current backend changes are wired with the frontend but basic operations still work

Checklist ✅

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

@NahomAlemu NahomAlemu self-assigned this Jan 20, 2026
@beimnettes beimnettes self-requested a review January 23, 2026 02:41
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.

HY @NahomAlemu, I pulled the branch locally and reviewed the backend changes. The new structure with the user, tag, and media entities makes the schema much cleaner and easier to follow, and it lines up well with the updated ERD. The database reset note is clear. Since this is an ongoing PR, I’ll continue reviewing as updates come in.

@TVW96 TVW96 self-requested a review January 23, 2026 22:45
@NahomAlemu NahomAlemu added the enhancement New feature or request label Jan 27, 2026
@NahomAlemu NahomAlemu marked this pull request as ready for review January 27, 2026 17:38
@github-actions
Copy link

github-actions bot commented Feb 4, 2026

E2E Test Results

  • Total: 86
  • Passed: 72
  • Failed: 14

View detailed report

@TVW96
Copy link
Contributor

TVW96 commented Feb 9, 2026

@NahomAlemu
I'm having two issues with this PR.

  1. The database is filled with loads of test events from an unknown location. (created on: 2026/02/06)
Screenshot 2026-02-09 at 11 32 27 AM 2. Event data not visible on frontend. Receiving 500 error when attempting to fetch events from events api. Screenshot 2026-02-09 at 11 35 18 AM Screenshot 2026-02-09 at 11 37 30 AM

@NahomAlemu
Copy link
Contributor Author

NahomAlemu commented Feb 9, 2026

@NahomAlemu I'm having two issues with this PR.

  1. The database is filled with loads of test events from an unknown location. (created on: 2026/02/06)

Hey Troy, thanks for reviewing this PR.

Yes, the test data is coming from Playwright. We do have a teardown step in place; I’ll double-check whether it’s being triggered correctly. Either way, the data is isolated to the test environment and isn’t harmful.

This is a known frontend issue. Since this PR focuses on the database schema redesign, the frontend hasn’t been fully wired yet. I’ve created tracking issues for the UI work, and we’ll address those once this PR is merged. UI inconsistencies in this branch are expected.

Tracking issue example:
#152 (comment)

@TVW96
Copy link
Contributor

TVW96 commented Feb 9, 2026

@NahomAlemu

I think I see what’s happening here, but the current approach introduces new issues rather than resolving the underlying one.

The test were failing earlier because you had attempt to mask them rather than address the entire schema/test while adjusting the ERD.

The recent commits I pushed:

  • Update package-lock, remove quoted identifiers, transform tags-->eventTags (0819b87)
  • fix: update tests for PostgreSQL column names and tags transformation (5ffb918)

were to fix the broken backend connection that was identified earlier today and align the test suite with the updated schema.

As a result of the changes, both backend-ci and frontend-ci are now failing event though no deployment configuration was modified in this PR. This suggests the failures are originating from a CI/CD workflow configuraiton issue rather than from the changes introduced here.

I'm going to dig further into the GitHub Actions setup to see what adjustments are needed to support the repository's current state and ensure the pipelines reflect the updated schema and test expectations.

@github-actions
Copy link

E2E Test Results

  • Total: 86
  • Passed: 72
  • Failed: 14

View detailed report

@NahomAlemu
Copy link
Contributor Author

@NahomAlemu

I think I see what’s happening here, but the current approach introduces new issues rather than resolving the underlying one.

The test were failing earlier because you had attempt to mask them rather than address the entire schema/test while adjusting the ERD.

The recent commits I pushed:

were to fix the broken backend connection that was identified earlier today and align the test suite with the updated schema.

As a result of the changes, both backend-ci and frontend-ci are now failing event though no deployment configuration was modified in this PR. This suggests the failures are originating from a CI/CD workflow configuraiton issue rather than from the changes introduced here.

I'm going to dig further into the GitHub Actions setup to see what adjustments are needed to support the repository's current state and ensure the pipelines reflect the updated schema and test expectations.

The issues related to the E2E tests were resolved in a separate PR, which was expected since I merged the main branch into this PR branch. The E2E tests were still referencing the old schema, which is why they were failing.

This PR addresses that issue and additional related fixes:
#154 (comment)

I’ve also discussed the UI issues with Troy. This PR should be good to merge alongside the upcoming PR that fixes the GitHub Actions failure, assuming there are no additional concerns.

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.

I will approve this PR for schema adjustments. However, additional PRs will be required to rectify any misconfigurations after merging with your testing branch.

Copy link
Contributor

@IsaacJrTypes IsaacJrTypes left a comment

Choose a reason for hiding this comment

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

Great work on the ERD redesign and tests! Thanks for creating and reporting UI bug issues so we can track and revisit these issues in later PRs. Its awesome to see you focused on test driven development since its an essential practice when writing quality code.

My intention with this task was only to produce the ERD, but you definitely went ham with this PR. We will use the learning to help guide the migration to Supabase.

I am approving so we can move forward since the issues are documented.

@NahomAlemu NahomAlemu merged commit 8d3837a into main Feb 10, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants