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

Group and ungroup split transactions #2805

Merged
merged 19 commits into from
Jun 26, 2024
Merged

Conversation

joel-jeremy
Copy link
Contributor

@joel-jeremy joel-jeremy commented May 28, 2024

Easily turn multiple transactions into a split transaction or separate a split transaction into multiple individual ones.

Make as split transaction

image

Unsplit transaction

image

Copy link

netlify bot commented May 28, 2024

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit 7d18017
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/66760022a702ae0008693550
😎 Deploy Preview https://deploy-preview-2805.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.

@github-actions github-actions bot changed the title Group and ungroup split transactions [WIP] Group and ungroup split transactions May 28, 2024
Copy link
Contributor

github-actions bot commented May 28, 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.76 MB → 4.76 MB (+7.35 kB) +0.15%
Changeset
File Δ Size
src/components/transactions/SelectedTransactionsButton.jsx 🆕 +7.44 kB 0 B → 7.44 kB
home/runner/work/actual/actual/packages/loot-core/src/shared/transactions.ts 📈 +1.47 kB (+22.19%) 6.63 kB → 8.1 kB
src/components/modals/PayeeAutocompleteModal.tsx 📈 +111 B (+7.84%) 1.38 kB → 1.49 kB
src/components/accounts/Account.jsx 📈 +2.72 kB (+5.97%) 45.53 kB → 48.25 kB
src/components/transactions/TransactionsTable.jsx 📈 +1.17 kB (+2.13%) 55.21 kB → 56.38 kB
src/components/schedules/ScheduleLink.tsx 📈 +33 B (+1.16%) 2.77 kB → 2.8 kB
src/components/accounts/Header.jsx 📈 +144 B (+0.92%) 15.33 kB → 15.47 kB
src/components/mobile/transactions/TransactionEdit.jsx 📈 +249 B (+0.77%) 31.77 kB → 32.01 kB
src/components/transactions/TransactionList.jsx 📈 +25 B (+0.56%) 4.37 kB → 4.4 kB
src/components/Modals.tsx 📉 -41 B (-0.26%) 15.18 kB → 15.14 kB
src/components/transactions/SelectedTransactions.jsx 🔥 -5.96 kB (-100%) 5.96 kB → 0 B
View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset File Size % Changed
static/js/wide.js 266.96 kB → 272.5 kB (+5.54 kB) +2.08%
static/js/index.js 3.02 MB → 3.02 MB (+1.81 kB) +0.06%

Smaller

No assets were smaller

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/usePreviewTransactions.js 790 B 0%
static/js/BackgroundImage.js 122.29 kB 0%
static/js/AppliedFilters.js 27.22 kB 0%
static/js/narrow.js 75.73 kB 0%
static/js/ReportRouter.js 1.23 MB 0%

Copy link
Contributor

github-actions bot commented May 28, 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.13 MB → 1.13 MB (-10 B) -0.00%
Changeset
File Δ Size
packages/loot-core/src/shared/transactions.ts 📈 +1.61 kB (+19.07%) 8.43 kB → 10.04 kB
packages/loot-core/src/server/main.ts 📉 -59 B (-0.10%) 60.27 kB → 60.21 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
kcab.worker.js 1.13 MB → 1.13 MB (-10 B) -0.00%

Unchanged

No assets were unchanged

@joel-jeremy joel-jeremy changed the title [WIP] Group and ungroup split transactions Group and ungroup split transactions May 28, 2024
@psybers
Copy link
Contributor

psybers commented May 29, 2024

Right now if I want to pull one of the splits out, I have to separate the entire transaction, then join back the other splits together.

Is there a way, since individual splits can be selected, to just have it so you can select a subset of the splits and pull those out into separate transactions, leaving the rest together as a split?

@psybers
Copy link
Contributor

psybers commented May 29, 2024

Here are some bugs:

  • It seems to let me pick two transactions from different accounts and make them into a split.
  • When you make a new split from two transactions, the 'parent' transaction that is created does not have a payee. Only the splits do.
  • I am not sure how the undo/redo system works but if you can somehow group several steps into a single "undo" operation, that might make sense. Otherwise you have to ctrl-z twice to actually undo the join operation.

@psybers
Copy link
Contributor

psybers commented May 29, 2024

@joel-jeremy Instead of always making a "Split" payee, could it be a bit smarter? If the transactions being grouped are all the same Payee, it should use that. If not, I am not sure of the best fallback case, but perhaps we fall back to either the one that is most common or the newest? I do not have any payee named "Split" and would not like having that, personally.

@psybers
Copy link
Contributor

psybers commented May 29, 2024

I am unsure why it renders differently. Here is a real split (from the test data):
image

Here is one I created:
image

Notice the payees of the children are not greyed out like the first one.

@joel-jeremy
Copy link
Contributor Author

@psybers Thank you for testing this out! Pushed some changes. Seems to fix most of the issues, and now it's going to allow pulling out one or more child transactions, not just as a whole.

@psybers
Copy link
Contributor

psybers commented May 29, 2024

Here's another edge case:

image

It gives the option to unsplit here.

@psybers
Copy link
Contributor

psybers commented May 29, 2024

If I join several into a split, it keeps them selected. Is it possible to keep them selected when making them individual? It looks like if I pick a subset of the splits it does that.

But if I pick the parent and then unsplit, can you have it select the children that are now individual?

@joel-jeremy
Copy link
Contributor Author

@psybers The code handles that edge-case, it ignores the non-split transactions.

@psybers
Copy link
Contributor

psybers commented May 29, 2024

If I pick all but 1 of the splits and then pull them out, I am left with this parent with one child:

