From 5369a5a8928a44f6134d0ff0506e73ee640482db Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Mon, 21 Oct 2024 13:51:25 -0400 Subject: [PATCH] feat: add tags to conversations on removal (#16933) --- .../unit/admin/views/test_malware_reports.py | 19 +++-- tests/unit/helpdesk/test_services.py | 69 +++++++++++++++++++ warehouse/admin/views/malware_reports.py | 17 ++++- warehouse/helpdesk/interfaces.py | 5 ++ warehouse/helpdesk/services.py | 33 +++++++++ 5 files changed, 137 insertions(+), 6 deletions(-) diff --git a/tests/unit/admin/views/test_malware_reports.py b/tests/unit/admin/views/test_malware_reports.py index ec72b6efef75..cad8e71af6ba 100644 --- a/tests/unit/admin/views/test_malware_reports.py +++ b/tests/unit/admin/views/test_malware_reports.py @@ -135,7 +135,13 @@ def test_malware_reports_project_verdict_remove_malware(self, db_request): owner_user = UserFactory.create(is_frozen=False) project = ProjectFactory.create() RoleFactory(user=owner_user, project=project, role_name="Owner") - report = ProjectObservationFactory.create(kind="is_malware", related=project) + report = ProjectObservationFactory.create( + kind="is_malware", + related=project, + additional={ + "helpscout_conversation_url": "https://example.com/conversation/123" + }, + ) db_request.POST["confirm_project_name"] = project.name db_request.route_path = lambda a: "/admin/malware_reports/" @@ -236,7 +242,13 @@ def test_detail_remove_malware_for_project(self, db_request): owner_user = UserFactory.create(is_frozen=False) project = ProjectFactory.create() RoleFactory(user=owner_user, project=project, role_name="Owner") - report = ProjectObservationFactory.create(kind="is_malware", related=project) + report = ProjectObservationFactory.create( + kind="is_malware", + related=project, + additional={ + "helpscout_conversation_url": "https://example.com/conversation/123" + }, + ) db_request.matchdict["observation_id"] = str(report.id) db_request.POST["confirm_project_name"] = project.name @@ -254,8 +266,7 @@ def test_detail_remove_malware_for_project(self, db_request): assert db_request.session.flash.calls == [ pretend.call(f"Deleted the project '{project.name}'", queue="success"), pretend.call( - f"Malware Project {report.related.name} removed.\n" - "Please update related Help Scout conversations.", + f"Malware Project {report.related.name} removed and report updated", queue="success", ), ] diff --git a/tests/unit/helpdesk/test_services.py b/tests/unit/helpdesk/test_services.py index 5e92768af3d3..42f79271a274 100644 --- a/tests/unit/helpdesk/test_services.py +++ b/tests/unit/helpdesk/test_services.py @@ -10,6 +10,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import http + from textwrap import dedent import pretend @@ -125,3 +127,70 @@ def test_create_conversation(self): assert resp == "https://api.helpscout.net/v2/conversations/123" assert len(responses.calls) == 1 + + @responses.activate + def test_add_tag(self): + responses.get( + "https://api.helpscout.net/v2/conversations/123", + headers={"Content-Type": "application/hal+json"}, + json={"tags": [{"id": 9150, "color": "#929499", "tag": "existing_tag"}]}, + ) + responses.put( + "https://api.helpscout.net/v2/conversations/123/tags", + match=[ + responses.matchers.json_params_matcher( + {"tags": ["existing_tag", "added_tag"]} + ) + ], + status=http.HTTPStatus.NO_CONTENT, + ) + + service = HelpScoutService( + session=requests.Session(), + bearer_token="fake token", + mailbox_id="12345", + ) + + service.add_tag( + conversation_url="https://api.helpscout.net/v2/conversations/123", + tag="added_tag", + ) + + assert len(responses.calls) == 2 + # GET call + get_call = responses.calls[0] + assert get_call.request.url == "https://api.helpscout.net/v2/conversations/123" + assert get_call.request.headers["Authorization"] == "Bearer fake token" + assert get_call.response.json() == { + "tags": [{"id": 9150, "color": "#929499", "tag": "existing_tag"}] + } + # PUT call + put_call = responses.calls[1] + assert ( + put_call.request.url + == "https://api.helpscout.net/v2/conversations/123/tags" + ) + assert put_call.request.headers["Authorization"] == "Bearer fake token" + assert put_call.response.status_code == http.HTTPStatus.NO_CONTENT + + @responses.activate + def test_add_tag_with_duplicate(self): + responses.get( + "https://api.helpscout.net/v2/conversations/123", + headers={"Content-Type": "application/hal+json"}, + json={"tags": [{"id": 9150, "color": "#929499", "tag": "existing_tag"}]}, + ) + + service = HelpScoutService( + session=requests.Session(), + bearer_token="fake token", + mailbox_id="12345", + ) + + service.add_tag( + conversation_url="https://api.helpscout.net/v2/conversations/123", + tag="existing_tag", + ) + + # No PUT call should be made + assert len(responses.calls) == 1 diff --git a/warehouse/admin/views/malware_reports.py b/warehouse/admin/views/malware_reports.py index efd03c3686ca..55a86e4bea27 100644 --- a/warehouse/admin/views/malware_reports.py +++ b/warehouse/admin/views/malware_reports.py @@ -20,6 +20,7 @@ from pyramid.view import view_config from warehouse.authnz import Permissions +from warehouse.helpdesk.interfaces import IHelpDeskService from warehouse.observations.models import Observation from warehouse.utils.project import ( confirm_project, @@ -180,6 +181,13 @@ def malware_reports_project_verdict_remove_malware(project, request): # prohibit the project prohibit_and_remove_project(project, request, comment="malware") + # tell helpdesk to add a tag to all related conversations + helpdesk_service = request.find_service(IHelpDeskService) + for observation in observations: + helpdesk_service.add_tag( + conversation_url=observation.additional["helpscout_conversation_url"], + tag="removed_as_malware_via_admin", + ) request.session.flash( f"Malware Project {project.name} removed.\n" @@ -293,9 +301,14 @@ def remove_malware_for_project(request): prohibit_and_remove_project(project, request, comment="malware") + helpdesk_service = request.find_service(IHelpDeskService) + helpdesk_service.add_tag( + conversation_url=observation.additional["helpscout_conversation_url"], + tag="removed_as_malware_via_admin", + ) + request.session.flash( - f"Malware Project {project.name} removed.\n" - "Please update related Help Scout conversations.", + f"Malware Project {project.name} removed and report updated", queue="success", ) return HTTPSeeOther(request.route_path("admin.malware_reports.list")) diff --git a/warehouse/helpdesk/interfaces.py b/warehouse/helpdesk/interfaces.py index 82e94421c343..528643ad1ae3 100644 --- a/warehouse/helpdesk/interfaces.py +++ b/warehouse/helpdesk/interfaces.py @@ -25,3 +25,8 @@ def create_conversation(*, request_json: dict) -> str: """ Create a new conversation in the helpdesk service. """ + + def add_tag(*, conversation_url: str, tag: str) -> None: + """ + Add a tag to a conversation. + """ diff --git a/warehouse/helpdesk/services.py b/warehouse/helpdesk/services.py index 4cb51441ae49..d26ff33ad954 100644 --- a/warehouse/helpdesk/services.py +++ b/warehouse/helpdesk/services.py @@ -50,6 +50,10 @@ def create_conversation(self, *, request_json: dict) -> str: print(dedent(pprint.pformat(request_json))) return "localhost" + def add_tag(self, *, conversation_url: str, tag: str) -> None: + print(f"Adding tag '{tag}' to conversation '{conversation_url}'") + return + @implementer(IHelpDeskService) class HelpScoutService: @@ -120,3 +124,32 @@ def create_conversation(self, *, request_json: dict) -> str: resp.raise_for_status() # return the API-friendly location of the conversation return resp.headers["Location"] + + def add_tag(self, *, conversation_url: str, tag: str) -> None: + """ + Add a tag to a conversation in HelpScout + https://developer.helpscout.com/mailbox-api/endpoints/conversations/tags/update/ + """ + # Get existing tags and append new one + resp = self.http.get( + conversation_url, headers={"Authorization": f"Bearer {self.bearer_token}"} + ) + resp.raise_for_status() + + # collect tag strings from response + tags = [tag["tag"] for tag in resp.json()["tags"]] + + if tag in tags: + # tag already exists, no need to add it + return + + tags.append(tag) + + resp = self.http.put( + f"{conversation_url}/tags", + headers={"Authorization": f"Bearer {self.bearer_token}"}, + json={"tags": tags}, + timeout=10, + ) + resp.raise_for_status() + return