Feedback system design by nverbic · Pull Request #578 · Women-Coding-Community/wcc-backend

nverbic · 2026-03-22T22:13:43Z

Description

Implement feedback APIs.

Three Feedback Types:

MENTOR_REVIEW - Mentees review mentors (requires rating, reviewee, cycle)
COMMUNITY_GENERAL - General community feedback (no rating required)
MENTORSHIP_PROGRAM - Program feedback (requires rating, cycle)

Architecture

┌─────────────────────────────────────────────────────────────┐
│                    REST API Layer                            │
│  FeedbackController - 7 endpoints                           │
│  - GET /feedback (with filters)                             │
│  - GET /feedback/{id}                                       │
│  - POST /feedback                                           │
│  - PUT /feedback/{id}                                       │
│  - PUT /feedback/{id}/approve                               │
│  - PUT /feedback/{id}/anonymous-status                      │
│  - DELETE /feedback/{id}                                    │
└─────────────────────────────────────────────────────────────┘
                            ↓
┌─────────────────────────────────────────────────────────────┐
│                   Business Logic Layer                       │
│  FeedbackService - Validation & Orchestration               │
│  - Member validation (reviewer/reviewee exists)             │
│  - Data enrichment (fetch member names)                     │
│  - Search with multiple criteria                            │
│  - Approval workflow management                             │
└─────────────────────────────────────────────────────────────┘
                            ↓
┌─────────────────────────────────────────────────────────────┐
│                  Data Access Layer                           │
│  FeedbackRepository (Interface)                             │
│  PostgresFeedbackRepository (JDBC Implementation)           │
│  FeedbackMapper - Row mapping & SQL generation             │
└─────────────────────────────────────────────────────────────┘
                            ↓
┌─────────────────────────────────────────────────────────────┐
│                    Database Layer                            │
│  PostgreSQL                                                  │
│  - feedback_types (lookup table)                            │
│  - feedback (main table with 6 indexes)                     │
│  - Foreign keys to members, mentorship_cycles               │
└─────────────────────────────────────────────────────────────┘

Tables Created

1. feedback_types (Lookup Table)

- id (SERIAL PRIMARY KEY)
- name (VARCHAR(100) UNIQUE) - MENTOR_REVIEW, COMMUNITY_GENERAL, MENTORSHIP_PROGRAM
- description (TEXT)

2. feedback (Main Table)

- id (BIGSERIAL PRIMARY KEY)
- reviewer_id (INTEGER, FK to members, NOT NULL)
- reviewee_id (INTEGER, FK to members, nullable)
- mentorship_cycle_id (INTEGER, FK to mentorship_cycles, nullable)
- feedback_type_id (INTEGER, FK to feedback_types, NOT NULL)
- rating (INTEGER, CHECK 0-5)
- feedback_text (TEXT, NOT NULL)
- feedback_year (INTEGER)
- is_anonymous (BOOLEAN, DEFAULT true)
- is_approved (BOOLEAN, DEFAULT false)
- created_at (TIMESTAMP WITH TIME ZONE)
- updated_at (TIMESTAMP WITH TIME ZONE)

Constraints

MENTOR_REVIEW must have revieweeId, mentorshipCycleId, and rating
MENTORSHIP_PROGRAM must have mentorshipCycleId and rating
Rating must be 1-5 when provided
FeedbackType must be valid enum value

Data Privacy

isAnonymous=true → reviewerName not shown when feedback is published.
Protects reviewer identity
Reviewer can toggle anonymity
Default: isApproved=false
Only approved feedback visible to public
Admin controls publication

Feedback API Coverage Analysis Report

Generated: April 6, 2026
Source: build/reports/jacoco/test/jacocoTestReport.xml

Executive Summary

✅ PASS: Feedback API meets 70% coverage threshold on all metrics!

The Feedback API implementation has excellent test coverage across all layers:

Metric	Coverage	Status
Lines	97.9% (276/282)	✅ PASS
Branches	75.8% (185/244)	✅ PASS
Methods	94.5% (137/145)	✅ PASS
Classes	18 total	-

TODO:

change PUT to PATCH for anonymus and approved ✅ Completed
test more in Swagger ✅ Completed
add tests to reach 70% ✅ Completed
restrict APPROVE for specific roles (ADMIN, ...)
restrict only specific mentee to mentor review / now any member can give a review to another member (mentor) - the only restriction is that both are members.

Related Issue

Please link to the issue here

Change Type

Screenshots

