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

Issue 423 improve dashboard and asset listing for admins #461

Merged
merged 14 commits into from Jul 21, 2022

Conversation

nhoening
Copy link
Contributor

  • Admins can group assets on dashboard by accounts
  • Admins list all assets (from all accounts) in /assets overview

…better name

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
… not unique (anymore)

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening nhoening requested a review from Flix6x July 12, 2022 19:57
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening nhoening changed the title Issue 423 improve dashboard annd asset listing for admins Issue 423 improve dashboard and asset listing for admins Jul 12, 2022
@nhoening
Copy link
Contributor Author

I see there is a testing problem, but the PR is nonetheless reviewable.

…ttribute added earlier

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
…lexMeasures/flexmeasures into Issue-423-improve-dashboard-for-admins
@coveralls
Copy link
Collaborator

coveralls commented Jul 14, 2022

Pull Request Test Coverage Report for Build 2696744299

  • 50 of 56 (89.29%) changed or added relevant lines in 9 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 67.541%

Changes Missing Coverage Covered Lines Changed/Added Lines %
flexmeasures/ui/crud/assets.py 17 18 94.44%
flexmeasures/ui/views/new_dashboard.py 7 8 87.5%
flexmeasures/data/queries/generic_assets.py 2 4 50.0%
flexmeasures/data/services/asset_grouping.py 9 11 81.82%
Totals Coverage Status
Change from base Build 2679559260: 0.06%
Covered Lines: 7179
Relevant Lines: 10057

💛 - Coveralls

Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

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

Here's 3/4 of a review.

I think we could also discuss how the dashboard could be developed later. For example, I imagine the underlying asset data as a table (not shown, but served to the browser as a data frame) with columns name, type and owner, and the toggle button let's you switch between defining map layers based on type or based on owner. It seems like it would be cleaner to serve up the entire table once, and switch between layers on the client side only. Also, interesting applications may want to extend the table with new columns that can serve as map layers, like (from the top of my head) filtering assets into tiers based on their nominal capacity, or by how congested the area is in which they reside.

documentation/changelog.rst Outdated Show resolved Hide resolved
flexmeasures/data/queries/utils.py Outdated Show resolved Hide resolved
flexmeasures/data/queries/utils.py Show resolved Hide resolved
flexmeasures/ui/templates/views/new_dashboard.html Outdated Show resolved Hide resolved
flexmeasures/ui/templates/views/new_dashboard.html Outdated Show resolved Hide resolved
flexmeasures/ui/templates/views/new_dashboard.html Outdated Show resolved Hide resolved
flexmeasures/ui/templates/views/new_dashboard.html Outdated Show resolved Hide resolved
flexmeasures/ui/templates/views/new_dashboard.html Outdated Show resolved Hide resolved
@nhoening nhoening added this to the 0.11.0 milestone Jul 15, 2022
@nhoening nhoening added the UI label Jul 15, 2022
@nhoening nhoening self-assigned this Jul 15, 2022
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
Copy link
Contributor Author

You are right with your general comment, but that is outside the scope of this PR.

I believe this PR (next to adding some features we didn't have) opens up the way to implement such bigger steps, e.g. by refactoring get_asset_group_queries and enabling the dashboard to layer by something else.

@nhoening nhoening requested a review from Flix6x July 15, 2022 21:35
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

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

Here's the final 1/4 of my review.

Stuff about having map layers correspond to asset properties..

I agree it's for a different PR. Could you already open an issue for that and note roughly what code would need to be updated?

flexmeasures/ui/crud/assets.py Show resolved Hide resolved
flexmeasures/ui/crud/assets.py Show resolved Hide resolved
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
…oint, so public assets can be read as well (/assets needs an account, so this is the only clean solution).

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
Copy link
Contributor Author

I don't see the need to open a new issue at this point. We don't know what we'll really want when we re-build the frontend. The code might change, and as it is now, it is clearer than before.
I believe that in the end, calling the /assets API directly is the right way, and then do the various possibilities of grouping in the frontend.

@nhoening nhoening requested a review from Flix6x July 19, 2022 10:05
@nhoening nhoening merged commit 0a4f300 into main Jul 21, 2022
@nhoening nhoening deleted the Issue-423-improve-dashboard-for-admins branch July 21, 2022 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants