From a0053a4c88eedf67a6fd97053256960accbff887 Mon Sep 17 00:00:00 2001 From: JDev <45713692+joshuaunity@users.noreply.github.com> Date: Mon, 23 Sep 2024 16:38:42 +0100 Subject: [PATCH] Feat users pagination (#1160) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: dynamic account colors Added options to add a priamary and secondary color when creating an account Also fixed typo comment A line in the css wasnt commented properly * chore: removed experimental code * refactor: edgecase handling and more - Handled edge case for fetching color setting when accoutn has no consultant - wrote test for color computation utils - type hints additons - ignored some docker volume folders * chore: clearing unwanted DB changes * refactor: code improvements - type hinting - segregattion of fucntionalities The color hex validation is now in a seperate fuciton in the coding_utils file * chore: removed Alembic comments * chore: linting code * fix: removed unwanted classes * fix: removed unwanted css clases * chore: initialize repo * chore: added '#' to default secondary color * feat: account logo This feature gives the option to add a logo url to an account * fix: revert utils.py accidentally replaced its content with that of another file * chore: remove unnecessary validation * chore: typo fix and migration file cleanup * feat: added extra validation to check logo url length * feat: logourl fallback this feature checks te consultant account if the main account desint have a logo set If both arent set it fallback to the flexmeasures account logo * fix: url regex validation error additional pattern was needed to compliment the existing pattern * refactor: regex patterns for path and port * refactor: regex patterns for query params * docs: updated change log with new features changes (white labeling - logo url) * docs: fixed typo with wrong url linking * feat: api to edit account details * refactor: fallback for empty account - handle edgecase for rendering brand logo incase user is not logged it * feat: edit accotun popup - also modified preremdered variables to also send over authtoken * chore: consolidating migration files pt1 * chore: consolidating migration files pt2 * feat: added logo_url field to edit account form * chore: track account editing action with auditlog * docs: updated changelogs on details on new edit account feature * Revert "chore: consolidating migration files pt2" This reverts commit 4bf3df314123b9ef66c8e3c8f6081c4e76f32930. * Revert "chore: consolidating migration files pt1" This reverts commit 4a345dfc63ccd17e9707cf6bd3e7137a220bd588. * chore: db migration consolidate * refactor: detailed event message * refactor: removed auth_token from server rendered variables * feat: added validation to edit account API * refactor: edit form - reordered form inputs - form data code linting(reduced to one line) * chore: added extra info to edit form inputs * refactor: switch to select-option for consultancy account input field * refactor: logic to display consultancy account name into select field default if it exist * refactor: edit account form logic - show proper bootstarp popup for API responses - show consuntant account input field for only admin * chore: removed print statements * chore: checking GH actions * fix: handle empty logo url for add accoutn command * refactor: moved shard utils to general utils location * feat: pagination UI integration * feat: accounts pagination API integration * feat: users pagination API integration * chore: seperating accoutns pagainaion integration * chore: clean up UI * refactor: integrated pagination logic to users sections in account detail page * chore: reduced users per page to improve UI * chore: imported annotiation to suppoer or datatypes declarations * refactori: updated users test with newchanges due to api changes * refactor: modificatins to tests due to API changes * refactor: modificatins to tests due to API changes pt2 * refactor: modified tests relating to users API * chore: clearing print statements * refactor: mofdified test due to user API changes Signed-off-by: joshuaunity * refactor: post merge refactoring stabilizing user pagination feature after confflict resolution - integrated user pagination and direct API call in accounts detail page - updated some test and fix some typos Signed-off-by: joshuaunity * chore: clear unused code Signed-off-by: joshuaunity * perf: user pagination improvements Signed-off-by: joshuaunity * refactor: addressing change requests - removed unused function and test - fixed unclickeable user row Signed-off-by: joshuaunity * chore: removed unused validation Signed-off-by: joshuaunity * refactor: users table - refactored users tables on both users and account detial page - integrate with dataTable package for pagaination styling and logic(navigation) Signed-off-by: joshuaunity * chore: discard renderTable function Signed-off-by: joshuaunity * refactor: code cleanup and simplification Signed-off-by: joshuaunity * fix: roles not displaying Signed-off-by: joshuaunity * refactor: raise FMvalidation error instead Signed-off-by: joshuaunity * chore: table item vertical alignment Signed-off-by: joshuaunity * refactor: page and per page validation refactoring Signed-off-by: joshuaunity * refactor: tokenize the search terms for user API Signed-off-by: joshuaunity --------- Signed-off-by: JDev <45713692+joshuaunity@users.noreply.github.com> Signed-off-by: joshuaunity Co-authored-by: Nicolas Höning --- documentation/changelog.rst | 2 +- flexmeasures/api/v3_0/accounts.py | 11 +- flexmeasures/api/v3_0/users.py | 102 +++++++++++++---- flexmeasures/data/queries/users.py | 27 +++++ flexmeasures/data/services/users.py | 37 ------ flexmeasures/ui/crud/accounts.py | 3 - flexmeasures/ui/crud/users.py | 26 +---- .../ui/templates/admin/logged_in_user.html | 2 +- flexmeasures/ui/templates/base.html | 107 ++++++++++++++++++ flexmeasures/ui/templates/crud/account.html | 101 ++++------------- .../ui/templates/crud/account_audit_log.html | 2 +- flexmeasures/ui/templates/crud/accounts.html | 2 +- .../ui/templates/crud/asset_audit_log.html | 2 +- .../ui/templates/crud/user_audit_log.html | 2 +- flexmeasures/ui/templates/crud/users.html | 94 ++++----------- flexmeasures/ui/templates/views/status.html | 4 +- flexmeasures/ui/tests/test_user_crud.py | 13 --- 17 files changed, 276 insertions(+), 261 deletions(-) create mode 100644 flexmeasures/data/queries/users.py diff --git a/documentation/changelog.rst b/documentation/changelog.rst index 3c43393e9..f9797949a 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -10,9 +10,9 @@ v0.24.0 | October XX, 2024 New features ------------- * The asset beliefs chart can now be configured to not combine legends, but show them per plot [see `PR #1176 `_] +* Speed up loading the users page, by making the pagination backend-based and adding support for that in the API [see `PR #1160 `_] * X-axis labels in CLI plots show datetime values in a readable and informative format [see `PR #1172 `_] - Infrastructure / Support ---------------------- diff --git a/flexmeasures/api/v3_0/accounts.py b/flexmeasures/api/v3_0/accounts.py index 00005a33c..e74d410f7 100644 --- a/flexmeasures/api/v3_0/accounts.py +++ b/flexmeasures/api/v3_0/accounts.py @@ -200,13 +200,14 @@ def patch(self, account_data: dict, id: int, account: Account): "logo_url", "consultancy_account_id", ] - - # Compile modified fields string - modified_fields_str = ", ".join( - field + modified_fields = { + field: getattr(account, field) for field in fields_to_check if account_data.get(field) != getattr(account, field) - ) + } + + # Compile modified fields string + modified_fields_str = ", ".join(modified_fields.keys()) for k, v in account_data.items(): setattr(account, k, v) diff --git a/flexmeasures/api/v3_0/users.py b/flexmeasures/api/v3_0/users.py index 287697507..ff6ea6926 100644 --- a/flexmeasures/api/v3_0/users.py +++ b/flexmeasures/api/v3_0/users.py @@ -1,27 +1,33 @@ +from __future__ import annotations from flask_classful import FlaskView, route from marshmallow import fields +import marshmallow.validate as validate from sqlalchemy.exc import IntegrityError -from sqlalchemy import select +from sqlalchemy import and_, select, func +from flask_sqlalchemy.pagination import SelectPagination from webargs.flaskparser import use_kwargs from flask_security import current_user, auth_required from flask_security.recoverable import send_reset_password_instructions from flask_json import as_json -from werkzeug.exceptions import Forbidden, Unauthorized +from werkzeug.exceptions import Forbidden from flexmeasures.auth.policy import check_access from flexmeasures.data.models.audit_log import AuditLog from flexmeasures.data.models.user import User as UserModel, Account from flexmeasures.api.common.schemas.users import AccountIdField, UserIdField +from flexmeasures.api.common.schemas.generic_assets import SearchFilterField +from flexmeasures.api.v3_0.assets import get_accessible_accounts +from flexmeasures.data.queries.users import query_users_by_search_terms +from flexmeasures.data.schemas.account import AccountSchema from flexmeasures.data.schemas.users import UserSchema from flexmeasures.data.services.users import ( - get_users, set_random_password, remove_cookie_and_token_access, get_audit_log_records, ) from flexmeasures.auth.decorators import permission_required_for_context from flexmeasures.data import db -from flexmeasures.utils.time_utils import server_now +from flexmeasures.utils.time_utils import server_now, naturalized_datetime_str """ API endpoints to manage users. @@ -33,6 +39,7 @@ user_schema = UserSchema() users_schema = UserSchema(many=True) partial_user_schema = UserSchema(partial=True) +account_schema = AccountSchema() class UserAPI(FlaskView): @@ -45,13 +52,27 @@ class UserAPI(FlaskView): { "account": AccountIdField(data_key="account_id", load_default=None), "include_inactive": fields.Bool(load_default=False), + "page": fields.Int( + required=False, validate=validate.Range(min=1), default=1 + ), + "per_page": fields.Int( + required=False, validate=validate.Range(min=1), default=1 + ), + "filter": SearchFilterField(required=False, default=None), }, location="query", ) @as_json - def index(self, account: Account, include_inactive: bool = False): - """API endpoint to list all users. - + def index( + self, + account: Account, + include_inactive: bool = False, + page: int | None = None, + per_page: int | None = None, + filter: str | None = None, + ): + """ + API endpoint to list all users. .. :quickref: User; Download user list @@ -91,26 +112,63 @@ def index(self, account: Account, include_inactive: bool = False): :status 403: INVALID_SENDER :status 422: UNPROCESSABLE_ENTITY """ - if account is not None: check_access(account, "read") accounts = [account] else: - accounts = [] - for account in db.session.scalars(select(Account)).all(): - try: - check_access(account, "read") - accounts.append(account) - except (Forbidden, Unauthorized): - pass - - users = [] - for account in accounts: - users += get_users( - account_name=account.name, - only_active=not include_inactive, + accounts = get_accessible_accounts() + + filter_statement = UserModel.account_id.in_([a.id for a in accounts]) + + if include_inactive is False: + filter_statement = and_(filter_statement, UserModel.active.is_(True)) + + query = query_users_by_search_terms( + search_terms=filter, filter_statement=filter_statement + ) + + if page is not None: + num_records = db.session.scalar( + select(func.count(UserModel.id)).where(filter_statement) + ) + paginated_users: SelectPagination = db.paginate( + query, per_page=per_page, page=page ) - return users_schema.dump(users), 200 + + users_response: list = [ + { + **user_schema.dump(user), + "account": account_schema.dump(user.account), + "flexmeasures_roles": [ + role.name for role in user.flexmeasures_roles + ], + "last_login_at": naturalized_datetime_str(user.last_login_at), + "last_seen_at": naturalized_datetime_str(user.last_seen_at), + } + for user in paginated_users.items + ] + response: dict | list = { + "data": users_response, + "num-records": num_records, + "filtered-records": paginated_users.total, + } + else: + users = db.session.execute(query).scalars().all() + + response = [ + { + **user_schema.dump(user), + "account": account_schema.dump(user.account), + "flexmeasures_roles": [ + role.name for role in user.flexmeasures_roles + ], + "last_login_at": naturalized_datetime_str(user.last_login_at), + "last_seen_at": naturalized_datetime_str(user.last_seen_at), + } + for user in users + ] + + return response, 200 @route("/") @use_kwargs({"user": UserIdField(data_key="id")}, location="path") diff --git a/flexmeasures/data/queries/users.py b/flexmeasures/data/queries/users.py new file mode 100644 index 000000000..84b0e04f6 --- /dev/null +++ b/flexmeasures/data/queries/users.py @@ -0,0 +1,27 @@ +from __future__ import annotations + +from sqlalchemy import select, Select, or_, and_ + +from flexmeasures.data.models.user import User as UserModel, Account + + +def query_users_by_search_terms( + search_terms: list[str] | None, + filter_statement: bool = True, +) -> Select: + select_statement = select(UserModel) + if search_terms is not None: + filter_statement = filter_statement & and_( + *( + or_( + UserModel.email.ilike(f"%{term}%"), + UserModel.username.ilike(f"%{term}%"), + UserModel.account.has(Account.name.ilike(f"%{term}%")), + ) + for term in search_terms + ) + ) + query = select_statement.where(filter_statement).order_by(UserModel.id) + else: + query = select_statement.where(filter_statement).order_by(UserModel.id) + return query diff --git a/flexmeasures/data/services/users.py b/flexmeasures/data/services/users.py index d483eba1d..c00e1f8e5 100644 --- a/flexmeasures/data/services/users.py +++ b/flexmeasures/data/services/users.py @@ -35,43 +35,6 @@ def get_user(id: str) -> User: return user -def get_users( - account_name: str | None = None, - role_name: str | None = None, - account_role_name: str | None = None, - only_active: bool = True, -) -> list[User]: - """Return a list of User objects. - The role_name parameter allows to filter by role. - Set only_active to False if you also want non-active users. - """ - user_query = select(User) - - if account_name is not None: - account = db.session.execute( - select(Account).filter_by(name=account_name) - ).scalar_one_or_none() - if not account: - raise NotFound(f"There is no account named {account_name}!") - user_query = user_query.filter_by(account=account) - - if only_active: - user_query = user_query.filter(User.active.is_(True)) - - if role_name is not None: - role = db.session.execute( - select(Role).filter_by(name=role_name) - ).scalar_one_or_none() - if role: - user_query = user_query.filter(User.flexmeasures_roles.contains(role)) - - users = db.session.scalars(user_query).all() - if account_role_name is not None: - users = [u for u in users if u.account.has_role(account_role_name)] - - return users - - def find_user_by_email(user_email: str, keep_in_session: bool = True) -> User: user_datastore = SQLAlchemySessionUserDatastore(db.session, User, Role) user = user_datastore.find_user(email=user_email) diff --git a/flexmeasures/ui/crud/accounts.py b/flexmeasures/ui/crud/accounts.py index 6130ca937..994216833 100644 --- a/flexmeasures/ui/crud/accounts.py +++ b/flexmeasures/ui/crud/accounts.py @@ -7,7 +7,6 @@ from flexmeasures.ui.crud.api_wrapper import InternalApi from flexmeasures.ui.utils.view_utils import render_flexmeasures_template from flexmeasures.ui.crud.assets import get_assets_by_account -from flexmeasures.ui.crud.users import get_users_by_account from flexmeasures.data.models.user import Account from flexmeasures.data import db @@ -58,14 +57,12 @@ def get(self, account_id: str): account["consultancy_account_name"] = consultancy_account.name assets = get_assets_by_account(account_id) assets += get_assets_by_account(account_id=None) - users = get_users_by_account(account_id, include_inactive=include_inactive) accounts = get_accounts() return render_flexmeasures_template( "crud/account.html", account=account, accounts=accounts, assets=assets, - users=users, include_inactive=include_inactive, ) diff --git a/flexmeasures/ui/crud/users.py b/flexmeasures/ui/crud/users.py index 9da810c71..bfb0764b4 100644 --- a/flexmeasures/ui/crud/users.py +++ b/flexmeasures/ui/crud/users.py @@ -74,23 +74,6 @@ def process_internal_api_response( return user_data -def get_users_by_account( - account_id: int | str, include_inactive: bool = False -) -> list[User]: - get_users_response = InternalApi().get( - url_for( - "UserAPI:index", - account_id=account_id, - include_inactive=include_inactive, - ) - ) - users = [ - process_internal_api_response(user, make_obj=True) - for user in get_users_response.json() - ] - return users - - def get_all_users(include_inactive: bool = False) -> list[User]: get_users_response = InternalApi().get( url_for( @@ -98,10 +81,7 @@ def get_all_users(include_inactive: bool = False) -> list[User]: include_inactive=include_inactive, ) ) - users = [ - process_internal_api_response(user, make_obj=True) - for user in get_users_response.json() - ] + users = [user for user in get_users_response.json()] return users @@ -113,10 +93,8 @@ class UserCrudUI(FlaskView): def index(self): """/users""" include_inactive = request.args.get("include_inactive", "0") != "0" - users = get_all_users(include_inactive) - return render_flexmeasures_template( - "crud/users.html", users=users, include_inactive=include_inactive + "crud/users.html", include_inactive=include_inactive ) @login_required diff --git a/flexmeasures/ui/templates/admin/logged_in_user.html b/flexmeasures/ui/templates/admin/logged_in_user.html index bd78a2900..9395c9255 100644 --- a/flexmeasures/ui/templates/admin/logged_in_user.html +++ b/flexmeasures/ui/templates/admin/logged_in_user.html @@ -24,7 +24,7 @@

User Overview

Logged-in user: {{ logged_in_user.username }}
- +
diff --git a/flexmeasures/ui/templates/base.html b/flexmeasures/ui/templates/base.html index 7c5a776a4..a456f187c 100644 --- a/flexmeasures/ui/templates/base.html +++ b/flexmeasures/ui/templates/base.html @@ -805,6 +805,113 @@ } + diff --git a/flexmeasures/ui/templates/crud/account.html b/flexmeasures/ui/templates/crud/account.html index 735ebcc15..93a717a31 100644 --- a/flexmeasures/ui/templates/crud/account.html +++ b/flexmeasures/ui/templates/crud/account.html @@ -188,7 +188,7 @@

Edit {{ account.name }}

Account

Account: {{ account.name }}
-
Email address
+
@@ -251,90 +251,30 @@

Account

-

Users

- -
-
- -
- +

All users

+
+ +
ID
- - - - - - - - - - - - - - {% for user in users %} - - - - - - - - - - - {% endfor %} - -
UsernameEmailRolesTimezoneLast LoginLast SeenActiveURL
{{ user.username }} - {{ user.email }} - > - - {% for role in user.flexmeasures_roles %} {{ role.name }}{{ - "," if not loop.last }} {% endfor %} - {{ user.timezone }} - {{ user.last_login_at | naturalized_datetime}} - - {{ user.last_seen_at | naturalized_datetime}} - {{ user.active }}/users/{{ user.id }}
+ class="table table-striped paginate nav-on-click" + title="View this asset" + id="usersTable" + >

Assets

@@ -400,11 +340,15 @@

Assets

{% block paginate_tables_script %} {{ super() }} {% endblock %}