Feature/229 - Avoid display of "Back to subscriptions" button when no subscription available to the user.

[Reedyseth]

This pull request has some improvements on the wpsharks/comment-mail#229

Using the variable $has_subscriptions controls the display of the back image

And My Comment Subscription Message

The purpose of using the $valid_sub_key variable is to control the display of the message Back to subscriptions

So for some reason when I am testing $valid_sub_key always is null.

@raamdev if you can give me some insights on this that would be very helpful 😺

By the way, I apologize for all the commits below, those are because I was keeping my repo updated.

[jaswrks]

@Reedyseth On this line you're querying to see if the sub key is actually associated with a subscription in the DB, but this has already been determined by $this->sub as seen in the constructor here. So I would try to avoid making another DB query since this is already a known.

[jaswrks]

By the way, I apologize for all the commits below, those are because I was keeping my repo updated.

No worries. I love seeing the thought process behind the work; i.e., small commits as you work through the task at hand is very helpful to others in my view. Thanks! :-)

[Reedyseth]

@jaswsinc

No worries. I love seeing the thought process behind the work; i.e., small commits as you work through the task at hand is very helpful to others in my view. Thanks! :-)

Awesome, I did a few more before finish the issue 😄

@raamdev @jaswsinc I have finish this issue wpsharks/comment-mail#229 , last commit went OK with Travis CI, the branch is at the tip of the head and ready to be merge, I apologize for the late response, I spent a lot of time working with Phing and Componser, I still not able to build the project, I left some comments on the Phing Channel 2 Slack.

[raamdev]

@Reedyseth I suggest type casting this as a boolean, since the currentEmail() method can return an empty string—we want to make sure that the empty string gets interpreted as false here:

$current_email     = (boolean) $this->plugin->utils_sub->currentEmail();

[Reedyseth]

I suggest type casting this as a boolean

Roger ! 👍

[raamdev]

@Reedyseth I left you some feedback above. Please note that it's important that you are able to test the changes in this PR, so you'll need to get the build system working so that you can test your PR before I merge it in.

[Reedyseth]

@raamdev

Please note that it's important that you are able to test the changes in this PR, so you'll need to get the build system working so that you can test your PR before I merge it in.

Alright, I also added some questions, thanks !

[kristineds]

@raamdev @jaswsinc Submitting my PR: 09170be. Please review when you get a chance. 😄

Updated codebase to include feedback from both @raamdev and @jaswsinc. Tested on both scenarios:

User not logged in

• Visit a post as a user who IS NOT logged in
• Click the "Subscribe without Commenting" link on a post
• When you get to the "Add New Subscription" page, Back icon is disabled

User logged in with no subscriptions

• Visit a post as a user who IS logged in but has NO subscriptions
• Click the "Subscribe without Commenting" link on a post
• When you get to the "Add New Subscription" page, Back icon is disabled

[raamdev]

@kristineds Thanks! Was this tested with users who do have subscriptions? I.e., did you confirm that the back button still works as expected when it's visible?

[kristineds]

Was this tested with users who do have subscriptions

@raamdev I tested this and the Back button (same with the My Comment Subscriptions link) is not working as expected, i.e. I'm seeing the "Missing subscription key" error message for a user with subscriptions who is logged in.

---

Gonna work on this for a little bit more over the weekend. Thanks for the feedback!

[raamdev]

@kristineds Any update here? As I mentioned in the Comment Mail Project Status Update on Slack, I've bumped the Next Release by 1 week, putting the RC deadline at this Friday, August 5th. I'd like to have this ready to merge before then.

[kristineds]

@raamdev Since the error message that I'm getting (when clicking on the Back button) is because of a missing $sub_key, https://github.com/websharks/comment-mail-pro/blob/6d2629d4c2815bc6a6c3448992896c2a82517121/src/includes/templates/type-a/site/sub-actions/manage-summary.php#L104

Missing subscription key; unable to display summary

I'm trying to fix the issue you pointed out here. #71 (comment)

If I'm looking at it the wrong way, please point me to the right direction. 😄

---

Have you confirmed there's even an issue there? :-)

I used this line of code (based on Israel's previous PR) on my previous PR and the issue of the Back button displaying an error message for logged-in users with subscriptions is evident, so yes. Either that, or some other line of code is causing this issue and I can't find it. 😞

$sub_summary_return_url = $has_subscriptions && $plugin->utils_url->subManageSummaryUrl(!empty($sub_key) ? $sub_key : '', null, true);