Add 'Duplicate Transaction' to bulk editor for #548 #582
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| @@ -480,6 +483,14 @@ function SelectedTransactionsButton({ | |||
| }; | |||
| }, [selectedItems]); | |||
|
|
|||
| let ambiguousDuplication = useMemo(() => { | |||
| let transactions = [...selectedItems].map(id => getTransaction(id)); | |||
There was a problem hiding this comment.
If anyone has any suggestions for how this ambitiousDuplication fn can be improved, would love to hear them, seems a little unfortunate that we have to get ALL the selected transactions to determine if we can enable this functionality or not.
There was a problem hiding this comment.
You could show an error message after hitting the duplicate button (since the case where the duplication is invalid is unlikely, right?)
It might also make sense to duplicate just the top level transaction if it is selected along with some of its children (since that most closely marches the expected behavior IMO).
There was a problem hiding this comment.
I think an error message makes more sense here, agreed.
I can see either way with this behavior to be honest. I think overall duplicating child transactions seems like a bit of an unnecessary feature in my opinion, especially with updating the parents amount automatically, it just seems a bit backwards to how I personally go about splitting a transaction.
Would be super interested to hear if anyone does have a workflow where duplicating individual child transactions would be useful!
There was a problem hiding this comment.
Would be super interested to hear if anyone does have a workflow where duplicating individual child transactions would be useful!
Maybe start by not duplicating child transactions and wait for someone to complain?
There was a problem hiding this comment.
Yeah, I think that sounds like a better plan! I'll go ahead remove the child duplication code, easy to add back in later if it is a desired feature
@MatissJanis, wanted to ping you here since we spoke about this earlier on Discord!
…gsumpster.duplicate-transactions
|
Adding myself into the loop here as this will need docs. |
| @@ -480,6 +482,14 @@ function SelectedTransactionsButton({ | |||
| }; | |||
| }, [selectedItems]); | |||
|
|
|||
| let ambiguousDuplication = useMemo(() => { | |||
| let transactions = [...selectedItems].map(id => getTransaction(id)); | |||
There was a problem hiding this comment.
🥜 nitpick:
| let transactions = [...selectedItems].map(id => getTransaction(id)); | |
| let transactions = selectedItems.map(id => getTransaction(id)); |
There was a problem hiding this comment.
@MatissJanis, had to add this back, selectedItems is a Set and can't use .map
trafico-bot
bot
added
⚠️ Changes requested
Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>
Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>
trafico-bot
bot
added
🔍 Ready for Review
and removed
⚠️ Changes requested
Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>
trafico-bot
bot
added
✅ Approved
✨ Merged
Completes #548!
This function will duplicate root level transactions that are selected, it will also duplicate children (even from different parents). This function will be disabled if a user attempts to select both parent transactions and children transactions as that's a little ambiguous as to what they want us to do there.
Thanks to @MatissJanis for helping me with some of the edge cases on how this functionality should work!