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

[UI] Add configurability to the menu, what root view to show and what name to use #163

Merged
merged 20 commits into from Sep 3, 2021

Conversation

nhoening
Copy link
Contributor

@nhoening nhoening commented Aug 30, 2021

Let the root view ('/') and menu be configurable by which account role(s) the current user's account has.
Let menu titles and icons be configurable.

This gives more control for supporting different kinds of services on one server. We introduce account roles for this, so that accounts can be treated differently.

To test this, one could play with the relevant config variables:

FLEXMEASURES_ROOT_VIEW = [("details", ["MDC"],), "assets"]
FLEXMEASURES_MENU_LISTED_VIEWS = ["dashboard", "details", ("clusters", ["MDC"])]
FLEXMEASURES_MENU_LISTED_VIEW_ICONS = {"details": "snowflake-o"}
FLEXMEASURES_MENU_LISTED_VIEW_TITLES = {"details": "DEAILS", "dashboard": "DASHBOARD"}

But first, a database upgrade:

flexmeasures db upgrade

Also, you can assign account roles to your user, e.g.

insert into roles_accounts (role_id, account_id) VALUES (2,1);

Alternatively, making a fresh account now can create account roles alongside:

flexmeasures add account --name "Some company" --roles MDC,Prosumer

@nhoening nhoening marked this pull request as ready for review Aug 31, 2021
@nhoening nhoening requested a review from Flix6x Aug 31, 2021
Flix6x
Flix6x approved these changes Aug 31, 2021
Copy link
Contributor

@Flix6x Flix6x left a comment

Minor comments.

documentation/changelog.rst Outdated Show resolved Hide resolved
documentation/configuration.rst Outdated Show resolved Hide resolved
documentation/configuration.rst Outdated Show resolved Hide resolved
flexmeasures/data/scripts/cli_tasks/data_delete.py Outdated Show resolved Hide resolved
flexmeasures/ui/templates/defaults.jinja Outdated Show resolved Hide resolved
flexmeasures/ui/__init__.py Outdated Show resolved Hide resolved
@nhoening
Copy link
Contributor Author

@nhoening nhoening commented Aug 31, 2021

I just noticed that there is another setting (FLEXMEASURES_PLATFORM_NAME) which we might want to treat much the same as the two settings I already worked on

@nhoening
Copy link
Contributor Author

@nhoening nhoening commented Aug 31, 2021

We also still need CLI command for adding a new account role. Deleting a role can be tricky if accounts are connected to it.

flexmeasures/utils/app_utils.py Outdated Show resolved Hide resolved
Flix6x added 2 commits Sep 1, 2021
…ion (#164)

This gives more flexibility in naming the plugin, and less worrying about consistently naming plugins and blueprints in different places (thereby facilitating renaming plugins).

The possibility of defining multiple Blueprints in a plugin is more of a perk, but I can imagine it comes in handy if you want to register separate Blueprints for your UI and API, for example, with a different url_prefix.
@Flix6x Flix6x added this to In progress in Pluggability via automation Sep 1, 2021
nhoening and others added 10 commits Sep 2, 2021
* Allow to set view icons and titles as a config setting

* Add config defaults

* Allow Font Awesome icons to be used in the documentation

* Add documentation for the two new config settings

Co-authored-by: Nicolas Höning <iam@nicolashoening.de>
…verything, not just to menu entries not in the original list
@nhoening nhoening changed the title Let the root view ('/') and menu be configurable by which account role(s) the current user's account has. [UI] Add configurability to the menu, what root view to show and what name to use Sep 2, 2021
@nhoening nhoening requested a review from Flix6x Sep 2, 2021
Flix6x
Flix6x approved these changes Sep 3, 2021
Copy link
Contributor

@Flix6x Flix6x left a comment

I reverted 164 so this can be tackled in a separate PR to main.

Just 1 more comment (to fix a docstring or the delete_account_role function's logic, according to whatever you had in mind), otherwise this PR has my approval.

flexmeasures/data/scripts/cli_tasks/data_delete.py Outdated Show resolved Hide resolved
Pluggability automation moved this from In progress to Reviewer approved Sep 3, 2021
nhoening
Copy link
Contributor

@nhoening nhoening commented on 3d83ee2 Sep 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong I believe. That setting is just a name, not to be used in URLs, and it won't get a slash.

nhoening
Copy link
Contributor

@nhoening nhoening commented on 3d83ee2 Sep 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the occurrence of "see the name" and a slash was a copy-paste mistake by me.

I agree with your phrasing convention. Should I look those over?

Flix6x
Copy link
Contributor Author

@Flix6x Flix6x commented on 3d83ee2 Sep 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then please be consistent. There's a couple places that could use some clarity. What you had before:

  • would use the name "MyMDCApp" ("use" and no slash, this is meant to be used as a name only right?)
  • would see the name "/MyApp" ("see" and slash, this is meant to be used as a name only right?)
  • would show "/mdc-dashboard" ("show" and slash, this is meant to be used as a url only right?)
  • would be routed to "/default-dashboard" ("be routed to" and slash, this is meant to be used as a url only right?)
  • the "/" view will be shown ("will be shown" and slash)

I propose we use the phrase "routed to" for the url setting, and "see the name" for the name setting.

@nhoening nhoening merged commit 1cb33ea into main Sep 3, 2021
2 checks passed
Pluggability automation moved this from Reviewer approved to Done Sep 3, 2021
@nhoening nhoening deleted the views-by-accounts branch Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Pluggability
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants