From 59470cda669ecbbd66507b320d45a98a70a6028d Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Wed, 10 Apr 2024 14:18:36 -0600 Subject: [PATCH 1/4] backend: Add ability to delete a user Signed-off-by: Taylor Smock --- backend/api/users/resources.py | 47 ++++++ backend/models/postgis/application.py | 6 + backend/models/postgis/message.py | 4 +- backend/services/users/osm_service.py | 21 +++ backend/services/users/user_service.py | 56 ++++++- .../integration/api/users/test_resources.py | 150 +++++++++++++++++- .../services/users/test_osm_service.py | 4 + .../services/users/test_user_service.py | 65 ++++++++ 8 files changed, 343 insertions(+), 10 deletions(-) diff --git a/backend/api/users/resources.py b/backend/api/users/resources.py index ab08d78fd9..a57d26d422 100644 --- a/backend/api/users/resources.py +++ b/backend/api/users/resources.py @@ -1,11 +1,17 @@ from distutils.util import strtobool +from typing import Optional + +from flask import stream_with_context, Response from flask_restful import Resource, current_app, request from schematics.exceptions import DataError from backend.models.dtos.user_dto import UserSearchQuery +from backend.models.postgis.user import User from backend.services.users.authentication_service import token_auth from backend.services.users.user_service import UserService from backend.services.project_service import ProjectService +from backend.services.users.osm_service import OSMService +from backend.exceptions import Unauthorized class UsersRestAPI(Resource): @@ -44,6 +50,28 @@ def get(self, user_id): user_dto = UserService.get_user_dto_by_id(user_id, token_auth.current_user()) return user_dto.to_primitive(), 200 + @token_auth.login_required + def delete(self, user_id: Optional[int] = None): + """ + Delete user information by id. + :param user_id: The user to delete + :return: RFC7464 compliant sequence of user objects deleted + 200: User deleted + 401: Unauthorized - Invalid credentials + 404: User not found + 500: Internal Server Error + """ + if user_id == token_auth.current_user() or UserService.is_user_an_admin( + token_auth.current_user() + ): + return ( + UserService.delete_user_by_id( + user_id, token_auth.current_user() + ).to_primitive(), + 200, + ) + raise Unauthorized() + class UsersAllAPI(Resource): @token_auth.login_required @@ -115,6 +143,25 @@ def get(self): users_dto = UserService.get_all_users(query) return users_dto.to_primitive(), 200 + @token_auth.login_required + def delete(self): + if UserService.is_user_an_admin(token_auth.current_user()): + + def delete_users(): + for user in User.get_all_users_not_paginated(): + # We specifically want to remove users that have deleted their OSM accounts. + if OSMService.is_osm_user_gone(user.id): + data = UserService.delete_user_by_id( + user.id, token_auth.current_user() + ).to_primitive() + yield f"\u001e{data}\n" + + return Response( + stream_with_context(delete_users()), + headers={"Content-Type": "application/json-seq"}, + ) + raise Unauthorized() + class UsersQueriesUsernameAPI(Resource): @token_auth.login_required diff --git a/backend/models/postgis/application.py b/backend/models/postgis/application.py index 75fb8c913d..b0d7cf9712 100644 --- a/backend/models/postgis/application.py +++ b/backend/models/postgis/application.py @@ -60,6 +60,12 @@ def get_all_for_user(user: int): applications_dto.applications.append(application_dto) return applications_dto + @staticmethod + def delete_all_for_user(user: int): + for r in db.session.query(Application).filter(Application.user == user): + db.session.delete(r) + db.session.commit() + def as_dto(self): app_dto = ApplicationDTO() app_dto.user = self.user diff --git a/backend/models/postgis/message.py b/backend/models/postgis/message.py index 32516d5a57..5bc4ad88b9 100644 --- a/backend/models/postgis/message.py +++ b/backend/models/postgis/message.py @@ -1,3 +1,5 @@ +from typing import List + from sqlalchemy.sql.expression import false from backend import db @@ -178,7 +180,7 @@ def delete_multiple_messages(message_ids: list, user_id: int): db.session.commit() @staticmethod - def delete_all_messages(user_id: int, message_type_filters: list = None): + def delete_all_messages(user_id: int, message_type_filters: List[int] = None): """Deletes all messages to the user ----------------------------------- :param user_id: user id of the user whose messages are to be deleted diff --git a/backend/services/users/osm_service.py b/backend/services/users/osm_service.py index 33feb7ae17..87d57ff8cf 100644 --- a/backend/services/users/osm_service.py +++ b/backend/services/users/osm_service.py @@ -13,6 +13,25 @@ def __init__(self, message): class OSMService: + @staticmethod + def is_osm_user_gone(user_id: int) -> bool: + """ + Check if OSM details for the user from OSM API are available + :param user_id: user_id in scope + :raises OSMServiceError + """ + osm_user_details_url = ( + f"{current_app.config['OSM_SERVER_URL']}/api/0.6/user/{user_id}.json" + ) + response = requests.head(osm_user_details_url) + + if response.status_code == 410: + return True + if response.status_code != 200: + raise OSMServiceError("Bad response from OSM") + + return False + @staticmethod def get_osm_details_for_user(user_id: int) -> UserOSMDTO: """ @@ -25,6 +44,8 @@ def get_osm_details_for_user(user_id: int) -> UserOSMDTO: ) response = requests.get(osm_user_details_url) + if response.status_code == 410: + raise OSMServiceError("User no longer exists on OSM") if response.status_code != 200: raise OSMServiceError("Bad response from OSM") diff --git a/backend/services/users/user_service.py b/backend/services/users/user_service.py index 8324cd1477..48c23990f0 100644 --- a/backend/services/users/user_service.py +++ b/backend/services/users/user_service.py @@ -1,3 +1,5 @@ +from typing import Optional + from cachetools import TTLCache, cached from flask import current_app import datetime @@ -27,14 +29,14 @@ from backend.models.postgis.task import TaskHistory, TaskAction, Task from backend.models.dtos.user_dto import UserTaskDTOs from backend.models.dtos.stats_dto import Pagination +from backend.models.postgis.project_chat import ProjectChat from backend.models.postgis.statuses import TaskStatus, ProjectStatus -from backend.services.users.osm_service import OSMService, OSMServiceError from backend.services.messaging.smtp_service import SMTPService from backend.services.messaging.template_service import ( get_txt_template, template_var_replacing, ) - +from backend.services.users.osm_service import OSMService, OSMServiceError user_filter_cache = TTLCache(maxsize=1024, ttl=600) @@ -190,6 +192,56 @@ def get_user_dto_by_id(user: int, request_user: int) -> UserDTO: return user.as_dto(request_username) return user.as_dto() + @staticmethod + def delete_user_by_id(user_id: int, request_user_id: int) -> Optional[UserDTO]: + if user_id == request_user_id or UserService.is_user_an_admin(request_user_id): + user = User.get_by_id(user_id) + original_dto = UserService.get_user_dto_by_id(user_id, request_user_id) + user.accepted_licenses = [] + user.city = None + user.country = None + user.email_address = None + user.facebook_id = None + user.gender = None + user.interests = [] + user.irc_id = None + user.is_email_verified = False + user.is_expert = False + user.linkedin_id = None + user.name = None + user.picture_url = None + user.self_description_gender = None + user.skype_id = None + user.slack_id = None + user.twitter_id = None + # FIXME: Should we keep user_id since that will make conversations easier to follow? + # Keep in mind that OSM uses user_ on deleted accounts. + user.username = f"user_{user_id}" + + # Remove permissions from admin users, keep role for blocked users. + if UserService.is_user_an_admin(user_id): + user.set_user_role(UserRole.MAPPER) + user.save() + + # Remove messages that might contain user identifying information. + for message in ProjectChat.query.filter_by(user_id=user_id): + # TODO detect image links and try to delete them + message.message = f"[Deleted user_{user_id} message]" + db.session.commit() + + # Drop application keys + from backend.models.postgis.application import Application + + Application.delete_all_for_user(user_id) + + # Delete all messages (AKA notifications) for the user + Message.delete_all_messages( + user_id, [message_type.value for message_type in MessageType] + ) + # Leave interests, licenses, organizations, and tasks alone for now. + return original_dto + return None + @staticmethod def get_interests_stats(user_id): # Get all projects that the user has contributed. diff --git a/tests/backend/integration/api/users/test_resources.py b/tests/backend/integration/api/users/test_resources.py index fadd26e264..6173a18523 100644 --- a/tests/backend/integration/api/users/test_resources.py +++ b/tests/backend/integration/api/users/test_resources.py @@ -1,8 +1,9 @@ +from typing import Optional + from backend.models.postgis.task import Task, TaskStatus from backend.models.postgis.statuses import UserGender, UserRole, MappingLevel from backend.exceptions import get_message_from_sub_code - from tests.backend.base import BaseTestCase from tests.backend.helpers.test_helpers import ( return_canned_user, @@ -11,7 +12,6 @@ create_canned_interest, ) - TEST_USERNAME = "test_user" TEST_USER_ID = 1111111 TEST_EMAIL = "test@hotmail.com" @@ -93,11 +93,11 @@ def test_returns_404_if_user_not_found(self): @staticmethod def assert_user_detail_response( response, - user_id=TEST_USER_ID, - username=TEST_USERNAME, - email=TEST_EMAIL, - gender=None, - own_info=True, + user_id: Optional[int] = TEST_USER_ID, + username: Optional[str] = TEST_USERNAME, + email: Optional[str] = TEST_EMAIL, + gender: Optional[str] = None, + own_info: bool = True, ): assert response.status_code == 200 assert response.json["id"] == user_id @@ -556,3 +556,139 @@ def test_email_and_gender_not_returned_if_requested_by_other(self): TestUsersQueriesUsernameAPI.assert_user_detail_response( response, TEST_USER_ID, TEST_USERNAME, None, None, False ) + + def test_user_can_delete_self(self): + """Check that a user can delete (redact personal information) themselves""" + # Arrange + self.user.email_address = TEST_EMAIL + self.user.gender = UserGender.FEMALE.value + self.user.save() + # Act + response = self.client.delete( + self.url, headers={"Authorization": self.user_session_token} + ) + next_response = self.client.get( + f"/api/v2/users/{TEST_USER_ID}/", + headers={"Authorization": self.user_session_token}, + ) + # Assert + # Note that we return the deleted user information at this time + TestUsersQueriesUsernameAPI.assert_user_detail_response( + response, + TEST_USER_ID, + TEST_USERNAME, + TEST_EMAIL, + UserGender.FEMALE.name, + True, + ) + TestUsersQueriesUsernameAPI.assert_user_detail_response( + next_response, TEST_USER_ID, f"user_{TEST_USER_ID}", None, None, True + ) + + def test_other_user_cannot_delete_self(self): + """Check that another user cannot delete (redact personal information) about a different user""" + # Arrange + self.user.email_address = TEST_EMAIL + self.user.gender = UserGender.FEMALE.value + self.user.save() + user_2 = return_canned_user("user_2", 2222222) + user_2.create() + user_2_session_token = generate_encoded_token(user_2.id) + # Act + response = self.client.delete( + self.url, headers={"Authorization": user_2_session_token} + ) + # Assert + self.assertEqual(401, response.status_code) + rjson = response.json + self.assertDictEqual( + rjson, + { + "error": { + "code": 401, + "details": {}, + "message": "Authentication credentials were missing or incorrect.", + "sub_code": "UNAUTHORIZED", + } + }, + ) + + def test_other_admin_user_can_delete_self(self): + """Check that another user cannot delete (redact personal information) about a different user""" + # Arrange + self.user.email_address = TEST_EMAIL + self.user.gender = UserGender.FEMALE.value + self.user.save() + user_2 = return_canned_user("user_2", 2222222) + user_2.set_user_role(UserRole.ADMIN) + user_2.create() + user_2_session_token = generate_encoded_token(user_2.id) + # Act + response = self.client.delete( + self.url, headers={"Authorization": user_2_session_token} + ) + next_response = self.client.get( + f"/api/v2/users/{TEST_USER_ID}/", + headers={"Authorization": user_2_session_token}, + ) + # Assert + # Note that we return the deleted user information at this time + TestUsersQueriesUsernameAPI.assert_user_detail_response( + response, + TEST_USER_ID, + TEST_USERNAME, + TEST_EMAIL, + UserGender.FEMALE.name, + False, + ) + TestUsersQueriesUsernameAPI.assert_user_detail_response( + next_response, TEST_USER_ID, f"user_{TEST_USER_ID}", None, None, False + ) + + def test_admin_user_can_remove_redacted_osm_accounts(self): + """Check that an admin can redact redacted OSM accounts""" + # Arrange + self.user.email_address = TEST_EMAIL + self.user.gender = UserGender.FEMALE.value + self.user.id = 4 + self.user.save() + user_2 = return_canned_user("user_2", 2222222) + user_2.set_user_role(UserRole.ADMIN) + user_2.create() + user_2_session_token = generate_encoded_token(user_2.id) + # Act + response = self.client.delete( + "/api/v2/users/", headers={"Authorization": user_2_session_token} + ) + next_response = self.client.get( + "/api/v2/users/4/", headers={"Authorization": user_2_session_token} + ) + # Assert + self.assertEqual(200, response.status_code) + TestUsersQueriesUsernameAPI.assert_user_detail_response( + next_response, 4, "user_4", None, None, False + ) + + def test_user_cannot_remove_redacted_osm_accounts(self): + """Check that a user cannot redact redacted OSM accounts""" + # Arrange + self.user.email_address = TEST_EMAIL + self.user.gender = UserGender.FEMALE.value + self.user.id = 4 + self.user.save() + user_2 = return_canned_user("user_2", 2222222) + user_2.set_user_role(UserRole.MAPPER) + user_2.create() + user_2_session_token = generate_encoded_token(user_2.id) + # Act + response = self.client.delete( + "/api/v2/users/", headers={"Authorization": user_2_session_token} + ) + next_response = self.client.get( + "/api/v2/users/4/", headers={"Authorization": user_2_session_token} + ) + # Assert + self.assertEqual(401, response.status_code) + TestUsersQueriesUsernameAPI.assert_user_detail_response( + next_response, 4, TEST_USERNAME, None, None, False + ) diff --git a/tests/backend/integration/services/users/test_osm_service.py b/tests/backend/integration/services/users/test_osm_service.py index f10eb328b6..de8ec0cb2a 100644 --- a/tests/backend/integration/services/users/test_osm_service.py +++ b/tests/backend/integration/services/users/test_osm_service.py @@ -13,3 +13,7 @@ def test_get_osm_details_for_user_returns_user_details_if_valid_user_id(self): dto = OSMService.get_osm_details_for_user(13526430) # Assert self.assertEqual(dto.account_created, "2021-06-10T01:27:18Z") + + def test_is_user_deleted(self): + self.assertTrue(OSMService.is_osm_user_gone(535043)) + self.assertFalse(OSMService.is_osm_user_gone(2078753)) diff --git a/tests/backend/integration/services/users/test_user_service.py b/tests/backend/integration/services/users/test_user_service.py index 3857625a84..74d67c595c 100644 --- a/tests/backend/integration/services/users/test_user_service.py +++ b/tests/backend/integration/services/users/test_user_service.py @@ -2,6 +2,7 @@ from tests.backend.base import BaseTestCase from backend.models.postgis.message import Message +from backend.models.postgis.statuses import UserRole, UserGender from backend.services.users.user_service import ( UserService, MappingLevel, @@ -97,3 +98,67 @@ def test_register_user_creates_new_user(self): # Assert self.assertEqual(expected_user.username, test_user.username) self.assertEqual(expected_user.mapping_level, MappingLevel.INTERMEDIATE.value) + + @staticmethod + def add_user_identifying_information(user: User) -> User: + user.username = "Test User" + user.email_address = "test@example.com" + user.twitter_id = "@Test" + user.facebook_id = "@FBTest" + user.linkedin_id = "@LinkedIn" + user.slack_id = "@Slack" + user.skype_id = "@Skype" + user.irc_id = "IRC" + user.name = "Some name here" + user.city = "Some city" + user.country = "Some country" + user.picture_url = "https://test.com/path/to/picture.png" + user.gender = UserGender.MALE.value + user.self_description_gender = "I am male" + user.default_editor = "ID" + user.save() + return user + + def check_user_details_deleted(self, user: User, deleted: bool): + if deleted: + check = self.assertIsNone + self.assertNotEquals(UserRole.ADMIN.value, user.role) + self.assertEqual(f"user_{user.id}", user.username) + else: + self.assertNotEquals(f"user_{user.id}", user.username) + check = self.assertIsNotNone + check(user.email_address) + check(user.twitter_id) + check(user.facebook_id) + check(user.linkedin_id) + check(user.slack_id) + check(user.skype_id) + check(user.irc_id) + check(user.name) + check(user.city) + check(user.country) + check(user.picture_url) + check(user.gender) + check(user.self_description_gender) + self.assertEqual([], user.accepted_licenses) + self.assertEqual([], user.interests) + + def test_delete_user_same_user(self): + test_user = self.add_user_identifying_information(create_canned_user()) + UserService.delete_user_by_id(test_user.id, test_user.id) + self.check_user_details_deleted(User().get_by_id(test_user.id), deleted=True) + + def test_delete_user_different_user(self): + test_user = self.add_user_identifying_information(create_canned_user()) + other_user = return_canned_user("someone", test_user.id + 1) + other_user.create() + UserService.delete_user_by_id(test_user.id, other_user.id) + self.check_user_details_deleted(User().get_by_id(test_user.id), deleted=False) + + def test_delete_user_different_admin_user(self): + test_user = self.add_user_identifying_information(create_canned_user()) + other_user = return_canned_user("someone", test_user.id + 1) + other_user.set_user_role(UserRole.ADMIN) + other_user.create() + UserService.delete_user_by_id(test_user.id, other_user.id) + self.check_user_details_deleted(User().get_by_id(test_user.id), deleted=True) From 5cffa2cca65415e04b7c3ab3f6469e725a2503bc Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 11 Apr 2024 15:14:19 -0600 Subject: [PATCH 2/4] backend: Use planet.osm.org/users_deleted/users_deleted.txt to avoid hammering API Signed-off-by: Taylor Smock --- backend/api/users/resources.py | 37 +++++++++++++------ backend/models/postgis/user.py | 7 +++- backend/services/users/osm_service.py | 30 +++++++++++++++ .../services/users/test_osm_service.py | 12 ++++++ 4 files changed, 73 insertions(+), 13 deletions(-) diff --git a/backend/api/users/resources.py b/backend/api/users/resources.py index a57d26d422..881de46eba 100644 --- a/backend/api/users/resources.py +++ b/backend/api/users/resources.py @@ -1,5 +1,6 @@ from distutils.util import strtobool from typing import Optional +from collections.abc import Generator from flask import stream_with_context, Response from flask_restful import Resource, current_app, request @@ -146,22 +147,36 @@ def get(self): @token_auth.login_required def delete(self): if UserService.is_user_an_admin(token_auth.current_user()): - - def delete_users(): - for user in User.get_all_users_not_paginated(): - # We specifically want to remove users that have deleted their OSM accounts. - if OSMService.is_osm_user_gone(user.id): - data = UserService.delete_user_by_id( - user.id, token_auth.current_user() - ).to_primitive() - yield f"\u001e{data}\n" - return Response( - stream_with_context(delete_users()), + stream_with_context(UsersAllAPI._delete_users()), headers={"Content-Type": "application/json-seq"}, ) raise Unauthorized() + @staticmethod + def _delete_users() -> Generator[str, None, None]: + # Updated daily + deleted_users = OSMService.get_deleted_users() + if deleted_users: + last_deleted_user = 0 + for user in User.get_all_users_not_paginated(User.id): + while last_deleted_user < user.id: + last_deleted_user = next(deleted_users) + if last_deleted_user == user.id: + data = UserService.delete_user_by_id( + user.id, token_auth.current_user() + ).to_primitive() + yield f"\u001e{data}\n" + return + # Fall back to hitting the API (if the OSM API is not the default) + for user in User.get_all_users_not_paginated(): + # We specifically want to remove users that have deleted their OSM accounts. + if OSMService.is_osm_user_gone(user.id): + data = UserService.delete_user_by_id( + user.id, token_auth.current_user() + ).to_primitive() + yield f"\u001e{data}\n" + class UsersQueriesUsernameAPI(Resource): @token_auth.login_required diff --git a/backend/models/postgis/user.py b/backend/models/postgis/user.py index e07cb59e42..2bd7f642c6 100644 --- a/backend/models/postgis/user.py +++ b/backend/models/postgis/user.py @@ -182,9 +182,12 @@ def get_all_users(query: UserSearchQuery) -> UserSearchDTO: return dto @staticmethod - def get_all_users_not_paginated(): + def get_all_users_not_paginated(*order): """Get all users in DB""" - return db.session.query(User.id).all() + query = db.session.query(User.id) + if not order: + return query.all() + return query.order_by(*order).all() @staticmethod def filter_users(user_filter: str, project_id: int, page: int) -> UserFilterDTO: diff --git a/backend/services/users/osm_service.py b/backend/services/users/osm_service.py index 87d57ff8cf..cb4a13a7cf 100644 --- a/backend/services/users/osm_service.py +++ b/backend/services/users/osm_service.py @@ -1,3 +1,7 @@ +import re +from collections.abc import Generator +from typing import Optional + import requests from flask import current_app @@ -32,6 +36,32 @@ def is_osm_user_gone(user_id: int) -> bool: return False + @staticmethod + def get_deleted_users() -> Optional[Generator[int, None, None]]: + """ + Get the list of deleted users from OpenStreetMap. + This only returns users from the https://www.openstreetmap.org instance. + :return: The deleted users + """ + if current_app.config["OSM_SERVER_URL"] == "https://www.openstreetmap.org": + + def get_planet_osm_deleted_users() -> Generator[int, None, None]: + response = requests.get( + "https://planet.openstreetmap.org/users_deleted/users_deleted.txt", + stream=True, + ) + username = re.compile(r"^\s*(\d+)\s*$") + try: + for line in response.iter_lines(decode_unicode=True): + match = username.fullmatch(line) + if match: + yield int(match.group(1)) + finally: + response.close() + + return get_planet_osm_deleted_users() + return None + @staticmethod def get_osm_details_for_user(user_id: int) -> UserOSMDTO: """ diff --git a/tests/backend/integration/services/users/test_osm_service.py b/tests/backend/integration/services/users/test_osm_service.py index de8ec0cb2a..8012b7a7c2 100644 --- a/tests/backend/integration/services/users/test_osm_service.py +++ b/tests/backend/integration/services/users/test_osm_service.py @@ -17,3 +17,15 @@ def test_get_osm_details_for_user_returns_user_details_if_valid_user_id(self): def test_is_user_deleted(self): self.assertTrue(OSMService.is_osm_user_gone(535043)) self.assertFalse(OSMService.is_osm_user_gone(2078753)) + + def test_get_deleted_users(self): + # These are the first 10 deleted users on 2024-04-16. This should ensure that the test finishes quickly. + # Otherwise, it can take 6s+ (dependent upon network speed) + deleted_users = [4, 142, 593, 601, 1769, 2161, 2238, 2782, 2868] + generator = OSMService.get_deleted_users() + for deleted_user in generator: + if deleted_user in deleted_users: + deleted_users.remove(deleted_user) + if len(deleted_users) == 0: + break + self.assertEquals(0, len(deleted_users)) From 5ddcdcc46e1b29ca843196abc55e7a7961cff687 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Wed, 17 Apr 2024 11:53:22 -0600 Subject: [PATCH 3/4] Initial UI work Signed-off-by: Taylor Smock --- frontend/src/components/deleteModal/index.js | 41 ++++++++++++++++--- .../src/components/deleteModal/messages.js | 12 ++++++ .../src/components/teamsAndOrgs/management.js | 21 ++++++++-- .../user/forms/personalInformation.js | 13 +++++- frontend/src/components/user/list.js | 17 ++++++-- frontend/src/components/user/messages.js | 4 ++ 6 files changed, 94 insertions(+), 14 deletions(-) diff --git a/frontend/src/components/deleteModal/index.js b/frontend/src/components/deleteModal/index.js index cd36fefea9..e230c90636 100644 --- a/frontend/src/components/deleteModal/index.js +++ b/frontend/src/components/deleteModal/index.js @@ -12,7 +12,34 @@ import { AlertIcon } from '../svgIcons'; const DeleteTrigger = forwardRef((props, ref) => ); -export function DeleteModal({ id, name, type, className, endpointURL, onDelete }: Object) { +/** + * Called when an object is deleted + * @callback onDelete + * @param success The success object + */ + +/** + * Create a delete modal + * @param {number} [id] The id of the object to delete. Ignored if className is defined. + * @param {str} [name] The name of the object (unused) + * @param {('notifications'|'comments'|'users'|'interests'|'categories')} [type] The type of the object to delete. Ignored if className is defined. + * @param {str} [className] The additional css class names + * @param {str} [endpointURL] The endpoint to call + * @param {onDelete} [onDelete] Called when the object is deleted + * @typedef {import('@formatjs/intl').MessageDescriptor} MessageDescriptor + * @param {MessageDescriptor} [message] The message to show the user + * @returns {Element} The delete modal + * @constructor + */ +export function DeleteModal({ + id, + name, + type, + className, + endpointURL, + onDelete, + message = messages.delete, +}: Object) { const navigate = useNavigate(); const modalRef = useRef(); const token = useSelector((state) => state.auth.token); @@ -28,9 +55,9 @@ export function DeleteModal({ id, name, type, className, endpointURL, onDelete } setDeleteStatus('success'); if (type === 'notifications') { setTimeout(() => navigate(`/inbox`), 750); - } else if (type === 'comments') { + } else if (type === 'comments' || type === 'users') { setTimeout(() => { - onDelete(); + onDelete(success); modalRef.current.close(); }, 750); return; @@ -48,7 +75,11 @@ export function DeleteModal({ id, name, type, className, endpointURL, onDelete } + } modal closeOnDocumentClick @@ -86,7 +117,7 @@ export function DeleteModal({ id, name, type, className, endpointURL, onDelete } diff --git a/frontend/src/components/deleteModal/messages.js b/frontend/src/components/deleteModal/messages.js index f491dfcc0f..88641dd7dc 100644 --- a/frontend/src/components/deleteModal/messages.js +++ b/frontend/src/components/deleteModal/messages.js @@ -20,6 +20,10 @@ export default defineMessages({ id: 'deleteModal.status.success.teams', defaultMessage: 'Team deleted successfully.', }, + success_users: { + id: 'deleteModal.status.success.users', + defaultMessage: 'User deleted successfully.', + }, success_organisations: { id: 'deleteModal.status.success.organisations', defaultMessage: 'Organisation deleted successfully.', @@ -76,6 +80,10 @@ export default defineMessages({ id: 'deleteModal.status.failure.teams', defaultMessage: 'An error occurred when trying to delete this team.', }, + failure_users: { + id: 'deleteModal.status.failure.users', + defaultMessage: 'An error occurred when trying to delete this user.', + }, failure_comments: { id: 'deleteModal.status.failure.comments', defaultMessage: 'An error occurred when trying to delete this comment.', @@ -129,6 +137,10 @@ export default defineMessages({ id: 'deleteModal.title.teams', defaultMessage: 'Are you sure you want to delete this team?', }, + confirmDeleteTitle_users: { + id: 'deleteModal.title.users', + defaultMessage: "Are you sure you want to redact this user's information?", + }, confirmDeleteTitle_comments: { id: 'deleteModal.title.comments', defaultMessage: 'Are you sure you want to delete this comment?', diff --git a/frontend/src/components/teamsAndOrgs/management.js b/frontend/src/components/teamsAndOrgs/management.js index ec7b9561c5..db21737e81 100644 --- a/frontend/src/components/teamsAndOrgs/management.js +++ b/frontend/src/components/teamsAndOrgs/management.js @@ -23,18 +23,33 @@ export const AddButton = () => ( ); -export const DeleteButton = ({ className, onClick, showText = true }: Object) => { +/** + * A button for deleting something + * @param {string} className Additional css classes + * @param {MouseEventHandler} onClick The action to call on click + * @param {boolean} [showText=true] true if the message should be shown + * @typedef {import('@formatjs/intl').MessageDescriptor} MessageDescriptor + * @param {MessageDescriptor} message The message to show + * @returns {Element} The delete button + * @constructor + */ +export const DeleteButton = ({ + className, + onClick, + showText = true, + message = messages.delete, +}: Object) => { const intl = useIntl(); return (
{showText && ( - + )}
diff --git a/frontend/src/components/user/forms/personalInformation.js b/frontend/src/components/user/forms/personalInformation.js index bd6a20a202..3a25119ff3 100644 --- a/frontend/src/components/user/forms/personalInformation.js +++ b/frontend/src/components/user/forms/personalInformation.js @@ -1,5 +1,5 @@ import { useState } from 'react'; -import { connect } from 'react-redux'; +import { connect, useDispatch } from 'react-redux'; import { Form, Field } from 'react-final-form'; import { Tooltip } from 'react-tooltip'; import { FormattedMessage, useIntl } from 'react-intl'; @@ -8,9 +8,10 @@ import messages from '../messages'; import { FormSubmitButton } from '../../button'; import { InfoIcon, CheckIcon, CloseIcon } from '../../svgIcons'; import { UserCountrySelect, RadioField } from '../../formInputs'; -import { pushUserDetails } from '../../../store/actions/auth'; +import { logout, pushUserDetails } from '../../../store/actions/auth'; import { fetchLocalJSONAPI } from '../../../network/genericJSONRequest'; import { ORG_CODE } from '../../../config'; +import { DeleteModal } from '../../deleteModal'; export const PROFILE_RELEVANT_FIELDS = [ 'name', @@ -75,6 +76,7 @@ const RequiredIndicator = () => *; function _PersonalInformationForm({ userDetails, token, pushUserDetails }) { const intl = useIntl(); + const dispatch = useDispatch(); const labelClasses = 'db pt3 pb2'; const fieldClasses = 'blue-dark w-100 pv2 ph2 input-reset ba br1 b--grey-light bg-transparent lh-copy'; @@ -257,6 +259,13 @@ function _PersonalInformationForm({ userDetails, token, pushUserDetails }) { > + dispatch(logout())} + />

diff --git a/frontend/src/components/user/list.js b/frontend/src/components/user/list.js index 1ddc5651db..41f9ab6a55 100644 --- a/frontend/src/components/user/list.js +++ b/frontend/src/components/user/list.js @@ -12,6 +12,7 @@ import { PaginatorLine } from '../paginator'; import { SearchIcon, CloseIcon, SettingsIcon, CheckIcon } from '../svgIcons'; import { Dropdown } from '../dropdown'; import { nCardPlaceholders } from './usersPlaceholder'; +import { DeleteModal } from '../deleteModal'; const UserFilter = ({ filters, setFilters, updateFilters, intl }) => { const inputRef = useRef(null); @@ -289,7 +290,7 @@ export const UserEditMenu = ({ user, token, close, setStatus }) => { ); })} -

