wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.96k stars 1.31k forks source link

Stats keep failing to load on Jetpack Site when in "Author" role #13476

Open malinajirka opened 3 years ago

malinajirka commented 3 years ago

Expected behavior

~Stats are not visible in Calypso in this scenario, so I assume we should hide the menu item.~

~Note: We are adding Jetpack label in this PR. So if we decide to hide the Stats menu item, we will want to hide the Jetpack label as well.~

See https://github.com/wordpress-mobile/WordPress-Android/issues/13476#issuecomment-734012115 -> stats should load even for users in non-admin roles.

Actual behavior

The stats keep failing to load with com.android.volley.AuthFailureError no matter how many times I try.

Steps to reproduce the behavior

  1. Log in using a .com credentials and switch to a jetpack site on which you are in "Author" role
  2. Click on Stats
  3. Notice an error screen is shown
Tested on [device], Android [version], WPAndroid [version]

Emulator API 28, develop 3ea66f6fbb0ef2f6e86a6eb5ce3a5462b28c9055

designsimply commented 3 years ago

This also happens if you log in to a Jetpack site using the site address flow and then try to view Stats. (1m26s)

I think there may also be a bug in Calypso regarding whether all user roles should have access to stats. Please see https://github.com/Automattic/wp-calypso/issues/14151#issuecomment-416632733.

All of your site’s user roles can see the stats: Administrators, Editors, Authors, and Contributors. Source: https://wordpress.com/support/user-roles/#summary

If it's not expected for all roles to be able to view stats then we should nudge Calypso to remove that expectation from the support docs accordingly.

malinajirka commented 3 years ago

Ohh, good to know. Thank you! I've updated the description.

ravishanker commented 2 years ago

I can reproduce this issue. Calypso is just showing a spinner with no message. App is at least showing Data not loaded as shown below. Log is showing 403 Auth Failure Error as show below. It also happens on iOS too.

What should be done here? Should permissions be fixed on the backend?

Insights Weeks
Screenshot_20220214_183416 Screenshot_20220214_183641

2022-02-14 18:34:10.497 11144-11196/org.wordpress.android.prealpha E/Volley: [59] NetworkUtility.shouldRetryException: Unexpected response code 403 for https://public-api.wordpress.com/rest/v1.1/sites/189860817/stats/post/28/?locale=en_AU 2022-02-14 18:34:10.885 11144-11196/org.wordpress.android.prealpha E/Volley: [59] NetworkUtility.shouldRetryException: Unexpected response code 403 for https://public-api.wordpress.com/rest/v1.1/sites/189860817/stats/post/28/?locale=en_AU 2022-02-14 18:34:10.887 11144-11144/org.wordpress.android.prealpha E/WordPress-API: Volley error on https://public-api.wordpress.com/rest/v1.1/sites/189860817/stats/post/28/?locale=en_AU com.android.volley.AuthFailureError at com.android.volley.toolbox.NetworkUtility.shouldRetryException(NetworkUtility.java:189) at com.android.volley.toolbox.BasicNetwork.performRequest(BasicNetwork.java:145) at com.android.volley.NetworkDispatcher.processRequest(NetworkDispatcher.java:132) at com.android.volley.NetworkDispatcher.processRequest(NetworkDispatcher.java:111) at com.android.volley.NetworkDispatcher.run(NetworkDispatcher.java:90)

ravishanker commented 2 years ago

👋 Hi @wordpress-mobile/owl-team - Pinging you about a resolution on this issue for permissions fix on the backend. Please escalate it to appropriate person/team if required. Thank you. cc: @hannahtinkler

hannahtinkler commented 2 years ago

Hi @ravishanker 👋 I think this might need to be fixed in the Jetpack plugin. The TL;DR is that the Jetpack plugin is explicitly telling us to only allow administrators to view stats.

When the user has installed and activated Jetpack, they are prompted to 'Set up' Jetpack by connecting the Jetpack install with the user's WordPress account. During this setup Jetpack creates a stats_options option array which defaults the roles property to just 'administrator' (i.e. specifying that only administrators can view stats).

When the app/Calypso attempts to get the stats from the API, the API runs the stats_is_blog_user() function which checks whether the user should be able to access the stats. For Jetpack sites, part of this checks to see if the stats_options option has been synced from the site running the Jetpack install. If so, the roles property of the synced value is used to determine which roles should be able to view the stats (I guess so site owners can control this from their external site). The comment for this piece of code says If the blog has synced its stats_options we must honor the roles array so seems like an explicit decision to defer to this value. Given this, it seems like Jetpack might need to alter the default roles it uses in order for the stats to work as described in the docs for Jetpack sites.

All of your site’s user roles can see the stats: Administrators, Editors, Authors, and Contributors.

So at present this is true for WordPress sites, but not sites with the Jetpack plugin activated and connected (unless the owner has changed the stats_options value themselves after connecting).

ravishanker commented 2 years ago

Hi @hannahtinkler Thank you for detailed analysis. That explains the reason behind the issue clearly. I need to ping Jetpack plugin team then.

kraftbj commented 2 years ago

Within Jetpack, there is a setting to configure which roles are allowed to view stats: 2022-02-15 at 9 01 PM

Per Jetpack docs, it is expected that only admin users have access, see https://jetpack.com/support/wordpress-com-stats/ 2022-02-15 at 9 03 PM

The default has been since the first release of Jetpack ( https://github.com/Automattic/jetpack/blob/99e10babcb00e4d5f9a8d9d91584f71626fceb2f/modules/stats.php#L138 ) with other roles not being allowed to see stats, by default.

If the issue is for WordPress.com-on-Atomic sites, that may be a gap that needs to be addressed with a filter and wpcomsh.

ravishanker commented 2 years ago

Given that it is confirmed as not a bug. We're going to look at enhancing the messaging with the help of design and replace current Data not loaded generic message.