Skip to content

Conversation

@AnujChhikara
Copy link
Contributor

@AnujChhikara AnujChhikara commented Jan 16, 2026

Date: 18 Jan, 2026

Developer Name: @AnujChhikara


Issue Ticket Number

Tech Doc Link

Business Doc Link

  • NA

Description

  • Implemented a new endpoint to submit feedback for applications, allowing users to provide status updates and comments.
  • Enhanced validation for application updates to require feedback when the status is set to 'changes_requested'.
  • Updated routes to reflect the new feedback submission functionality.

Documentation Updated?

  • Yes
  • No

Under Feature Flag

  • Yes
  • No

Database Changes

  • Yes
  • No

Breaking Changes

  • Yes
  • No

Development Tested?

  • Yes
  • No

Screenshots

Screenshot 1 image image image image image

Test Coverage

Screenshot 1 image image

Additional Notes

@coderabbitai
Copy link

coderabbitai bot commented Jan 16, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

A new endpoint for submitting application feedback was introduced. The PATCH route /applications/:applicationId/feedback handles feedback submission with a new controller handler. The validator schema now enforces specific status values and makes feedback conditionally required when status is CHANGES_REQUESTED. All related tests were updated to reflect the new endpoint path and validation behavior.

Changes

Cohort / File(s) Summary
Controller & Route Implementation
controllers/applications.ts, routes/applications.ts
Added submitApplicationFeedback handler to process feedback submissions, including application retrieval, feedback entry creation with reviewer info and timestamp, and database updates. Route updated from /:applicationId to /:applicationId/feedback with new handler binding.
Validation & Middleware
middlewares/validators/application.ts
Enhanced validateApplicationUpdateData schema to enforce required status with allowed values (ACCEPTED, REJECTED, CHANGES_REQUESTED) and conditional feedback requirement—feedback becomes mandatory when status is CHANGES_REQUESTED; replaced manual validation with Joi's .valid() and .when() logic.
Test Suite
test/integration/application.test.ts, test/unit/middlewares/application-validator.test.ts
Updated integration tests to target new /feedback endpoint path and adjusted success message expectations; corrected validation error messages to match new schema constraints; updated unit test to use valid "accepted" status value.

Sequence Diagram(s)

sequenceDiagram
    Client->>+API: PATCH /applications/:applicationId/feedback
    API->>+Validator: validateApplicationUpdateData(body)
    Validator-->>-API: validation result
    alt Validation Failed
        API-->>Client: 400 Bad Request
    else Validation Passed
        API->>+Controller: submitApplicationFeedback(req, res)
        Controller->>+Database: getApplication(applicationId)
        Database-->>-Controller: application
        alt Application Not Found
            Controller-->>Client: 404 Not Found
        else Application Found
            Controller->>Controller: buildFeedbackEntry(status, feedback, reviewerName, timestamp)
            Controller->>Controller: appendToExistingFeedback()
            Controller->>+Database: updateApplication(newFeedbackList, status)
            Database-->>-Controller: updated application
            Controller-->>-API: 200 Success + feedback entry
            API-->>Client: 200 Success Response
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • MayankBansal12
  • iamitprakash

Poem

🐰 A feedback path hops into place,
With validation rules set at the pace,
Status checked, conditions bind tight,
Reviewers rejoice at the new route's might! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: adding a new application feedback submission endpoint.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description clearly relates to the changeset, detailing implementation of a new feedback submission endpoint and enhanced validation for application updates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Base automatically changed from anuj/application-nudge-test to anuj/nudge-functionality January 17, 2026 18:12
Base automatically changed from anuj/nudge-functionality to develop January 17, 2026 18:13
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
test/integration/application.test.ts (1)

359-435: Add coverage for changes_requested feedback requirement.
Consider adding tests that assert:

  • status: "changes_requested" with empty/whitespace feedback returns 400.
  • status: "changes_requested" with valid feedback succeeds.
🤖 Fix all issues with AI agents
In `@controllers/applications.ts`:
- Around line 134-173: The handler submitApplicationFeedback reads feedback,
mutates it and calls ApplicationModel.updateApplication which can cause lost
updates under concurrency; instead implement and call an atomic append in the
model (e.g., ApplicationModel.appendFeedback or a transactional method) that
takes applicationId and newFeedbackEntry and uses the DB's atomic array-append
operator (or a transaction) to push the entry and optionally update status, then
update the controller to stop reading/merging existingFeedback and call that new
model method (keep newFeedbackEntry creation and response unchanged).

In `@middlewares/validators/application.ts`:
- Around line 93-99: The Biome lint rule `noThenProperty` is flagging the `then`
usage inside the `joi.when` call (see joi.when with is:
APPLICATION_STATUS_TYPES.CHANGES_REQUESTED) causing CI failures; fix by adding a
local Biome suppression around that expression (disable `noThenProperty` for
that block) or update the Biome config to allow `then` on Joi conditions so the
validator in middlewares/validators/application.ts using
`joi.when(...).then(...)` is not flagged.

@AnujChhikara AnujChhikara force-pushed the anuj/superuser-application-feedback branch from 7cf9cb9 to 572ef6e Compare January 17, 2026 19:08
ERROR_SUBMITTING_FEEDBACK: "Error while submitting the application feedback",
};

const APPLICATION_STATUS = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

rename this object as it's make more sense to have one generic status object for the application rather than create different for each functionality.By doing this we can use this for nudge , feedback and other functions

it("should call next function if only status and feedback is passed, and status has any of the allowed values", async function () {
const req = {
body: {
status: "pending",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed this as earlier we are allowing pending status update by superuser with the new flow they can use changes_requested status

@AnujChhikara AnujChhikara self-assigned this Jan 18, 2026
@AnujChhikara AnujChhikara force-pushed the anuj/superuser-application-feedback branch from e57c612 to a02baf1 Compare January 18, 2026 15:42
@iamitprakash iamitprakash merged commit 97bda6b into develop Jan 20, 2026
4 checks passed
@iamitprakash iamitprakash deleted the anuj/superuser-application-feedback branch January 20, 2026 19:02
@AnujChhikara AnujChhikara mentioned this pull request Jan 21, 2026
10 tasks
iamitprakash added a commit that referenced this pull request Jan 21, 2026
* feat: add application feedback submission functionality (#2552)

* test: implement comprehensive application feedback tests (#2555)

* feat: add application feedback submission functionality

* feat: enhance application feedback functionality with comprehensive tests

* refactor: update application validator tests for improved clarity and consistency

* test: remove unauthorized user test case for application feedback
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.

5 participants