Conversation
- Complete rebrand from SpendLens to FinanceMate - New balance scale app icon (financemate-icon.png) - Dynamic currency system with 60+ currency symbols - Improved UI: solid dropdown backgrounds, better navigation visibility - Enhanced budget progress bars (more red in light mode) - Manual total override in expense form - Fixed calendar icon positioning and form alignment - Updated all branding across components and documentation
Summary of ChangesHello @mou514, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant upgrade to the application's core functionality and user experience. Beyond a complete visual and brand refresh, it introduces a suite of powerful features for personal finance management, including advanced budgeting, custom categorization, recurring expenses, and comprehensive reporting. The backend has been substantially expanded to support these features, alongside robust admin tools, enhanced security measures, and a solid testing foundation. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant redesign and a vast number of new features, including a revamped UI, database migrations for budgets, categories, tags, and notifications, and an extensive admin dashboard. The addition of both E2E and unit tests is a great step towards improving code quality. However, there are a few critical issues that need to be addressed. The new E2E tests are failing due to incorrect assertions and outdated test logic that doesn't match the new application flow. Additionally, there's a critical bug in the database service layer related to updating notifications that will cause runtime errors. I've also included some suggestions to improve code maintainability and fix placeholder URLs.
| test('should allow user to login and see dashboard', async ({ page }) => { | ||
| // Mock login API | ||
| await page.route('**/api/auth/login', async route => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ token: 'fake-jwt-token', user: { id: 1, email: 'test@example.com' } }), | ||
| }); | ||
| }); | ||
|
|
||
| // Mock expenses API (empty list initially) | ||
| await page.route('**/api/expenses', async route => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify([]), | ||
| }); | ||
| }); | ||
|
|
||
| // Mock User API if needed (often checked on load) | ||
| await page.route('**/api/auth/me', async route => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ id: 1, email: 'test@example.com' }), | ||
| }); | ||
| }); | ||
|
|
||
| await page.goto('/login'); | ||
|
|
||
| // Fill login form | ||
| await page.getByLabel('Email').fill('test@example.com'); | ||
| await page.getByLabel('Password').fill('password123'); | ||
| await page.getByRole('button', { name: 'Sign in' }).click(); | ||
|
|
||
| // Verify redirect to dashboard | ||
| await expect(page).toHaveURL('/'); | ||
| await expect(page.getByText('Current Balance')).toBeVisible(); | ||
| }); |
There was a problem hiding this comment.
This test is failing because the assertions don't match the application's behavior after the redesign. The application now redirects to /home after login, not /. Additionally, the expected text 'Current Balance' is no longer present on the new home page. The test needs to be updated to reflect the new UI and routing logic.
For example, the URL assertion should be updated:
// Verify redirect to dashboard
await expect(page).toHaveURL('/home');
await expect(page.getByText('Track Your Spending with Ease')).toBeVisible();| const result = await this.db.prepare( | ||
| 'UPDATE notifications SET is_read = 1 WHERE id = ? AND user_id = ?' | ||
| ).bind(id, userId, userId).run(); |
There was a problem hiding this comment.
There's a mismatch between the SQL query and the bound parameters. The query UPDATE notifications SET is_read = 1 WHERE id = ? AND user_id = ? expects two parameters, but the .bind() call provides three (id, userId, userId). This will cause a runtime error. The extra userId should be removed.
).bind(id, userId).run();| await page.route('**/api/expenses', async route => { | ||
| // Handle both GET (refresh) and POST (save) | ||
| if (route.request().method() === 'GET') { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify([{ id: 101, amount: 5.5, total: 5.5, merchant: 'Coffee Shop', category: 'Food & Drink', date: new Date().toISOString(), currency: 'USD' }]), | ||
| }); | ||
| } else if (route.request().method() === 'POST') { | ||
| await route.fulfill({ | ||
| status: 201, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ success: true, data: { id: 101 } }), | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Re-mocking an API route (**/api/expenses) inside a test after navigation has already occurred is unreliable and can lead to flaky tests. The initial page load might use the first mock, while subsequent interactions use the second, creating race conditions. All necessary API mocks should be defined once at the beginning of the test, before any page interactions occur, to ensure a consistent and predictable test environment.
| # Clone repository | ||
| git clone https://github.com/Creative-Geek/Focal.git | ||
| cd Focal | ||
| git clone https://github.com/yourusername/FinanceMate.git |
There was a problem hiding this comment.
The repository URL contains a placeholder yourusername. Please update this to the correct repository path to ensure the clone command works for users.
| <div align="center"> | ||
|
|
||
| **[Live Demo](https://focal.creative-geek.tech)** • **[Documentation](docs/DEVELOPMENT.md)** • **[Report Bug](https://github.com/Creative-Geek/Focal/issues)** | ||
| **[Documentation](docs/DEVELOPMENT.md)** • **[Report Bug](https://github.com/yourusername/FinanceMate/issues)** |
There was a problem hiding this comment.
The URL for reporting a bug also uses the yourusername placeholder. This should be updated to the correct repository issues URL.
| await page.route('**/api/auth/me', async route => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ id: 1, email: 'test@example.com' }), | ||
| }); | ||
| }); | ||
|
|
||
| await page.route('**/api/expenses', async route => { | ||
| if (route.request().method() === 'GET') { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify([]), | ||
| }); | ||
| } else if (route.request().method() === 'POST') { | ||
| await route.fulfill({ | ||
| status: 201, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ id: 101, amount: 50, description: 'Lunch', date: new Date().toISOString() }), | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| // Bypass login by setting token (if app checks localstorage on load) | ||
| // Or just re-login. Re-login is safer with mocks. | ||
| await page.route('**/api/auth/login', async route => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ token: 'fake-jwt-token', user: { id: 1, email: 'test@example.com' } }), | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The API mocking logic within this test is duplicated from the test above. To improve maintainability and reduce code duplication, this common setup should be extracted into a test.beforeEach block. This will ensure all tests in this suite start with the same consistent mock environment.
| <meta property="og:url" content="https://focal.creative-geek.tech/" /> | ||
| <meta property="og:title" content="FinanceMate: AI Expense Tracker" /> | ||
| <meta property="og:description" | ||
| content="Track your spending with AI-powered receipt scanning. FinanceMate makes expense management effortless." /> | ||
| <meta property="og:image" content="https://focal.creative-geek.tech/images/dashboard.png" /> | ||
| <meta property="og:image:alt" content="Focal Dashboard - Expense tracking with AI receipt scanning" /> | ||
| <meta property="og:site_name" content="FinanceMate" /> | ||
|
|
||
| <!-- PWA --> | ||
| <link rel="manifest" href="/manifest.webmanifest" /> | ||
| <meta name="mobile-web-app-capable" content="yes" /> | ||
| <meta name="apple-mobile-web-app-capable" content="yes" /> | ||
| <meta | ||
| name="apple-mobile-web-app-status-bar-style" | ||
| content="black-translucent" | ||
| /> | ||
| <meta name="apple-mobile-web-app-title" content="Focal" /> | ||
| <!-- Twitter --> | ||
| <meta property="twitter:card" content="summary_large_image" /> | ||
| <meta property="twitter:url" content="https://focal.creative-geek.tech/" /> | ||
| <meta property="twitter:title" content="FinanceMate: AI Expense Tracker" /> | ||
| <meta property="twitter:description" | ||
| content="Track your spending with AI-powered receipt scanning. FinanceMate makes expense management effortless." /> | ||
| <meta property="twitter:image" content="https://focal.creative-geek.tech/images/dashboard.png" /> | ||
| <meta property="twitter:image:alt" content="Focal Dashboard - Expense tracking with AI receipt scanning" /> |
There was a problem hiding this comment.
The og:url, twitter:url, og:image, and alt text meta tags contain hardcoded URLs and text from the old 'Focal' project. These should be updated to reflect the new 'FinanceMate' branding and the correct production URL, or removed if a live demo is not available.
Update name in Wrangler configuration file to match deployed Worker
1 participant
No description provided.