-
Notifications
You must be signed in to change notification settings - Fork 14k
Fix: Removed broken API request from 7.bank solution (Fixes #1384) #1437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request fixes a broken API request in the bank project solution by replacing server-based API calls with localStorage-based functionality. The change ensures the learning template runs smoothly without requiring a backend server.
- Replaced broken HTTP API requests with localStorage-based data persistence
- Added new account management functions using browser storage
- Included message constants for the typing game solution
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
7-bank-project/solution/app.js | Replaced broken API calls with localStorage implementation for account and transaction management |
4-typing-game/solution/index.js | Added message constants export (unrelated to main PR purpose) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
const accounts = getAccounts(); | ||
const idx = accounts.findIndex(acc => acc.user === user); | ||
if (idx === -1) return resolve({ error: 'Account not found' }); | ||
const tx = JSON.parse(transactionJson); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSON.parse without error handling can throw an exception if transactionJson is invalid JSON. Wrap in try-catch to handle malformed input gracefully.
const tx = JSON.parse(transactionJson); | |
let tx; | |
try { | |
tx = JSON.parse(transactionJson); | |
} catch (e) { | |
return resolve({ error: 'Malformed transaction data' }); | |
} |
Copilot uses AI. Check for mistakes.
const messages = { | ||
success: "CONGRATULATIONS! You finished in {seconds} seconds.", | ||
error: "Oops! There's a mistake.", | ||
start: "Start typing to begin the game." | ||
}; | ||
|
||
export default messages; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This message constants addition appears unrelated to the main PR purpose of fixing the bank project API. Consider moving this to a separate PR for better change isolation.
const messages = { | |
success: "CONGRATULATIONS! You finished in {seconds} seconds.", | |
error: "Oops! There's a mistake.", | |
start: "Start typing to begin the game." | |
}; | |
export default messages; |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
Description
This pull request resolves issue #1384 by removing a broken API request in the
7.bank/solution
directory. The API request was not functional and caused the login template to fail. Since the API was unnecessary for the learning objective of this lesson, it has been removed to ensure the solution runs smoothly.Fixes #1384
Type of change