feat: integrate patch API to allow edit application#1147
Merged
iamitprakash merged 7 commits intodevelopfrom Feb 19, 2026
Merged
feat: integrate patch API to allow edit application#1147iamitprakash merged 7 commits intodevelopfrom
iamitprakash merged 7 commits intodevelopfrom
Conversation
Deploying www-rds with
|
| Latest commit: |
7e1e72e
|
| 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 Use the checkbox below for a quick retry:
WalkthroughThis PR adds edit mode functionality to the application form, allowing users to modify existing pending applications. It renames the phone number field for consistency, converts static storage key properties to getters across form steps, and integrates API-driven data loading and submission for the edit workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DetailHeader as Detail Header<br/>(Component)
participant JoinController as Join<br/>(Controller)
participant NewStepper as New Stepper<br/>(Component)
participant OnboardingService as Onboarding<br/>(Service)
participant API as Backend API
User->>DetailHeader: Click Edit Button
alt isEditAllowed == false
DetailHeader->>User: Show error toast
else isEditAllowed == true
DetailHeader->>JoinController: Navigate to join route<br/>with edit=true, dev=true
JoinController->>JoinController: Set edit flag<br/>isEditMode = true
JoinController->>NewStepper: Pass `@edit`=true
NewStepper->>NewStepper: Detect editMode
NewStepper->>OnboardingService: Call loadExistingData()
OnboardingService->>API: Fetch application details
API-->>OnboardingService: Return application data
OnboardingService->>OnboardingService: Map data to step storage<br/>populateLocalStorageFromApplication()
OnboardingService->>OnboardingService: Set editDataLoaded=true
User->>NewStepper: Edit form fields & Submit
NewStepper->>OnboardingService: Collect application data<br/>filtered by EDIT_MODE_ALLOWED_FIELDS
NewStepper->>API: Send PATCH request<br/>UPDATE_APPLICATION_URL
API-->>NewStepper: Return 200 success
NewStepper->>JoinController: Navigate to<br/>applications.detail
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ 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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/components/new-join-steps/new-step-four.js (1)
22-24: 🧹 Nitpick | 🔵 TrivialPre-existing: hardcoded
'newStepOneData'should useSTEP_DATA_STORAGE_KEY.stepOne.Since this file already imports
STEP_DATA_STORAGE_KEY, the hardcoded string on Line 23 should be replaced for consistency — especially given the PR-wide effort to standardize storage key access.♻️ Proposed fix
get userRole() { const stepOneData = JSON.parse( - localStorage.getItem('newStepOneData') || '{}', + localStorage.getItem(STEP_DATA_STORAGE_KEY.stepOne) || '{}', ); return stepOneData.role || ''; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/new-join-steps/new-step-four.js` around lines 22 - 24, Replace the hardcoded storage key string used when reading step one data with the standardized constant: change the localStorage.getItem call that populates stepOneData to use STEP_DATA_STORAGE_KEY.stepOne instead of 'newStepOneData' (locate usage in new-step-four.js where stepOneData is defined and the call to localStorage.getItem occurs).tests/integration/components/application/detail-header-test.js (1)
67-80: 🧹 Nitpick | 🔵 TrivialConsider asserting that the edit button is hidden when status is
'accepted'.This test sets
status: 'accepted'and verifies the nudge button is disabled, but doesn't assert that the edit button is absent. SinceisEditAllowedreturnsfalsefor'accepted', addingassert.dom('[data-test-button="edit-button"]').doesNotExist()would strengthen coverage for the new conditional rendering.💚 Proposed addition
assert.dom('[data-test-button="nudge-button"]').hasAttribute('disabled'); + assert.dom('[data-test-button="edit-button"]').doesNotExist(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/components/application/detail-header-test.js` around lines 67 - 80, The test "it disables nudge button when status is not pending" should also assert that the edit button is hidden for non-editable statuses; update the test in tests/integration/components/application/detail-header-test.js (the test block with the application having status: 'accepted') to add an assertion that the edit button does not exist (e.g., assert.dom('[data-test-button="edit-button"]').doesNotExist()), referencing the Application::DetailHeader component rendering and the fact that isEditAllowed should be false for 'accepted' status.
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/new-join-steps/new-step-four.js`:
- Around line 22-24: Replace the hardcoded storage key string used when reading
step one data with the standardized constant: change the localStorage.getItem
call that populates stepOneData to use STEP_DATA_STORAGE_KEY.stepOne instead of
'newStepOneData' (locate usage in new-step-four.js where stepOneData is defined
and the call to localStorage.getItem occurs).
In `@app/components/new-stepper.js`:
- Around line 74-200: The loadExistingData method in NewStepper must mark
onboarding.editDataLoaded=true to avoid the controller's loading getter (in
app/controllers/join.js) staying true and blocking rendering; update the end of
the try block in loadExistingData (after setting localStorage and
this.isValid/preValid) to set this.onboarding.editDataLoaded = true (or call a
small helper e.g., this.markEditDataLoaded()) so the controller's loading check
(isEditMode && !this.onboarding.editDataLoaded) can become false and the stepper
can mount.
- Around line 57-65: The constructor uses a truthy check (if (this.args.edit))
while the editMode getter uses strict equality (this.args.edit === true), which
can lead to inconsistent behavior if a non-boolean value like "true" is passed;
update the constructor to use the same strict boolean check (or call the
editMode getter) so both checks align—e.g., replace the truthy if in the
constructor with if (this.args.edit === true) or if (this.editMode) and keep the
editMode getter as the single source of truth; ensure the branch that calls
loadExistingData() references those symbols (this.args.edit / editMode /
loadExistingData) consistently.
- Around line 155-157: The forEach callback currently uses an expression-bodied
arrow that returns a value due to "!value && delete stepFourData[key]" which
static analysis warns about; change the callback in
Object.entries(stepFourData).forEach(...) to a block-bodied function and perform
the delete imperatively (e.g., if (!value) delete stepFourData[key]) so the
callback does not return a value; reference the existing call to
Object.entries(stepFourData).forEach and the stepFourData variable when making
the edit.
- Around line 19-29: The allow-list EDIT_MODE_ALLOWED_FIELDS currently contains
'professional' which never exists in the flattened object produced by
collectApplicationData (it spreads stepTwoData as top-level keys like skills and
college), so professional fields are excluded from edit payloads; fix this by
either adding 'skills' and 'college' to EDIT_MODE_ALLOWED_FIELDS or by modifying
collectApplicationData to assemble a professional object from stepTwoData (e.g.,
{ professional: { skills, college, ... } }) before the filtering step so the
existing 'professional' entry matches; update the code paths that rely on
EDIT_MODE_ALLOWED_FIELDS and collectApplicationData to remain consistent (refer
to EDIT_MODE_ALLOWED_FIELDS, collectApplicationData, and stepTwoData).
- Around line 305-318: The PATCH handler may receive a response without
application.id, causing newApplicationId to be undefined and breaking
router.replaceWith; update the logic around newApplicationId (the const
newApplicationId = data.application?.id) to fall back to
this.onboarding.applicationData?.id when isEditMode is true or when
data.application?.id is missing, then call
this.router.replaceWith('applications.detail', <the resolved id>); ensure you
still clearAllStepData() and set this.isSubmitting = false as before.
In `@app/services/onboarding.js`:
- Line 232: The hardcoded storage key 'isValid' in the call to
setLocalStorageItem should be replaced with a named constant to match the
existing pattern (like STEP_DATA_STORAGE_KEY); declare a descriptive constant
(e.g., IS_VALID_STORAGE_KEY) near the other storage keys and use that constant
in the setLocalStorageItem call inside the function where setLocalStorageItem is
used to persist step state so future refactors won't miss the key.
- Around line 13-17: Remove the duplicated APPLICATION_STATUS_TYPES object from
app/services/onboarding.js and import the shared constant from
app/constants/join.js instead; update the top of onboarding.js to add an import
for APPLICATION_STATUS_TYPES and replace any local references to the literal
object with the imported APPLICATION_STATUS_TYPES to ensure a single source of
truth.
- Around line 144-147: getApplicationDetails() currently calls
populateLocalStorageFromApplication() unconditionally for pending applications,
which overwrites user-entered form data; change the logic in the
constructor/getApplicationDetails() so populateLocalStorageFromApplication()
runs only when the service is in edit mode (detect ?edit=true or an explicit
this.editMode flag) or when localStorage has no existing application data (check
the specific keys your app uses before overwriting). Also ensure editDataLoaded
is set appropriately (set it before or only after a deliberate edit-mode
population) so it can guard against accidental population; references to update:
getApplicationDetails(), populateLocalStorageFromApplication(), editDataLoaded,
this.applicationData, and APPLICATION_STATUS_TYPES.pending.
In `@tests/integration/components/application/detail-header-test.js`:
- Around line 67-80: The test "it disables nudge button when status is not
pending" should also assert that the edit button is hidden for non-editable
statuses; update the test in
tests/integration/components/application/detail-header-test.js (the test block
with the application having status: 'accepted') to add an assertion that the
edit button does not exist (e.g.,
assert.dom('[data-test-button="edit-button"]').doesNotExist()), referencing the
Application::DetailHeader component rendering and the fact that isEditAllowed
should be false for 'accepted' status.
In `@tests/integration/components/new-stepper-test.js`:
- Around line 324-334: The test assumes this.fetchStub.lastCall is the
submission fetch but loadExistingData (called when NewStepper is rendered with
`@edit`={{true}}) also issues a fetch, so make the assertion explicit: verify
this.fetchStub.callCount (or use this.fetchStub.getCall(index)) to ensure you
inspect the fetch made by the submit action. Update the test around the submit
block to assert the stub call count after
click('[data-test-button="submit-review"]') and replace this.fetchStub.lastCall
with this.fetchStub.getCall(<submissionIndex>) (or assert callCount === expected
and then use lastCall) to target the PATCH request to /applications/app-123 from
the submit flow.
- Around line 312-322: Replace the fragile setTimeout-based wait in the test for
the NewStepper component with a deterministic wait using settled() from
`@ember/test-helpers`: remove the await new Promise(resolve => setTimeout(resolve,
100)) call and replace it with await settled() so the test waits for all async
behavior (including NewStepper's loadExistingData) to finish before reading
localStorage (STEP_DATA_STORAGE_KEY.stepOne) and asserting city/state/role;
settled is already imported at the top of the file.
MayankBansal12
force-pushed
the
feat/edit-application
branch
from
eafbd9b to
9be81e0
Compare
MayankBansal12
force-pushed
the
feat/edit-application
branch
from
46c3642 to
71d0000
Compare
MayankBansal12
commented
Feb 19, 2026
Member
Author
There was a problem hiding this comment.
updated this expectation since a new query param is added for edit
AnujChhikara
previously approved these changes
Feb 19, 2026
* test: add tests for base step * test: add test for new stepper for edit mode * fix: replace college with institution
iamitprakash
approved these changes
Feb 19, 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: 17-02-25
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
demo.mp4