Skip to content
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

Custom Reports show activity fix #2785

Merged
merged 5 commits into from
Jun 3, 2024

Conversation

carkom
Copy link
Contributor

@carkom carkom commented May 20, 2024

There was a regression created with #2643 which broke the "showActivity" elements of custom reports since they were using negative numbers for the filters. Easy fix, just switched to "inflow/outflow"

Also had an issue with weekly show activity clicks not filtering dates correctly. To duplicate on edge Open new custom report, click table graph, click time, change interval to weekly, click any cell, notice the "filtered balance" doesn't match the cell you clicked. Can also be seen on weekly stacked bar graph and weekly line graph.

@github-actions github-actions bot changed the title Custom Reports show activity fixes [WIP] Custom Reports show activity fixes May 20, 2024
Copy link

netlify bot commented May 20, 2024

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit 3070d0b
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/665cc70fdd249c0008e0e276
😎 Deploy Preview https://deploy-preview-2785.demo.actualbudget.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

github-actions bot commented May 20, 2024

Bundle Stats — desktop-client

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count Total bundle size % Changed
9 4.72 MB → 4.71 MB (-2.68 kB) -0.06%
Changeset
File Δ Size
src/components/reports/graphs/showActivity.ts 📈 +307 B (+21.71%) 1.38 kB → 1.68 kB
src/components/reports/spreadsheets/recalculate.ts 📈 +153 B (+10.49%) 1.42 kB → 1.57 kB
src/components/reports/spreadsheets/custom-spreadsheet.ts 📈 +171 B (+3.90%) 4.28 kB → 4.45 kB
src/components/reports/graphs/tableGraph/ReportTableRow.tsx 📈 +165 B (+2.47%) 6.52 kB → 6.68 kB
src/components/reports/ChooseGraph.tsx 📈 +56 B (+1.60%) 3.41 kB → 3.47 kB
src/components/reports/graphs/tableGraph/ReportTable.tsx 📈 +32 B (+1.06%) 2.96 kB → 2.99 kB
src/components/reports/spreadsheets/grouped-spreadsheet.ts 📈 +40 B (+1.02%) 3.84 kB → 3.88 kB
src/components/reports/reports/CustomReport.tsx 📈 +27 B (+0.14%) 19.49 kB → 19.52 kB
src/components/reports/ReportOptions.ts 📈 +3 B (+0.05%) 6.41 kB → 6.42 kB
src/components/reports/graphs/AreaGraph.tsx 📈 +1 B (+0.01%) 8.26 kB → 8.26 kB
src/components/reports/graphs/StackedBarGraph.tsx 📉 -792 B (-9.98%) 7.75 kB → 6.97 kB
src/components/reports/graphs/BarGraph.tsx 📉 -1001 B (-10.67%) 9.16 kB → 8.19 kB
src/components/reports/graphs/DonutGraph.tsx 📉 -1 kB (-12.09%) 8.29 kB → 7.29 kB
src/components/reports/graphs/LineGraph.tsx 📉 -875 B (-12.20%) 7.01 kB → 6.15 kB
View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

No assets were bigger

Smaller

Asset File Size % Changed
static/js/ReportRouter.js 1.23 MB → 1.23 MB (-2.68 kB) -0.21%

Unchanged

Asset File Size % Changed
static/js/indexeddb-main-thread-worker-e59fee74.js 13.5 kB 0%
static/js/resize-observer.js 18.37 kB 0%
static/js/BackgroundImage.js 122.29 kB 0%
static/js/usePreviewTransactions.js 790 B 0%
static/js/AppliedFilters.js 20.41 kB 0%
static/js/narrow.js 59.97 kB 0%
static/js/wide.js 263.11 kB 0%
static/js/index.js 3 MB 0%

Copy link
Contributor

github-actions bot commented May 20, 2024

Bundle Stats — loot-core

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count Total bundle size % Changed
1 1.2 MB 0%

Changeset

No files were changed

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

No assets were bigger

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
kcab.worker.js 1.2 MB 0%

@trafico-bot trafico-bot bot added 🚧 WIP Still work-in-progress, please don't review and don't merge 🔍 Ready for Review Pull Request is not reviewed yet and removed 🚧 WIP Still work-in-progress, please don't review and don't merge labels May 20, 2024
@carkom carkom changed the title [WIP] Custom Reports show activity fixes Custom Reports show activity fixes May 20, 2024
@trafico-bot trafico-bot bot added 🚧 WIP Still work-in-progress, please don't review and don't merge and removed 🔍 Ready for Review Pull Request is not reviewed yet labels May 20, 2024
@carkom carkom changed the title Custom Reports show activity fixes Custom Reports show activity fixe May 20, 2024
@carkom carkom changed the title Custom Reports show activity fixe Custom Reports show activity fix May 20, 2024
@trafico-bot trafico-bot bot added 🔍 Ready for Review Pull Request is not reviewed yet and removed 🚧 WIP Still work-in-progress, please don't review and don't merge labels May 20, 2024
@youngcw youngcw added this to the v24.6.0 milestone May 22, 2024
@youngcw
Copy link
Contributor

youngcw commented May 27, 2024

I think this issue is also present in schedules. Would you be willing to look into putting in the same fix there?

@youngcw
Copy link
Contributor

youngcw commented May 28, 2024

Actually, if all users schedules will be broken, then we probably need to either roll back that commit that changed the filter, or put in a migration. Both probably involve rolling back the commit for this next release.

@Teprifer
Copy link

Actually, if all users schedules will be broken, then we probably need to either roll back that commit that changed the filter, or put in a migration. Both probably involve rolling back the commit for this next release.

@MatissJanis FYI if you don't see it in discord

@carkom carkom removed this from the v24.6.0 milestone May 28, 2024
@MatissJanis
Copy link
Member

Thanks for the heads up @Teprifer . Looks like @youngcw already got this situation under control, so I won't bud in :)

@carkom
Copy link
Contributor Author

carkom commented May 29, 2024

There's been lot's of discussion about the regression PR.

I'd welcome any code reviews anyone can offer as this PR is still relevant I'd like to get it merged with existing changes. The switch to inflow/outflow actually hardens the code and prevents any future bugs from affecting the custom reports functionality. Cheers!

@trafico-bot trafico-bot bot added the ⚠️ Changes requested Pull Request needs changes before it can be reviewed again label May 30, 2024
@trafico-bot trafico-bot bot removed the 🔍 Ready for Review Pull Request is not reviewed yet label May 30, 2024
@trafico-bot trafico-bot bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Jun 2, 2024
@carkom carkom requested a review from psybers June 2, 2024 21:20
@trafico-bot trafico-bot bot removed the ⚠️ Changes requested Pull Request needs changes before it can be reviewed again label Jun 2, 2024
@trafico-bot trafico-bot bot added ✅ Approved Pull Request has been approved and can be merged and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Jun 3, 2024
@carkom carkom added 🔍 Ready for Review Pull Request is not reviewed yet and removed ✅ Approved Pull Request has been approved and can be merged labels Jun 3, 2024
@carkom carkom requested review from youngcw and removed request for MatissJanis and youngcw June 3, 2024 15:28
@trafico-bot trafico-bot bot added ✅ Approved Pull Request has been approved and can be merged and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Jun 3, 2024
@carkom carkom merged commit e7c6611 into actualbudget:master Jun 3, 2024
19 checks passed
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved Pull Request has been approved and can be merged labels Jun 3, 2024
@carkom carkom deleted the weeklyIntervalFix branch June 6, 2024 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Merged Pull Request has been merged successfully
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants