LaunchDesk Class - Debugging Exercise#5
LaunchDesk Class - Debugging Exercise#5Kavishka Weerasinghe (KavishkaWeerasinghe2001) wants to merge 12 commits into
Conversation
📝 WalkthroughWalkthroughThis pull request fixes twelve interconnected bugs in the launch-checks application and updates instructor documentation. Key fixes include unifying localStorage keys, correcting form validation and submit handler spelling, expanding search filtering across multiple fields, fixing status filter logic, adjusting metrics calculations, aligning row action dataset keys, persisting status changes, and correcting demo fetch and CSV export paths. ChangesLaunch-Checks Application Fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 2
🧹 Nitpick comments (2)
js/app.js (1)
1-2: ⚡ Quick winRemove or clarify the outdated comment.
The comment on Line 2 states "this key should match STORAGE_KEY," but there is only one
STORAGE_KEYconstant defined. This comment appears to be a remnant from when there were two separate keys (for saving and loading). Since the bug is now fixed, the comment is misleading.🧹 Proposed fix
const STORAGE_KEY = "launchdesk-v1-items"; -// Intentional bug: this key should match STORAGE_KEY.Or update to clarify the teaching intent:
const STORAGE_KEY = "launchdesk-v1-items"; -// Intentional bug: this key should match STORAGE_KEY. +// Fixed: unified storage key (previously had separate save/load keys).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/app.js` around lines 1 - 2, The trailing comment next to the STORAGE_KEY constant is outdated and misleading; remove the comment or replace it with a short, accurate note (e.g., "key used for local storage of items" or a brief teaching note) so it no longer implies a second key exists; locate the const STORAGE_KEY declaration and update or delete the adjacent comment accordingly.docs/instructor-solution-guide.md (1)
240-241: ⚡ Quick winClarify the broken code example for localStorage keys.
Lines 240-241 both declare
const STORAGE_KEY =with different values, which would be a syntax error. The documentation should clarify that these represent two different constants that were originally in the code (e.g.,STORAGE_SAVE_KEYandSTORAGE_LOAD_KEY), or show this as a before/after comparison more clearly.📝 Suggested clarification
-- Broken constants: +- Broken code (two different keys were defined): ```js -const STORAGE_KEY = "launchdesk-v1-items"; -const STORAGE_KEY = "launchdesk-items-v1"; +const STORAGE_SAVE_KEY = "launchdesk-v1-items"; +const STORAGE_LOAD_KEY = "launchdesk-items-v1";</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/instructor-solution-guide.mdaround lines 240 - 241, The docs show two
conflicting const declarations for STORAGE_KEY which is a syntax error; update
the example to clarify intent by renaming them to distinct identifiers (e.g.,
STORAGE_SAVE_KEY and STORAGE_LOAD_KEY) or present them explicitly as a
before/after diff so readers see the change; locate the duplicated STORAGE_KEY
lines and replace the second with a clear alternative name (or wrap the pair in
a before/after block) to remove the duplicate declaration and make the example
unambiguous.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@js/app.js:
- Line 242: The dueSoon calculation currently counts overdue items because it
only checks daysUntil(check.dueDate) <= 7; update the filter in the dueSoon
computation (the checks.filter callback where daysUntil is called) to ensure the
returned days are non-negative and at most 7 (e.g., compute days =
daysUntil(check.dueDate) and require days >= 0 && days <= 7) so only items due
within the next 7 days are counted and overdue items are excluded.In
@package.json:
- Around line 7-8: The npm scripts are inconsistent: "dev" uses python while
"start" uses python3; update the package.json scripts so both "dev" and "start"
use the same Python executable (prefer python3 for modern environments) to avoid
Python2 fallback; edit the "dev" entry to match "start" (or vice versa) so both
scripts run "python3 -m http.server 5173" and save the file.
Nitpick comments:
In@docs/instructor-solution-guide.md:
- Around line 240-241: The docs show two conflicting const declarations for
STORAGE_KEY which is a syntax error; update the example to clarify intent by
renaming them to distinct identifiers (e.g., STORAGE_SAVE_KEY and
STORAGE_LOAD_KEY) or present them explicitly as a before/after diff so readers
see the change; locate the duplicated STORAGE_KEY lines and replace the second
with a clear alternative name (or wrap the pair in a before/after block) to
remove the duplicate declaration and make the example unambiguous.In
@js/app.js:
- Around line 1-2: The trailing comment next to the STORAGE_KEY constant is
outdated and misleading; remove the comment or replace it with a short, accurate
note (e.g., "key used for local storage of items" or a brief teaching note) so
it no longer implies a second key exists; locate the const STORAGE_KEY
declaration and update or delete the adjacent comment accordingly.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `b7d3b7da-e8ad-4feb-a960-b2205a3ed381` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between ccb0d19576026476f7d69e2381775937746f70e2 and 95bf7b7911329865a4d83e8049c9c5c5e9a076bb. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `docs/instructor-solution-guide.md` * `js/app.js` * `package.json` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| (check) => check.priority === "Critical" && check.status !== "Fixed", | ||
| ).length; | ||
| const dueSoon = checks.filter((check) => daysUntil(check.dueDate) > 7).length; // Intentional bug: this should count items due within 7 days. | ||
| const dueSoon = checks.filter((check) => daysUntil(check.dueDate) <= 7).length; // Intentional bug: this should count items due within 7 days. |
There was a problem hiding this comment.
Fix incomplete: "Due soon" metric includes overdue items.
The current logic daysUntil(check.dueDate) <= 7 will incorrectly count items with negative days (overdue items). According to the instructor guide (lines 181-184), the fix should also verify that days are non-negative.
🐛 Complete the fix
- const dueSoon = checks.filter((check) => daysUntil(check.dueDate) <= 7).length; // Intentional bug: this should count items due within 7 days.
+ const dueSoon = checks.filter((check) => {
+ const days = daysUntil(check.dueDate);
+ return days >= 0 && days <= 7;
+ }).length;This ensures only items due in the future within 7 days are counted, excluding overdue items.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const dueSoon = checks.filter((check) => daysUntil(check.dueDate) <= 7).length; // Intentional bug: this should count items due within 7 days. | |
| const dueSoon = checks.filter((check) => { | |
| const days = daysUntil(check.dueDate); | |
| return days >= 0 && days <= 7; | |
| }).length; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/app.js` at line 242, The dueSoon calculation currently counts overdue
items because it only checks daysUntil(check.dueDate) <= 7; update the filter in
the dueSoon computation (the checks.filter callback where daysUntil is called)
to ensure the returned days are non-negative and at most 7 (e.g., compute days =
daysUntil(check.dueDate) and require days >= 0 && days <= 7) so only items due
within the next 7 days are counted and overdue items are excluded.
| "dev": "python -m http.server 5173", | ||
| "start": "python3 -m http.server 5173" |
There was a problem hiding this comment.
Inconsistent Python command between dev and start scripts.
The dev script uses python while start uses python3. On systems where python points to Python 2.x, the dev script will fail. For consistency and compatibility, both should use the same command.
🔧 Proposed fix
- "dev": "python -m http.server 5173",
+ "dev": "python3 -m http.server 5173",
"start": "python3 -m http.server 5173"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dev": "python -m http.server 5173", | |
| "start": "python3 -m http.server 5173" | |
| "dev": "python3 -m http.server 5173", | |
| "start": "python3 -m http.server 5173" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 7 - 8, The npm scripts are inconsistent: "dev"
uses python while "start" uses python3; update the package.json scripts so both
"dev" and "start" use the same Python executable (prefer python3 for modern
environments) to avoid Python2 fallback; edit the "dev" entry to match "start"
(or vice versa) so both scripts run "python3 -m http.server 5173" and save the
file.
1 participant
Summary by CodeRabbit
Release Notes
Documentation
Bug Fixes
Chores