+

@@ -315,6 +316,7 @@ export const UserEditMenu = ({ user, token, close, setStatus }) => { export function UserListCard({ user, token, username, setStatus }: Object) { const [isHovered, setHovered] = useState(false); + const [other_username, setUserName] = useState(user.username); return (
  • - {user.username} + {other_username}
    • @@ -363,6 +365,13 @@ export function UserListCard({ user, token, username, setStatus }: Object) { )} + setUserName('user_' + user.id)} + />
    )} diff --git a/frontend/src/components/user/messages.js b/frontend/src/components/user/messages.js index e310092431..d4ece11a08 100644 --- a/frontend/src/components/user/messages.js +++ b/frontend/src/components/user/messages.js @@ -334,6 +334,10 @@ export default defineMessages({ id: 'users.list.actions.setLevel', defaultMessage: 'Set mapper level', }, + redactUser: { + id: 'users.list.actions.redact', + defaultMessage: 'Redact user information', + }, userAttributeUpdationSuccess: { id: 'users.list.attribute.updation.success', defaultMessage: From da9810fff423cefd226a7f3e0b770012beeb76eb Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 2 May 2024 11:14:26 -0600 Subject: [PATCH 4/4] Update list.test.js Of specific note, the background-image check passes prior to the previous commit because two renders happen before the test continues. With the new code, only one render happens. This may have been a source of intermittent test failures in the past. Signed-off-by: Taylor Smock --- frontend/src/components/user/tests/list.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/user/tests/list.test.js b/frontend/src/components/user/tests/list.test.js index c764b005d1..447289fbe3 100644 --- a/frontend/src/components/user/tests/list.test.js +++ b/frontend/src/components/user/tests/list.test.js @@ -37,10 +37,12 @@ describe('User list card', () => { expect(getByText('Beginner')).toBeInTheDocument(); expect(getByText('Shyam')).toBeInTheDocument(); expect(screen.getByText('Shyam').closest('a')).toHaveAttribute('href', '/users/Shyam'); - expect(screen.getByTitle(/Ram/i)).toHaveStyle( + // The first render does not have background-image. The second render does. + await waitFor(() => expect(screen.getByTitle('Ram')).toHaveStyle( `background-image: url(https://www.openstreetmap.org/rails/active_storage/representations/redirect/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBBNXQ2Q3c9PSIsImV4cCI6bnVsbCwicHVyIjoiYmxvYl9pZCJ9fQ==--fe41f1b2a5d6cf492a7133f15c81f105dec06ff7/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaDdCem9MWm05eWJXRjBPZ2h3Ym1jNkZISmxjMmw2WlY5MGIxOXNhVzFwZEZzSGFXbHBhUT09IiwiZXhwIjpudWxsLCJwdXIiOiJ2YXJpYXRpb24ifX0=--058ac785867b32287d598a314311e2253bd879a3/unnamed.webp)`, - ); - expect(container.querySelectorAll('svg').length).toBe(2); + )); + // Two gears (for mapper settings), two trash cans (for user personal information removal) + expect(container.querySelectorAll('svg').length).toBe(4); }); });