fix: resolve three script.js defects breaking the recommendation flow#437
Open
anshul23102 wants to merge 1 commit into
Open
fix: resolve three script.js defects breaking the recommendation flow#437anshul23102 wants to merge 1 commit into
anshul23102 wants to merge 1 commit into
Conversation
…malharshita#338) Bug 1 (renderResults - cards never render): The outer if(!projects) block was never closed before the success-path code that hides the empty state, shows the grid, and runs forEach. A nested duplicate if with an early return made those lines unreachable on every code path. Removed the duplicate inner if and all repeated display assignments, added a single clean early return, and moved the success path to execute after the guard. Bug 2 (duplicate fetch on error): The .catch() handler contained a full second fetch("/api/recommend", ...) call with a re-declared payload. On a network error the second request fired and unconditionally showed "Something went wrong" regardless of its own response. Lines after the catch closure referenced data which was out of scope - dead code that never ran. Removed the entire duplicate fetch block and the dead code; replaced them with a lean .catch that only calls setLoadingState(false), displays the error message, and logs the error. Bug 3 (spinner display overwritten): btnLoading.style.display was assigned twice in immediate succession. The correct "inline-flex" value on the first line was immediately overwritten with "inline", breaking the spinner's flex alignment. Same double-assignment applied to btnLabel. Removed both duplicate lines, keeping only the first correct assignment for each element.
|
@anshul23102 is attempting to deploy a commit to the komalsony234-1530's projects Team on Vercel. A member of the Team first needs to authorize it. |
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
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.
Summary [required]
Three independent defects in
static/script.jscombined to make the core recommendation flow completely broken. Project cards never appeared, a network failure would silently retry the API, and the loading spinner rendered incorrectly. All three issues are isolated tostatic/script.jswith no changes to any other file.Bug 1 (renderResults - cards never render, CRITICAL)
The outer
if (!projects || projects.length === 0)block was never closed before the code that hides the empty state, shows the results grid, and runsprojects.forEach. A nested duplicateifwith an earlyreturnmade those lines unreachable on every code path: when projects exist the outer condition is false so the whole block is skipped; when there are no projects the innerreturnfires first. The success path never executed. The fix collapses the duplicated conditions into a single guard with an earlyreturnfor the empty case, leaving the success path unconditionally reachable when projects are present.Bug 2 (duplicate fetch inside
.catch(), CRITICAL)The
.catch()of the primaryfetch("/api/recommend", ...)contained a full secondfetchcall with a re-declaredpayload. On any network error the second request fired and then unconditionally displayed "Something went wrong" regardless of its own response. The lines after the catch closure referenceddatawhich was out of scope - unreachable dead code. The fix removes the entire duplicate fetch block and the dead code, replacing them with a lean.catchthat callssetLoadingState(false), shows the error message, and logs the error.Bug 3 (spinner display value overwritten, MEDIUM)
btnLoading.style.displayandbtnLabel.style.displaywere each assigned twice in immediate succession insidesetLoadingState. The correct"inline-flex"value for the spinner was immediately overwritten with"inline", breaking its flex-based alignment. The fix removes both duplicate assignments, keeping only the first correct line for each element.Related Issue [required]
Closes #338
Type of Change [required]
What Was Changed [required]
static/script.jsrenderResultsguard logic; removed duplicate fetch from.catch(); removed duplicate display assignments fromsetLoadingStateHow to Test This PR [required]
git checkout fix/script-three-defects-338pip install -r requirements.txtpython app.pypython tests/test_basic.pyExpected test output:
Test Results [required]
Self-Review Checklist [required]
feat/,fix/,docs/,data/,style/,test/python tests/test_basic.pyand all tests passprint()orconsole.log()debug statementsvardeclarations and ES5-compatible patterns per the project JS style guideNotes for Reviewer
This PR is a net deletion of 45 lines. No new logic was introduced; the fix restores the code to match what the surrounding comments and function docstrings describe as intended behaviour. Each of the three bugs is independently reproducible using the steps in the issue.