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
Improve transaction commands #85
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
If possible could you do up the TransactionEditCommand asap? Rules still depend on it to have any effect at all.
docs/UserGuide.adoc
Outdated
|
||
Edits the specified transaction, setting the specified fields. | ||
Edits the specified txn, setting the specified fields. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for text perhaps use "transaction" instead?
docs/UserGuide.adoc
Outdated
|
||
Deletes the transactions with the specified IDs. | ||
Deletes the txn with the specified ID. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here perhaps
src/main/java/budgetbuddy/logic/commands/transactioncommands/TransactionDeleteCommand.java
Show resolved
Hide resolved
…ion of transactions
…rebasing changes.
67520b2
to
45b3dfa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two issues:
The default account handling is very weird. It should definitely not be kept as a static field in AccountsManager
.
Transaction add also doesn't work. TransactionAddCommandParser
calls CommandParserUtil#parseAccount
which creates a new Account
, and then sets, on the Transaction
, the Account
. Then TransactionAddCommand
calls AccountsManager#addTransaction
, which gets the Account
set on the Transaction
, adds the Transaction
to that account, and then the Account
is promptly lost.
I'm going to merge first, and we can fix it after the other PR is merged.
No description provided.