Issue 423 improve dashboard and asset listing for admins (#461)

* improve auth behaviour of query utility function, and also give it a better name

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* add account attribute to generic_asset

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* For admins, show next to assets on dashboard what account they are on

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* add switch button to group assets on dashboard by accounts (for admins)

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* using asset IDs in marker variable names is safer, as asset names are not unique (anymore)

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* Assets Ui page shows all account assets for admin(-reader)s

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* add changelog entry

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* correct treatmnt of session in CRUD asset views, remove unnecessary attribute added earlier

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* fix one more test

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* implement review comments

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* Let the asets page display public assets. Add GET /assets/public endpoint, 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>

documentation/changelog.rst

@@ -9,8 +9,9 @@ New features
 -------------
 * The asset page now shows the most relevant sensor data for the asset [see `PR #449 <http://www.github.com/FlexMeasures/flexmeasures/pull/449>`_]
 * Individual sensor charts show available annotations [see `PR #428 <http://www.github.com/FlexMeasures/flexmeasures/pull/428>`_]
-* Add CLI command ``flexmeasures jobs show-queues`` [see `PR #455 <http://www.github.com/FlexMeasures/flexmeasures/pull/455>`_]
+* Admins can group assets by account on dashboard & assets page [see `PR #461 <http://www.github.com/FlexMeasures/flexmeasures/pull/461>`_]
 * Collapsible side-panel (hover/swipe) used for date selection on sensor charts, and various styling improvements [see `PR #447 <http://www.github.com/FlexMeasures/flexmeasures/pull/447>`_ and `PR #448 <http://www.github.com/FlexMeasures/flexmeasures/pull/448>`_]
+* Add CLI command ``flexmeasures jobs show-queues`` [see `PR #455 <http://www.github.com/FlexMeasures/flexmeasures/pull/455>`_]
 * Switched from 12-hour AM/PM to 24-hour clock notation for time series chart axis labels [see `PR #446 <http://www.github.com/FlexMeasures/flexmeasures/pull/446>`_]
 * Get data in a given resolution [see `PR #458 <http://www.github.com/FlexMeasures/flexmeasures/pull/458>`_]
 
@@ -24,6 +25,7 @@ Infrastructure / Support
 * Docker compose stack now with Redis worker queue [see `PR #455 <http://www.github.com/FlexMeasures/flexmeasures/pull/455>`_]
 * Allow access tokens to be passed as env vars as well [see `PR #443 <http://www.github.com/FlexMeasures/flexmeasures/pull/443>`_]
 * Queue workers can get initialised without a custom name and name collisions are handled [see `PR #455 <http://www.github.com/FlexMeasures/flexmeasures/pull/455>`_]
+* New API endpoint to get public assets [see `PR #461 <http://www.github.com/FlexMeasures/flexmeasures/pull/461>`_]
 
 
 v0.10.1 | June XX, 2022

flexmeasures/api/v3_0/assets.py

@@ -2,6 +2,7 @@
 
 from flask import current_app
 from flask_classful import FlaskView, route
+from flask_security import login_required
 from flask_json import as_json
 from marshmallow import fields
 from webargs.flaskparser import use_kwargs, use_args
@@ -77,6 +78,27 @@ def index(self, account: Account):
         """
         return assets_schema.dump(account.generic_assets), 200
 
+    @route("/public", methods=["GET"])
+    @login_required
+    @as_json
+    def public(self):
+        """Return all public assets.
+
+        .. :quickref: Asset; Return all public assets.
+
+        This endpoint returns all public assets.
+
+        :reqheader Authorization: The authentication token
+        :reqheader Content-Type: application/json
+        :resheader Content-Type: application/json
+        :status 200: PROCESSED
+        :status 400: INVALID_REQUEST
+        :status 401: UNAUTHORIZED
+        :status 422: UNPROCESSABLE_ENTITY
+        """
+        assets = GenericAsset.query.filter(GenericAsset.account_id.is_(None)).all()
+        return assets_schema.dump(assets), 200
+
     @route("", methods=["POST"])
     @permission_required_for_context(
         "create-children", arg_loader=AccountIdField.load_current

flexmeasures/api/v3_0/tests/test_assets_api.py

@@ -100,6 +100,17 @@ def test_get_assets(
         assert turbine["account_id"] == setup_accounts["Supplier"].id
 
 
+def test_get_public_assets(client, setup_api_test_data, setup_accounts):
+    auth_token = get_auth_token(client, "test_admin_user@seita.nl", "testtest")
+    get_assets_response = client.get(
+        url_for("AssetAPI:public"),
+        headers={"content-type": "application/json", "Authorization": auth_token},
+    )
+    print("Server responded with:\n%s" % get_assets_response.json)
+    assert len(get_assets_response.json) == 1
+    assert get_assets_response.json[0]["name"] == "troposphere"
+
+
 def test_alter_an_asset(client, setup_api_test_data, setup_accounts):
     # without being an account-admin, no asset can be created ...
     with UserContext("test_prosumer_user@seita.nl") as prosumer1:

flexmeasures/data/queries/generic_assets.py

@@ -4,7 +4,7 @@
 from sqlalchemy.orm import Query
 
 from flexmeasures.data.models.generic_assets import GenericAsset, GenericAssetType
-from flexmeasures.data.queries.utils import potentially_limit_query_to_account_assets
+from flexmeasures.data.queries.utils import potentially_limit_assets_query_to_account
 
 
 def query_assets_by_type(
@@ -28,7 +28,7 @@ def query_assets_by_type(
         query = query.filter(GenericAssetType.name == type_names)
     else:
         query = query.filter(GenericAssetType.name.in_(type_names))
-    query = potentially_limit_query_to_account_assets(query, account_id)
+    query = potentially_limit_assets_query_to_account(query, account_id)
     return query
 
 
@@ -52,7 +52,7 @@ def get_location_queries(account_id: Optional[int] = None) -> Dict[str, Query]:
     :param account_id: Pass in an account ID if you want to query an account other than your own. This only works for admins. Public assets are always queried.
     """
     asset_queries = {}
-    all_assets = potentially_limit_query_to_account_assets(
+    all_assets = potentially_limit_assets_query_to_account(
         GenericAsset.query, account_id
     ).all()
     loc_groups = group_assets_by_location(all_assets)
@@ -71,7 +71,7 @@ def get_location_queries(account_id: Optional[int] = None) -> Dict[str, Query]:
         location_query = GenericAsset.query.filter(
             GenericAsset.name.in_([asset.name for asset in loc_group])
         )
-        asset_queries[location_name] = potentially_limit_query_to_account_assets(
+        asset_queries[location_name] = potentially_limit_assets_query_to_account(
             location_query, account_id
         )
     return asset_queries

flexmeasures/data/queries/sensors.py

@@ -3,7 +3,7 @@
 from sqlalchemy.orm import Query
 
 from flexmeasures.data.models.generic_assets import GenericAsset, GenericAssetType
-from flexmeasures.data.queries.utils import potentially_limit_query_to_account_assets
+from flexmeasures.data.queries.utils import potentially_limit_assets_query_to_account
 
 
 def query_sensor_by_name_and_generic_asset_type_name(
@@ -30,7 +30,7 @@ def query_sensor_by_name_and_generic_asset_type_name(
             .filter(GenericAsset.generic_asset_type_id == GenericAssetType.id)
             .filter(Sensor.generic_asset_id == GenericAsset.id)
         )
-    query = potentially_limit_query_to_account_assets(query, account_id)
+    query = potentially_limit_assets_query_to_account(query, account_id)
     return query
 
 
@@ -59,7 +59,7 @@ def query_sensors_by_proximity(
     closest_sensor_query = closest_sensor_query.order_by(
         GenericAsset.great_circle_distance(lat=latitude, lng=longitude).asc()
     )
-    closest_sensor_query = potentially_limit_query_to_account_assets(
+    closest_sensor_query = potentially_limit_assets_query_to_account(
         closest_sensor_query, account_id
     )
     return closest_sensor_query

flexmeasures/data/queries/utils.py

@@ -42,8 +42,9 @@ def create_beliefs_query(
     return query
 
 
-def potentially_limit_query_to_account_assets(
-    query: Query, account_id: Optional[int]
+def potentially_limit_assets_query_to_account(
+    query: Query,
+    account_id: Optional[int] = None,
 ) -> Query:
     """Filter out all assets that are not in the current user's account.
     For admins and CLI users, no assets are filtered out, unless an account_id is set.
@@ -55,7 +56,7 @@ def potentially_limit_query_to_account_assets(
     user_is_admin = (
         running_as_cli()
         or current_user.has_role(ADMIN_ROLE)
-        or current_user.has_role(ADMIN_READER_ROLE)
+        or (query.statement.is_select and current_user.has_role(ADMIN_READER_ROLE))
     )
     if account_id is None and user_is_admin:
         return query  # allow admins to query assets across all accounts

flexmeasures/data/services/asset_grouping.py

@@ -5,16 +5,19 @@
 
 from __future__ import annotations
 from typing import List, Dict, Optional
+from flask_login import current_user
 import inflect
 
 from sqlalchemy.orm import Query
+from flexmeasures.auth.policy import user_has_admin_access
 
 from flexmeasures.utils.flexmeasures_inflection import parameterize, pluralize
 from flexmeasures.data.models.generic_assets import (
     GenericAssetType,
     GenericAsset,
     assets_share_location,
 )
+from flexmeasures.data.models.user import Account
 from flexmeasures.data.queries.generic_assets import (
     query_assets_by_type,
     get_location_queries,
@@ -24,35 +27,49 @@
 
 
 def get_asset_group_queries(
-    custom_additional_groups: Optional[Dict[str, List[str]]] = None,
+    group_by_type: bool = True,
+    group_by_account: bool = False,
     group_by_location: bool = False,
+    custom_aggregate_type_groups: Optional[Dict[str, List[str]]] = None,
 ) -> Dict[str, Query]:
     """
-    An asset group is defined by Asset queries. Each query has a name, and we prefer pluralised names.
-    They still need an executive call, like all(), count() or first().
+    An asset group is defined by Asset queries, which this function can generate.
+
+    Each query has a name (for the asset group it represents).
+    These queries still need an executive call, like all(), count() or first().
 
     This function limits the assets to be queried to the current user's account,
     if the user is not an admin.
 
-    Note: Make sure the current user has the "read" permission on his account (on GenericAsset.__class__?? See https://github.com/FlexMeasures/flexmeasures/issues/200).
+    Note: Make sure the current user has the "read" permission on their account (on GenericAsset.__class__?? See https://github.com/FlexMeasures/flexmeasures/issues/200) or is an admin.
 
-    :param custom_additional_groups: dict of asset type groupings (mapping group names to names of asset types). See also the setting FLEXMEASURES_ASSET_TYPE_GROUPS.
+    :param group_by_type: If True, groups will be made for assets with the same type. We prefer pluralised group names here. Defaults to True.
+    :param group_by_account: If True, groups will be made for assets within the same account. This makes sense for admins, as they can query across accounts.
     :param group_by_location: If True, groups will be made for assets at the same location. Naming of the location currently supports charge points (for EVSEs).
+    :param custom_aggregate_type_groups: dict of asset type groupings (mapping group names to names of asset types). See also the setting FLEXMEASURES_ASSET_TYPE_GROUPS.
     """
     asset_queries = {}
 
     # 1. Custom asset groups by combinations of asset types
-    if custom_additional_groups:
-        for asset_type_group_name, asset_types in custom_additional_groups.items():
+    if custom_aggregate_type_groups:
+        for asset_type_group_name, asset_types in custom_aggregate_type_groups.items():
             asset_queries[asset_type_group_name] = query_assets_by_type(asset_types)
 
-    # 2. We also include a group per asset type - using the pluralised asset type name
-    for asset_type in GenericAssetType.query.all():
-        asset_queries[pluralize(asset_type.name)] = query_assets_by_type(
-            asset_type.name
-        )
-
-    # 3. Finally, we group assets by location
+    # 2. Include a group per asset type - using the pluralised asset type name
+    if group_by_type:
+        for asset_type in GenericAssetType.query.all():
+            asset_queries[pluralize(asset_type.name)] = query_assets_by_type(
+                asset_type.name
+            )
+
+    # 3. Include a group per account (admins only)  # TODO: we can later adjust this for accounts who admin certain others, not all
+    if group_by_account and user_has_admin_access(current_user, "read"):
+        for account in Account.query.all():
+            asset_queries[account.name] = GenericAsset.query.filter(
+                GenericAsset.account_id == account.id
+            )
+
+    # 4. Finally, we can group assets by location
     if group_by_location:
         asset_queries.update(get_location_queries())
 

flexmeasures/ui/__init__.py

@@ -12,6 +12,7 @@
 from flexmeasures.utils.flexmeasures_inflection import (
     capitalize,
     parameterize,
+    pluralize,
 )
 from flexmeasures.utils.time_utils import (
     localized_datetime_str,
@@ -128,6 +129,7 @@ def add_jinja_filters(app):
     app.jinja_env.filters["naturalized_datetime"] = naturalized_datetime_str
     app.jinja_env.filters["naturalized_timedelta"] = naturaldelta
     app.jinja_env.filters["capitalize"] = capitalize
+    app.jinja_env.filters["pluralize"] = pluralize
     app.jinja_env.filters["parameterize"] = parameterize
     app.jinja_env.filters["isnull"] = pd.isnull
     app.jinja_env.filters["hide_nan_if_desired"] = (

flexmeasures/ui/crud/assets.py

@@ -1,4 +1,4 @@
-from typing import Union, Optional, Tuple
+from typing import Union, Optional, List, Tuple
 import copy
 
 from flask import url_for, current_app
@@ -7,7 +7,7 @@
 from flask_security import login_required, current_user
 from wtforms import StringField, DecimalField, SelectField
 from wtforms.validators import DataRequired
-from flexmeasures.auth.policy import ADMIN_ROLE
+from flexmeasures.auth.policy import user_has_admin_access
 
 from flexmeasures.data import db
 from flexmeasures.auth.error_handling import unauthorized_handler
@@ -112,6 +112,8 @@ def process_internal_api_response(
     """
     Turn data from the internal API into something we can use to further populate the UI.
     Either as an asset object or a dict for form filling.
+
+    If we add other data by querying the database, we make sure the asset is not in the session afterwards.
     """
 
     def expunge_asset():
@@ -127,12 +129,14 @@ def expunge_asset():
         asset.generic_asset_type = GenericAssetType.query.get(
             asset.generic_asset_type_id
         )
+        expunge_asset()
+        asset.owner = Account.query.get(asset_data["account_id"])
+        expunge_asset()
         if "id" in asset_data:
-            expunge_asset()
             asset.sensors = Sensor.query.filter(
                 Sensor.generic_asset_id == asset_data["id"]
             ).all()
-        expunge_asset()
+            expunge_asset()
         return asset
     return asset_data
 
@@ -149,16 +153,33 @@ class AssetCrudUI(FlaskView):
 
     @login_required
     def index(self, msg=""):
-        """/assets"""
-        get_assets_response = InternalApi().get(
-            url_for("AssetAPI:index"), query={"account_id": current_user.account_id}
-        )
-        assets = [
-            process_internal_api_response(ad, make_obj=True)
-            for ad in get_assets_response.json()
-        ]
+        """GET from /assets
+
+        List the user's assets. For admins, list across all accounts.
+        """
+        assets = []
+
+        def get_asset_by_account(account_id) -> List[GenericAsset]:
+            if account_id is not None:
+                get_assets_response = InternalApi().get(
+                    url_for("AssetAPI:index"), query={"account_id": account_id}
+                )
+            else:
+                get_assets_response = InternalApi().get(url_for("AssetAPI:public"))
+            return [
+                process_internal_api_response(ad, make_obj=True)
+                for ad in get_assets_response.json()
+            ]
+
+        if user_has_admin_access(current_user, "read"):
+            for account in Account.query.all():
+                assets += get_asset_by_account(account.id)
+            assets += get_asset_by_account(account_id=None)
+        else:
+            assets = get_asset_by_account(current_user.account_id)
+
         return render_flexmeasures_template(
-            "crud/assets.html", account=current_user.account, assets=assets, message=msg
+            "crud/assets.html", assets=assets, message=msg
         )
 
     @login_required
@@ -347,7 +368,7 @@ def _set_account(asset_form: NewAssetForm) -> Tuple[Optional[Account], Optional[
     account_error = None
 
     if asset_form.account_id.data == -1:
-        if current_user.has_role(ADMIN_ROLE):
+        if user_has_admin_access(current_user, "update"):
             return None, None  # Account can be None (public asset)
         else:
             account_error = "Please pick an existing account."