Rethink App state vs. Session state about fava HOT 6 CLOSED

I would prefer 1., because there may be more query strings in the future (think table-column-ordering, etc.) and managing every link on every page to always have the correct query strings sounds like a huge task.

For the "transparency"-argument in 2.: The filter-permalink shows all the selected filters in a way 2. would do, so that kind of transparency would be given in 1. as well. Do you agree?

from fava.

I would prefer 1., because there may be more query strings in the future (think table-column-ordering, etc.) and managing every link on every page to always have the correct query strings sounds like a huge task.

It would only be feasible to implement 1. if we can patch url_for to automatically append the query string (and app.url_defaults looks like it does exactly that). I would reserve query strings for things like filters that massively change the data that is displayed. I think the URL should be a precise (as far as possible) identifier for the page that's displayed; and /income_statement/ and /income_statement/?time=2015 are two very different things, so they deserve unique URLs IMHO.

For the "transparency"-argument in 2.: The filter-permalink shows all the selected filters in a way 2. would do, so that kind of transparency would be given in 1. as well. Do you agree?

Well on a website I personally couldn't be bothered with a separate link to set a bookmark. Since beancount-web is more of a web app, I don't mind as much; still my browser has a nice button to set a bookmark, so if there is no good reason not to have the URL be indentifying for that page, I think it's an unnecessary nuisance.

Edit: playing around with a url_defaults function, it seems really trivial to inject the filters into every needed url. It could actually simplify the application code behind it drastically as there would be no more need for any filter_remove and connected application logic.

from fava.

I think the URL should be a precise (as far as possible) identifier for the page that's displayed;

True, and a good reason for 2.

still my browser has a nice button to set a bookmark, so if there is no good reason not to have the URL be indentifying for that page, I think it's an unnecessary nuisance.

Also very true, so again, looks more like 2. will be the winner after all 😉

It would only be feasible to implement 1. if we can patch url_for to automatically append the query string
Edit: playing around with a url_defaults function, it seems really trivial to inject the filters into every needed url. It could actually simplify the application code behind it drastically as there would be no more need for any filter_remove and connected application logic.

If this works and automatically adding the filters to url_for (with the possibility to add manual GET-parameters in the templates), then we should definitely go with the 2. option, keep everything in the URL query string.

I will create a new branch, and can you then create a PR to that branch where you show your experiments with app.url_defaults?

Edit: The new branch is called url-defaults where we can play with this.

from fava.

I will create a new branch, and can you then create a PR to that branch where you show your experiments with app.url_defaults?

Sounds good. My first steps are in my filter-refactoring branch which is a bit of a mess currently. As soon as I get it cleaned up and more complete I will submit a PR.

from fava.

@yagebu Are any parts missing? I tried it out extensively today and noticed no bugs.

from fava.

I think the url-defaults branch contains everything needed for this issue (and I also didn't find any more bugs) so I'll close this :)

from fava.