feat: update admin feedback API and add request changes button for application#1040
Merged
iamitprakash merged 6 commits intodevelopfrom Feb 26, 2026
Merged
Conversation
- Introduced a new button for requesting changes in application details. - Updated the application feedback submission logic to handle 'changes_requested' status. - Enhanced tests to cover new functionality, including success and error toast messages. - Adjusted styles for the new button to ensure consistent UI.
WalkthroughA "Request Changes" feature is added to the application details modal, enabling users to request modifications to pending applications with accompanying feedback. Changes include a new UI button, updated API integration to a feedback endpoint, validation logic for feedback text, status badge styling, and expanded test coverage for the new workflow. Changes
Sequence DiagramsequenceDiagram
participant User
participant Client as Client/UI
participant API as API Server
participant Toast as Toast Notifier
User->>Client: Click "Request Changes" button
activate Client
Client->>Client: Open application details modal<br/>(pending status)
Client->>Client: Display feedback textarea
User->>Client: Enter feedback text
User->>Client: Click confirm/submit
alt Feedback validation fails
Client->>Toast: Show error toast<br/>("feedback required")
Toast->>User: Display error message
else Feedback validation succeeds
Client->>API: PATCH /applications/{id}/feedback<br/>{ status: 'changes_requested', feedback: text }
activate API
API->>API: Process feedback submission
API->>Client: Return success response
deactivate API
Client->>Toast: Show success toast<br/>("Application feedback submitted successfully")
Toast->>User: Display success message
Client->>Client: Close modal & update UI<br/>(show changes requested badge)
end
deactivate Client
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
applications/script.js (1)
229-263: 🧹 Nitpick | 🔵 Trivial
applicationforEach parameter shadows the outeropenApplicationDetailsparameter.The loop variable
applicationat line 229 shadows the function parameter of the same name. While the status-check logic at lines 286–341 is correctly outside the loop and references the outer binding, the shadowing makes the code hard to reason about at a glance and risks future bugs if logic is moved inside the loop.♻️ Suggested rename
- selectedApplication.applicationDetails.forEach((application) => { + selectedApplication.applicationDetails.forEach((detail) => { const applicationSection = createElement({ ... }); const applicationSectionTitle = createElement({ ... - innerText: application.title, + innerText: detail.title, }); let applicationSectionDescription; - if (application.title === 'Status') { - const statusLabel = application.description + if (detail.title === 'Status') { + const statusLabel = detail.description ... applicationSectionDescription = createElement({ ... - attributes: { class: `status-badge status-badge--${application.description}` }, - innerText: statusLabel, + attributes: { class: `status-badge status-badge--${detail.description}` }, + innerText: statusLabel, }); } else { applicationSectionDescription = createElement({ ... - innerText: application.description, + innerText: detail.description, }); } ... });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@applications/script.js` around lines 229 - 263, The forEach loop parameter named application shadows the outer openApplicationDetails parameter; rename the loop variable used in selectedApplication.applicationDetails.forEach (and its internal references like application.title, application.description, applicationSection, applicationSectionDescription) to a nonconflicting name such as detail or appDetail, update every use inside that loop accordingly, and leave the outer openApplicationDetails binding and the separate status-check logic untouched so there is no variable shadowing or potential future confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/applications/applications.test.js`:
- Around line 75-94: The interceptor branch matching url
`${STAGING_API_URL}/applications/lavEduxsb2C5Bl4s289P` contains an unreachable
non-GET branch because submitApplicationFeedback issues a PATCH to the
`/feedback` suffix; remove the dead code by deleting the conditional branch that
handles method !== 'GET' (the JSON response with message 'Application feedback
submitted successfully') inside the interceptedRequest.respond block for that
exact URL, leaving only the GET response that returns pendingApplications[0];
ensure the actual PATCH handler remains in the separate interceptor that matches
the `/feedback` URL so submitApplicationFeedback is still tested.
In `@applications/index.html`:
- Around line 135-140: Update the Request changes button so its aria-label
matches the sibling buttons' pattern and casing: change the aria-label on the
element with class "application-details-request-changes" to "Request Application
Changes" (and ensure the visible button text follows the same capitalization if
needed) so it consistently uses the "[Action] Application" format like the
Accept and Reject buttons.
In `@applications/script.js`:
- Around line 281-283: The textarea's placeholder is never set because the
property is named "placeHolder" (wrong casing); update the element creation to
use the standard "placeholder" attribute (e.g., change placeHolder to
placeholder or call setAttribute('placeholder', 'Add Feedback here (required for
Request changes)')) in the code that builds the element (where the object with
placeHolder and innerText is used) so the browser renders the hint correctly.
- Around line 178-185: The Score and Nudge Count sections render the literal
"undefined" because createElement sets element.textContent to
application.score/application.nudgeCount even when they are undefined; update
the code that builds these sections (where application.score and
application.nudgeCount are used) to guard against missing values by either (a)
only creating/pushing the section if the value is not null/undefined, or (b)
coerce the value to a safe display string (e.g., empty string or "—") before
passing it into createElement; reference the createElement helper and the
application.score and application.nudgeCount usages to locate and apply this
conditional check.
---
Outside diff comments:
In `@applications/script.js`:
- Around line 229-263: The forEach loop parameter named application shadows the
outer openApplicationDetails parameter; rename the loop variable used in
selectedApplication.applicationDetails.forEach (and its internal references like
application.title, application.description, applicationSection,
applicationSectionDescription) to a nonconflicting name such as detail or
appDetail, update every use inside that loop accordingly, and leave the outer
openApplicationDetails binding and the separate status-check logic untouched so
there is no variable shadowing or potential future confusion.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
__tests__/applications/applications.test.jsapplications/index.htmlapplications/script.jsapplications/style.cssapplications/utils.js
Deploying dashboard-rds with
|
| Latest commit: |
4b421cc
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://708211b1.dashboard-rds.pages.dev |
| Branch Preview URL: | https://refactor-application-superus.dashboard-rds.pages.dev |
MayankBansal12
force-pushed
the
refactor/application-superuser-feedback
branch
from
e5d4a95 to
7c8f537
Compare
MayankBansal12
changed the title
AnujChhikara
approved these changes
Feb 26, 2026
iamitprakash
approved these changes
Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Date: 25-02-26
Developer Name: @MayankBansal12
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screencast
feedback-demo.mp4
tests