Conversation
- Updated `handleDateRangeUpdate` in `SpendingAnalysisView.vue` to safely handle both `Date` objects (from manual picker) and `YYYY-MM-DD` strings (from preset selections). - Reverted redundant call to `formatDateForInput` in `fetchData` as inputs are guaranteed strings. - Added a new unit test suite `frontend/tests/views/SpendingAnalysisView.spec.js` to verify both selection paths. - Verified that all frontend tests pass, including `DateRangePicker.spec.js` and the new test. This fixes the runtime error 'date.toISOString is not a function' when clicking presets like 'Last 30 Days' or 'This Year' on the Spending Analysis page. Co-authored-by: niyazmft <9331133+niyazmft@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello, 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 addresses a critical bug in the Spending Analysis page where selecting preset date ranges led to a runtime crash. The fix enhances the date range handling mechanism to gracefully accept various input formats (both strings and Date objects), ensuring the feature functions reliably. Comprehensive tests have been added to validate the solution and maintain stability. 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. 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 effectively addresses a crash on the Spending Analysis page by correctly handling different date input types. The addition of a comprehensive test suite is a valuable contribution that improves the stability of this component. I have one suggestion to simplify the implementation in SpendingAnalysisView.vue by removing redundant logic, which will enhance maintainability.
| const startStr = typeof start === 'string' ? start : utils.formatDateForInput(start); | ||
| const endStr = typeof end === 'string' ? end : utils.formatDateForInput(end); | ||
| fetchData(startStr, endStr); |
There was a problem hiding this comment.
While this change correctly fixes the issue, the explicit typeof check here is redundant. The utils.formatDateForInput function already contains logic to safely handle both string and Date inputs, so this implementation introduces unnecessary code duplication.
To improve maintainability and centralize the type-handling logic, it's best to rely on the utility function's existing capabilities. I recommend simplifying this back to a direct call to the utility function.
fetchData(utils.formatDateForInput(start), utils.formatDateForInput(end));
There was a problem hiding this comment.
I think the main fix should be in the shared DateRangePicker, not handled separately in each page. Since multiple views consume the same component, fixing only SpendingAnalysisView.vue is too narrow for the updated scope of issue #8. This PR helps that page, but I wouldn’t treat it as the complete fix unless the shared picker contract is normalized.
- Updated `applyRange` in `DateRangePicker.vue` to always emit `Date` objects for both manual and preset selections, providing a consistent contract for all consumers. - Reverted manual type checking in `SpendingAnalysisView.vue` as the picker now guarantees `Date` objects. - Updated `DateRangePicker.spec.js` to verify `Date` object emission and fixed existing assertions. - Added a new unit test suite `frontend/tests/views/SpendingAnalysisView.spec.js` using timezone-agnostic `Date.UTC` to verify the view correctly triggers data refreshes. - Verified that all 13 frontend tests pass correctly. This fixes the runtime error 'date.toISOString is not a function' on the Spending Analysis page when selecting presets like 'Last 30 Days' or 'This Year'. Co-authored-by: niyazmft <9331133+niyazmft@users.noreply.github.com>
niyazmft
deleted the
fix-spending-analysis-preset-dates-v20260320-1415-16626626647949589162
branch
1 participant
Fixed a bug where using preset date ranges (e.g., 'Last 30 Days', 'This Year') on the Spending Analysis page caused a runtime crash ('date.toISOString is not a function'). The crash occurred because the shared DateRangePicker emits YYYY-MM-DD strings for presets, while the page logic expected Date objects.
The fix involved updating
SpendingAnalysisView.vueto detect and normalize both string and Date object inputs before passing them to the API fetch logic. I also added a comprehensive test suite to ensure this scenario is covered and remains stable. All frontend tests pass.Fixes #8
PR created automatically by Jules for task 16626626647949589162 started by @niyazmft