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

Idea: Automatically open additional posting lines if not transaction is not balanced #1477

Closed
fdw opened this issue Sep 4, 2022 · 16 comments · Fixed by #1762
Closed

Idea: Automatically open additional posting lines if not transaction is not balanced #1477

fdw opened this issue Sep 4, 2022 · 16 comments · Fixed by #1762

Comments

@fdw
Copy link
Contributor

fdw commented Sep 4, 2022

Sometimes, a transaction has three or more postings. In the current dialog, two posting lines appear by default, and I have to add one manually when I need one.

While this works, the button to add a new one is on the very right and on small screen (phone, for example), I have to scroll. For some additional comfort, I'd love Fava to recognize that the transaction is not balanced yet and add another posting automatically.

Is this something you find valuable, or is it too complex?

@yagebu
Copy link
Member

yagebu commented Dec 17, 2022

This shouldn't be too hard to do (for the simple case of postings without cost and maybe all of the same currency). A PR would be welcome :)

@fdw
Copy link
Contributor Author

fdw commented Dec 29, 2022

Do you have a hint for me how/where this could be done? I'm still very new to Vue.

@yagebu
Copy link
Member

yagebu commented May 26, 2023

Fava uses Svelte (https://svelte.dev/), not Vue ;)

The transaction form is rendered by the following Svelte component:

https://github.com/beancount/fava/blob/main/frontend/src/entry-forms/Transaction.svelte

That would be the right place to add such functionality. If you haven't used Svelte before, it probably makes sense to go through its tutorial to get a feeling for it. A reactive statement (https://svelte.dev/tutorial/reactive-statements) would allow you to run some code (check whether a posting needs to be added and add it) whenever the transaction object changes.

@fdw
Copy link
Contributor Author

fdw commented Jun 23, 2023

Thanks for the pointers, they're very helpful 🙂

But to make this work, I need to sum the Postings using their currency. Is there already a method to do that?

@yagebu
Copy link
Member

yagebu commented Jul 8, 2023

No there isn't any such logic in the frontend yet (so far the amounts are only treated as strings). You'll have to split the amounts, convert to a number, group by currency and then sum them.

@fdw
Copy link
Contributor Author

fdw commented Jul 9, 2023

That means I would have to re-implement Beancount's balancing... do you have a pointer where I can leave such a general function that extracts the amount out of a posting? Can I do it in the same Posting.svelte?

@yagebu
Copy link
Member

yagebu commented Jul 25, 2023

I think entries.ts would be the right place for it. Feel free to refactor Amount and Posting to classes if that helps implementing that functionality.

@fdw
Copy link
Contributor Author

fdw commented Jul 30, 2023

Sorry, one more question: I've noticed that when I try to enter a total price (in contrast to a per-unit price) using {{<blah>}} double braces, it's transformed into { 0 # <blah>}. It works fine, but I can't find either syntax in the Beancount documentation. Have I used it wrong the whole time?

Edit: I've found at least a hint: https://beancount.github.io/docs/beancount_cheat_sheet.html and beancount/beancount#278 . However, these syntaxes seem more and more obscure. Would it be okay for you if I don't implement all of them?

@yagebu
Copy link
Member

yagebu commented Aug 29, 2023

I'd be fine if this logic in the frontend didn't support cost at all (and ignored anything between the {})

@fdw
Copy link
Contributor Author

fdw commented Oct 3, 2023

So, I've finally opened a PR for the simple case. I hope this is okay 🙂

@sohamshanbhag
Copy link
Contributor

A simpler fix for the usability problem would be to add an empty line whenever all lines in the input have text and/or amounts(irrespective of if they balance). Then, you could change the default 2 line input to one line input.

sohamshanbhag added a commit to sohamshanbhag/fava that referenced this issue Feb 17, 2024
This commit checks if information has been added to the last posting
when entering a transaction and adds an empty posting in this case.

Fixes beancount#1477.
sohamshanbhag added a commit to sohamshanbhag/fava that referenced this issue Feb 17, 2024
This commit checks if information has been added to the last posting
when entering a transaction and adds an empty posting in this case.

Fixes beancount#1477.
yagebu pushed a commit to sohamshanbhag/fava that referenced this issue Feb 18, 2024
This commit checks if information has been added to the last posting
when entering a transaction and adds an empty posting in this case.

Fixes beancount#1477.
@fdw
Copy link
Contributor Author

fdw commented Feb 18, 2024

This solution has one drawback in my eyes: If the new line is only added when the whole transaction is unbalanced, it also gives you feedback about errors (f.e. it's only unbalanced because you mistyped one amount).

@yagebu
Copy link
Member

yagebu commented Feb 18, 2024

Yes, more frontend validations to better catch invalid transactions are still possible

@sohamshanbhag
Copy link
Contributor

sohamshanbhag commented Feb 18, 2024

Another alternative would be to validate transaction on clicking save. Seems like that would be in https://github.com/beancount/fava/blob/f1171f3e186ecfe0a661d99a3028e340245e167e/frontend/src/api/index.ts#L162C23-L162C34

@fdw
Copy link
Contributor Author

fdw commented Feb 19, 2024

How about disabling save and showing maybe a red border around all amounts until they're balanced? That feels more streamlined and intuitive than only checking on save.

@sohamshanbhag
Copy link
Contributor

Beancount allows unbalanced postings in input transactions, so I'm not sure if this should be added. However, if this is to be added, the code is very short. I can add a PR if @yagebu wants this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants