Zendesk request override get tasks #5429

Vagoasdf · 2024-10-29T13:52:43Z

Description Of Changes

Creating a new Request Override that would check for the status of the ticket on a delete request and halt it if there is an open ticket

Currently we are using a default list to determine if a ticket is open. Basically, anything that its not "closed" its open. The list is ["new", "open", "pending", "hold", "solved"]

It could be a parametrized list on the future, but the current fides iteration does not support indicating a multi-select field from the config.yml file

Code Changes

Creating new Request Override
Creating tests for the new functionality
Adding fixtures that manage the new state

Steps to Confirm

Create a new user with a known email on Zendesk via Postman
Create a few new tickets for that username via Postman.
Set up the Zendesk Integration (This can be done copying the relevant config.yml on the paired PR)
Try to make a Delete request through the test site with the known email
Check the Status of the request. It should fail with the message ""User still has open tickets, halting request"
Update the tickets status to closed via Postman
Try to make a Delete Request through the test site with the known email
Check the status of the request. It should've succesed
Check on Postman and/or Zendesk if the user still exists. It should not

Pre-Merge Checklist

All CI Pipelines Succeeded
Issue Requirements are Met
Relevant Follow-Up Issues Created
Update CHANGELOG.md

Adding an override that checks on the status of the tickets, and skips the delete in case the user still has open tickets

…leting

vercel · 2024-10-29T13:52:47Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 6, 2024 0:46am

Vagoasdf · 2024-10-29T13:55:00Z

I'm not sure on which of the Two PR's the Changelog should be updated. Putting them here for now

cypress · 2024-10-29T14:09:03Z

fides Run #10820

Run Properties: Passed #10820 • 1a059d2868 ℹ️: Merge 161a7f13e6544dbe9ad966d16374605b12a6e694 into c11da91030fa3d057c1706d14ea7...

Project	`fides`
Branch Review	`refs/pull/5429/merge`
Run status	`Passed #10820`
Run duration	`00m 38s`
Commit	`1a059d2868 ℹ️: Merge 161a7f13e6544dbe9ad966d16374605b12a6e694 into c11da91030fa3d057c1706d14ea7...`
Committer	`Bruno Gutierrez Rios`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`4`
View all changes introduced in this branch ↗︎

codecov · 2024-10-29T20:41:30Z

Codecov Report

Attention: Patch coverage is 48.48485% with 17 lines in your changes missing coverage. Please review.

Project coverage is 85.41%. Comparing base (c11da91) to head (161a7f1).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...rride_implementations/zendesk_request_overrides.py	48.48%	17 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5429      +/-   ##
==========================================
- Coverage   85.47%   85.41%   -0.06%     
==========================================
  Files         384      385       +1     
  Lines       24119    24152      +33     
  Branches     2625     2630       +5     
==========================================
+ Hits        20615    20630      +15     
- Misses       2950     2967      +17     
- Partials      554      555       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Vagoasdf · 2024-10-30T15:05:09Z

Is it possible that Codecov is missing the test coverage because we are missing the Zendesk_config.yml file?

galvana · 2024-10-29T23:39:40Z

src/fides/api/service/saas_request/override_implementations/zendesk_request_overrides.py

+## Uplifted from Connectors-logitech
+## See https://github.com/ethyca/connectors-logitech/blob/main/connectors/zendesk/zendesk_request_overrides.py#L95


We don't want to mention customer's names in this public repo

galvana · 2024-10-30T18:02:08Z

src/fides/api/service/saas_request/override_implementations/zendesk_request_overrides.py

