-
Notifications
You must be signed in to change notification settings - Fork 3
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
Activating Pro version should auto-deactivate Lite version if active #270
Comments
@raamdev / @jaswsinc Hi, is there any example I can follow on how to setup this up in Comment Mail? |
Noting Jason's response on Slack:
|
It looks like Comment Mail already includes a conflicts check to handle auto-deactivation of the Lite/Pro version when the other is being installed (see So it looks like we have a bug in the Conflicts class. I seems like Comment Mail Lite is not being properly deactivated when Comment Mail Pro is installed. I suggest a review of the Conflicts class and some further testing to determine exactly what's going on. |
I tested changing the code in Comment Mail and testing out having Lite/Pro installed and installing the other version while it was activated. class_alias(__NAMESPACE__.'\\ApiBase', GLOBAL_NS); Comment Mail error:
I also tested Comet Cache to see if the Comet Cache error:
|
Steps to reproduce errorFor Comment Mail / Comment Mail Pro
Ideal Behaviour: The Lite/Pro version should deactivate upon activating the other version. Observed Behaviour: Displays Comment Mail error:
For Comet Cache / Comet Cache Pro
Ideal Behaviour: The Lite/Pro version should deactivate upon activating the other version. Observed Behaviour: Displays Comet Cache error:
|
@renzms I see you're using the latest public version of Comment Mail (v160215). Have you tested Comment Mail against the dev branch? A LOT of the Comment Mail codebase has changed since v160215, so I just want to confirm this hasn't already been fixed there. |
@raamdev
|
@renzms writes...
Have you investigated what might be causing that error? |
@raamdev Tried looking into this one and these are my findings: In https://github.com/websharks/comment-mail/blob/dev/src/includes/api.php shouldn't it also include: use WebSharks\CommentMail\Classes; after the Then have line 12 written instead as: class_alias(__NAMESPACE__.'\\Classes\\ApiBase', GLOBAL_NS); In https://github.com/websharks/comment-mail/blob/dev/src/includes/classes/ApiBase.php, should there be a function to get the current plugin instance like in Comet Cache here? |
Next Release Changelog:
|
@jaswsinc @raamdev Confirmed partly working with possible bugTested Using Comment Mail Lite / Pro Version 161210-RC Observed BehaviourInstalling/Activating Comment Mail Pro while Comment Mail Lite is active automatically disables Comment Mail Lite quietly. Prompt is Possible BugI did notice though, that the opposite is not true. When attempting to activate Comment Mail Lite when Comment Mail Pro is active, the plugin only keeps Comment Mail Pro active but header prompt says |
@renzms Thank you. Yes, that seems confusing and incorrect. @jaswsinc Can you add a Dashboard message that says "Thank you for upgrading" when going from Lite → Pro, and have it properly deactivate Pro when going from Pro → Lite? I realize the latter is a less common scenario, but it should still be handled gracefully, as that scenario will occur. |
I can do that but it's not a quick fix. It might take another day or so because there is some logic that needs to get worked out, and this is rather tricky given the loading sequence in core. Actually, I suggest not going this route. I think stuff like this is on the user to get right, and trying to put too much logic in code that can deal with something that is such a fundamental part of installing a piece of software to begin with, is asking for problems. For example, I'd hate to add logic that works out something so simple like this, only to have that break in a future release and actually prevent the software from being installed! That would be rather embarrassing, since all you need do is decide whether you're going to run the lite version or the pro version to begin with. I mean, really. I think they can get that part right. |
@jaswsinc writes...
That sounds exactly like something a customer would say referring to developers who "can't even produce software that can tell the Pro version from the Lite version!". :-) I'm fine with deferring that work to a later time if it's going to delay releasing today, but I'll have to open another bug report so that it's documented that we're aware of that issue. My feeling is that two variations of our software should play nicely with each other and be smart enough to deactivate the other when one is activated. I do still think we need a "Thank you for upgrading" Dashboard message when going from Lite → Pro; that should be easy enough to add, no? |
Oh, give me a break! The crazy thing here would be having pro installed, then trying to install the lite version on top of the pro version and expecting that to work. What we do right now is that the pro version takes precedence over the lite version. What you're asking for is that we also detect a case where a site owner has pro installed, then for some unknown reason tries to install the lite version (because they want less power?, and they also forget to deactivate the pro version before doing so), and in that case, the lite should version should win. That's the way I understand this request. So fine, we can do that, but it's not a simple thing to do, and my feeling is that it's a waste of time, since that's an extremely odd thing to do, and likely will never happen outside of testing or fiddling. If it does happen, pro wins the battle. If you don't want pro to win the battle, you can simply uninstall it.
I don't think we need one, no. I'm not against adding one, but again, this is little stuff that is already established in WordPress core. There's no rule that when you install a plugin it needs to reach out and hug you by specifically reinforcing something that a user will already know. |
If they just paid for the Pro version, it would be nice to say thank you, much like we do with s2Member Pro. |
Yep, nothing against that note. We thanked them for their purchase at comment-mail.com, but we can thank them again in the software, which is great. |
As I said, I'm fine with leaving that extra behavior out. But if you have the Pro version installed and you decide to ask for a refund and then choose to voluntarily revert back to the Lite version (just one example scenario where this might happen), being unable to activate the Lite version because the Pro version silently prevents you from activating it is not good UX. A Dashboard message saying "you must disable the Pro version first"? Auto-deactivating the Pro version in favor the version being activated? Either of those would make sense. But silently preventing the user from taking the action and assuming they're smart enough to figure it out doesn't make sense. Again, I see your point about not wanting to add unnecessary complexity to the code and I think this is an uncommon enough scenario that I'm not too concerned with fixing it. |
- **Bug Fix:** Prevent browser autocomplete in Comment Mail options. See [Issue #319](#319). - **Bug Fix:** Searching by email address alone should always narrow to the search to that specific email address and not result in any fuzzy or fulltext matching. See [Issue #226](#226). - **Bug Fix:** The conflict check for 'Subscribe to Comments Reloaded' was not working in the previous release; i.e., if you attempt to activate both Comment Mail and the 'Subscribe to Comments Reloaded' plugin at the same, this should result in a Dashboard warning. Fixed in this release. See [Issue #315](#315). - **Bug Fix:** Notify 'Subscribe to Comments Reloaded' users about the comment form template being disabled under certain scenarios. See [Issue #314](#314). - **Bug Fix:** Do not attempt to import 'Subscribe to Comments Reloaded' (StCR) settings if StCR is no longer installed, even if old StCR options exist in the database. See [Issue #294](#294). - **Bug Fix** (Pro): Do not show SparkPost partner image when Mandrill is selected as the RVE handler. See [Issue #318](#318). - **Bug Fix** (Pro): Conflict checks between lite and pro corrected. This was not working properly in the previous release; i.e., installing Comment Mail Pro when Comment Mail Lite is already running should result in Comment Mail Lite being deactivated automatically. See [Issue #270](#270). - **New Feature:** It is now possible to manually process the outgoing mail queue. See: **WP Dashboard → Comment Mail → Mail Queue**. See also [Issue #282](#282). - **New Feature** (Pro): In Comment Mail Pro it is now possible to enable/disable comment content clipping entirely; e.g., if you prefer that email notifications include the full original comment content in raw HTML instead of being clipped and displayed in the email as plain text. See: **WP Dashboard → Comment Mail → Config. Options → Email Notification Clips**. See also: [Issue #281](#281). - **Accessibility:** This release improves screen reader accessibility by adding `aria-hidden="true"` to all FontAwesome icons. See [Issue #304](#304). - **Accessibility:** This release improves screen reader accessibility by offering a new setting that allows a site owner to enable or disable select menu option enhancement via jQuery. Disabling select menu option enhancement has the benefit of improving accessibility for screen readers whenever accessibility is of more concern than presentation. See: **Dashboard → Comment Mail → Config Options → Misc. UI-Related Settings**. See also [Issue #304](#304).
Comment Mail v161213 has been released and includes changes from this GitHub Issue. See the v161213 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 (#270). |
If Comment Mail Pro is installed on a site where Comment Mail Lite is currently running, it should gracefully handle that scenario by deactivating Comment Mail Lite and displaying a "Thank you for upgrading to Comment Mail Pro!" message. As of now, the dev version does not handle this scenario gracefully and instead produces an error message.
Also, if activating Comment Mail Lite on a site running Comment Mail Pro, the Pro version should be automatically deactivated (no need to thank the site owner for downgrading though).
The text was updated successfully, but these errors were encountered: