feat: add application detail page #1130
Merged
Conversation
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
Deploying www-rds with
|
| Latest commit: |
484db8e
|
| Status: | ⚡️ Build in progress... |
|
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 WalkthroughThis pull request introduces a comprehensive application detail view feature, adding multiple Handlebars components, a new controller with computed getters, route modifications to pass currentUser data, and extensive styling. The implementation follows Ember patterns with component composition and template-driven rendering. Changes
Sequence DiagramsequenceDiagram
participant User
participant Route as ApplicationsDetailRoute
participant Controller as ApplicationsDetailController
participant Template as Detail Template
participant Components as UI Components
User->>Route: Navigate to application detail
Route->>Route: model(params) - fetch application & currentUser
Route->>Controller: Pass model with application & currentUser
Controller->>Controller: Compute getters (sections, links, fullName, etc.)
Controller->>Template: Provide computed data
Template->>Components: Render DetailHeader with profile/score
Template->>Components: Render InfoCard for basic info & about you
Template->>Components: Render SocialLinkPill for each social link
alt hasFeedback = true
Template->>Components: Render FeedbackCard for each feedback item
else hasFeedback = false
Template->>Components: Show "No feedback yet" placeholder
end
Components->>User: Display complete application detail view
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ 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. 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: 14
🤖 Fix all issues with AI agents
In @app/components/application/detail-header.hbs:
- Around line 26-37: The "Nudge" and "Edit" Reusables::Button instances are
permanently disabled (@disabled={{true}}) and need context for users; either
remove these buttons until implemented or add an explanatory affordance: replace
the static disabled attribute with a state-driven disabled flag and render a
tooltip/title and accessible reason (e.g., title or aria-describedby) tied to
the buttons with @text "Nudge" and @text "Edit", and ensure the
Reusables::Button component supports/display tooltips and aria-disabled so
screen readers get the explanation.
- Around line 4-8: Replace the static alt text on the <img> that uses
@model.imageUrl with a descriptive fallback that includes the user's name (e.g.,
use @model.name to generate "User Name profile image") and ensure you handle
missing images by providing a fallback alt and/or fallback src; update the <img>
alt attribute logic in the template that references @model.imageUrl so it uses
@model.name when available and supplies a sensible default alt when name or
imageUrl is absent.
- Around line 10-12: The template directly accesses @model.biodata.firstName /
lastName which can be null; update the template to avoid null errors by either
wrapping the name block in a conditional check for @model.biodata (e.g., {{#if
@model.biodata}} ... {{/if}}) OR, better, move the display logic into the
controller's fullName getter and use that in the template (replace direct
biodata access with the controller's fullName property) so missing biodata is
handled consistently.
In @app/components/application/feedback-card.js:
- Around line 4-6: The formattedDate getter should guard against missing or
invalid this.args.createdAt: update the formattedDate method to first check if
this.args.createdAt is null/undefined and return a safe fallback (e.g., an empty
string or placeholder), then construct a Date from this.args.createdAt and
verify it’s valid (e.g., !isNaN(date.getTime())) before calling
toLocaleDateString(); if the date is invalid return the same fallback. This
change keeps the logic inside the formattedDate getter and references the
existing symbol this.args.createdAt and the getter name formattedDate.
In @app/components/application/info-card.hbs:
- Around line 10-11: Replace the semantically incorrect <label> used for
display-only data with proper non-form elements: wrap key/value pairs in a <dl>
with <dt> for the label (item.label) and <dd> for the value (item.value), or at
minimum replace <label> with a <span> and keep the <p> for the value; update the
template block that renders item.label/item.value accordingly and make the same
change for the other identical occurrence in this component so all display-only
key/value pairs use semantic elements instead of <label>.
In @app/components/application/social-link-pill.js:
- Around line 4-12: The getIcon method can throw when platform is
null/undefined; update getIcon(platform) to guard before calling toLowerCase by
deriving a safe key (e.g., const key = typeof platform === 'string' ?
platform.toLowerCase() : '') and then return icons[key] || 'mdi:link'; ensure
you handle non-string inputs similarly so getIcon never directly calls
platform.toLowerCase().
In @app/controllers/applications/detail.js:
- Around line 8-10: Getter currentUser can throw if this.model is null; use
optional chaining to avoid runtime errors. Change the currentUser getter (get
currentUser()) to return this.model?.currentUser instead of
this.model.currentUser so it safely returns undefined when model is
null/undefined; update any callers expecting null vs undefined if needed and run
tests to ensure behavior remains acceptable.
- Around line 16-18: The isApplicant getter can throw if this.application is
null/undefined because it reads this.application.userId without a safety check;
update the isApplicant getter (referenced as isApplicant, currentUser, and
application.userId) to guard application access—e.g., use optional chaining or
an explicit null check on this.application before comparing ids so the
expression becomes safe when application is missing.
- Around line 4-6: The getter application can throw when this.model is null;
update the get application() accessor to defensively handle a null/undefined
model (e.g., return this.model?.application or check if this.model is truthy and
return null/undefined/default) so accessing this.model.application never causes
a TypeError; change the getter implementation in the application() method
accordingly and ensure callers handle a possibly null return.
- Around line 69-85: The getters access this.application without null safety;
update fullName, formattedLocation, score, and hasFeedback to use optional
chaining and safe defaults (e.g., use this.application?.biodata or
this.application?.location to destructure, this.application?.score ?? 'N/A' for
score, and this.application?.feedback?.length > 0 ?? false for hasFeedback) so
they won’t throw when this.application is null/undefined; target the get
fullName, get formattedLocation, get score, and get hasFeedback getters and
replace direct this.application accesses with optional chaining and fallback
empty objects/values as needed.
- Around line 20-40: The getter basicInfoSections can throw when
this.application is null/undefined; add a null guard at the top of
basicInfoSections (e.g., if (!this.application) return a safe default array) and
replace direct accesses this.application.role and
this.application.professional?.skills with guarded/default values derived from
that check (keep this.fullName and this.formattedLocation behavior intact),
ensuring the getter always returns the expected sections array even when
application is missing.
- Around line 54-67: The socialLinks getter lacks a null guard and can throw if
this.application or this.application.socialLink is null/undefined; update the
getter (socialLinks) to safely handle missing application or socialLink (e.g.,
use optional chaining like const social = this.application?.socialLink ?? {}),
ensure it returns an empty array when no links exist, and keep the existing push
logic for github/linkedin/twitter/peerlist so it never attempts to read
properties off undefined.
- Around line 42-52: The getter aboutYouSections can throw if this.application
is undefined; update it to guard against a missing application by returning a
sensible default (e.g., an array of 'N/A' values) when this.application is falsy
or by using optional chaining on this.application throughout; modify the
aboutYouSections getter to check if (!this.application) return [...default
entries...] or change each access to this.application?.intro?.<field> so it
never dereferences undefined.
In @app/styles/application-detail.module.css:
- Around line 268-276: Both .status-badge--changes and .status-badge--pending
use a hardcoded color (#f59e0b); replace that literal with a CSS variable (e.g.,
var(--color-amber) or var(--color-warning)) in those rules and add the new
variable definition (--color-amber: #f59e0b;) to your global design
tokens/variables file so the badges follow the design system.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
app/components/application/detail-header.hbsapp/components/application/feedback-card.hbsapp/components/application/feedback-card.jsapp/components/application/info-card.hbsapp/components/application/social-link-pill.hbsapp/components/application/social-link-pill.jsapp/components/application/status-badge.hbsapp/components/application/status-badge.jsapp/controllers/applications/detail.jsapp/routes/applications/detail.jsapp/styles/app.cssapp/styles/application-detail.module.cssapp/templates/applications/detail.hbs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build (18.x)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (10)
app/routes/applications/detail.js (1)
44-49: LGTM!The changes correctly fetch and return both
applicationandcurrentUserdata, aligning with the new controller structure. The existing error handling flow is preserved.app/styles/application-detail.module.css (1)
1-5: Well-structured CSS module.The file is well-organized with logical groupings for header, content columns, info cards, social links, feedback, and status badges. Good use of CSS variables for theming and responsive breakpoints for mobile support.
app/styles/app.css (1)
61-61: LGTM!The import follows the existing naming convention and placement pattern.
app/components/application/social-link-pill.hbs (1)
1-9: LGTM!Good security practices with
target="_blank"paired withrel="noopener noreferrer". The template is clean and follows Ember conventions.app/components/application/status-badge.hbs (1)
1-3: LGTM!The template correctly binds to the
statusClassgetter and renders the status text. The implementation is clean and follows Glimmer component patterns.app/components/application/status-badge.js (1)
3-14: LGTM!Good defensive handling with optional chaining on
this.args.status. The status mapping is clear and the fallback to'pending'provides sensible default behavior.app/components/application/feedback-card.hbs (1)
1-12: LGTM!Clean component composition with proper separation of concerns. The template correctly uses
this.formattedDatefrom the backing class and passes@statusto the child component.app/templates/applications/detail.hbs (3)
42-50: LGTM on feedback iteration.The feedback card rendering correctly passes all required arguments. The naming
feedback.feedbackfor@feedbackTextis slightly confusing but follows the data model where the feedback object has afeedbacktext property.
59-65: LGTM on error handling.Clean error state with appropriate messaging for missing applications.
2-8: No action required — controller properly defines theapplicationgetter.The controller correctly defines
get application() { return this.model.application; }(lines 4–6 ofapp/controllers/applications/detail.js), so the template's usage ofthis.applicationon line 4 and throughout is valid and follows proper Ember conventions.
MayankBansal12
force-pushed
the
feat/application-detail
branch
2 times, most recently
from
7554abd to
d608df4
Compare
MayankBansal12
force-pushed
the
feat/application-detail
branch
from
18b9a36 to
6c3f035
Compare
MayankBansal12
commented
Jan 19, 2026
Member
Author
There was a problem hiding this comment.
skipping module here, since the tests PR #1137 covers all test changes
MayankBansal12
commented
Jan 19, 2026
| data-test-home-img | ||
| class="nav__home__img" | ||
| src="assets/images/rds-logo.png" | ||
| src="/assets/images/rds-logo.png" |
Member
Author
There was a problem hiding this comment.
This change is required, since the assets weren't loading correctly for nested routes
MayankBansal12
force-pushed
the
feat/application-detail
branch
from
cb838ff to
6c3f035
Compare
MayankBansal12
force-pushed
the
feat/application-detail
branch
from
85a3d59 to
2aa61dd
Compare
10 tasks
10 tasks
Suvidh-kaushik
previously approved these changes
Jan 19, 2026
MayankBansal12
changed the title
* test: add unit tests for routes and controllers for detail page * test: add integration tests for application components * fix: add more expectation and cleanup tests * refactor: use constants for applications data
iamitprakash
approved these changes
Jan 20, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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: 13-01-2026
Developer Name: @MayankBansal12
Issue Ticket Number:-
Description:
Is Under Feature Flag
Database changes
Breaking changes (If your feature is breaking/missing something please mention pending tickets)
Is Development Tested?
Tested in staging?
Add relevant Screenshot below ( e.g test coverage etc. )
demo
Screencast.from.2026-01-19.04-40-56.mp4
Screencast.from.2026-01-19.04-58-52.mp4