handle Zendesk ticket deletions#7569
Conversation
escattone
force-pushed
the
handle-deleted-zendesk-tickets-3085
branch
4 times, most recently
from
cffe85d to
f5f71dd
Compare
escattone
force-pushed
the
handle-deleted-zendesk-tickets-3085
branch
from
f5f71dd to
08926f3
Compare
| self.assertIn("Test Subject", str_repr) | ||
| self.assertIn("processing_failed", str_repr) | ||
|
|
||
| def test_is_zendesk_deleted_reflects_zd_deleted_at(self): |
There was a problem hiding this comment.
This test is not needed - it's testing the language and not the behavior
|
|
||
| zd_ticket, zd_comments = fetch_zendesk_ticket_data(str(ticket_id)) | ||
| try: | ||
| ticket = qs.filter(zd_deleted_at__isnull=True).get() |
There was a problem hiding this comment.
Given that we introduce more cases on when it's appropriate to sync the tickets, I wonder if we need a manager to handle that. We could very well override the default manager in the model. This will allow us to make changes on what's updated in a single place
| return "direct_support" | ||
|
|
||
| @property | ||
| def is_zendesk_deleted(self): |
There was a problem hiding this comment.
Although having a timestamp makes a lot of sense and adds metadata that we can potentially need, this property also introduces another way of querying about the status of the ticket. Status should only be reflected by the status choices.
With this new addition, the status of a ticket is only derived by looking at two places and it adds complexity to templates and queries. A potential solution could be here either to collapse this into the statuses or use managers in a single place to get the right tickets
There was a problem hiding this comment.
I agree. Since Zendesk doesn't have a "deleted" status, I'd rather not fold that into zd_status, but I'll look into a clean solution.
| self.assertIsNotNone(self.ticket.zd_deleted_at) | ||
|
|
||
| @patch("kitsune.customercare.tasks.sync_ticket_from_zendesk") | ||
| def test_permanently_deleted_marks_ticket_deleted(self, mock_sync): |
|
@akatsoulas Ready for another review. |
2 participants
mozilla/sumo#3085