Skip to content

Conversation

@MichaelArestad
Copy link
Contributor

@MichaelArestad MichaelArestad commented Jul 18, 2019

Fixes #13035

Changes proposed in this Pull Request:

Updates these states:

  • free plan, Akismet not installed
  • free plan, Akismet installed but not activated
  • free plan, Akismet installed and activated but not configured
  • paid plan, Akismet not installed
  • paid plan, Aksimet installed but not activated
  • paid plan, Akismet installed and activated but not configured

The intention is that users on a free plan will clearly see the options both to upgrade to a paid Jetpack plan and to add a key for an existing Akismet subscription, and users on a paid plan with Akismet somehow misconfigured will clearly see the path to get it configured again.

This PR adds a new Jetpack endpoint: /plugins/akismet/activate. For now I have hard-coded it to only work with Akismet, because a Jetpack endpoint to activate arbitrary plugins would require a larger discussion and isn't relevant to this project. However, /plugins/akismet/activate would be compatible with such an endpoint.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • This not a new feature and neither adds or removes parts of Jetpack.

Testing instructions:

  • Go to Jetpack's dashboard.
  • In each of the following states, observe the Akismet card and verify that "Upgrade" takes you to buy a Jetpack plan if appropriate and "Activate Akismet" takes you to the Akismet settings page, with both installing and/or activating Akismet as necessary:
    • Jetpack on free plan, Akismet not installed.
    • Jetpack on free plan, Akismet installed but not activated.
    • Jetpack on free plan, Akismet installed and activated.
    • Jetpack on personal plan, Akismet not installed.
    • Jetpack on personal plan, Akismet installed but not activated.
    • Jetpack on personal plan, Akismet installed and activated.
    • Other paid plans should behave the same as the personal plan.

Before

image

After

image

Proposed changelog entry for your changes:

  • Improved the flow to purchase and activate Akismet from the At-a-Glance page.

@MichaelArestad MichaelArestad added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] In Progress [Pri] Normal [Feature] Akismet An anti-spam feature that automatically filters spam comments and forms, blocking unwanted content. labels Jul 18, 2019
@MichaelArestad MichaelArestad requested review from a team, crunnells and robertbpugh July 18, 2019 21:56
@MichaelArestad MichaelArestad self-assigned this Jul 18, 2019
@jetpackbot
Copy link
Collaborator

jetpackbot commented Jul 18, 2019

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: September 3, 2019.
Scheduled code freeze: August 27, 2019

Generated by 🚫 dangerJS against d22dd94

crunnells
crunnells previously approved these changes Jul 18, 2019
@MichaelArestad
Copy link
Contributor Author

Moving a comment from @jessefriedman here for continuity:

We evaluated all the possible states and I think I was able to wireframe something up that will reduce all the states to 2.

  1. No API Key inserted, all states of Akismet (installed, uninstalled, active, deactivated, etc…)
  2. API Key configured through either an Akismet plan or Jetpack plan

@robertf4 we chatted on this in Zoom and we feel pretty good about it. @robertbpugh, @marestad, @chrisrunnells, what do you think?
akismet

image

@MichaelArestad
Copy link
Contributor Author

@jessefriedman I'm hesitant to make a change to the Banner adding a link to input an Akismet key for two reasons:

  • It makes the assumption that our users need a key for Akismet.
  • It adds a second Call to Action that takes some attention away to the primary CTA.

Do you think that link is necessary?
How likely is it a user will come to the Jetpack Dashboard on a free plan to add a key to akismet?

In the meantime, I'm going to get my sandbox working again and get you a mockup similar to your proposal.

@MichaelArestad
Copy link
Contributor Author

MichaelArestad commented Aug 6, 2019

image

Note: It's unclear if we can even add this link where I did. I would need @robertf4 to weigh in.

@jessefriedman
Copy link
Member

@MichaelArestad

It makes the assumption that our users need a key for Akismet.