Pull request checklist

Please check if your PR fulfills the following requirements:

I checked and followed the contributor guide
I have tested my changes locally.

Implement API for Community, Mentorship and Mentor feedback.

Add feedback migration file.

Update migration file name.

- Accepted mentorship cycle implementation from main - Resolved GlobalExceptionHandler conflict - Feedback system integrated with cycle tracking - Fixed V34 migration to reference mentorship_cycles table - All 15 mentorship cycle files accepted from main

Update/add mentor.json, feedback.json, mentee.json

nverbic · 2026-03-22T22:16:21Z

I will remove two admin files that were updated by accident.

dricazenck · 2026-03-23T22:22:13Z

+        feedback.getIsApproved());
+
+    // Return the last inserted ID
+    return jdbc.queryForObject("SELECT LASTVAL()", Long.class);


I am not sure that is the best way to get the id of the feedback I would play safe, make void and if we need the id later it should be made select to get the id. If there are concurrency, this could be incorrect.

@dricazenck The id is needed in order to return feedback object when creating it, like in MemberMapper here

womencodingcommunity · 2026-03-23T22:26:25Z

@@ -0,0 +1,8 @@
+{
+  "reviewerId": 1,


[WARNING] Debug/test file committed to the repository root

feedback-post-request.json looks like a local Swagger/curl test file that was accidentally included. It should not be committed — it adds noise to the repo and would need maintenance as the API evolves.

Please delete this file before merging.

womencodingcommunity · 2026-03-23T22:26:53Z

+  @Override
+  public Feedback update(final Long id, final Feedback entity) {
+    feedbackMapper.updateFeedback(entity, id);
+    return entity;


[WARNING] update() returns the passed-in entity, not what is in the database

After updateFeedback() runs, the returned entity object will not have the updated_at timestamp set (it comes from CURRENT_TIMESTAMP in SQL), and reviewerName/revieweeName are not persisted at all. Callers of updateFeedback will see stale metadata in the response.

Consider re-fetching after update, similar to what create() already does:

@Override public Feedback update(final Long id, final Feedback entity) { feedbackMapper.updateFeedback(entity, id); return findById(id).orElseThrow(); }

womencodingcommunity · 2026-03-23T22:27:53Z

+import static org.hamcrest.Matchers.is;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.eq;


[WARNING] Tests are missing @DisplayName — project convention

Per the project conventions in CLAUDE.md, all tests should use @DisplayName with Given-When-Then format:

@Test @DisplayName("Given a valid MENTOR_REVIEW DTO, when createFeedback is called, then it returns HTTP 201 with the created feedback") void testCreateFeedbackReturnsCreated() { ... }

This applies to all test methods in FeedbackControllerTest, FeedbackDtoValidationTest, FeedbackServiceTest, PostgresFeedbackRepositoryTest, and FeedbackMapperTest. Also, test method names could follow the should prefix convention (e.g., shouldReturnCreatedWhenFeedbackIsValid).

womencodingcommunity

Great effort on this feature — the architecture is well-thought-out, the layered structure is clean, and the custom @ValidFeedback constraint is a nice pattern. The test coverage is a solid start too. There are three critical issues that need to be resolved before merging, and a few warnings worth addressing:

Must fix before merge:

@Valid missing on @RequestBody — neither field-level nor class-level validation triggers without it (createFeedback, updateFeedback)
SELECT LASTVAL() race condition — use RETURNING id in the INSERT instead
FeedbackType.fromId() silent fallback — throws IllegalArgumentException instead of returning COMMUNITY_GENERAL for unknown IDs

Should address:

feedback-post-request.json committed to repo root — please remove
DB rating CHECK >= 0 vs Java @Min(1) inconsistency; missing DB constraint for MENTORSHIP_PROGRAM requiring mentorship_cycle_id
update() returns in-memory entity — updated_at and names will be stale in response
reviewerName/revieweeName set in service but not persisted or joined on read — they will always be null from GET endpoints
isAnonymous privacy logic not enforced on GET responses
All test methods need @DisplayName with Given-When-Then format (project convention)

The TODO items in the PR description (PATCH methods, more Swagger testing, reaching 70% coverage) should be tracked as follow-up issues before this is considered production-ready.

Update Feedback.java and FeedbackSearchCriteria to use only needed lombok annotations Update FeedbackController.java - use Patch instead of Put for approveFeedback and setFeedbackAnonymousStatus Update/Add tests Feedback coverage is above 70% threshold. Implement PMD suggestions.

"chore: rename feedback migration to V36 to maintain chronological order after main merge"

# Conflicts: # src/main/java/com/wcc/platform/configuration/GlobalExceptionHandler.java

Fix SonarQube issues.

@Valid

Implement PR suggestions - add @Valid to FeedbackDTO create and update - add @DsiaplyName to tests - update FeedbackMapper addFeedback() to query database instead of using SELECT LASTVAL() when returning values. - update tests - Update FeedbackType to throw exception if nonexisting typeId passed to fromId() function - Fix PostgresFeedbackRepository update() to return feedback object from database and not just the same entity that is passed to update() as arg.

- Update of migration file. Change rating range and add constraint.

Update FeedbackMapper - set reviewer and reviewee names instead of nulls. Update FeedbackMapper tests. Update Feedback - Remove Setter for Reviewer Name and Reviewee Name. Update PostgresFeedbackRepository SELECT sql queries to join members and feedback to retrieve member's names. Update tests to reflect join between members and feedback tables. Update application.yml to contain also option to host admin app on 3001 port.

Implement PMD suggestions.

# Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

- change dev amd start scripts to "dev": "next dev -p 3001" and start": "next start -p 3001",

dricazenck · 2026-04-25T22:05:26Z

+package com.wcc.platform.repository.postgres.constants;
+
+/** Constants related to Feedback entity. */
+@SuppressWarnings({"PMD.DataClass", "PMD.LongVariable"})


don't suppress warnings, they are here for a good reason. Please fix them ;)

dricazenck · 2026-04-25T22:07:30Z

+  public static final String COLUMN_REVIEWER_NAME = "reviewer_name";
+  public static final String COLUMN_REVIEWEE_NAME = "reviewee_name";
+  public static final String COLUMN_MENTORSHIP_CYCLE_ID = "mentorship_cycle_id";
+  public static final String COLUMN_FEEDBACK_TYPE_ID = "feedback_type_id";


For example:

Suggested change

public static final String COLUMN_FEEDBACK_TYPE_ID = "feedback_type_id";

public static final String COL_FEEDBACK_TP = "feedback_type_id";

dricazenck · 2026-04-25T22:07:48Z

+
+/** Constants related to Feedback entity. */
+@SuppressWarnings({"PMD.DataClass", "PMD.LongVariable"})
+public final class FeedbackConstants {


this can be a record ;)

@dricazenck I have chated with Claude. The suggestion is to keep DataClass. MemberConstants and MentorConstants in the same package are final class as well. What do you think?

dricazenck · 2026-04-25T22:09:32Z

+ * the result sets to Feedback objects with the help of FeedbackRowMapper.
+ */
+@Repository
+@Primary


No need to add primary, when only one class implements the interface ;)

dricazenck · 2026-04-25T22:14:08Z

@@ -0,0 +1,49 @@
+-- Table for feedback types
+CREATE TABLE IF NOT EXISTS feedback_types


make sure to review the data and version of the migration to avoid conflicts in case anyone create another migration, otherwise this migration won't work.

- Restrict Year parameter to min and max range (2000 - 2100). Validate Year already in controller. Update tests. - Shorter names for feedback column names constants - Remove Primary from PostgresFeedbackRepository class

- fix PMD warnings

nverbic · 2026-05-07T10:40:27Z

@ Feedback is ready for merge.
I have two tasks opened for next PRs:

restrict APPROVE for specific roles (ADMIN, ...) - discuss with team how to proceed
restrict only specific mentee to mentor review / now any member can give a review to another member - the only restriction is that both are members. There is no check if the mentee is/was working with the mentor being reviewed.

# Conflicts: # src/main/resources/init-data/mentor.json

sonarqubecloud · 2026-05-07T16:31:09Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.7% Duplication on New Code

See analysis details on SonarQube Cloud

nverbic · 2026-06-10T15:36:44Z

@dricazenck @goelsonali @Sowmiya07 Ladies, just a litte reminder for this PR. It is completed. Some time has passed so I will have to do changes (at least in the migration). Pleas just give me green light, so we can merge.

nverbic added 5 commits March 17, 2026 11:10

Feedback API

a4a1986

Implement API for Community, Mentorship and Mentor feedback.

Feedback API

d088391

Add feedback migration file.

Feedback API

409fb56

Update migration file name.

Feedback API

8f4a5ab

Update/add mentor.json, feedback.json, mentee.json

dricazenck reviewed Mar 23, 2026

View reviewed changes