Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support updating existing channels for Facebook and Instagram, remove… #5692

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

norkans7
Copy link

… old refresh token views

@norkans7 norkans7 marked this pull request as ready for review November 26, 2024 14:07
Copy link

codecov bot commented Nov 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (8f298ad) to head (92da17f).

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #5692   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          573       573           
  Lines        25798     25735   -63     
=========================================
- Hits         25798     25735   -63     

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

@@ -94,6 +94,7 @@ class Category(Enum):
beta_only = False

unique_addresses = False
matching_addresses_updates = False
Copy link
Author

Choose a reason for hiding this comment

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

To allow update the existing channel with matching address

),
]
menu_items = [
dict(label=_("Reconnect Facebook Page"), view_name="channels.types.facebookapp.claim", obj_view=False)
Copy link
Author

Choose a reason for hiding this comment

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

This keep the link to reconnect on the channel page menu instead of trying to use add new channel that could be confusing to users

Copy link
Author

Choose a reason for hiding this comment

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

Added obj_view as we are linking to a view that will not take the obj uuid as parameter

error_connect = False
if resp.status_code != 200:
error_connect = True
existing_channel = Channel.objects.filter(
Copy link
Author

Choose a reason for hiding this comment

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

We first check for an existing channel and update it.
If none found we create a new one

@rowanseymour
Copy link
Member

So this changes claiming a channel so that it can update an existing channel if the address is the same? I'm not sure that's the most intuitive workflow. I was thinking that we could 1) show that a channel is broken 2) allow reconnecting it. And generally allow updating credentials on existing channels instead of treating them like immutable objects. Isn't that possible in the FB/IG case?

@norkans7
Copy link
Author

On the Facebook app we need to add URLs that are allowed to receive the token from the Facebook login wizard, we can only use the claim URL for now, so that is why I use the approach to use the same view.

@norkans7
Copy link
Author

Instead of the refresh token view we had that used to check the channel credentials is valid, We can have CheckValid view that will do the same but to reconnect the user will need to use the same claim view as adding a new channel

I do not think we want to always use the token on the read page to check the token is valid,

@rowanseymour
Copy link
Member

We already have ChannelType.check_credentials that we use for Twilio and Vonage.. what if we expanded that? We could check on the read page and display an alert with a link to fixing them.

@norkans7
Copy link
Author

ChannelType.check_credentials for Twilio and Vonage is used to check the update credentials are valid,
I think we want to avoid calling the API each time the user goes to the read page however we can use the other view instead as we had.

@norkans7 norkans7 force-pushed the refresh-token-meta-channels branch 2 times, most recently from ace1e1a to b1d5402 Compare November 26, 2024 23:01
@@ -0,0 +1,18 @@
{% extends "smartmin/read.html" %}
{% load smartmin temba humanize channels i18n tz %}
Copy link
Author

Choose a reason for hiding this comment

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

Monosnap Facebook 2024-11-27 00-56-19 Monosnap Facebook 2024-11-27 00-54-39

{% blocktrans trimmed with name=branding.name %}
You can connect your Facebook page to {{ name }} in just a few simple steps.
{% endblocktrans %}
{% if update_existing %}
Copy link
Author

Choose a reason for hiding this comment

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

Monosnap Connect Facebook 2024-11-27 01-38-30

Copy link
Author

Choose a reason for hiding this comment

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

Clarification when the user gets to the page from reconnecting an existing channel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants