From fabf1a4af60a0088686d99015647159a163dcdb7 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Wed, 11 Dec 2024 17:04:40 +0100 Subject: [PATCH 01/36] Fix: counts just for active projects and users --- server/mergin/auth/api.yaml | 2 +- server/mergin/auth/controller.py | 13 ++++---- server/mergin/stats/tasks.py | 7 +++-- server/mergin/tests/test_auth.py | 30 ++++++++++++++++++- server/mergin/tests/test_statistics.py | 2 ++ .../admin-lib/src/modules/admin/store.ts | 9 +++--- .../admin-lib/src/modules/admin/types.ts | 2 +- .../src/modules/admin/views/OverviewView.vue | 21 +++++-------- 8 files changed, 55 insertions(+), 31 deletions(-) diff --git a/server/mergin/auth/api.yaml b/server/mergin/auth/api.yaml index 39c92946..e400fbfd 100644 --- a/server/mergin/auth/api.yaml +++ b/server/mergin/auth/api.yaml @@ -924,7 +924,7 @@ components: type: object properties: active_monthly_contributors: - type: array + type: integer description: count of users who made a project change last months items: type: integer diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 6e0e9dd5..fdc12310 100644 --- a/server/mergin/auth/controller.py +++ b/server/mergin/auth/controller.py @@ -545,15 +545,12 @@ def create_user(): @auth_required(permissions=["admin"]) def get_server_usage(): data = { - "active_monthly_contributors": [ - current_app.ws_handler.monthly_contributors_count(), - current_app.ws_handler.monthly_contributors_count(month_offset=1), - current_app.ws_handler.monthly_contributors_count(month_offset=2), - current_app.ws_handler.monthly_contributors_count(month_offset=3), - ], - "projects": Project.query.count(), + "active_monthly_contributors": current_app.ws_handler.monthly_contributors_count(), + "projects": Project.query.filter(Project.removed_at.is_(None)).count(), "storage": files_size(), - "users": User.query.count(), + "users": User.query.filter( + is_(User.username.ilike("deleted_%"), False) | is_(User.active, True) + ).count(), "workspaces": current_app.ws_handler.workspace_count(), } return data, 200 diff --git a/server/mergin/stats/tasks.py b/server/mergin/stats/tasks.py index 762517f8..f45681f5 100644 --- a/server/mergin/stats/tasks.py +++ b/server/mergin/stats/tasks.py @@ -7,6 +7,7 @@ import json import logging from flask import current_app +from sqlalchemy.sql.operators import is_ from .models import MerginInfo from ..celery import celery @@ -60,8 +61,10 @@ def send_statistics(): "url": current_app.config["MERGIN_BASE_URL"], "contact_email": current_app.config["CONTACT_EMAIL"], "licence": current_app.config["SERVER_TYPE"], - "projects_count": Project.query.count(), - "users_count": User.query.count(), + "projects_count": Project.query.filter(Project.removed_at.is_(None)).count(), + "users_count": User.query.filter( + is_(User.username.ilike("deleted_%"), False) | is_(User.active, True) + ).count(), "workspaces_count": current_app.ws_handler.workspace_count(), "last_change": str(last_change_item.updated) + "Z" if last_change_item else "", "server_version": current_app.config["VERSION"], diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 2613c68f..7365c12a 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -10,6 +10,8 @@ from sqlalchemy import desc from unittest.mock import patch +from mergin.tests import test_workspace + from ..auth.models import User, UserProfile, LoginHistory from ..auth.tasks import anonymize_removed_users from ..app import db @@ -21,7 +23,14 @@ test_workspace_name, test_project, ) -from .utils import add_user, login_as_admin, login +from .utils import ( + add_user, + create_project, + create_workspace, + login_as_admin, + login, + upload_file_to_project, +) @pytest.fixture(scope="function") @@ -838,3 +847,22 @@ def test_username_generation(client): user = add_user("user25", "user") assert User.generate_username(user.email) == user.username + "1" + + +def test_server_usage(client): + """Test server usage endpoint""" + login_as_admin(client) + workspace = create_workspace() + user = add_user() + admin = User.query.filter_by(username="mergin").first() + # create new project + project = create_project("project", workspace, admin) + project.set_role(user.id, ProjectRole.READER) + upload_file_to_project(project, "test.txt", client) + resp = client.get("/app/admin/usage") + assert resp.status_code == 200 + assert resp.json["users"] == 2 + assert resp.json["workspaces"] == 1 + assert resp.json["projects"] == 2 + assert resp.json["storage"] == "454 kB" + assert resp.json["active_monthly_contributors"] == 1 diff --git a/server/mergin/tests/test_statistics.py b/server/mergin/tests/test_statistics.py index 2c25116e..01a4c5c2 100644 --- a/server/mergin/tests/test_statistics.py +++ b/server/mergin/tests/test_statistics.py @@ -54,6 +54,8 @@ def test_send_statistics(app, caplog): assert data["service_uuid"] == app.config["SERVICE_ID"] assert data["licence"] == "ce" assert data["monthly_contributors"] == 1 + assert data["users_count"] == 1 + assert data["projects_count"] == 1 assert data["contact_email"] == "test@example.com" # repeated action does not do anything diff --git a/web-app/packages/admin-lib/src/modules/admin/store.ts b/web-app/packages/admin-lib/src/modules/admin/store.ts index a86cb042..7b9281ff 100644 --- a/web-app/packages/admin-lib/src/modules/admin/store.ts +++ b/web-app/packages/admin-lib/src/modules/admin/store.ts @@ -42,6 +42,7 @@ export interface AdminState { checkForUpdates?: boolean isServerConfigHidden: boolean latestServerVersion?: LatestServerVersionResponse + usage: ServerUsageResponse } const cookies = new Cookies() @@ -62,7 +63,8 @@ export const useAdminStore = defineStore('adminModule', { user: null, checkForUpdates: undefined, isServerConfigHidden: false, - latestServerVersion: undefined + latestServerVersion: undefined, + usage: undefined }), getters: { displayUpdateAvailable: (state) => { @@ -112,9 +114,6 @@ export const useAdminStore = defineStore('adminModule', { setIsServerConfigHidden(value: boolean) { this.isServerConfigHidden = value }, - setUsage(data: ServerUsageResponse){ - this.usage = data - }, async fetchUsers(payload: { params: PaginatedUsersParams }) { const notificationStore = useNotificationStore() @@ -322,7 +321,7 @@ export const useAdminStore = defineStore('adminModule', { const notificationStore = useNotificationStore() try { const response = await AdminApi.getServerUsage() - this.setUsage(response.data) + this.usage = response.data } catch (e) { notificationStore.error({ text: errorUtils.getErrorMessage(e) }) } diff --git a/web-app/packages/admin-lib/src/modules/admin/types.ts b/web-app/packages/admin-lib/src/modules/admin/types.ts index d099f070..31e0a02d 100644 --- a/web-app/packages/admin-lib/src/modules/admin/types.ts +++ b/web-app/packages/admin-lib/src/modules/admin/types.ts @@ -72,7 +72,7 @@ export interface PaginatedAdminProjectsParams extends PaginatedRequestParams { export type ServerUsageResponse = ServerUsage export interface ServerUsage { - active_monthly_contributors: number[] + active_monthly_contributors: number projects: number storage: string users: number diff --git a/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue b/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue index 6c9fbbfc..fdc4d5bf 100644 --- a/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue +++ b/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue @@ -14,10 +14,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
- +
{{ usage?.active_monthly_contributors }} @@ -67,7 +62,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
{{ usage?.users }} @@ -109,7 +104,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial {{ usage?.workspaces }} - - - +
{{ usage?.storage }} - +
- +
+ + +
+ +
+ + + +
Date: Thu, 12 Dec 2024 15:34:22 +0100 Subject: [PATCH 04/36] Do not count admins --- server/mergin/sync/workspace.py | 6 +----- server/mergin/tests/test_workspace.py | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 3fd506df..826445f9 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -354,13 +354,9 @@ def project_access(self, project: Project) -> List[ProjectAccessDetail]: def server_editors_count(self) -> int: if Configuration.GLOBAL_ADMIN or Configuration.GLOBAL_WRITE: - return User.query.filter( - is_(User.username.ilike("deleted_%"), False) | User.active - ).count() - if Configuration.GLOBAL_READ: return User.query.filter( is_(User.username.ilike("deleted_%"), False) | User.active, - User.is_admin.is_(True), + User.is_admin.is_(False), ).count() return ( diff --git a/server/mergin/tests/test_workspace.py b/server/mergin/tests/test_workspace.py index 2aafc268..47c62981 100644 --- a/server/mergin/tests/test_workspace.py +++ b/server/mergin/tests/test_workspace.py @@ -45,7 +45,7 @@ def test_workspace_implementation(client): Configuration.GLOBAL_WRITE = True assert ws.user_has_permissions(user, "write") assert ws.user_has_permissions(user, "read") - assert handler.server_editors_count() == 2 + assert handler.server_editors_count() == 1 assert not ws.user_has_permissions(user, "admin") assert not ws.user_has_permissions(user, "owner") @@ -76,7 +76,7 @@ def test_workspace_implementation(client): project.removed_by = user.id db.session.commit() assert ws.disk_usage() == default_project_usage - assert handler.server_editors_count() == 2 + assert handler.server_editors_count() == 1 current_time = datetime.datetime.now(datetime.timezone.utc) latest_version.created = datetime.datetime.combine( From e5055982d8ff3c83c840edc7234699460ae688c7 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Thu, 12 Dec 2024 15:40:16 +0100 Subject: [PATCH 05/36] Fix storage usage for no files --- server/mergin/sync/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/mergin/sync/utils.py b/server/mergin/sync/utils.py index e8c9f367..72426d01 100644 --- a/server/mergin/sync/utils.py +++ b/server/mergin/sync/utils.py @@ -336,4 +336,4 @@ def files_size(): SELECT pg_size_pretty(SUM(sum)) FROM partials; """ ) - return db.session.execute(files_size).scalar() + return db.session.execute(files_size).scalar() or "0 bytes" From c5099773f3ce6e6b95109d6990f78e1622b44fcb Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Thu, 12 Dec 2024 16:22:41 +0100 Subject: [PATCH 06/36] Revert admin handling --- server/mergin/sync/workspace.py | 1 - server/mergin/tests/test_workspace.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 826445f9..3f2bc397 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -356,7 +356,6 @@ def server_editors_count(self) -> int: if Configuration.GLOBAL_ADMIN or Configuration.GLOBAL_WRITE: return User.query.filter( is_(User.username.ilike("deleted_%"), False) | User.active, - User.is_admin.is_(False), ).count() return ( diff --git a/server/mergin/tests/test_workspace.py b/server/mergin/tests/test_workspace.py index 47c62981..2aafc268 100644 --- a/server/mergin/tests/test_workspace.py +++ b/server/mergin/tests/test_workspace.py @@ -45,7 +45,7 @@ def test_workspace_implementation(client): Configuration.GLOBAL_WRITE = True assert ws.user_has_permissions(user, "write") assert ws.user_has_permissions(user, "read") - assert handler.server_editors_count() == 1 + assert handler.server_editors_count() == 2 assert not ws.user_has_permissions(user, "admin") assert not ws.user_has_permissions(user, "owner") @@ -76,7 +76,7 @@ def test_workspace_implementation(client): project.removed_by = user.id db.session.commit() assert ws.disk_usage() == default_project_usage - assert handler.server_editors_count() == 1 + assert handler.server_editors_count() == 2 current_time = datetime.datetime.now(datetime.timezone.utc) latest_version.created = datetime.datetime.combine( From d3598a38a369a0115178be34185da27b9f397cc5 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Fri, 13 Dec 2024 08:51:04 +0100 Subject: [PATCH 07/36] Add fallback 0 size for no files in storage server usage --- server/mergin/sync/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/mergin/sync/utils.py b/server/mergin/sync/utils.py index 72426d01..9efcd284 100644 --- a/server/mergin/sync/utils.py +++ b/server/mergin/sync/utils.py @@ -333,7 +333,7 @@ def files_size(): LEFT OUTER JOIN file_history fh ON fh.id = lf.file_id WHERE fh.change = 'update_diff'::push_change_type ) - SELECT pg_size_pretty(SUM(sum)) FROM partials; + SELECT pg_size_pretty(COALESCE(SUM(sum), 0)) FROM partials; """ ) - return db.session.execute(files_size).scalar() or "0 bytes" + return db.session.execute(files_size).scalar() From 6cd9639fb884a24932d5ca8c47cd4e2153b6c718 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Fri, 13 Dec 2024 16:33:45 +0100 Subject: [PATCH 08/36] Add editor limit hit blocking --- server/mergin/sync/public_api.yaml | 17 +++++++++++++++ server/mergin/sync/public_api_controller.py | 23 +++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/server/mergin/sync/public_api.yaml b/server/mergin/sync/public_api.yaml index 48d39c13..c8072ad6 100644 --- a/server/mergin/sync/public_api.yaml +++ b/server/mergin/sync/public_api.yaml @@ -1044,6 +1044,23 @@ components: detail: Maximum number of people in this workspace is reached. Please upgrade your subscription to add more people (UsersLimitHit) rejected_emails: [ rejected@example.com ] users_quota: 6 + EditorsLimitHit: + allOf: + - $ref: '#/components/schemas/CustomError' + type: object + properties: + rejected_emails: + nullable: true + type: array + items: + type: string + editors_quota: + type: integer + example: + code: EditorsLimitHit + detail: Maximum number of editors in this workspace is reached. Please upgrade your subscription to add more (EditorsLimitHit) + rejected_emails: [ rejected@example.com ] + editors_quota: 6 UpdateProjectAccessError: allOf: - $ref: '#/components/schemas/CustomError' diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index 77faeec2..6a39e441 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -680,11 +680,34 @@ def update_project(namespace, project_name): # noqa: E501 # pylint: disable=W0 """ project = require_project(namespace, project_name, ProjectPermissions.Update) parsed_access = parse_project_access_update_request(request.json.get("access", {})) + + # get current status for easier rollback + modified_user_ids = [] + for role in list(ProjectRole.__reversed__()): + modified_user_ids.extend(parsed_access.get(role, [])) + current_permissions_map = { + user_id: project.get_role(user_id) for user_id in modified_user_ids + } + # get set of modified user_ids and possible (custom) errors id_diffs, error = current_app.ws_handler.update_project_members( project, parsed_access ) + # revert back rejected changes + if error and hasattr(error, "rejected_emails"): + rejected_users = ( + db.session.query(User.id) + .filter(User.email.in_(error.rejected_emails)) + .all() + ) + for user in rejected_users: + if current_permissions_map[user.id] is None: + project.unset_role(user.id) + else: + project.set_role(user.id, current_permissions_map[user.id]) + db.session.commit() + if not id_diffs and error: # nothing was done but there are errors return jsonify(error.to_dict()), 422 From 8a518aff31a0271e3dce3bad2156ea24015954af Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Fri, 13 Dec 2024 16:47:42 +0100 Subject: [PATCH 09/36] Adjust projectApi --- .../lib/src/modules/project/projectApi.ts | 28 ++++++++++++++++--- .../packages/lib/src/modules/project/types.ts | 6 ++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/web-app/packages/lib/src/modules/project/projectApi.ts b/web-app/packages/lib/src/modules/project/projectApi.ts index ec9b4789..01b63cd8 100644 --- a/web-app/packages/lib/src/modules/project/projectApi.ts +++ b/web-app/packages/lib/src/modules/project/projectApi.ts @@ -25,7 +25,7 @@ import { ProjectVersion, ProjectAccessDetail, ProjectAccess, - ProjectVersionFileChange + ProjectVersionFileChange, CreateProjectAccessParams } from '@/modules/project/types' export const ProjectApi = { @@ -175,12 +175,32 @@ export const ProjectApi = { async updateProjectAccess( id: string, + userId: number, data: UpdateProjectAccessParams, withRetry?: boolean ): Promise> { - return ProjectModule.httpService.patch(`/app/project/${id}/access`, data, { - ...(withRetry ? getDefaultRetryOptions() : {}) - }) + return ProjectModule.httpService.patch( + `/v2/projects/${id}/collaborators/${userId}`, + data, + { + ...(withRetry ? getDefaultRetryOptions() : {}) + } + ) + }, + + async createProjectAccess( + id: string, + userId: number, + data: CreateProjectAccessParams, + withRetry?: boolean + ): Promise> { + return ProjectModule.httpService.post( + `/v2/projects/${id}/collaborators/${userId}`, + data, + { + ...(withRetry ? getDefaultRetryOptions() : {}) + } + ) }, async pushProjectChanges( diff --git a/web-app/packages/lib/src/modules/project/types.ts b/web-app/packages/lib/src/modules/project/types.ts index 8260991f..3f471334 100644 --- a/web-app/packages/lib/src/modules/project/types.ts +++ b/web-app/packages/lib/src/modules/project/types.ts @@ -298,6 +298,12 @@ export interface UpdateProjectAccessParams { public?: boolean } +export interface CreateProjectAccessParams { + user_id?: number + role?: ProjectRoleName + public?: boolean +} + export interface DownloadPayload { url: string } From 1d4c45ac86c9cfdbcff8461e564948badb853798 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 16 Dec 2024 10:38:36 +0100 Subject: [PATCH 10/36] Address commnets @varmar05 - updatte returning storage size to number not formated string --- server/mergin/auth/api.yaml | 4 ++-- server/mergin/sync/utils.py | 2 +- server/mergin/sync/workspace.py | 2 +- server/mergin/tests/test_auth.py | 7 ++++++- .../src/modules/admin/views/OverviewView.vue | 16 +++++----------- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/server/mergin/auth/api.yaml b/server/mergin/auth/api.yaml index e400fbfd..df924109 100644 --- a/server/mergin/auth/api.yaml +++ b/server/mergin/auth/api.yaml @@ -934,9 +934,9 @@ components: description: total number of projects example: 12 storage: - type: string + type: number description: projest files size in bytes - example: 1024 kB + example: 1024 users: type: integer description: count of registered accounts diff --git a/server/mergin/sync/utils.py b/server/mergin/sync/utils.py index 9efcd284..b4120acc 100644 --- a/server/mergin/sync/utils.py +++ b/server/mergin/sync/utils.py @@ -333,7 +333,7 @@ def files_size(): LEFT OUTER JOIN file_history fh ON fh.id = lf.file_id WHERE fh.change = 'update_diff'::push_change_type ) - SELECT pg_size_pretty(COALESCE(SUM(sum), 0)) FROM partials; + SELECT COALESCE(SUM(sum), 0) FROM partials; """ ) return db.session.execute(files_size).scalar() diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 3f2bc397..8cc0a27a 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -355,7 +355,7 @@ def project_access(self, project: Project) -> List[ProjectAccessDetail]: def server_editors_count(self) -> int: if Configuration.GLOBAL_ADMIN or Configuration.GLOBAL_WRITE: return User.query.filter( - is_(User.username.ilike("deleted_%"), False) | User.active, + is_(User.username.ilike("deleted_%"), False), ).count() return ( diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 569def6e..71a35e29 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial from datetime import datetime, timedelta +import os import pytest import json from flask import url_for @@ -30,6 +31,7 @@ login_as_admin, login, upload_file_to_project, + test_project_dir, ) @@ -853,6 +855,9 @@ def test_server_usage(client): """Test server usage endpoint""" login_as_admin(client) workspace = create_workspace() + init_project = Project.query.filter_by(workspace_id=workspace.id).first() + resp = client.get("/app/admin/usage") + print(resp.json) user = add_user() admin = User.query.filter_by(username="mergin").first() # create new project @@ -865,7 +870,7 @@ def test_server_usage(client): assert resp.json["users"] == 2 assert resp.json["workspaces"] == 1 assert resp.json["projects"] == 2 - assert resp.json["storage"] == "454 kB" + assert resp.json["storage"] == project.disk_usage + init_project.disk_usage assert resp.json["active_monthly_contributors"] == 1 assert resp.json["editors"] == 1 project.set_role(user.id, ProjectRole.EDITOR) diff --git a/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue b/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue index 36bf6211..39bacdec 100644 --- a/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue +++ b/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue @@ -31,15 +31,17 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
{{ + $filters.filesize(usage.storage, null, 0) + }} @@ -98,14 +100,6 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial >{{ usage?.active_monthly_contributors }} - From 3c2c186f6fa3c114f8943cdf384bb47d4e357552 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 16 Dec 2024 11:42:38 +0100 Subject: [PATCH 11/36] Fix removed filtering based just on deleted_ and add tests --- server/mergin/auth/controller.py | 2 +- server/mergin/stats/tasks.py | 2 +- server/mergin/tests/test_auth.py | 9 ++++++-- server/mergin/tests/test_statistics.py | 31 ++++++++++++++++++++++---- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 05c52d9a..8b693ed3 100644 --- a/server/mergin/auth/controller.py +++ b/server/mergin/auth/controller.py @@ -549,7 +549,7 @@ def get_server_usage(): "projects": Project.query.filter(Project.removed_at.is_(None)).count(), "storage": files_size(), "users": User.query.filter( - is_(User.username.ilike("deleted_%"), False) | User.active, + is_(User.username.ilike("deleted_%"), False), ).count(), "workspaces": current_app.ws_handler.workspace_count(), "editors": current_app.ws_handler.server_editors_count(), diff --git a/server/mergin/stats/tasks.py b/server/mergin/stats/tasks.py index 34bb24f5..da7a9cb6 100644 --- a/server/mergin/stats/tasks.py +++ b/server/mergin/stats/tasks.py @@ -63,7 +63,7 @@ def send_statistics(): "licence": current_app.config["SERVER_TYPE"], "projects_count": Project.query.filter(Project.removed_at.is_(None)).count(), "users_count": User.query.filter( - is_(User.username.ilike("deleted_%"), False) | is_(User.active, True) + is_(User.username.ilike("deleted_%"), False) ).count(), "workspaces_count": current_app.ws_handler.workspace_count(), "last_change": str(last_change_item.updated) + "Z" if last_change_item else "", diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 71a35e29..91027fb4 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -856,8 +856,6 @@ def test_server_usage(client): login_as_admin(client) workspace = create_workspace() init_project = Project.query.filter_by(workspace_id=workspace.id).first() - resp = client.get("/app/admin/usage") - print(resp.json) user = add_user() admin = User.query.filter_by(username="mergin").first() # create new project @@ -877,3 +875,10 @@ def test_server_usage(client): db.session.commit() resp = client.get("/app/admin/usage") assert resp.json["editors"] == 2 + user.inactivate() + user.anonymize() + project.delete() + resp = client.get("/app/admin/usage") + assert resp.json["editors"] == 1 + assert resp.json["users"] == 1 + assert resp.json["projects"] == 1 diff --git a/server/mergin/tests/test_statistics.py b/server/mergin/tests/test_statistics.py index 3982535e..b73cb042 100644 --- a/server/mergin/tests/test_statistics.py +++ b/server/mergin/tests/test_statistics.py @@ -5,11 +5,15 @@ import json from unittest.mock import patch import requests +from sqlalchemy.sql.operators import is_ + +from mergin.auth.models import User +from mergin.sync.models import Project, ProjectRole from ..app import db from ..stats.tasks import send_statistics from ..stats.models import MerginInfo -from .utils import Response +from .utils import Response, add_user, create_project, create_workspace def test_send_statistics(app, caplog): @@ -24,6 +28,12 @@ def test_send_statistics(app, caplog): mock.return_value = Response(True, {}) app.config["COLLECT_STATISTICS"] = False app.config["CONTACT_EMAIL"] = "test@example.com" + user = add_user() + admin = User.query.filter_by(username="mergin").first() + # create new project + workspace = create_workspace() + project = create_project("project", workspace, admin) + project.set_role(user.id, ProjectRole.EDITOR) task = send_statistics.s().apply() # nothing was done assert task.status == "SUCCESS" @@ -55,16 +65,29 @@ def test_send_statistics(app, caplog): assert data["service_uuid"] == app.config["SERVICE_ID"] assert data["licence"] == "ce" assert data["monthly_contributors"] == 1 - assert data["users_count"] == 1 - assert data["projects_count"] == 1 + assert data["users_count"] == 2 + assert data["projects_count"] == 2 assert data["contact_email"] == "test@example.com" - assert data["editors"] == 1 + assert data["editors"] == 2 # repeated action does not do anything task = send_statistics.s().apply() assert task.status == "SUCCESS" assert info.last_reported == ts + # project removed / users removed in time + info.last_reported = None + project.delete() + user.inactivate() + user.anonymize() + db.session.commit() + task = send_statistics.s().apply() + url, data = mock.call_args + data = json.loads(data["data"]) + assert data["projects_count"] == 1 + assert data["users_count"] == 1 + assert data["editors"] == 1 + info.last_reported = None db.session.commit() # server responds with non-ok status From c2aedea823bd740c3bb9ab1df132abc423bf5d34 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 17 Dec 2024 14:19:28 +0100 Subject: [PATCH 12/36] Update project access api to v2 endpoint --- .../components/ProjectMembersTable.vue | 3 +- .../lib/src/modules/project/projectApi.ts | 23 +++++++---- .../packages/lib/src/modules/project/store.ts | 41 +++++++++++++++---- .../packages/lib/src/modules/project/types.ts | 10 ++--- .../views/ProjectSettingsViewTemplate.vue | 4 +- 5 files changed, 56 insertions(+), 25 deletions(-) diff --git a/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue b/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue index 3b7c284f..f96f9e6d 100644 --- a/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue +++ b/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue @@ -163,7 +163,8 @@ function removeMember(item: ProjectAccessDetail) { function roleUpdate(item: ProjectAccessDetail, value: ProjectRoleName) { projectStore.updateProjectAccess({ projectId: projectStore.project.id, - data: { role: value, user_id: item.id } + userId: item.id, + data: { role: value } }) } diff --git a/web-app/packages/lib/src/modules/project/projectApi.ts b/web-app/packages/lib/src/modules/project/projectApi.ts index 01b63cd8..fbd77fdd 100644 --- a/web-app/packages/lib/src/modules/project/projectApi.ts +++ b/web-app/packages/lib/src/modules/project/projectApi.ts @@ -21,11 +21,12 @@ import { PushProjectChangesParams, PushProjectChangesResponse, SaveProjectSettings, - UpdateProjectAccessParams, ProjectVersion, ProjectAccessDetail, ProjectAccess, - ProjectVersionFileChange, CreateProjectAccessParams + ProjectVersionFileChange, + UpdateProjectPayload, + UpdatePublicFlagParams } from '@/modules/project/types' export const ProjectApi = { @@ -176,7 +177,7 @@ export const ProjectApi = { async updateProjectAccess( id: string, userId: number, - data: UpdateProjectAccessParams, + data: UpdateProjectPayload, withRetry?: boolean ): Promise> { return ProjectModule.httpService.patch( @@ -188,15 +189,23 @@ export const ProjectApi = { ) }, - async createProjectAccess( + async updatePublicFlag( + id: string, + data: UpdatePublicFlagParams, + withRetry?: boolean + ): Promise> { + return ProjectModule.httpService.patch(`/app/project/${id}/access`, data, { + ...(withRetry ? getDefaultRetryOptions() : {}) + }) + }, + + async removeProjectAccess( id: string, userId: number, - data: CreateProjectAccessParams, withRetry?: boolean ): Promise> { - return ProjectModule.httpService.post( + return ProjectModule.httpService.delete( `/v2/projects/${id}/collaborators/${userId}`, - data, { ...(withRetry ? getDefaultRetryOptions() : {}) } diff --git a/web-app/packages/lib/src/modules/project/store.ts b/web-app/packages/lib/src/modules/project/store.ts index 831e9adb..363d2ec7 100644 --- a/web-app/packages/lib/src/modules/project/store.ts +++ b/web-app/packages/lib/src/modules/project/store.ts @@ -43,7 +43,9 @@ import { ProjectAccessDetail, UpdateProjectAccessParams, ProjectVersionFileChange, - ProjectVersionListItem + ProjectVersionListItem, + UpdateProjectPayload, + UpdatePublicFlagParams } from '@/modules/project/types' import { useUserStore } from '@/modules/user/store' @@ -758,10 +760,10 @@ export const useProjectStore = defineStore('projectModule', { const notificationStore = useNotificationStore() this.accessLoading = true try { - const response = await ProjectApi.updateProjectAccess(this.project.id, { - user_id: item.id, - role: 'none' - }) + const response = await ProjectApi.removeProjectAccess( + this.project.id, + item.id + ) this.access = this.access.filter((access) => access.id !== item.id) this.project.access = response.data } catch { @@ -774,24 +776,26 @@ export const useProjectStore = defineStore('projectModule', { }, /** - * Updates the access for a user on the given project. `project.access` contains also public attribute. This attribute can be changed also. + * Updates the access for a user on the given project. * * @param payload - Object containing the project ID and access update details. * @returns Promise resolving when the API call completes. */ async updateProjectAccess(payload: { projectId: string - data: UpdateProjectAccessParams + userId: number + data: UpdateProjectPayload }) { const notificationStore = useNotificationStore() this.accessLoading = true try { const response = await ProjectApi.updateProjectAccess( payload.projectId, + payload.userId, payload.data ) this.access = this.access.map((access) => { - if (access.id === payload.data.user_id) { + if (access.id === payload.userId) { access.project_permission = payload.data.role } return access @@ -806,6 +810,27 @@ export const useProjectStore = defineStore('projectModule', { } }, + async updatePublicFlag(payload: { + projectId: string + data: UpdatePublicFlagParams + }) { + const notificationStore = useNotificationStore() + this.accessLoading = true + try { + const response = await ProjectApi.updatePublicFlag( + payload.projectId, + payload.data + ) + this.project.access = response.data + } catch { + notificationStore.error({ + text: `Failed to update public flag` + }) + } finally { + this.accessLoading = false + } + }, + async getVersionChangeset(payload: { workspace: string projectName: string diff --git a/web-app/packages/lib/src/modules/project/types.ts b/web-app/packages/lib/src/modules/project/types.ts index 3f471334..e8c39aaa 100644 --- a/web-app/packages/lib/src/modules/project/types.ts +++ b/web-app/packages/lib/src/modules/project/types.ts @@ -292,15 +292,11 @@ export type EnhancedProjectDetail = ProjectDetail & { path: string } -export interface UpdateProjectAccessParams { - user_id?: number - role?: ProjectRoleName - public?: boolean +export interface UpdateProjectPayload { + role: ProjectRoleName } -export interface CreateProjectAccessParams { - user_id?: number - role?: ProjectRoleName +export interface UpdatePublicFlagParams { public?: boolean } diff --git a/web-app/packages/lib/src/modules/project/views/ProjectSettingsViewTemplate.vue b/web-app/packages/lib/src/modules/project/views/ProjectSettingsViewTemplate.vue index a4c55ab3..e8673a7b 100644 --- a/web-app/packages/lib/src/modules/project/views/ProjectSettingsViewTemplate.vue +++ b/web-app/packages/lib/src/modules/project/views/ProjectSettingsViewTemplate.vue @@ -95,10 +95,10 @@ export default defineComponent({ } }, methods: { - ...mapActions(useProjectStore, ['deleteProject', 'updateProjectAccess']), + ...mapActions(useProjectStore, ['deleteProject', 'updatePublicFlag']), ...mapActions(useDialogStore, { showDialog: 'show' }), togglePublicPrivate() { - this.updateProjectAccess({ + this.updatePublicFlag({ projectId: this.project.id, data: { public: !this.project.access.public From 7d6b336fc9168cc8a0f1b3d870b6495e08b387b9 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Wed, 18 Dec 2024 09:52:59 +0100 Subject: [PATCH 13/36] Added method for error handling which could be reused in other applications --- .../lib/src/modules/project/projectApi.ts | 6 ++++-- .../packages/lib/src/modules/project/store.ts | 17 ++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/web-app/packages/lib/src/modules/project/projectApi.ts b/web-app/packages/lib/src/modules/project/projectApi.ts index fbd77fdd..67ff0e4e 100644 --- a/web-app/packages/lib/src/modules/project/projectApi.ts +++ b/web-app/packages/lib/src/modules/project/projectApi.ts @@ -184,7 +184,8 @@ export const ProjectApi = { `/v2/projects/${id}/collaborators/${userId}`, data, { - ...(withRetry ? getDefaultRetryOptions() : {}) + ...(withRetry ? getDefaultRetryOptions() : {}), + validateStatus } ) }, @@ -207,7 +208,8 @@ export const ProjectApi = { return ProjectModule.httpService.delete( `/v2/projects/${id}/collaborators/${userId}`, { - ...(withRetry ? getDefaultRetryOptions() : {}) + ...(withRetry ? getDefaultRetryOptions() : {}), + validateStatus } ) }, diff --git a/web-app/packages/lib/src/modules/project/store.ts b/web-app/packages/lib/src/modules/project/store.ts index 363d2ec7..d52baeae 100644 --- a/web-app/packages/lib/src/modules/project/store.ts +++ b/web-app/packages/lib/src/modules/project/store.ts @@ -41,7 +41,6 @@ import { SaveProjectSettings, ErrorCodes, ProjectAccessDetail, - UpdateProjectAccessParams, ProjectVersionFileChange, ProjectVersionListItem, UpdateProjectPayload, @@ -757,8 +756,8 @@ export const useProjectStore = defineStore('projectModule', { async removeProjectAccess( item: Pick ) { - const notificationStore = useNotificationStore() this.accessLoading = true + const notificationStore = useNotificationStore() try { const response = await ProjectApi.removeProjectAccess( this.project.id, @@ -786,7 +785,6 @@ export const useProjectStore = defineStore('projectModule', { userId: number data: UpdateProjectPayload }) { - const notificationStore = useNotificationStore() this.accessLoading = true try { const response = await ProjectApi.updateProjectAccess( @@ -801,15 +799,20 @@ export const useProjectStore = defineStore('projectModule', { return access }) this.project.access = response.data - } catch { - notificationStore.error({ - text: `Failed to update project access` - }) + } catch (err) { + this.handleProjectAccessError(err, 'Failed to update project access') } finally { this.accessLoading = false } }, + handleProjectAccessError(err: unknown, defaultMessage: string) { + const notificationStore = useNotificationStore() + notificationStore.error({ + text: getErrorMessage(err, defaultMessage) + }) + }, + async updatePublicFlag(payload: { projectId: string data: UpdatePublicFlagParams From 14b95ab019d625583c7d4c446a51baecc44089e9 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Wed, 18 Dec 2024 12:14:22 +0100 Subject: [PATCH 14/36] Use v2 api to get project collaborators and their project permission --- .../lib/src/common/permission_utils.ts | 29 ++++++++++++++++++- .../components/ProjectMembersTable.vue | 13 ++++++--- .../lib/src/modules/project/projectApi.ts | 6 ++++ .../packages/lib/src/modules/project/store.ts | 16 ++++++++++ .../packages/lib/src/modules/project/types.ts | 7 ++--- 5 files changed, 62 insertions(+), 9 deletions(-) diff --git a/web-app/packages/lib/src/common/permission_utils.ts b/web-app/packages/lib/src/common/permission_utils.ts index e810b2db..4877e2b7 100644 --- a/web-app/packages/lib/src/common/permission_utils.ts +++ b/web-app/packages/lib/src/common/permission_utils.ts @@ -4,7 +4,7 @@ import { DropdownOption } from './components/types' -import { ProjectAccess } from '@/modules' +import {ProjectAccess, ProjectAccessDetail} from '@/modules' export enum WorkspaceRole { guest, @@ -41,6 +41,14 @@ export type ProjectRoleName = | Extract | 'none' +const ROLE_HIERARCHY: ProjectRoleName[] = [ + 'none', + 'reader', + 'editor', + 'writer', + 'owner' +] + export type ProjectPermissionName = 'owner' | 'write' | 'edit' | 'read' export const USER_ROLE_NAME_BY_ROLE: Record = @@ -209,3 +217,22 @@ export function getProjectPermissionByRoleName( } return mapper[roleName] } + +export function calculateProjectPermission( + project_role: ProjectRoleName, + workspace_role: WorkspaceRoleName +): ProjectRoleName { + const mappedWorkspaceRole: ProjectRoleName = + workspace_role === 'admin' ? 'owner' : (workspace_role as ProjectRoleName) + + if (project_role === 'none' && workspace_role === 'guest') { + return 'none' + } + + const projectRoleIndex = ROLE_HIERARCHY.indexOf(project_role) + const workspaceRoleIndex = ROLE_HIERARCHY.indexOf(mappedWorkspaceRole) + + return projectRoleIndex > workspaceRoleIndex + ? project_role + : mappedWorkspaceRole +} diff --git a/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue b/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue index f96f9e6d..0ea7d66f 100644 --- a/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue +++ b/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue @@ -47,7 +47,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial ]" > { projectStore.access = [] }) -projectStore.getProjectAccess(projectStore.project?.id) +projectStore.getProjectCollaborators(projectStore.project?.id) diff --git a/web-app/packages/lib/src/modules/project/projectApi.ts b/web-app/packages/lib/src/modules/project/projectApi.ts index fbd77fdd..c1e3aba0 100644 --- a/web-app/packages/lib/src/modules/project/projectApi.ts +++ b/web-app/packages/lib/src/modules/project/projectApi.ts @@ -317,5 +317,11 @@ export const ProjectApi = { return ProjectModule.httpService.get( `/v1/resource/changesets/${workspace}/${projectName}/${versionId}?path=${path}` ) + }, + + async getProjectCollaborators( + projectId: string + ): Promise> { + return ProjectModule.httpService.get(`/v2/projects/${projectId}/collaborators`) } } diff --git a/web-app/packages/lib/src/modules/project/store.ts b/web-app/packages/lib/src/modules/project/store.ts index 363d2ec7..3193b155 100644 --- a/web-app/packages/lib/src/modules/project/store.ts +++ b/web-app/packages/lib/src/modules/project/store.ts @@ -749,6 +749,22 @@ export const useProjectStore = defineStore('projectModule', { } }, + async getProjectCollaborators(projectId: string) { + const notificationStore = useNotificationStore() + + try { + this.accessLoading = true + const response = await ProjectApi.getProjectCollaborators(projectId) + this.access = response.data + } catch { + notificationStore.error({ + text: 'Failed to get project access' + }) + } finally { + this.accessLoading = false + } + }, + /** * Removes the given user's access to the current project. * diff --git a/web-app/packages/lib/src/modules/project/types.ts b/web-app/packages/lib/src/modules/project/types.ts index e8c39aaa..1763fd3a 100644 --- a/web-app/packages/lib/src/modules/project/types.ts +++ b/web-app/packages/lib/src/modules/project/types.ts @@ -5,7 +5,7 @@ /* eslint-disable camelcase */ import { ProjectRoleName, - ProjectPermissionName + ProjectPermissionName, WorkspaceRoleName } from '@/common/permission_utils' import { PaginatedRequestParamsApi, @@ -343,9 +343,8 @@ export type ErrorCodes = 'UpdateProjectAccessError' export interface ProjectAccessDetail { id: number - type: 'member' email: string username: string - project_permission: ProjectRoleName - name: string + workspace_role: WorkspaceRoleName + project_role: ProjectRoleName } From ec4284059c62b677d91c75dabce96bdca7abf2ab Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Wed, 18 Dec 2024 12:21:29 +0100 Subject: [PATCH 15/36] Keep original interfaces --- web-app/packages/lib/src/common/permission_utils.ts | 2 +- .../project/components/ProjectMembersTable.vue | 10 +++++----- .../packages/lib/src/modules/project/projectApi.ts | 5 +++-- web-app/packages/lib/src/modules/project/types.ts | 12 +++++++++++- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/web-app/packages/lib/src/common/permission_utils.ts b/web-app/packages/lib/src/common/permission_utils.ts index 4877e2b7..a86c4123 100644 --- a/web-app/packages/lib/src/common/permission_utils.ts +++ b/web-app/packages/lib/src/common/permission_utils.ts @@ -4,7 +4,7 @@ import { DropdownOption } from './components/types' -import {ProjectAccess, ProjectAccessDetail} from '@/modules' +import { ProjectAccess } from '@/modules' export enum WorkspaceRole { guest, diff --git a/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue b/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue index 0ea7d66f..734a6cee 100644 --- a/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue +++ b/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue @@ -86,7 +86,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import { computed, ref, onUnmounted } from 'vue' import { useProjectStore } from '../store' -import { ProjectAccessDetail } from '../types' +import { ProjectCollaborators } from '../types' import AppContainer from '@/common/components/AppContainer.vue' import AppDropdown from '@/common/components/AppDropdown.vue' @@ -153,15 +153,15 @@ const searchedItems = computed(() => }) ) -function canRemoveMember(item: ProjectAccessDetail) { +function canRemoveMember(item: ProjectCollaborators) { return props.allowRemove && item.id !== loggedUser.value?.id } -function removeMember(item: ProjectAccessDetail) { +function removeMember(item: ProjectCollaborators) { projectStore.removeProjectAccess(item) } -function roleUpdate(item: ProjectAccessDetail, value: ProjectRoleName) { +function roleUpdate(item: ProjectCollaborators, value: ProjectRoleName) { projectStore.updateProjectAccess({ projectId: projectStore.project.id, userId: item.id, @@ -169,7 +169,7 @@ function roleUpdate(item: ProjectAccessDetail, value: ProjectRoleName) { }) } -function ProjectPermission(item: ProjectAccessDetail) { +function ProjectPermission(item: ProjectCollaborators) { return calculateProjectPermission(item.project_role, item.workspace_role) } diff --git a/web-app/packages/lib/src/modules/project/projectApi.ts b/web-app/packages/lib/src/modules/project/projectApi.ts index c1e3aba0..9537d61c 100644 --- a/web-app/packages/lib/src/modules/project/projectApi.ts +++ b/web-app/packages/lib/src/modules/project/projectApi.ts @@ -26,7 +26,8 @@ import { ProjectAccess, ProjectVersionFileChange, UpdateProjectPayload, - UpdatePublicFlagParams + UpdatePublicFlagParams, + ProjectCollaborators } from '@/modules/project/types' export const ProjectApi = { @@ -321,7 +322,7 @@ export const ProjectApi = { async getProjectCollaborators( projectId: string - ): Promise> { + ): Promise> { return ProjectModule.httpService.get(`/v2/projects/${projectId}/collaborators`) } } diff --git a/web-app/packages/lib/src/modules/project/types.ts b/web-app/packages/lib/src/modules/project/types.ts index 1763fd3a..8b1e4bb7 100644 --- a/web-app/packages/lib/src/modules/project/types.ts +++ b/web-app/packages/lib/src/modules/project/types.ts @@ -5,7 +5,8 @@ /* eslint-disable camelcase */ import { ProjectRoleName, - ProjectPermissionName, WorkspaceRoleName + ProjectPermissionName, + WorkspaceRoleName } from '@/common/permission_utils' import { PaginatedRequestParamsApi, @@ -342,6 +343,15 @@ export interface ProjectVersionFileChange { export type ErrorCodes = 'UpdateProjectAccessError' export interface ProjectAccessDetail { + id: number + type: 'member' + email: string + username: string + project_permission: ProjectRoleName + name: string +} + +export interface ProjectCollaborators { id: number email: string username: string From dfb6ad72bf757f9c5f31e44236c9d7442395b62e Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Wed, 18 Dec 2024 16:54:13 +0100 Subject: [PATCH 16/36] Added project_role to access --- server/mergin/sync/models.py | 1 + server/mergin/sync/private_api.yaml | 8 ++++++++ server/mergin/sync/schemas.py | 1 + 3 files changed, 10 insertions(+) diff --git a/server/mergin/sync/models.py b/server/mergin/sync/models.py index de81676a..ff1aa137 100644 --- a/server/mergin/sync/models.py +++ b/server/mergin/sync/models.py @@ -360,6 +360,7 @@ class ProjectAccessDetail: username: str name: Optional[str] project_permission: str + project_role: Optional[ProjectRole] type: str diff --git a/server/mergin/sync/private_api.yaml b/server/mergin/sync/private_api.yaml index 4160ed07..40af2e9b 100644 --- a/server/mergin/sync/private_api.yaml +++ b/server/mergin/sync/private_api.yaml @@ -594,6 +594,14 @@ components: - writer - editor - reader + project_role: + type: string + nullable: true + enum: + - owner + - writer + - editor + - reader invitation: description: Present only for type `invitation` type: object diff --git a/server/mergin/sync/schemas.py b/server/mergin/sync/schemas.py index c782c5ca..2faaae13 100644 --- a/server/mergin/sync/schemas.py +++ b/server/mergin/sync/schemas.py @@ -362,6 +362,7 @@ class ProjectAccessDetailSchema(Schema): username = fields.String() name = fields.String() project_permission = fields.String() + project_role = fields.String() type = fields.String() invitation = fields.Nested(ProjectInvitationAccessSchema()) From 4f3c23d6a847c2074b2d8bc4e1dc6ac0887c1168 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Wed, 18 Dec 2024 16:54:37 +0100 Subject: [PATCH 17/36] Cleanup of none ProjecRole --- .../lib/src/common/permission_utils.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/web-app/packages/lib/src/common/permission_utils.ts b/web-app/packages/lib/src/common/permission_utils.ts index e810b2db..97179ffd 100644 --- a/web-app/packages/lib/src/common/permission_utils.ts +++ b/web-app/packages/lib/src/common/permission_utils.ts @@ -16,7 +16,6 @@ export enum WorkspaceRole { } export enum ProjectRole { - none, reader, editor, writer, @@ -37,9 +36,10 @@ export type WorkspaceRoleName = | 'admin' | 'owner' -export type ProjectRoleName = - | Extract - | 'none' +export type ProjectRoleName = Extract< + WorkspaceRoleName, + 'reader' | 'editor' | 'writer' | 'owner' +> export type ProjectPermissionName = 'owner' | 'write' | 'edit' | 'read' @@ -63,7 +63,6 @@ export const USER_ROLE_BY_NAME: Record = { } export const PROJECT_ROLE_NAME_BY_ROLE: Record = { - [ProjectRole.none]: 'none', [ProjectRole.reader]: 'reader', [ProjectRole.editor]: 'editor', [ProjectRole.writer]: 'writer', @@ -71,7 +70,6 @@ export const PROJECT_ROLE_NAME_BY_ROLE: Record = { } export const PROJECT_ROLE_BY_NAME: Record = { - none: ProjectRole.none, reader: ProjectRole.reader, editor: ProjectRole.editor, writer: ProjectRole.writer, @@ -191,8 +189,7 @@ export function getProjectAccessKeyByRoleName( owner: 'ownersnames', writer: 'writersnames', editor: 'editorsnames', - reader: 'readersnames', - none: undefined + reader: 'readersnames' } return mapper[roleName] } @@ -200,12 +197,11 @@ export function getProjectAccessKeyByRoleName( export function getProjectPermissionByRoleName( roleName: ProjectRoleName ): ProjectPermissionName { - const mapper: Record = { + const mapper: Record = { owner: 'owner', writer: 'write', editor: 'edit', - reader: 'read', - none: undefined + reader: 'read' } return mapper[roleName] } From 67f146ed96fc368241c4f23b232168bbbc6e7076 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Wed, 18 Dec 2024 16:55:32 +0100 Subject: [PATCH 18/36] Updated: - remove project collaborator/update project collaboratr methods in store.ts - replace for access methods --- .../modules/admin/views/AccountDetailView.vue | 3 +- .../components/AccessRequestTableTemplate.vue | 4 +- .../components/ProjectAccessRequests.vue | 4 +- .../components/ProjectMembersTable.vue | 2 +- .../components/ProjectsTableDataLoader.vue | 7 ---- .../lib/src/modules/project/projectApi.ts | 35 ++++++++++------ .../packages/lib/src/modules/project/store.ts | 32 +++++++++------ .../packages/lib/src/modules/project/types.ts | 41 +++++++++++++++---- 8 files changed, 82 insertions(+), 46 deletions(-) diff --git a/web-app/packages/admin-lib/src/modules/admin/views/AccountDetailView.vue b/web-app/packages/admin-lib/src/modules/admin/views/AccountDetailView.vue index 86b9cd2b..9032e85b 100644 --- a/web-app/packages/admin-lib/src/modules/admin/views/AccountDetailView.vue +++ b/web-app/packages/admin-lib/src/modules/admin/views/AccountDetailView.vue @@ -215,7 +215,8 @@ const changeStatusDialog = () => { await adminStore.updateUser({ username: user.value.username, data: { - active: !user.value.active + active: !user.value.active, + is_admin: user.value.is_admin } }) } diff --git a/web-app/packages/lib/src/modules/project/components/AccessRequestTableTemplate.vue b/web-app/packages/lib/src/modules/project/components/AccessRequestTableTemplate.vue index 91eb6f22..e0e813c2 100644 --- a/web-app/packages/lib/src/modules/project/components/AccessRequestTableTemplate.vue +++ b/web-app/packages/lib/src/modules/project/components/AccessRequestTableTemplate.vue @@ -187,7 +187,7 @@ export default defineComponent({ await this.acceptProjectAccessRequest({ data, itemId: request.id, - namespace: this.namespace + workspace: this.namespace }) await this.updatePaginationOrFetch() } catch (err) { @@ -200,7 +200,7 @@ export default defineComponent({ async cancelRequest(request) { await this.cancelProjectAccessRequest({ itemId: request.id, - namespace: this.namespace + workspace: this.namespace }) await this.updatePaginationOrFetch() }, diff --git a/web-app/packages/lib/src/modules/project/components/ProjectAccessRequests.vue b/web-app/packages/lib/src/modules/project/components/ProjectAccessRequests.vue index 80d0eb3e..bc7af479 100644 --- a/web-app/packages/lib/src/modules/project/components/ProjectAccessRequests.vue +++ b/web-app/packages/lib/src/modules/project/components/ProjectAccessRequests.vue @@ -158,7 +158,7 @@ export default defineComponent({ await this.acceptProjectAccessRequest({ data, itemId: request.id, - namespace: this.project.namespace + workspace: this.project.namespace }) await this.updatePaginationOrFetch() } catch (err) { @@ -173,7 +173,7 @@ export default defineComponent({ async cancelRequest(request) { await this.cancelProjectAccessRequest({ itemId: request.id, - namespace: this.project.namespace + workspace: this.project.namespace }) await this.updatePaginationOrFetch() }, diff --git a/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue b/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue index f96f9e6d..95b0edaf 100644 --- a/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue +++ b/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue @@ -163,7 +163,7 @@ function removeMember(item: ProjectAccessDetail) { function roleUpdate(item: ProjectAccessDetail, value: ProjectRoleName) { projectStore.updateProjectAccess({ projectId: projectStore.project.id, - userId: item.id, + access: item, data: { role: value } }) } diff --git a/web-app/packages/lib/src/modules/project/components/ProjectsTableDataLoader.vue b/web-app/packages/lib/src/modules/project/components/ProjectsTableDataLoader.vue index 04229c27..9e79484b 100644 --- a/web-app/packages/lib/src/modules/project/components/ProjectsTableDataLoader.vue +++ b/web-app/packages/lib/src/modules/project/components/ProjectsTableDataLoader.vue @@ -60,10 +60,6 @@ export default defineComponent({ default: false }, namespace: String, - asAdmin: { - type: Boolean, - default: false - }, public: { type: Boolean, default: true @@ -136,9 +132,6 @@ export default defineComponent({ if (projectGridState.namespace) { params.only_namespace = projectGridState.namespace } - if (this.asAdmin) { - params.as_admin = true - } if (!this.public) { params.public = false } diff --git a/web-app/packages/lib/src/modules/project/projectApi.ts b/web-app/packages/lib/src/modules/project/projectApi.ts index 67ff0e4e..76f4d832 100644 --- a/web-app/packages/lib/src/modules/project/projectApi.ts +++ b/web-app/packages/lib/src/modules/project/projectApi.ts @@ -25,8 +25,10 @@ import { ProjectAccessDetail, ProjectAccess, ProjectVersionFileChange, - UpdateProjectPayload, - UpdatePublicFlagParams + UpdateProjectCollaboratorPayload, + UpdatePublicFlagParams, + ProjectCollaborator, + AddProjectCollaboratorPayload } from '@/modules/project/types' export const ProjectApi = { @@ -174,17 +176,28 @@ export const ProjectApi = { ) }, - async updateProjectAccess( + async addProjectCollaborator( + id: string, + data: AddProjectCollaboratorPayload + ): Promise> { + return ProjectModule.httpService.post( + `/v2/projects/${id}/collaborators`, + data, + { + validateStatus + } + ) + }, + + async updateProjectCollaborator( id: string, userId: number, - data: UpdateProjectPayload, - withRetry?: boolean - ): Promise> { + data: UpdateProjectCollaboratorPayload + ): Promise> { return ProjectModule.httpService.patch( `/v2/projects/${id}/collaborators/${userId}`, data, { - ...(withRetry ? getDefaultRetryOptions() : {}), validateStatus } ) @@ -200,15 +213,13 @@ export const ProjectApi = { }) }, - async removeProjectAccess( + async removeProjectCollaborator( id: string, - userId: number, - withRetry?: boolean - ): Promise> { + userId: number + ): Promise> { return ProjectModule.httpService.delete( `/v2/projects/${id}/collaborators/${userId}`, { - ...(withRetry ? getDefaultRetryOptions() : {}), validateStatus } ) diff --git a/web-app/packages/lib/src/modules/project/store.ts b/web-app/packages/lib/src/modules/project/store.ts index d52baeae..011b33c5 100644 --- a/web-app/packages/lib/src/modules/project/store.ts +++ b/web-app/packages/lib/src/modules/project/store.ts @@ -43,7 +43,7 @@ import { ProjectAccessDetail, ProjectVersionFileChange, ProjectVersionListItem, - UpdateProjectPayload, + UpdateProjectCollaboratorPayload, UpdatePublicFlagParams } from '@/modules/project/types' import { useUserStore } from '@/modules/user/store' @@ -759,12 +759,11 @@ export const useProjectStore = defineStore('projectModule', { this.accessLoading = true const notificationStore = useNotificationStore() try { - const response = await ProjectApi.removeProjectAccess( + await ProjectApi.removeProjectCollaborator( this.project.id, - item.id + Number(item.id) ) this.access = this.access.filter((access) => access.id !== item.id) - this.project.access = response.data } catch { notificationStore.error({ text: `Failed to update project access for user ${item.username}` @@ -782,23 +781,30 @@ export const useProjectStore = defineStore('projectModule', { */ async updateProjectAccess(payload: { projectId: string - userId: number - data: UpdateProjectPayload + access: ProjectAccessDetail + data: UpdateProjectCollaboratorPayload }) { this.accessLoading = true try { - const response = await ProjectApi.updateProjectAccess( - payload.projectId, - payload.userId, - payload.data - ) + if (!payload.access.project_role) { + await ProjectApi.addProjectCollaborator(payload.projectId, { + ...payload.data, + username: payload.access.username + }) + } else { + await ProjectApi.updateProjectCollaborator( + payload.projectId, + Number(payload.access.id), + payload.data + ) + } this.access = this.access.map((access) => { - if (access.id === payload.userId) { + if (access.id === payload.access.id) { access.project_permission = payload.data.role + access.project_role = payload.data.role } return access }) - this.project.access = response.data } catch (err) { this.handleProjectAccessError(err, 'Failed to update project access') } finally { diff --git a/web-app/packages/lib/src/modules/project/types.ts b/web-app/packages/lib/src/modules/project/types.ts index e8c39aaa..025ccdd8 100644 --- a/web-app/packages/lib/src/modules/project/types.ts +++ b/web-app/packages/lib/src/modules/project/types.ts @@ -5,7 +5,8 @@ /* eslint-disable camelcase */ import { ProjectRoleName, - ProjectPermissionName + ProjectPermissionName, + WorkspaceRoleName } from '@/common/permission_utils' import { PaginatedRequestParamsApi, @@ -184,12 +185,12 @@ export interface GetAccessRequestsPayload extends GetUserAccessRequestsPayload { export interface AcceptProjectAccessRequestPayload { itemId: number data: AcceptProjectAccessRequestData - namespace?: string + workspace?: string } export interface CancelProjectAccessRequestPayload { itemId: number - namespace: string + workspace: string } export interface CreateProjectParams { @@ -292,7 +293,12 @@ export type EnhancedProjectDetail = ProjectDetail & { path: string } -export interface UpdateProjectPayload { +export interface AddProjectCollaboratorPayload { + role: ProjectRoleName + username: string +} + +export interface UpdateProjectCollaboratorPayload { role: ProjectRoleName } @@ -341,11 +347,30 @@ export interface ProjectVersionFileChange { export type ErrorCodes = 'UpdateProjectAccessError' +export type ProjectAccessDetailType = 'invitation' | 'member' + export interface ProjectAccessDetail { + id: number | string + type: ProjectAccessDetailType + role: WorkspaceRoleName + name?: string + email: string + username?: string + project_permission?: ProjectRoleName + project_role?: ProjectRoleName | null + invitation?: { + expires_at: string + projects?: { + ids: string[] + permissions: ProjectPermissionName + } + } +} + +export interface ProjectCollaborator { id: number - type: 'member' + usernaname: string email: string - username: string - project_permission: ProjectRoleName - name: string + workspace_role: WorkspaceRoleName + project_role: ProjectRoleName } From 13e68b413d5397b20ebf0691eb3505638f489657 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Wed, 18 Dec 2024 17:51:38 +0100 Subject: [PATCH 19/36] Fix tests for project_role --- server/mergin/sync/private_api_controller.py | 1 - server/mergin/sync/workspace.py | 9 ++++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/server/mergin/sync/private_api_controller.py b/server/mergin/sync/private_api_controller.py index 616ac69d..31d3e2af 100644 --- a/server/mergin/sync/private_api_controller.py +++ b/server/mergin/sync/private_api_controller.py @@ -26,7 +26,6 @@ AdminProjectSchema, ProjectAccessSchema, ProjectAccessDetailSchema, - ProjectVersionListSchema, ) from .permissions import ( require_project_by_uuid, diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 8cc0a27a..3f0b5eec 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -323,17 +323,19 @@ def project_access(self, project: Project) -> List[ProjectAccessDetail]: direct_members_ids = [u.user_id for u in project.project_users] users = User.query.filter(User.active.is_(True)).order_by(User.email) - direct_members = users.filter(User.id.in_(direct_members_ids)).all() + direct_members: list[User] = users.filter(User.id.in_(direct_members_ids)).all() for dm in direct_members: - project_role = ProjectPermissions.get_user_project_role(project, dm) + project_permission = ProjectPermissions.get_user_project_role(project, dm) + project_role = project.get_role(dm.id) member = ProjectAccessDetail( id=dm.id, username=dm.username, role=ws.get_user_role(dm).value, name=dm.profile.name(), email=dm.email, - project_permission=project_role and project_role.value, + project_permission=project_permission and project_permission.value, + project_role=project_role.value if project_role else None, type="member", ) result.append(member) @@ -347,6 +349,7 @@ def project_access(self, project: Project) -> List[ProjectAccessDetail]: email=gm.email, role=global_role, project_permission=global_role, + project_role=None, type="member", ) result.append(member) From e61f7b5f4cea1c1852e77e3d9194ffb81a23588f Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Thu, 19 Dec 2024 13:45:02 +0100 Subject: [PATCH 20/36] Address comments @varmar05 - renaming attributes --- server/mergin/sync/models.py | 2 +- server/mergin/sync/private_api.yaml | 48 +++++++++---------- server/mergin/sync/schemas.py | 2 +- server/mergin/sync/workspace.py | 6 +-- .../mergin/tests/test_private_project_api.py | 28 +++++------ .../components/ProjectMembersTable.vue | 2 +- .../packages/lib/src/modules/project/store.ts | 2 +- .../packages/lib/src/modules/project/types.ts | 4 +- 8 files changed, 47 insertions(+), 47 deletions(-) diff --git a/server/mergin/sync/models.py b/server/mergin/sync/models.py index ff1aa137..cccd7b43 100644 --- a/server/mergin/sync/models.py +++ b/server/mergin/sync/models.py @@ -359,7 +359,7 @@ class ProjectAccessDetail: role: str username: str name: Optional[str] - project_permission: str + workspace_role: str project_role: Optional[ProjectRole] type: str diff --git a/server/mergin/sync/private_api.yaml b/server/mergin/sync/private_api.yaml index 40af2e9b..0116cd65 100644 --- a/server/mergin/sync/private_api.yaml +++ b/server/mergin/sync/private_api.yaml @@ -551,8 +551,7 @@ components: - id - type - email - - project_permission - - role + - workspace_role properties: id: description: User/Invitation (uu)id @@ -569,16 +568,9 @@ components: type: string format: email example: john.doe@example.com - role: + workspace_role: description: Workspace role - type: string - enum: - - owner - - admin - - writer - - editor - - reader - - guest + $ref: "#/components/schemas/WorkspaceRole" username: description: Present only for type `member` type: string @@ -587,21 +579,13 @@ components: description: Present only for type `member` type: string example: John Doe - project_permission: - type: string - enum: - - owner - - writer - - editor - - reader + role: + description: Project role defined as combination of project and workspace roles + $ref: "#/components/schemas/ProjectRole" project_role: - type: string nullable: true - enum: - - owner - - writer - - editor - - reader + description: Project role defined in database, not calculated version + $ref: "#/components/schemas/ProjectRole" invitation: description: Present only for type `invitation` type: object @@ -666,3 +650,19 @@ components: items: type: integer example: [1] + WorkspaceRole: + type: string + enum: + - owner + - admin + - writer + - editor + - reader + - guest + ProjectRole: + type: string + enum: + - owner + - writer + - editor + - reader diff --git a/server/mergin/sync/schemas.py b/server/mergin/sync/schemas.py index 2faaae13..c282b56c 100644 --- a/server/mergin/sync/schemas.py +++ b/server/mergin/sync/schemas.py @@ -361,7 +361,7 @@ class ProjectAccessDetailSchema(Schema): role = fields.String() username = fields.String() name = fields.String() - project_permission = fields.String() + workspace_role = fields.String() project_role = fields.String() type = fields.String() invitation = fields.Nested(ProjectInvitationAccessSchema()) diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 3f0b5eec..25b8e6fe 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -331,10 +331,10 @@ def project_access(self, project: Project) -> List[ProjectAccessDetail]: member = ProjectAccessDetail( id=dm.id, username=dm.username, - role=ws.get_user_role(dm).value, + workspace_role=ws.get_user_role(dm).value, name=dm.profile.name(), email=dm.email, - project_permission=project_permission and project_permission.value, + role=project_permission and project_permission.value, project_role=project_role.value if project_role else None, type="member", ) @@ -347,8 +347,8 @@ def project_access(self, project: Project) -> List[ProjectAccessDetail]: username=gm.username, name=gm.profile.name(), email=gm.email, + workspace_role=global_role, role=global_role, - project_permission=global_role, project_role=None, type="member", ) diff --git a/server/mergin/tests/test_private_project_api.py b/server/mergin/tests/test_private_project_api.py index c8aabd9e..f475255f 100644 --- a/server/mergin/tests/test_private_project_api.py +++ b/server/mergin/tests/test_private_project_api.py @@ -490,7 +490,7 @@ def test_get_project_access(client): resp = client.get(url) assert resp.status_code == 200 assert len(resp.json) == 1 - assert resp.json[0]["project_permission"] == "owner" + assert resp.json[0]["role"] == "owner" project.set_role(users[0].id, ProjectRole.OWNER) project.set_role(users[1].id, ProjectRole.WRITER) project.set_role(users[2].id, ProjectRole.READER) @@ -498,9 +498,9 @@ def test_get_project_access(client): resp = client.get(url) assert resp.status_code == 200 assert len(resp.json) == 4 - assert sum(map(lambda x: int(x["project_permission"] == "owner"), resp.json)) == 2 - assert sum(map(lambda x: int(x["project_permission"] == "writer"), resp.json)) == 1 - assert sum(map(lambda x: int(x["project_permission"] == "reader"), resp.json)) == 1 + assert sum(map(lambda x: int(x["role"] == "owner"), resp.json)) == 2 + assert sum(map(lambda x: int(x["role"] == "writer"), resp.json)) == 1 + assert sum(map(lambda x: int(x["role"] == "reader"), resp.json)) == 1 # user3 does not have access to the project assert not any(users[3].email == access["email"] for access in resp.json) assert any(users[2].email == access["email"] for access in resp.json) @@ -508,27 +508,27 @@ def test_get_project_access(client): resp = client.get(url) assert resp.status_code == 200 assert len(resp.json) == 6 - assert sum(map(lambda x: int(x["project_permission"] == "owner"), resp.json)) == 2 - assert sum(map(lambda x: int(x["project_permission"] == "writer"), resp.json)) == 1 - assert sum(map(lambda x: int(x["project_permission"] == "reader"), resp.json)) == 3 + assert sum(map(lambda x: int(x["role"] == "owner"), resp.json)) == 2 + assert sum(map(lambda x: int(x["role"] == "writer"), resp.json)) == 1 + assert sum(map(lambda x: int(x["role"] == "reader"), resp.json)) == 3 Configuration.GLOBAL_WRITE = True resp = client.get(url) assert resp.status_code == 200 assert len(resp.json) == 6 - assert sum(map(lambda x: int(x["project_permission"] == "owner"), resp.json)) == 2 - assert sum(map(lambda x: int(x["project_permission"] == "writer"), resp.json)) == 4 - assert sum(map(lambda x: int(x["project_permission"] == "reader"), resp.json)) == 0 + assert sum(map(lambda x: int(x["role"] == "owner"), resp.json)) == 2 + assert sum(map(lambda x: int(x["role"] == "writer"), resp.json)) == 4 + assert sum(map(lambda x: int(x["role"] == "reader"), resp.json)) == 0 Configuration.GLOBAL_ADMIN = True resp = client.get(url) assert resp.status_code == 200 assert len(resp.json) == 6 - assert sum(map(lambda x: int(x["project_permission"] == "owner"), resp.json)) == 6 - assert sum(map(lambda x: int(x["project_permission"] == "writer"), resp.json)) == 0 - assert sum(map(lambda x: int(x["project_permission"] == "reader"), resp.json)) == 0 + assert sum(map(lambda x: int(x["role"] == "owner"), resp.json)) == 6 + assert sum(map(lambda x: int(x["role"] == "writer"), resp.json)) == 0 + assert sum(map(lambda x: int(x["role"] == "reader"), resp.json)) == 0 # pretend a user was deleted to test that api can handle it users[3].inactivate() users[3].anonymize() resp = client.get(url) assert resp.status_code == 200 assert len(resp.json) == 5 - assert sum(map(lambda x: int(x["project_permission"] == "owner"), resp.json)) == 5 + assert sum(map(lambda x: int(x["role"] == "owner"), resp.json)) == 5 diff --git a/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue b/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue index 95b0edaf..6f066f2b 100644 --- a/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue +++ b/web-app/packages/lib/src/modules/project/components/ProjectMembersTable.vue @@ -71,7 +71,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial