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

Review subscription workflow messages #1155

Closed
Jesus89 opened this issue Nov 5, 2019 · 21 comments · Fixed by #1391
Closed

Review subscription workflow messages #1155

Jesus89 opened this issue Nov 5, 2019 · 21 comments · Fixed by #1391

Comments

@Jesus89
Copy link
Member

Jesus89 commented Nov 5, 2019

Now when a user wants to subscribe to a Dataset/Geography, after pressing the button the user gets an exception:

We should add a field in the subscription_info endpoint with this information do_enabled: true/false so we can show a better message with a contact link instead of the buttons to subscribe.

When should we show the request access message?

  • Catalog().subscriptions()
  • dataset/geography.subscription_info()
  • dataset/geography.subscribe()
  • dataset/geography.download()
  • dataset/geography..get_all() (when you pass a credentials instance, it will check your subscriptions)
  • Enrichment()

Message to show when the DO is not enabled

We are sorry, the Data Observatory is not enabled for your account yet. Please contact your customer success manager or send an email to sales@carto.com to request access to it.

Message to show in the subscription flow

<h3>Subscription contract</h3>
You are about to subscribe to <b>{id}</b>.
The cost of this {type} is <b>${subscription_list_price}</b>.
If you want to proceed, a Request will be sent to CARTO who will
order the data and load it into your account.
if (${estimated_delivery_days} == 0) {
	This {type} is available for Instant Subscription for your organization,
	so it will automatically process the order and you will get immediate access to the {type}.
} else {
	This {type} will be available in your account in about ${estimated_delivery_days} days.
	We will contact you shortly to complete the subscription details.
}
In order to proceed we need you to agree to the License of the {type}
available at <b><a href="{tos_link}" target="_blank">this link</a></b>.
<br>Do you want to proceed?

cc @gonzaloriestra

@Jesus89 Jesus89 added this to the [1.0rc1] Feedback enhancements milestone Nov 5, 2019
@cmongut
Copy link
Contributor

cmongut commented Nov 18, 2019

Message suggestion:

"We are sorry. The Data Observatory is not enabled for your account. Contact your customer success manager or send an email to sales@carto.com to get access to it."

@xavipereztr could you confirm this can be the message to show to people that don't have the DO enabled?

@xavipereztr
Copy link

w/ some minor changes in case you find them useful:

"We are sorry, the Data Observatory is not enabled for your account yet. Please contact your customer success manager or send an email to sales@carto.com to request access to it."

@cmongut
Copy link
Contributor

cmongut commented Nov 18, 2019

👍

@alasarr
Copy link
Contributor

alasarr commented Nov 18, 2019

We need to extend this feature to all the methods of DO: subscribe, enrich, download, etc... Not only when we want to subscribe. Thus, we need to add a more generic endpoint that we could request to check the status of the user.

@alrocar where should be the right place to place this endpoint?

@alrocar
Copy link
Contributor

alrocar commented Nov 18, 2019

If I'm not wrong, right now if a user does not have DO enabled the APIs raise an exception as stated in the description of the issue. Shouldn't be enough with capturing the error in each case?

This concrete issue is a different case, what @Jesus89 proposes is to know in advance if the user has DO enabled or not to adapt the UI. In case we want to implement that, I think a good place could be the public /api/v4/me endpoint.

@alasarr
Copy link
Contributor

alasarr commented Nov 18, 2019

Cool, /api/v4/me makes lots of sense for me.

I'd like to show an error if the user tries to perform an enrichment for example. CF should check it before running an operation and raises the exception if users have not data to DO.

@alrocar
Copy link
Contributor

alrocar commented Nov 18, 2019

I think we can decide about this on a per case basis. Right now in enrichment, BQ throws an error (a 403 or similar) and we catch it in CF. Wouldn't be just enough with that?

For other cases, it might be interesting the APIs to be the ones doing the validation, so we can store usage/interest metrics even when the user does not have DO enabled (just thinking out loud, not sure if this makes any sense).

@alasarr
Copy link
Contributor

alasarr commented Nov 18, 2019

I think we can decide about this on a per case basis. Right now in enrichment, BQ throws an error (a 403 or similar) and we catch it in CF. Wouldn't be just enough with that?

I think the following workflow would be easier since you don't need to catch the error at each method.

  • To have an endpoint which returns if the user has access to DO
  • Storage the response in the credentials object.
  • Add a decorator for each method need to access to DO.

