Enhance the application model and render new fields#1039
Enhance the application model and render new fields#1039Dhirenderchoudhary wants to merge 11 commits intoRealDevSquad:developfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds a "Request Changes" button to the application details interface with accompanying feedback submission flow. Introduces new utility functions for nudging applications and adding feedback. Updates styling to support application status badges and a highlights section displaying application score and nudge count. Extends mock data with new application fields. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Request Changes<br/>Button
participant Script as script.js<br/>(updateUserApplication)
participant Utils as utils.js<br/>(addApplicationFeedback)
participant API as Backend API
User->>UI: Click "Request Changes"
UI->>Script: Trigger with isRequestChanges: true
Script->>Script: Validate feedback exists
Script->>Utils: Call addApplicationFeedback(id, feedbackData)
Utils->>API: PATCH /applications/{id}/feedback
API-->>Utils: Response
Utils-->>Script: Return feedback update result
Script->>API: updateApplication call
API-->>Script: Success response
Script->>UI: Show success toast
Script->>UI: Close application details
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 7
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)
121-156:⚠️ Potential issue | 🟠 Major
Promise.allcan leave the application in a partially updated state.If
addApplicationFeedbacksucceeds butupdateApplicationfails (or vice versa), the application ends up with feedback submitted but status unchanged, or status changed but feedback lost.Promise.allrejects on the first failure, so the success toast is never shown, but the successful call's side effect has already persisted server-side.Consider sequencing these calls (feedback first, then status update) so you can handle partial failure more gracefully, or at minimum document/accept this as a known trade-off.
🤖 Fix all issues with AI agents
In `@applications/index.html`:
- Around line 135-140: Update the button with class
"application-details-request-changes" to use a clearer aria-label: replace
"Request Changes Application" with "Request changes for this application" so
screen readers read a natural phrase; locate the <button> element that currently
sets aria-label and update that attribute value accordingly.
In `@applications/script.js`:
- Around line 92-95: The current branch that handles isRequestChanges silently
returns when applicationTextarea is missing or empty; change this so it
validates and informs the user instead of returning silently: inside the
isRequestChanges handling (the block referencing isRequestChanges and
applicationTextarea), replace the early return with a validation flow that
displays a visible error/validation message (e.g., render or call an existing
showValidationError/toast function), set focus on applicationTextarea, and
prevent submission until non-empty input is provided; keep the existing guard
for null textarea but ensure the user sees the error rather than no-op.
- Around line 89-116: In updateUserApplication, the isRequestChanges branch
incorrectly calls showToast directly instead of the dev-aware wrapper
showToastMessage; replace the showToast(...) calls inside the isRequestChanges
branch with showToastMessage(...) using the same message and type payload so the
request-changes flow uses the same toast system as the accept/reject path, and
ensure showToastMessage is available in the module scope (same usage pattern as
in the accept/reject branch).
In `@applications/style.css`:
- Around line 779-822: The button width rules for selectors
`.application-details .application-details-actions .application-details-reject`,
`.application-details .application-details-actions .application-details-accept`,
`.application-details .application-details-actions .application-details-nudge`,
and `.application-details .application-details-actions
.application-details-request-changes` are currently placed outside the `@media
(max-width: 850px)` block and therefore apply globally; wrap these width
declarations in an appropriate media query (e.g., a desktop/mid-range breakpoint
such as `@media (min-width: 851px)` or the same breakpoint group used for other
desktop styles) so the 18%/24% widths only apply on larger viewports, or
alternatively move them into the global button styles block if they truly are
intended to be global.
- Around line 462-496: The three rules for .description.status-pending,
.description.status-accepted, and .description.status-rejected duplicate
padding, border-radius, display, color, and text-transform; extract those shared
declarations into a single base class (e.g., .description.status-badge) and
leave only background-color in the specific selectors
(.description.status-pending, .description.status-accepted,
.description.status-rejected); update the CSS selectors to use
.description.status-badge alongside the existing nested context
(.application-details .application-details-main .application-section) so
specificity is preserved and only background-color varies per status.
In `@applications/utils.js`:
- Around line 120-140: Remove the unused exported function nudgeApplication from
applications/utils.js: delete the async function nudgeApplication and its export
so the module no longer exports an unused symbol; if you intend to keep it for a
future PR instead, move the implementation to a feature branch or comment it out
and add a TODO explaining where it will be wired up, but do not leave an
exported, unused function in this PR.
In `@mock-data/applications/index.js`:
- Around line 5-7: The mock data only includes applicationScore and nudgeCount
on the first object, so update the mock arrays fetchedApplications,
acceptedApplications, and pendingApplications to include those fields on
multiple entries with varied values (e.g., present with >0, present as 0, and
omitted) so tests exercise both the numeric rendering and the 'N/A' fallback;
locate the objects inside
fetchedApplications/acceptedApplications/pendingApplications in
mock-data/applications/index.js and add applicationScore and nudgeCount
properties across several entries accordingly.
…efactor application status badge styling, and update dependencies.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@applications/script.js`:
- Around line 134-141: The accept/reject branch currently checks
applicationTextarea.value directly, allowing whitespace-only feedback; mirror
the isRequestChanges path by trimming input first: compute const feedback =
applicationTextarea.value.trim() (or use applicationTextarea.value &&
applicationTextarea.value.trim()) and only push
addApplicationFeedback(currentApplicationId, { feedback, status:
applicationStatus }) when feedback is non-empty, ensuring you reference
applicationTextarea, addApplicationFeedback, currentApplicationId, and
applicationStatus so the validation is consistent across both code paths.
- Around line 739-741: The click handler passes a redundant isAccepted flag to
updateUserApplication even though updateUserApplication short-circuits when
isRequestChanges is true; remove the misleading parameter and call
updateUserApplication({ isRequestChanges: true }) from
applicationRequestChangesButton's listener so only the meaningful property is
supplied (reference: applicationRequestChangesButton and updateUserApplication
with the isRequestChanges early-return).
In `@applications/style.css`:
- Around line 790-819: Remove the orphaned CSS rules for the non-existent
.application-details-nudge selector or add the missing element; locate the rules
under the .application-details .application-details-actions
.application-details-nudge block and either delete all related selectors
(including the :is(:hover, :active, :focus-visible), :hover and :active variants
and the bounceBackAnimation usage) or implement a corresponding button element
(e.g., class="application-details-nudge") in the application details markup
where nudgeCount is shown so the styles apply.
…nager, improve API base URL loading, and refine application feedback and toast handling.
Dhirenderchoudhary
added
enhancement
…base URL environment configuration, and upgrade to Yarn 4.
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
…and refine API base URL loading logic.
There was a problem hiding this comment.
don't see the new format of the application here you can check the application response new one for the correct response structure or check the backend repo
There was a problem hiding this comment.
this is also not replied
There was a problem hiding this comment.
what you mean by new format i did that according to my knowledge and changes the test cases
AnujChhikara
left a comment
AnujChhikara
left a comment
There was a problem hiding this comment.
I don't see any test attached to this pr or any test pr link make sure to add them
AnujChhikara
left a comment
AnujChhikara
left a comment
There was a problem hiding this comment.
I don't see the admin feedback api changes in this PR , make sure to include them and test your changes with your local backend or staging backend not with mock data please
…and enhance styling for application statuses
Dhirenderchoudhary
force-pushed
the
develop
branch
from
e3ddb4f to
4209c6d
Compare
There was a problem hiding this comment.
this is also not replied
Still don't see the test or Test PR link attached |
…uttons after updates, add success toast for requesting changes, and update Yarn configuration.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/toast/script.js (1)
50-56:⚠️ Potential issue | 🔴 CriticalFix legacy toast functions to accept object payloads instead of strings.
The
showToasterfunctions inidentity-service-logs/utils.js(line 207) andgroups/script.js(line 250) expect a string parameter and setinnerText = message. However, they are passed{ message, type }objects fromshowToastMessagewhen their arity is 1, causing them to display[object Object]instead of the actual message.Update both functions to extract the message from the object:
function showToaster({ message }) { const toaster = document.querySelector('.toast__message'); toaster.innerText = message; toaster.classList.add('toast--show'); setTimeout(() => { toaster.classList.remove('toast--show'); }, 3000); }This affects 8 call sites: 2 in
identity-service-logs/utils.jsand 6 ingroups/script.js.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/toast/script.js` around lines 50 - 56, The legacy toaster functions need to accept an object payload (with message and type) instead of a plain string: update the showToaster implementations referenced in identity-service-logs/utils.js and groups/script.js so their signature destructures the payload (e.g. function showToaster({ message, type }) or at least { message }) and use message for toaster.innerText and any type-based styling; ensure these showToaster variants still work with showToastMessage (which passes { message, type } when oldToastFunction.length === 1) and adjust any local references within those functions accordingly across the 8 affected call sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@applications/style.css`:
- Around line 454-472: The CSS lacks a rule for the status class generated for
"changes_requested" (script builds status-${application.status.toLowerCase()}
producing status-changes_requested), so add a selector for
.description.status-changes_requested and set its background-color to the
desired value (e.g., var(--color-pending)) or normalize mapping to reuse
.description.status-pending; update the stylesheet by adding that class
alongside
.description.status-pending/.description.status-accepted/.description.status-rejected
so badges for changes_requested render with the correct background color.
---
Outside diff comments:
In `@components/toast/script.js`:
- Around line 50-56: The legacy toaster functions need to accept an object
payload (with message and type) instead of a plain string: update the
showToaster implementations referenced in identity-service-logs/utils.js and
groups/script.js so their signature destructures the payload (e.g. function
showToaster({ message, type }) or at least { message }) and use message for
toaster.innerText and any type-based styling; ensure these showToaster variants
still work with showToastMessage (which passes { message, type } when
oldToastFunction.length === 1) and adjust any local references within those
functions accordingly across the 8 affected call sites.
| ); | ||
| }); | ||
|
|
||
| it('should hide action buttons after successfully requesting changes', async function () { |
There was a problem hiding this comment.
what is this test about? are we hiding the button if the status is request changes
| await page.click('#application-details-request-changes'); | ||
| await page.waitForSelector('[data-testid="toast-component"].show'); | ||
|
|
||
| const acceptBtn = await page.$('#application-details-accept'); |
There was a problem hiding this comment.
use data test id here also to get the buttons
| let feedbackText = ''; | ||
| if (typeof application.feedback === 'string') { | ||
| feedbackText = application.feedback; | ||
| } else if (Array.isArray(application.feedback)) { | ||
| feedbackText = application.feedback | ||
| .map((f) => (typeof f === 'string' ? f : f.feedback || '')) | ||
| .filter((f) => f) | ||
| .join('\n'); | ||
| } else if (application.feedback && typeof application.feedback === 'object') { | ||
| feedbackText = application.feedback.feedback || ''; | ||
| } |
There was a problem hiding this comment.
why are we making this so complex from backend you will you get this response
feedback: [
{
"status": string,
"reviewerName": string,
"createdAt": timestamp
"feedback": string
}
],
I would suggest show the backend feedback at different part of the card not on the textarea as it can be multiple
| applicationAcceptButton.classList.add('hidden'); | ||
| applicationRejectButton.classList.add('hidden'); | ||
| applicationRequestChangesButton.classList.add('hidden'); |
There was a problem hiding this comment.
but admin can accept or reject or request changes if the status is request changes
2 participants
Date: 11 Feb 2026
Developer Name: Dhirender
Issue Ticket Number
#1037
Tech Doc Link
Business Doc Link
Description
We are revamping the flow to join RDS and as part of this revamp, new fields have been introduced such as nudge count, application score, etc. These fields are currently not rendered in the application details modal. Apart from this currently the admins can only accept/reject an application. But in this revamp we are introducing the option to request changes on an application. This issue ticket aims at adding the new fields which are being stored but not rendered in the application detail modal and adding a button to request changes on an application. Apart from this we will also improve the UI of the application.
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Screen.Recording.2026-02-11.at.6.52.17.PM.mov
Test Coverage
Screenshot 1
Additional Notes