I'm using Message Subscribe UI to implement the following UI at #1710688: Activity stream & Email notification system and ran into an issue:

Per the comp in that issue, I'm trying to have separate tabs for individual Node and group subscriptions. However, it appears that message_subscribe_ui_tab() uses a formula of prefix + flag entity type (rather than bundle, which makes sense).

As a result, the tabs for the Group subscription and the individual node subscription use the same view. Because the view is hard-coded to use one of the two flag relationships, only one of the two tabs has a functioning view.

Some possible resolutions:

A) Use a single flag type for following groups and define a new callback for the additional view
B) Don't use message_subscribe's menu callbacks and instead implement a Quicktab-based UI for the tabs and handle generation of the tabs in Commons_Follow. That would bring the additional benefit of allowing the user to switch between the tabs without a page refresh.
C) Something else ;)

Files: 
CommentFileSizeAuthor
#11 1845110-message-flag-rel-11.patch2.46 KBAmitaibu
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]
#7 1845110-message-subscribe-email-flag-7.patch1.11 KBezra-g
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]
#6 1845110-message-subscribe-email-flag-6.patch1.07 KBezra-g
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]
#4 message_subscribe_group.png5.52 KBezra-g
#4 message_subscribe_node.png7.64 KBezra-g
#4 1845110-message-subscribe-relationship-4.patch930 bytesezra-g
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]
#2 1845110-message-susbcribe-set-relationship-tab.patch1.34 KBezra-g
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]

Comments

Status:Active» Fixed

uses a formula of prefix + flag entity type

No, it's just the flag prefix. After that you can have any flag name you want. So you can create as many arbitrary tabs as you wish.

Because the view is hard-coded to use one of the two flag relationships

The view is allegedly hard-coded :P. See how in message_subscribe_ui_tab() we set the correct relationship.

So unless I didn't understand you question, you can already implement the above comps using Message-subscribe UI as-is.

Title:Implementing Separate tab for Group vs Regular node followingEmail relationship not set in UI tab
Category:support» bug
Status:Fixed» Needs review
StatusFileSize
new1.34 KB
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]

Thanks for the clarification. I think this leads me to the actual issue, which is that the relationship is not automatically set for the email_flag relationship, or whichever is the second relationship in the view.

This is because we do $rel_set = FALSE; before looping through the relationships, so only the first of 2 relationships in the view is set. Based on my understanding of the code, I'm not sure we need this check at all. Removing it makes the email flag links display as expected on a view that needs the relationship to be programmatically attached. A patch is attached for review.

@ezra-g,
I'm not sure I understand your issue - can you attach screenshots of what you see incorrect?

StatusFileSize
new930 bytes
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]
new7.64 KB
new5.52 KB

Sure - Screenshots attached showing output from two tabs - Both rendering the email_node view showing that the 2nd relationship (for the email_node flag) isn't set when the view is showing items from the follow_group flag.

Also, I've attached a revised patch that removes the $rel_set variable entirely.

Status:Needs review» Needs work

After further investigation, this fix isn't sufficient - It solves the problem of not setting the 2nd relationship in the view but still sets it to the wrong relationship.

<?php
$relationships
[$key]['flag'] = $flag_name;
?>

In the case of Commons Notify, $flag_name here is taken from the first relationship, and so it gets set to commons_follow_group rather than email_group.

Status:Needs work» Needs review
StatusFileSize
new1.07 KB
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]

We need a way to determine the proper flag to use for the email relationship. Message Subscribe assumes this will be email_user, email_group, email_node, or email_term. This patch checks the configured relationships for the 'label' property and computes the corresponding email flag name when the label is set to 'email_flag'.

StatusFileSize
new1.11 KB
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]

Here's a better patch with an appropriate isset() check.

+++ b/message_subscribe_ui/message_subscribe_ui.module
@@ -169,15 +169,13 @@ function message_subscribe_ui_tab($account, $flag_name = NULL) {
+        $flag_name = 'email_' . str_replace($prefix, '', $flag_name);

This doesn't look right, because the module doesn't need to know about message_subscribe_email.

This function is there also for when you are not using message_subscribe_email module -- The view should replace the flag on the fly, as you might have different flags on your Views.

If you don't have message_subscribe_email, then you won't be using a view with a relationship that has the email_flag. We're not adding any code that would fail without message_subscribe_email enabled, so I think we're OK here.

Do you agree?

The reason we have this code, is for example if you have subscribe_node and subscribe_og flags enabled. The tabs for both of them use the same View (message_subscribe), however we need to make sure that when you look at the subscribe_og tab, the View will use the correct relationship.

The code should be extended to cater also for the Message subscribe email, adding a 2nd relationship. I'll have a look.

StatusFileSize
new2.46 KB
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]

Ok, I understand what you try to do in #7 -- let me know if this patch works for you.

Status:Needs review» Fixed

Committed.

@ezra-g,
Let me know if it's ok, I'd like to tag a new alpha :)

On some initial testing this looks good to me. Thanks, Amitaibu!

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.