image

Probably it should just go to a normal, non-split transaction?

@psybers
Copy link
Contributor

psybers commented May 29, 2024

@psybers The code handles that edge-case, it ignores the non-split transactions.

I was thinking it makes more sense to not show the menu option in this edge case.

@psybers
Copy link
Contributor

psybers commented May 29, 2024

General UX principles state the menu item should probably always be shown, and just grey out when it isn't available. Similar to the "Make transfer" option.

The wording of the menu item could maybe be improved, but I am struggling with some good suggestions.

  • Join transactions
  • Merge into split
  • Make split transaction

@joel-jeremy
Copy link
Contributor Author

Maybe we should consider introducing a special "Split" payee just as we have a special "Transfer" category. I always found it a bit odd having to set a payee on the parent transaction when the child transactions can have differing payees that are unrelated.

You didn't really pay for anything with the parent transaction, you already denoted that on it's child transactions. So more likely than not, the parent payee is just duplicating the info on the child transactions.

@Teprifer
Copy link

Maybe we should consider introducing a special "Split" payee just as we have a special "Transfer" category. I always found it a bit odd having to set a payee on the parent transaction when the child transactions can have differing payees that are unrelated.

You didn't really pay for anything with the parent transaction, you already denoted that on it's child transactions. So more likely than not, the parent payee is just duplicating the info on the child transactions.

Maybe, but this logic doesn't quite follow when there is only one actual transaction and the split is to apportion parts to different categories - it would be annoying and superfluous having to set the same payee repeatedly vs the once you do now.

@joel-jeremy
Copy link
Contributor Author

Maybe, but this logic doesn't quite follow when there is only one actual transaction and the split is to apportion parts to different categories - it would be annoying and superfluous having to set the same payee repeatedly vs the once you do now.

To help with that, we can have the "Split" payee render as a button, which opens the payee autocomplete to set all child transactions to the same payee, but parent will still have the "Split" payee.

@psybers
Copy link
Contributor

psybers commented May 30, 2024

@joel-jeremy Can you provide a concrete use case of where it makes sense to take two separate transactions, from two different payees, and then join them together as a "split"? I am struggling to envision this case.

I can clearly see the other case, where you might want to pull part of a split out (I would use that every week!). An example is if you split a bill with your partner and want to pull their part out to track separately. You could leave it as a split and just categorize it differently but it is nice to see it separate. Then you could delete it.

@psybers
Copy link
Contributor

psybers commented May 30, 2024

Any ideas what this would/would not do if you are using a bank sync? If I pull out a split, or join two things together will it suddenly try to sync the original transaction back in?

@joel-jeremy
Copy link
Contributor Author

Example of how we could render the parent transaction payee:

image

When split button is clicked:
image\

Updates all the child transaction payees:
image

@joel-jeremy
Copy link
Contributor Author

Can you provide a concrete use case of where it makes sense to take two separate transactions, from two different payees, and then join them together as a "split"? I am struggling to envision this case.

@psybers Personally, I use split transaction for transactions that have fees. I add the fee as a child transaction with a different payee e.g.

image

@joel-jeremy
Copy link
Contributor Author

Any ideas what this would/would not do if you are using a bank sync? If I pull out a split, or join two things together will it suddenly try to sync the original transaction back in?

Haven't done a thorough testing but I believe as long as the imported_id is still set, the sync logic will match that regardless if the transaction became a child transaction or vice versa.

@joel-jeremy joel-jeremy requested review from psybers and MatissJanis and removed request for psybers June 21, 2024 22:49

return transactions.some(t => t && t.is_child);
}, [selectedItems]);
}, [selectedIds, getTransaction]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 praise: ‏good catch!

@joel-jeremy joel-jeremy merged commit 5951b92 into master Jun 26, 2024
22 of 23 checks passed
@joel-jeremy joel-jeremy deleted the group-ungroup-transactions branch June 26, 2024 19:39
@github-actions github-actions bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved labels Jun 26, 2024
@Zybrion
Copy link

Zybrion commented Jul 16, 2024

Hello @joel-jeremy,
I'd like to provide some feedback regarding the split transaction entry.
I have always used splits like a receipt mechanism. The parent transaction has the full amount and each item bought (for example in a store or in a restaurant) gets its own split entry underneath the parent entry.

The issue I am now facing after this change is that about half of my transactions now have split as the payee which is really annoying because with a quick glance I cannot see from where / what I bought there. I'd have to manually open each and every split transaction in order to gain knowledge from where / what I bought.

Is it possible to integrate the suggestion from @psybers mentioned here? #2805 (comment)
I heavily agree that when all child transactions have one common payee that this one payee should be shown when the split transaction is collapsed. If there are multiple payees in the child transaction, then I personally would be fine having split shown as a payee (although I'd still prefer to control it myself).

I have attached a screenshot to maybe better visualize my problem:
split_transactions

Thank you for your work!

@joel-jeremy
Copy link
Contributor Author

Hey there 👋 thank you for your feedback! There is an ongoing PR for that: #3049

@Zybrion
Copy link

Zybrion commented Jul 16, 2024

Oh, very awesome! I am sorry I didn't see that one. Thank you! That looks very nice

@warstellar
Copy link

I am not sure what I'm doing wrong, but it doesn't seem to make any sense as to why I can see or not see this option:
image
image

@vcar65
Copy link

vcar65 commented Oct 26, 2024

I can't see the options 'Mark as split transaction' nor 'Unspilt transactions'.
I'm running version v24.10.1 on Pikapods

CORRECTION
Now I can see these options
I presume that the transactions can´t be reconciled

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.

7 participants