We could just create a generic decorator to catch generic error responses from BQ and our API, but it will be more difficult.

@gonzaloriestra
Copy link

Right now we are checking if DO is enabled for token, subscribe and unsubscribe endpoints, but not for subscriptions or subscription_info.

If we also check it when calling subscription_info (I think it doesn't make sense to show the info if you can't do anything else), this issue would be solved, without having to make an extra call and add more information to /me.

And as token is also protected, I think all the flows from CARTOframes are covered, aren't they? @oleurud?

@alrocar
Copy link
Contributor

alrocar commented Nov 20, 2019

I have one question here... shouldn't be subscriptions open to anyone? Even when they don't have DO enabled, it'd be useful to understand possible users interested.

@simon-contreras-deel
Copy link
Contributor

simon-contreras-deel commented Nov 20, 2019

We have different cases about that.

User out of team account (without DO):

  • subscription_info: ForbiddenErrorException: Access denied
  • subscribe: ForbiddenErrorException: Access denied
  • subscriptions: Works!
  • enrichment: ServerErrorException: ['The user does not have Data Observatory enabled']
  • download: ServerErrorException: ['The user does not have Data Observatory enabled']

Team user without DO:

  • subscription_info: Works
  • subscribe: Message to subscribe and then ServerErrorException: ['The user does not have Data Observatory enabled']
  • subscriptions: Works!
  • enrichment: ServerErrorException: ['The user does not have Data Observatory enabled']
  • download: ServerErrorException: ['The user does not have Data Observatory enabled']

What I think we should offer to any user without DO:

  • subscription_info: Works
  • subscribe: You dont have access message. Contact us at xxx to bring you access
  • subscriptions: You dont have access message. Contact us at xxx to bring you access
  • enrichment: You dont have access message. Contact us at xxx to bring you access
  • download: You dont have access message. Contact us at xxx to bring you access

Makes sense for you?

@alasarr
Copy link
Contributor

alasarr commented Nov 20, 2019

After a call with @oleurud we've found a solution where we're going to use token endpoint to validate if the user has access to DO.

  1. We're going to create a decorator @do_access_required in CF.
  2. We decorator will call to token endpoint.
    2.1. If token is returned, the decorator will call call to the decorated function.
    2.2. If no token is returned because of lack of do permissions, we'll catch the error and we'll display it, the decorated function won't be executed
    2.3. In both cases CF will save the result (in memory) to avoid future calls to token endpoint.

Decorator is just a suggestion, feel free to use another approach if it fits better.

@Jesus89
Copy link
Member Author

Jesus89 commented Nov 20, 2019

In case the user has no access to DO, should we show the subscription info, with a different message to contact CARTO? #1155 (comment)

@alasarr
Copy link
Contributor

alasarr commented Nov 20, 2019

No, the subscription info cannot be fetched if the user has no access to DO.

The message to contact CARTO would be great any time we say the user has no access to DO: enrichment, subscribe, download, etc...

@gonzaloriestra
Copy link

Ok, then we have to check if it's enabled from the subscription_info endpoint as well and catch the error from CARTOframes properly 👍

Do you think we should improve the message The user does not have Data Observatory enabled to include instructions or something?

@alasarr
Copy link
Contributor

alasarr commented Nov 21, 2019

I think we can include the email to write to support or a link to a contact form. @cmongut please advise

@cmongut
Copy link
Contributor

cmongut commented Nov 21, 2019

We have already talked about messaging above. I have updated the first comment with the message agreed 🙂

@gonzaloriestra
Copy link

Oops, sorry! 😅

@alasarr
Copy link
Contributor

alasarr commented Nov 21, 2019

Because of an issue with speed licensing, instead of implementing this via the token endpoint we're going to do it via /api/v4/me

Blocked until CartoDB/cartodb#15254 will be completed

@Jesus89 Jesus89 changed the title Add friendly message when DO is disabled Review subscription workflow messages Dec 3, 2019
@Jesus89
Copy link
Member Author

Jesus89 commented Dec 12, 2019

This is unblocked with https://github.com/CartoDB/cartodb-central/pull/2620

@Jesus89 Jesus89 added the triage label Dec 12, 2019
@Jesus89
Copy link
Member Author

Jesus89 commented Dec 17, 2019

Related to #1331

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

Successfully merging a pull request may close this issue.

7 participants