+
+
+@register("zendesk_logitech_tickets_read", [SaaSRequestType.READ])
+def zendesk_logitech_tickets_read(


Same here, let's just call it zendesk_tickets_read

Suggested change

def zendesk_logitech_tickets_read(

def zendesk_tickets_read(

galvana · 2024-10-30T18:11:29Z

tests/fixtures/saas/zendesk_fixtures.py

@@ -40,7 +40,7 @@ def zendesk_erasure_identity_email() -> str:
 class ZendeskClient:
    def __init__(self, secrets: Dict[str, Any]):
        self.base_url = f"https://{secrets['domain']}"
-        self.auth = secrets["username"], secrets["api_key"]
+        self.auth = secrets["username"] + "/token", secrets["api_key"]


I commented on this somewhere else too, but if we don't have a migration in place to migrate the username, we should just leave things as they are. I agree this is cleaner, but we would break existing integrations

In that case, i think we should add it to the hoverable description on the config. Something that points to the user that they hadd to add the "/token" at the end without lookinv over the Zendesk Docs

galvana · 2024-10-30T18:15:58Z

CHANGELOG.md

@@ -21,6 +21,9 @@ The types of changes are:
 - Added DataHub integration config [#5401](https://github.com/ethyca/fides/pull/5401)
 - Added keepalive settings to the Redshift integration [#5433](https://github.com/ethyca/fides/pull/5433)

+### Changed
+- Adding a Request Override on Zendesk Ticket's that checks for the tickets statuses on a Delete Request


Suggested change

- Adding a Request Override on Zendesk Ticket's that checks for the tickets statuses on a Delete Request

- Preventing erasures for the Zendesk integration if there are any open tickets [#5429](https://github.com/ethyca/fides/pull/5429)

galvana · 2024-10-30T18:16:23Z

src/fides/api/service/saas_request/override_implementations/zendesk_request_overrides.py

+    """
+    if policy.get_rules_for_action(action_type=ActionType.erasure):
+        for ticket in tickets:
+            ## TODO: Check with Ramp that the Statuses list is the expected one


Same here, let's avoid putting customer names in the comments

Once we move to fidesplus, we're ok 😄

tests/ops/integration_tests/saas/test_zendesk_task.py

galvana · 2024-10-30T18:17:59Z

tests/ops/integration_tests/saas/test_zendesk_task.py

+            (
+                access_results,
+                erasure_results,
+            ) = await zendesk_runner.non_strict_erasure_request(
+                access_policy=policy,
+                erasure_policy=erasure_policy_string_rewrite,
+                identities={"email": zendesk_erasure_identity_email},
+            )


We can simplify this since we're not using the results

Suggested change

(

access_results,

erasure_results,

) = await zendesk_runner.non_strict_erasure_request(

access_policy=policy,

erasure_policy=erasure_policy_string_rewrite,

identities={"email": zendesk_erasure_identity_email},

)

await zendesk_runner.non_strict_erasure_request(

access_policy=policy,

erasure_policy=erasure_policy_string_rewrite,

identities={"email": zendesk_erasure_identity_email},

)

galvana

Just one small change and we're good to go!

galvana · 2024-11-04T23:58:02Z

src/fides/api/service/saas_request/override_implementations/zendesk_request_overrides.py

+    """
+    if policy.get_rules_for_action(action_type=ActionType.erasure):
+        for ticket in tickets:
+            if ticket["status"] in ["new", "open", "pending", "hold", "solved"]:


I gave this some more thought, we can be a bit safer about this. Let's just halt the request if any ticket is not set to "closed". It's better to check for an explicit "closed" than to try to enumerate over all statuses that can be considered open.

Suggested change

if ticket["status"] in ["new", "open", "pending", "hold", "solved"]:

if ticket["status"] != "closed":

cypress · 2024-11-06T12:59:18Z

fides Run #10821

Run Properties: Passed #10821 • 5ca600a13c: Zendesk request override get tasks (#5429)

Project	`fides`
Branch Review	`main`
Run status	`Passed #10821`
Run duration	`00m 38s`
Commit	`5ca600a13c: Zendesk request override get tasks (#5429)`
Committer	`Bruno Gutierrez Rios`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`4`
View all changes introduced in this branch ↗︎

Vagoasdf added 3 commits October 28, 2024 15:12

Uplifting Override from Logitech Connectors

367e604

Adding an override that checks on the status of the tickets, and skips the delete in case the user still has open tickets

Updated Fixtures and Tests. Testing for Ticket being closed before de…

1e8d2eb

…leting

Static Checks

8725742

Vagoasdf requested a review from a team as a code owner October 29, 2024 13:52

Updating Changelog

6bc4d17

Vagoasdf requested review from galvana and Roger-Ethyca and removed request for a team, galvana and Roger-Ethyca October 29, 2024 15:06

Merge branch 'main' into zendesk_request_override_get_tasks

dfc77ec

vercel bot deployed to Preview October 30, 2024 18:15 View deployment

galvana requested changes Oct 30, 2024

View reviewed changes

Vagoasdf added 4 commits October 30, 2024 16:32

Addressing Review Requests

2abf705

Updating Changelog

a3f767a

Correcting the reason of why zendesk is skipped

c8d37e2

Merge branch 'main' into zendesk_request_override_get_tasks

8a2e70b

vercel bot deployed to Preview October 31, 2024 20:31 View deployment

Vagoasdf requested a review from galvana November 1, 2024 12:33

Merge branch 'main' into zendesk_request_override_get_tasks

ef3d6c0

vercel bot deployed to Preview November 4, 2024 13:44 View deployment

Merge branch 'main' into zendesk_request_override_get_tasks

a080c0e

vercel bot deployed to Preview November 4, 2024 23:55 View deployment

galvana requested changes Nov 5, 2024

View reviewed changes

Using Not closed in favour of list of open status

a1054c8

Vagoasdf requested a review from galvana November 5, 2024 16:01

Merge branch 'main' into zendesk_request_override_get_tasks

a6609fb

galvana approved these changes Nov 5, 2024

View reviewed changes

Merge branch 'main' into zendesk_request_override_get_tasks

161a7f1

Vagoasdf merged commit 5ca600a into main Nov 6, 2024
12 of 14 checks passed

Vagoasdf deleted the zendesk_request_override_get_tasks branch November 6, 2024 12:43

vercel bot deployed to Preview November 6, 2024 12:46 View deployment

Vagoasdf mentioned this pull request Nov 6, 2024

Running static checks properly #5471

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zendesk request override get tasks #5429

Zendesk request override get tasks #5429

Vagoasdf commented Oct 29, 2024 •

edited

Loading

vercel bot commented Oct 29, 2024 •

edited

Loading

Vagoasdf commented Oct 29, 2024 •

edited

Loading

cypress bot commented Oct 29, 2024 •

edited

Loading

codecov bot commented Oct 29, 2024 •

edited

Loading

Vagoasdf commented Oct 30, 2024

galvana Oct 29, 2024

galvana Oct 30, 2024

galvana Oct 30, 2024

Vagoasdf Oct 30, 2024

galvana Oct 30, 2024

galvana Oct 30, 2024

galvana Oct 30, 2024

galvana Oct 30, 2024

galvana left a comment

galvana Nov 4, 2024

cypress bot commented Nov 6, 2024

		## Uplifted from Connectors-logitech
		## See https://github.com/ethyca/connectors-logitech/blob/main/connectors/zendesk/zendesk_request_overrides.py#L95



		@register("zendesk_logitech_tickets_read", [SaaSRequestType.READ])
		def zendesk_logitech_tickets_read(

	- Adding a Request Override on Zendesk Ticket's that checks for the tickets statuses on a Delete Request
	- Preventing erasures for the Zendesk integration if there are any open tickets [#5429](https://github.com/ethyca/fides/pull/5429)

	if ticket["status"] in ["new", "open", "pending", "hold", "solved"]:
	if ticket["status"] != "closed":

Zendesk request override get tasks #5429

Zendesk request override get tasks #5429

Conversation

Vagoasdf commented Oct 29, 2024 • edited Loading

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

vercel bot commented Oct 29, 2024 • edited Loading

Vagoasdf commented Oct 29, 2024 • edited Loading

cypress bot commented Oct 29, 2024 • edited Loading

codecov bot commented Oct 29, 2024 • edited Loading

Codecov Report

Vagoasdf commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

galvana left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Nov 6, 2024

Vagoasdf commented Oct 29, 2024 •

edited

Loading

vercel bot commented Oct 29, 2024 •

edited

Loading

Vagoasdf commented Oct 29, 2024 •

edited

Loading

cypress bot commented Oct 29, 2024 •

edited

Loading

codecov bot commented Oct 29, 2024 •

edited

Loading