search

redstreet / fava_investor

Comprehensive set of reports, analyses, and tools for investments, for Beancount and Fava (plain text, double entry accounting software). Includes Fava plugins, command line tools, and libraries for each module.
GNU General Public License v3.0
136 stars 20 forks source link

Fix assetalloc_class chart #93

Closed aldur closed 7 months ago

aldur commented 7 months ago

The chart showing asset allocation by class is currently broken on main because of a schema change that prevents correct deserialisation. This PR fixes the issue (took me a while to figure it out because the JS code swallows the error).

redstreet commented 7 months ago

Very nice, you get hero points for this! This was badly needed, and I haven't had a chance to get to it. Not a quick fix for me either as I'm unfamiliar with html/templating related code.

Merged, thank you a ton!

aldur commented 7 months ago

Thank you, happy to help :) The table representation of the tree is still broken unfortunately, but I didn't manage to fix that. Maybe next time :)

redstreet commented 7 months ago

Hmm, the table hasn't been broken for me at any point. I just checked with the latest version of fava and top of the tree of fava_investor, and it works fine. That's what the pythonanywhere site is also using, and it seems to work there too.

If you're seeing that it's broken, it'd be great if you could file a bug with a minimal test case + config. Is it broken only on the web or on the command line too?

aldur commented 7 months ago

My bad, sorry! I was running fava 1.26 against fava_investor 0.6.0 and that created the result attached against the example from pythonanywhere.

Bumping fava to 1.27 solved the issue and it now renders correctly.

Not sure if it possible, but would it be worth warning the user if running an unsupported pair of versions?

Screenshot 2024-04-12 at 18 56 53
redstreet commented 7 months ago

Good to know, thanks for reporting! We should just bump up the minimum fava version, and make a release. Will do, thank you!

redstreet commented 7 months ago

Done: https://github.com/redstreet/fava_investor/releases/tag/0.7.0

Thanks again.