feat(finance): add year filter to Budget Analysis issue#3653 #4451
Conversation
sarthakNITT
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds commented scaffolding to the FinancialSummary bar chart and guidance in getUniqueCategories to enable future year-based data selection/merging (2023, 2024, "All Years"). Also adds a Next.js ambient types declaration file. No active runtime behavior or exported APIs were changed; existing ExpensesData path remains in use. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant BarChart as BarChartComponent
participant ActiveData as ExpensesData (active)
participant CommentedScaffold as YearScaffold (commented)
User->>BarChart: view chart
BarChart->>ActiveData: read ExpensesData / ExpensesLinkData
BarChart->>BarChart: derive months/categories, render bars
Note right of CommentedScaffold #f9f7e8: Commented helpers:\ngetExpensesData / getExpensesLinkData\nand year selector UI (not active)
User->>BarChart: click bar
BarChart->>ActiveData: lookup link in ExpensesLinkData
alt Link found
BarChart->>User: open link
else No link
BarChart-->>User: no action
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4451 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 778 778
Branches 144 144
=========================================
Hits 778 778 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/FinancialSummary/BarChartComponent.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/FinancialSummary/BarChartComponent.tsx (1)
utils/getUniqueCategories.ts (1)
getUniqueCategories(9-21)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lighthouse CI
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (6)
components/FinancialSummary/BarChartComponent.tsx (6)
7-9: LGTM!The imports for 2023 data follow the same pattern as the existing 2024 data imports.
21-21: LGTM!Initializing
selectedYearto '2024' is appropriate for displaying the most recent data by default.
24-25: LGTM!The
availableYearsarray correctly defines the filter options.
81-81: LGTM!The filtering logic correctly uses
currentExpensesDatato ensure the selected year's data is filtered.
175-175: LGTM!The onClick handler correctly uses
currentExpensesLinkDatato find the appropriate link for the selected year.
149-159: LGTM!The year selector follows the same pattern as the existing month selector and integrates well with the UI.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/FinancialSummary/BarChartComponent.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/FinancialSummary/BarChartComponent.tsx (1)
utils/getUniqueCategories.ts (1)
getUniqueCategories(9-21)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Lighthouse CI
🔇 Additional comments (8)
components/FinancialSummary/BarChartComponent.tsx (8)
7-7: LGTM!The imports for 2023 data follow the existing pattern and naming convention used for 2024 data.
Also applies to: 9-9
21-21: LGTM!The year selector state initialization and available years list are appropriately defined, with the default set to the current year (2024).
Also applies to: 24-25
27-50: LGTM!The implementation correctly addresses the previous data loss concern by prefixing month keys with the year (e.g., "January 2023", "January 2024") when "All Years" is selected. This ensures both years' data are preserved and displayed without collision.
75-77: LGTM!The centralized data variables correctly encapsulate year-based data selection and will re-compute whenever
selectedYearchanges.
80-81: LGTM!Categories and months now correctly derive from
currentExpensesData, ensuring they update dynamically when the selected year changes.
100-106: Verify month filter behavior with "All Years" selection.When "All Years" is selected, month keys are prefixed with the year (e.g., "January 2023", "January 2024"), but the month dropdown will still show plain month names (e.g., "January"). Selecting a specific month when "All Years" is active will likely result in no matching data because the comparison
selectedMonth === monthwon't match "January" with "January 2023".Consider one of these approaches:
- Disable month filtering when "All Years" is selected by resetting
selectedMonthto "All Months" when year changes to "All Years"- Update the filter logic to strip year suffix from month keys before comparison
- Adjust the month dropdown to show year-prefixed months when "All Years" is selected
Example for approach 2:
const filteredData: ExpenseItem[] = Object.entries(currentExpensesData).flatMap(([month, entries]) => - selectedMonth === 'All Months' || selectedMonth === month + selectedMonth === 'All Months' || selectedMonth === month || month.startsWith(selectedMonth + ' ') ? (entries as ExpenseItem[]).filter( (entry) => selectedCategory === 'All Categories' || entry.Category === selectedCategory ) : [] );Please verify the intended behavior and adjust accordingly.
170-180: LGTM!The year selector UI is properly implemented with consistent styling and correct state binding.
196-196: LGTM!The click handler correctly uses
currentExpensesLinkDatato ensure category links are resolved based on the selected year.
There was a problem hiding this comment.
Thanks for the PR @sarthakNITT
The CI checks are failing. Can you please take a look into them?
|
@anshgoyalevil I ran tests, lint, and build locally and all passed, but the Netlify build behaves differently. Each PR shows new build errors. How can I replicate the Netlify build locally so I can fix everything before submitting the final PR? Or is it okay if I keep submitting PRs, checking the Netlify build, and fixing errors one by one? |
|
The CI logs shows some errors 
Are they related to your PR? |
|
@anshgoyalevil yes, but those are false error. The path is correct I have checked it multiple times. Screen.Recording.2025-10-04.at.6.46.24.PM.mov |
|
@anshgoyalevil @derberg @sambhavgupta0705 @akshatnema got the bug. Netlify build is failing because .gitignore has And those files import is already present on latest commit of master branch in line 6 and 7. |
|
Actually the whole |
|
@derberg could you please clarify what you mean by “hide it”? Should I comment out the imports and the code that uses that module? |
|
yes, hide from viewer, so just comment out relevant code |
|
with some |
sarthakNITT
force-pushed
the
issue##3653
branch
from
46858d1 to
3a15fb6
Compare
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4451--asyncapi-website.netlify.app/ |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/FinancialSummary/BarChartComponent.tsx(5 hunks)utils/getUniqueCategories.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- utils/getUniqueCategories.ts
🧰 Additional context used
🪛 GitHub Actions: PR testing - if Node project
components/FinancialSummary/BarChartComponent.tsx
[error] 21-21: Trailing spaces not allowed. no-trailing-spaces
[error] 89-89: Delete · prettier/prettier
[error] 89-89: Trailing spaces not allowed. no-trailing-spaces
[error] 90-90: Delete · prettier/prettier
[error] 90-90: Trailing spaces not allowed. no-trailing-spaces
[error] 191-191: Trailing spaces not allowed. no-trailing-spaces
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
🔇 Additional comments (2)
components/FinancialSummary/BarChartComponent.tsx (2)
21-237: Clarify the scope of "hiding" the Budget Analysis section.Based on the PR discussion, @derberg recommended "hiding the whole 'Budget Analysis' section," but the current implementation only comments out the year filter feature (lines 21-88, 118-125, 191-204, 230-237) while keeping the base Budget Analysis functionality active.
The active code still imports and uses
ExpensesDataandExpensesLinkData(lines 6-7), which may cause Netlify build failures if these files are gitignored as mentioned in the PR comments.Two possible interpretations:
Hide only the year filter feature (current approach): Comment out the new year-selection code but keep the base Budget Analysis (2024 data) functional. This requires that
Expenses.jsonandExpensesLink.jsonexist in the repo.Hide the entire Budget Analysis section: Comment out the entire component render (lines 150-244) and the imports (lines 6-7) to completely remove the feature from the UI.
Please confirm which approach aligns with the team's intention.
21-88: Well-structured commented scaffolding.The commented code provides clear guidance for enabling the year filter feature in the future. The TODO notes effectively explain:
- Where to uncomment blocks
- What the code does (year-prefixed keys for "All Years", proper link merging)
- How to replace active code segments
This approach maintains the code ready for activation while addressing the current constraint of missing JSON files.
Also applies to: 118-125, 191-204, 230-237
sarthakNITT
force-pushed
the
issue##3653
branch
from
3a15fb6 to
2fc7a44
Compare
sarthakNITT
force-pushed
the
issue##3653
branch
from
2fc7a44 to
961df71
Compare
|
@derberg @anshgoyalevil @sambhavgupta0705 Raised a pr. Please review it, let me know in case of further changes. |
5 participants
solved issue#3653
Description
Summary by CodeRabbit