All users of Akismet need a key. I think that is a pretty safe assumption.

How likely is it a user will come to the Jetpack Dashboard on a free plan to add a key to Akismet?

I don't think we need to assume the path is "I'm coming to the At a Glance page to add a key". I agree that is unlikely to be the intention. However, it is very possible that a user arrives at the AaG page and is prompted to upgrade and already has a key. They may be "looking around" especially since Akismet is put under Jetpack in the main nav.

It adds a second Call to Action that takes some attention away to the primary CTA.

I believe it to be supportive of the first call-to-action. Users will likely know if they already have a key, so we aren't putting a fork in the road. It's a very clear path both supporting the need to upgrade to get a key. You either have one or you don't but either way, you know you need to upgrade to get it working.

@robertbpugh
Copy link
Contributor

robertbpugh commented Aug 7, 2019 via email

@MichaelArestad
Copy link
Contributor Author

All users of Akismet need a key. I think that is a pretty safe assumption.

That's true. My concern is more around the flow and perception. If they do already have a key, they likely got it by going through an Akismet-specific flow. I suspect it's unlikely they will be going through the Jetpack Dashboard to complete that flow. In addition, any users that purchase a paid plan have Akismet keys automagically set for them. They never actually input a key. This means that, for Jetpack users, there's likely very little need to set an Akismet key via this route. Perhaps if the key expired or something? CC @jeherve Can you weigh in on this. I might be way off here.

Can we not make it responsive to whether or not they have a key?

@robertbpugh That's a great question. I'm not sure about this one. Perhaps @cfinke can weigh in on whether we can show different UI to someone who has created a key for Akismet, but hasn't entered it yet.

@robertf4 How hard would it be to add the link to the text in the description prop in my above mockup?

@robertf4
Copy link
Contributor

robertf4 commented Aug 8, 2019

@MichaelArestad, adding a simple link is very easy, it just requires adding a prop like this to the JetpackBanner in akismet.jsx, substituting the Akismet settings link:

description={ __( 'Already have a key? {{a}}Activate Akismet{{/a}}', {
    components: { a: <a href="https://www.wordpress.com" /> },
})}

image

If we want to try for more robust behavior such as installing/activating the plugin then we can still do it via a REST API call, which won't be a whole lot of dev but should probably be a separate PR.

@jeherve
Copy link
Member

jeherve commented Aug 8, 2019

This means that, for Jetpack users, there's likely very little need to set an Akismet key via this route. Perhaps if the key expired or something? CC @jeherve Can you weigh in on this. I might be way off here.

That's a super tricky question. I'm really only making assumptions here and I am not very comfortable doing that.

@eoigal and his team are probably in a better position to help, since they've already worked on the different states a site can be in when they designed the Akismet admin page.
They may also have some stats about the proportions of folks landing on the Akismet admin page and choosing the manual API key field vs. the link to connect.

I think that overall, I would keep that card as short and concise as possible. I would delegate as much as possible to the actual Akismet admin page when the plugin is active. When it is not active, I think our focus should be to get them to install / activate the plugin. That can be done as part of the Jetpack Plan purchase flow in most scenarios.

If someone already has a key, I guess that would place them in 3 buckets:

  • They use the same key on multiple sites. In this case, I think it's safe to assume that they are already pretty familiar with Akismet and won't be coming to the Jetpack dashboard to install, activate, and configure the Akismet plugin with their key. I don't know if we should aim to support this scenario in the Jetpack AAG dashboard.
  • They purchased a Jetpack plan but for some reason did not go through the automatic setup that adds Akismet and the key to the site for them. That will happen on a Multisite network for example. In this case, maybe we should detect that they currently have a paid Jetpack plan, and then replace the main call to action from "Upgrade" to "Set up", to help them get set up.
  • They have a legacy Akismet plan for that site. That's the problematic scenario here. Ideally I think we should also detect that by making a call to the Akismet API to get that data for the linked wpcom user, or the site URL, and if we find a plan, we change the main CTA to help them get set up. Or maybe we shouldn't focus on that use-case as it may be an edge case or a legacy scenario that will be phased out eventually?

@jessefriedman
Copy link
Member

@MichaelArestad at this point I think we can only make assumptions about whether they are there with a key or not. The reality of the situation is that the AaG view showcases undone tasks. It's specifically designed to show you what steps you have left to take. So the very nature of this view is one to prompt a user to input a key they may not have entered elsewhere.

Without taking too drastic of an action here we figured we'd do a step process of improving the flow by adding a link that is trackable. This means we'll be able to quickly prove whether people arrive at this view key-in-hand or not. If we see extremely low usage of the link then we have the data we need to remove it. If we don't have that link at all then we are betting on assumption that users don't have a key and we don't really have a way to prove otherwise.

To @robertbpugh's point, @robertf4 and I discussed pinging Akismet with the domain to see if they have a key set and we determined that was out of scope for this project, which takes us back to finding a solution that would allow us to better understand the needs of a user on this page and being able to test into a solution. :)

@robertf4 robertf4 added this to the 7.7 milestone Aug 14, 2019
@robertf4 robertf4 force-pushed the update/akismet-dashitem branch from c4d94f2 to 54f25e9 Compare August 20, 2019 14:07
@robertf4 robertf4 self-assigned this Aug 20, 2019
@robertf4 robertf4 changed the title Akismet Dashitem - update copy for various states At a Glance: Improve Akismet Upgrade and Activate Flow Aug 20, 2019
@robertf4 robertf4 added the [Status] Needs Review This PR is ready for review. label Aug 21, 2019
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

I see the following card when Akismet is installed and active, but not configured, on a free site:

image

The "Activate Akismet" feels a bit out of place?



After going through the flow on a new site, purchasing a Personal plan, and letting Jetpack install, activate, and configure Akismet for me, here is what I see in the Akismet card:

image

Yet the Akismet plugin seems to be configured on the site:

image


On a site with a Professional plan, Akismet active, but no API key set up, here is what I see:

image

Interestingly, there was actually a flashing banner while the page loaded:
http://cl.ly/3694e95fc8be


Do you think you could get rid of the short array syntax here as well?

I would also recommend setting up your editor to get PHPCS recommendations inside your editor, that should help you catch stuff before you commit.

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Aug 21, 2019
@robertf4
Copy link
Contributor

I see the following card when Akismet is installed and active, but not configured, on a free site:
[...]
The "Activate Akismet" feels a bit out of place?

We were trying to use a single description to cover all the cases, but I can see how using "activate" could confuse this as "activated" is a plugin state. What do you think about using "Set up Akismet" instead? It would be a catch-all for all the states the Akismet plugin could be in.

@robertf4
Copy link
Contributor

robertf4 commented Aug 21, 2019

After going through the flow on a new site, purchasing a Personal plan, and letting Jetpack install, activate, and configure Akismet for me, here is what I see in the Akismet card:
[...]
Yet the Akismet plugin seems to be configured on the site:
[...]

This looks like the API key wasn't set, as when you have a Jetpack plan and the API key isn't set you get the "Connect with Jetpack" option that you saw.

I tested this flow a couple of times, and I found that if I load the Jetpack dashboard at the right time during the setup process, I see this:

image

This indicates that the site is on a paid Jetpack plan, but Akismet is somehow not installed, activated, or configured yet.

Then if I wait just a few moments and reload, I see this:

image

So I think what is happening is the dashboard is being loaded as Akismet is being installed, and it should be resolved quickly. In your case you didn't see the Set Up button because you happened to load the page when the plugin was installed and active but the key wasn't set, which was due to the below bug.


On a site with a Professional plan, Akismet active, but no API key set up, here is what I see:
[...]

This ended up being a bug, where that case wasn't correctly showing the "Set Up" option. It has been fixed.

