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

Enable plugins to decide which view is shown after logging in. #106


Copy link

@create-issue-branch create-issue-branch bot commented Apr 29, 2021

closes #100

@nhoening nhoening changed the title Config setting to decide which view is shown after logging in. Enable plugins to decide which view is shown after logging in. Apr 29, 2021
@nhoening nhoening marked this pull request as ready for review Apr 29, 2021
@nhoening nhoening requested a review from Flix6x Apr 29, 2021
Flix6x approved these changes May 1, 2021
Copy link

@Flix6x Flix6x left a comment

I like it, but it wasn't obvious to me that the PR title reflects its contents. As is, should this really close #100?

Maybe consider adding an example that actually overrides the "/" route. Just adding @flexmeasures_ui.route("/") before an existing (plugin) route should do the trick, right? Theoretically, could one set one of the other existing FM routes to become the default after logging in, for example, the portfolio view? Perhaps by doing this in the plugin:

from flexmeasures.ui.views.portfolio import portfolio_view

def default_portfolio_view():

Copy link

@nhoening nhoening commented May 1, 2021

Yes I believe this PR could have a different title, like "Plugins can override existing routes", and one consequence is that they could rewrite "/", which is usually where you'll go after logging in (you go to where you wanted to go before you were rerouted to the login form, and that usually is "/").

@nhoening nhoening merged commit 3dc87dd into main May 1, 2021
2 checks passed
@nhoening nhoening deleted the issue-100-Config_setting_to_decide_which_view_is_shown_after_logging_in branch May 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

2 participants