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

Detect when Jetpack Subscriptions module is active and show Dashboard warning #113

Closed
raamdev opened this Issue Jun 26, 2015 · 29 comments

Comments

Projects
None yet
2 participants
@raamdev
Contributor

raamdev commented Jun 26, 2015

The Jetpack Subscriptions module provides similar features as Comment Mail and, just as we do with StCR (see #97) we should detect when that Jetpack module is active and Comment Mail is enabled and show an appropriate warning message.

2015-06-25_22-03-47

@raamdev raamdev added the todo label Jun 26, 2015

@jaswrks jaswrks self-assigned this Jun 26, 2015

@jaswrks

This comment has been minimized.

@jaswrks

This comment has been minimized.

@jaswrks

This comment has been minimized.

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 26, 2015

All set in the dev branch.

@jaswrks jaswrks closed this Jun 26, 2015

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 26, 2015

@jaswsinc I just realized that the Jetpack Subscriptions modules does more than allow subscribing to comments: it also allows subscribing to future posts on the blog.

So what if a site owner wants to use the Jetpack Subscriptions module for allowing subscriptions to the Blog but wants to use Comment Mail for comment subscriptions? I don't see any inherent problem with doing that, except that now Comment Mail is going to scream whenever the Subscriptions module is active.

What if we instead simply check if the 'follow comments' option of the Jetpack Subscriptions module is enabled and then show the warning message (which will need to be updated after making this change to mention specifically the 'follow comments' option as what's causing the message to show)?

2015-06-26_10-59-34

In fact, this might be good ground for a KB Article that describes and shows a screenshot of exactly what needs to be disabled, so if you agree with making this change I think the Jetpack Subscriptions module Dashboard message should be updated to include a link to a new KB Article called "Can I use Jetpack Subscriptions with Comment Mail?"

@raamdev raamdev reopened this Jun 26, 2015

@raamdev raamdev added this to the Initial Public Release milestone Jun 26, 2015

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 26, 2015

Ah, I see what you mean. Yep, that sounds like a good idea to me! :-)


Might be worth opening a feature request for this too. That would make a great addition to Comment Mail, I think. That's the perfect time to pick up a new subscriber, so for us to make that possible with CM would be great.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 26, 2015

Might be worth opening a feature request for this too. That would make a great addition to Comment Mail, I think. That's the perfect time to pick up a new subscriber, so for us to make that possible with CM would be great.

Done! #114

jaswrks pushed a commit that referenced this issue Jun 26, 2015

jaswrks pushed a commit to websharks/comment-mail-pro that referenced this issue Jun 26, 2015

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 26, 2015

I'm ignoring stb_enabled (Blog subscriptions) for now as a short-term solution, just to prevent CM from nagging when you only have blog subscriptions enabled.

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 26, 2015

My suggestion is that we close this now, and let that short-term solution suffice. Instead of putting more work into a KB article or a special notice about this, we can put our focus on #114 instead. If you agree, please feel free to close.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 26, 2015

@jaswsinc Yep, that works for me. I noticed something in your commit (see websharks/comment-mail-pro@110455b#commitcomment-11887387) that could be fixed. Also, can we update the Dashboard message then to specifically mention disabling "Jetpack Follow Comments in your WordPress Discussion Settings"? Right now the Dashboard message implies that you have to disable the entire Subscriptions module.

Otherwise I agree, this issue can be closed.

jaswrks pushed a commit to websharks/comment-mail-pro that referenced this issue Jun 26, 2015

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 26, 2015

@raamdev writes...

I noticed something in your commit (see websharks/comment-mail-pro@110455b#commitcomment-11887387) that could be fixed

Thank you for catching that! :-)

@raamdev writes...

Also, can we update the Dashboard message then to specifically mention disabling "Jetpack Follow Comments in your WordPress Discussion Settings"?

Done. Thanks :-)

@jaswrks jaswrks closed this Jun 26, 2015

@raamdev raamdev modified the milestones: v150709, Initial Public Release Jul 15, 2015

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jul 23, 2015

It appears this Dashboard message is no longer working. There's no warning when the Jetpack Subscriptions module is active with Follow Comments enabled.

  • WordPress v4.2.3
  • Comment Mail Pro v150709
  • Jetpack v3.6

2015-07-23_11-06-22
2015-07-23_11-07-04

@raamdev raamdev reopened this Jul 23, 2015

@raamdev raamdev added this to the Next Release milestone Jul 23, 2015

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jul 24, 2015

I just got a JetPack update to v3.6.1 and I was also unable to reproduce it with the latest version.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jul 24, 2015

I just got a JetPack update to v3.6.1 and I was also unable to reproduce it with the latest version.

Ah, good, so it was related to the latest Jetpack update. Mystery solved!

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jul 24, 2015

No, I was unable to reproduce it with JetPack v3.6.1 either.

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jul 24, 2015

Just for kicks, if that's a test site that you're experimenting with, you might try restarting the server and see if that makes any difference. Perhaps there is an opcode cache that contains the previous release of Comment Mail in memory still. That only comes to mind, because I had two issues earlier today that were related to that problem with PHP 5.5 running the Opcache extension with timestamp checking off.

http://php.net/manual/en/opcache.configuration.php#ini.opcache.validate-timestamps

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jul 24, 2015

unable

Oh! Interesting. Let me run a few more tests and see if I can reproduce it.

you might try restarting the server

Will do.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jul 24, 2015

@jaswsinc I checked the server where I initially discovered this issue and that server does not have APC or Opcache installed.

I then used my raam.websharks-inc.net test site:

  1. Disabled Comment Mail Plugin Deletion Safeguards
  2. Deactivated and deleted Comment Mail Pro
  3. Updated Jetpack to 3.6.1 and confirmed "Follow Comments" was enabled in Dashboard → Settings → Discussion
  4. Installed and activated Comment Mail Pro v150709
  5. Enabled Comment Mail Pro

At that point I would've expected to see the warning about Jetpack + Comment Mail, but nothing. Only a message that said my settings were updated.


I'm not able to get that Jetpack + Comment Mail warning to show no matter what I do.

Can you give me a list of steps you're following to get it to show?

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jul 24, 2015

Tested Comment Mail Lite v150709; same issue. No Dashboard message warning about potential Jetpack conflict.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jul 24, 2015

Ah Ha! I I think I figured out the problem!

When I go into Dashboard → Settings → Discussion and simply click "Save Changes" at the bottom, then the notice shows up!

2015-07-24_19-35-37

So, it sounds like the option that Comment Mail is checking for (maybe stb_enabled on this line does't exist, until the Discussion Settings are actually saved, even though the Jetpack Follow Comments option is enabled (it's enabled by default when you install Jetpack).

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jul 24, 2015

That's it! Nice work tracking that down.
Referencing: https://github.com/Automattic/jetpack/blob/202d1b80e9386aa6ed2dceb6ab9a4f362413e695/modules/comments/comments.php#L210

We need to pass a default option value in the second argument to get_option() in our check.

jaswrks pushed a commit that referenced this issue Jul 24, 2015

@jaswrks jaswrks referenced this issue Jul 24, 2015

Merged

PR: feature/113 #129

@jaswrks jaswrks added the in progress label Jul 24, 2015

jaswrks pushed a commit to websharks/comment-mail-pro that referenced this issue Jul 24, 2015

@jaswrks jaswrks referenced this issue Jul 24, 2015

Merged

PR: feature/113 #7

jaswrks pushed a commit to websharks/comment-mail-pro that referenced this issue Jul 24, 2015

jaswrks pushed a commit that referenced this issue Jul 24, 2015

@jaswrks jaswrks closed this in #129 Jul 24, 2015

@jaswrks jaswrks removed the in progress label Jul 24, 2015

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jul 24, 2015

Next Lite Release:

  • JetPack Conflict Detection: This release fixes a bug in the automatic JetPack conflict detection; i.e., if you enable JetPack Subscriptions w/ Follow Comments enabled together with Comment Mail at the same, a warning is displayed to notify you of a possible conflict in your configuration. See this GitHub issue if you'd like to learn more.
@jaswrks

This comment has been minimized.

Member

jaswrks commented Jul 24, 2015

Next Pro Release:

  • JetPack Conflict Detection: This release fixes a bug in the automatic JetPack conflict detection; i.e., if you enable JetPack Subscriptions w/ Follow Comments enabled together with Comment Mail at the same, a warning is displayed to notify you of a possible conflict in your configuration. See this GitHub issue if you'd like to learn more.
@raamdev

This comment has been minimized.

Contributor

raamdev commented Jul 25, 2015

@jaswsinc Thanks for fixing that! :-)

One other thing I just noticed about that Dashboard notice is that it doesn't say where you can update those Jetpack settings. I suggest we add the following to that message:

2015-07-25_11-43-03

I'll submit a PR for this now.

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jul 27, 2015

@raamdev Merged. Thanks! :-)

@jaswrks jaswrks closed this Jul 27, 2015

@websharks websharks locked and limited conversation to collaborators Dec 24, 2015

@raamdev

This comment has been minimized.

Contributor

raamdev commented Dec 24, 2015

Comment Mail v151224 has been released and includes changes from this GitHub Issue: See the v151224 announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#113).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.