Interestingly, there was actually a flashing banner while the page loaded:
http://cl.ly/3694e95fc8be

I haven't been able to get that to reproduce, are you still seeing that?


Do you think you could get rid of the short array syntax here as well?

Sure, done.

I would also recommend setting up your editor to get PHPCS recommendations inside your editor, that should help you catch stuff before you commit.

I do have phpcbf set up to run on save, but I think I'm using the wpcom ruleset. I'll have to see if I can find a Jetpack ruleset, or perhaps a core-only ruleset.

@robertf4 robertf4 force-pushed the update/akismet-dashitem branch from dab48e9 to 904942b Compare August 21, 2019 21:59
@robertf4 robertf4 force-pushed the update/akismet-dashitem branch from 904942b to d22dd94 Compare August 21, 2019 22:01
@robertf4 robertf4 added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Aug 21, 2019
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

What do you think about using "Set up Akismet" instead? It would be a catch-all for all the states the Akismet plugin could be in.

I like it! 👍 This is not a blocker though, so can be done in a future PR if you'd like.

I do have phpcbf set up to run on save, but I think I'm using the wpcom ruleset. I'll have to see if I can find a Jetpack ruleset, or perhaps a core-only ruleset.

If you run yarn build you should get all you need from Jetpack. Once you have that, you may still have to configure your IDE so it picks up the project's ruleset when available, instead of the global ruleset installed on your machine. My own editor has a "Auto Config Search" option that picks up project's rulesets and uses those when possible, for example.

For more info:
https://github.com/Automattic/jetpack/blob/master/docs/development-environment.md#use-php-codesniffer-and-eslint-to-make-sure-your-code-respects-coding-standards

I haven't been able to get that to reproduce, are you still seeing that?

It does not seem to happen anymore! 👍

All in all, it tests well for me now. This is good to go imo. I would still appreciate it if someone from Catalyst could review it too, to get another pair of eyes on this.

For reference, here are the different statuses right now:

  • No Akismet

no-akismet

  • Akismet installed, but not active

image

  • Akismet active, but not configured

image

  • Akismet active, but not configured on a site with a plan

image

  • Akismet active and configured

image

@MichaelArestad
Copy link
Contributor Author

LGTM! Thanks!

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Aug 23, 2019
@jeherve
Copy link
Member

jeherve commented Aug 23, 2019

Merging. Only this should be left to do now, but it can be done in another PR:

#13085 (comment)

@jeherve jeherve merged commit 659e8bd into master Aug 23, 2019
@jeherve jeherve deleted the update/akismet-dashitem branch August 23, 2019 07:59
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 23, 2019
jeherve added a commit that referenced this pull request Aug 23, 2019
jeherve added a commit that referenced this pull request Aug 27, 2019
* 7.7 changelog: initial set of changes

* Changelog: add #13154

* Changelog: add #13134

* Changelog: add #12699 and many others

* Changelog: add #13127

* Changelog: add #13167

* Changelog: add #13225

* Changelog: add #13179

* Changelog: add #13173

* Changelog: add #13232

* Changelog: add #13137

* Changelog: add #13172

* Changelog: add #13182

* Changelog: add #13200

* Changelog: add #13244

* Changelog: add #13267

* Changelog: add #13204

* changelog: add #13205

* Changelog: add #13183

* Changelog: add #13278

* Changelog: add #13162

* Changelog: add #13268

* Changelog: add #13286

* Changelog: add #13273

* Changelog: add #12474

* Changelog: add #13085

* Changelog: add #13266

* Changelog: add #13306

* Changelog: add #13311

* Changelog: add #13302

* Changelog: add #13196

* Changelog: add #12733

* Changelog: add #13261

* Changelog: add #13322

* Changelog: add #13333

* Changelog: add #13335
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Akismet An anti-spam feature that automatically filters spam comments and forms, blocking unwanted content. [Pri] Normal [Status] Design Review Complete [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

At a Glance: Akismet Anti-spam flow improvement

9 participants