Skip to content

Delete request event view#5852

Closed
norkans7 wants to merge 1 commit into
mainfrom
delete-request-event-view
Closed

Delete request event view#5852
norkans7 wants to merge 1 commit into
mainfrom
delete-request-event-view

Conversation

@norkans7
Copy link
Copy Markdown

@norkans7 norkans7 commented Feb 11, 2025

After #5867

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (cda385c) to head (d396851).
Report is 289 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #5852   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          490       490           
  Lines        25251     25264   +13     
=========================================
+ Hits         25251     25264   +13     

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@norkans7 norkans7 force-pushed the delete-request-event-view branch from 08552ab to ebbe09c Compare February 11, 2025 16:27
@norkans7 norkans7 marked this pull request as ready for review February 11, 2025 16:49
@norkans7
Copy link
Copy Markdown
Author

Monosnap Data deletion request 2025-02-11 18-26-29

@norkans7 norkans7 force-pushed the delete-request-event-view branch from ebbe09c to c9a3c01 Compare February 11, 2025 16:55
@norkans7 norkans7 marked this pull request as draft February 12, 2025 17:40
@norkans7 norkans7 force-pushed the delete-request-event-view branch from c9a3c01 to ab28187 Compare February 12, 2025 17:49
@norkans7 norkans7 changed the base branch from main to channel-event-delete-request-3 February 12, 2025 17:49
@norkans7 norkans7 force-pushed the channel-event-delete-request-3 branch 2 times, most recently from 98778aa to a0060e5 Compare February 14, 2025 16:14
@norkans7 norkans7 force-pushed the delete-request-event-view branch 2 times, most recently from 44fd9b8 to 13b713b Compare February 14, 2025 16:31
@norkans7 norkans7 force-pushed the channel-event-delete-request-3 branch from a0060e5 to 893df30 Compare February 14, 2025 16:32
@norkans7 norkans7 force-pushed the delete-request-event-view branch from 13b713b to 55c8264 Compare February 14, 2025 16:49
@norkans7 norkans7 force-pushed the channel-event-delete-request-3 branch from 893df30 to b6de6c9 Compare February 14, 2025 17:17
@norkans7 norkans7 force-pushed the delete-request-event-view branch from 55c8264 to c7d045d Compare February 14, 2025 17:18
@norkans7 norkans7 force-pushed the channel-event-delete-request-3 branch from b6de6c9 to 83e8802 Compare February 17, 2025 17:00
@norkans7 norkans7 force-pushed the delete-request-event-view branch from c7d045d to 69e84ce Compare February 17, 2025 17:00
Base automatically changed from channel-event-delete-request-3 to main February 17, 2025 17:17
@norkans7 norkans7 force-pushed the delete-request-event-view branch from 69e84ce to d396851 Compare February 17, 2025 17:32
@norkans7 norkans7 marked this pull request as ready for review February 17, 2025 17:32
Comment thread temba/channels/views.py

class ChannelEventCRUDL(SmartCRUDL):
model = ChannelEvent
path = "events" # urls like /channels/events/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@norkans7 @ericnewcomer maybe this url.. should be something in the contacts app? E.g.

contact/forgetme/<contact-uuid>

And then when the contact is deleted... it says contact deleted

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we use the channel event UUID since that is the one we are creating on courier at the moment we receive the callback, retrieving the contact UUID i would be a challenge I guess and an issue if we cannot find the contact

I think if we can use the contact/forgetme/<event-uuid> with the channel event UUID that would be fine so not the contact UUID.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can't find the contact.. then there's no channel event and nothing to return and we noop

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This URL is the one we return as confirmation URL in courier
https://github.com/nyaruka/courier/pull/834/files#diff-45e8b1f3d84682ed837b92e0ff2e697b5d885ca171ecc8c3e0d766debedb7f81R258

Do you mean we need to make courier query for the contact before adding the channel event?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Courier has to query to know if we even own the contact in question and channel events are associated with a contact.. so I think I've convinced myself that the delete status URL is just a public facing view with the contact UUID in the URL... @ericnewcomer ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked on the current approach to query the contact, and the logic we have is that we try to get a contact for a URN and if it does not exists we create a contact for it as that is the logic we want for messages
But we can adjust that to make the creation optional

@norkans7
Copy link
Copy Markdown
Author

norkans7 commented Mar 6, 2025

Replaced by #5928

1 similar comment
@norkans7
Copy link
Copy Markdown
Author

Replaced by #5928

@norkans7 norkans7 closed this Mar 18, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants