CUNY Academic Commons

Given the wide impact of the bug, I suggest we convert everyone and let users who want to change their settings back do so manually. Since every email notification includes a link to email settings and since the group homepage makes it easy to change them, I think we wouldn't be asking too much of users to redo their settings and that the number of cases we'd fix make the unfortunate necessity of overriding the set preferences of a small number of users worthwhile.

This assumes that there isn't an easy way to distinguish cases, as there doesn't appear to be.

The related question is whether we compile a list of all users whose subscriptions would be affected by this bug and notify them of the change and give them an easy way to adjust their settings if they're unhappy with it. I'd favor that, too.

I agree with Matt completely. We reset all "No Email" subscriptions to "Supersub." Then send a blanket form email notifying users that a recent bug fix necessitated a reset of email subscription settings. The users can then reset their settings as they see fit.

I think the work involved in locating and altering all email settings would require a time investment that just isn't justified. If the error affected personal profile info or something more specific it might be worth it. I don't think there is an easy way to determine who is and is not affected. We don't even have a definite date to go back to where we believe the bug began. If I understand correctly, this could go all the way back to the addition of the GES plugin?

If I understand correctly, this could go all the way back to the addition of the GES plugin?

More likely, all the way back to the addition of the plugin that auto-adds group admins to the Group For Group Admins. Either way, it's going to be hard to pin down a date, and in any case, there's no way to track in the database when a subscription level changed.

Dom, do you have a sense of how the script will work, so that you can start working on it? I'm happy to do it, but it will have to wait until after Cbox launches. In a nutshell:
- Loop through all groups
- For each group, pull up subscribed members
- For each No Email, change to supersub and resave
- Each time you do this, you may want to log somewhere that a change has been made. That way we will can convert this into a list of recipients for any notification email we plan to send to affected members.

Boone,

Maybe you and I should both revisit this script after CBox? My Calc midterm is only a few days before the CBox release. I also don't mind writing it but I haven't done much PHP scripting, let alone specialized communication with the DB from a custom script. I would like to do it so I can learn more about the lower level interface between PHP and SQL (mysqli_*), but I have a feeling I will have many questions.

That'll be fine, Dom. FWIW, I would write this as a script that runs after the WP bootstrap, so there will be no direct db manipulation - we'll be using the WP/BP/GES API functions. Let's revisit in a few weeks.

Kinda like my long-subdomains plugin? I can write a similar single use plugin that we can use to query for member's email sub-status and then manipulate them right there in the Network Admin? That would give me more practice in working with the admin section as well.

Kinda like my long-subdomains plugin? I can write a similar single use plugin that we can use to query for member's email sub-status and then manipulate them right there in the Network Admin? That would give me more practice in working with the admin section as well.

Yes, similar to that, though it's not necessary for our purposes to build a lot of infrastructure for displaying information in a pretty way on the Network Admin - this'll be a one-time use plugin.

Understood

Boone,

Through some incredibly detailed code in an example plugin and some strategic Googling about WP_List_Table, I've managed to create a pretty cool plugin that runs in Network Users admin and neatly organizes all the info we'll need to close out this issue. There's only one more step that I need help with. I need to learn how to create a url with query parameters that can be used to adjust the actual subscription settings in the DB. Once you see the plugin you'll understand what I'm talking about. I want to use it in the "Bulk Actions" dropdown.

I've added the new plugin in a topic branch so we can test on cdev.

I'm in school all day today so I won't be able to do any testing until tonight or tomorrow. Let me know what you think of the code whenever you get a chance to look at it, no rush.

No prob. I'll keep researching and will hopefully have an answer before you even get a chance to look. :-)

Boone,

I've spent a few hours trying to knock this issue out. It's been around WAY too long. I've recreated my topic branch branched off the current 1.4.x code. The plugin's name is cac-member-email-status and the whole thing works fine with the exception of two parts.

1.) Column sorting doesn't work properly. This isn't a big deal and I should be able to fix it on my own.

2.) The bulk action to update the actual subscription status doesn't work. This is what I need help with.

Working inside the WP DB is still a skill I'm working on. When you get a chance could you please take a look at the code? I don't need you to do it for me (unless you want to), if you could just point me in the right direction to handle the updates that would be great. The function that handles the bulk actions (process_bulk_action()) starts on line 94 of cac-member-email-loader.php. You can see that I am grabbing all the user IDs that have been checked in the table and then I just output an admin notice div. If I can figure out how to update the DB right in that function we can resolve this issue and move on from broken email status.

All the code has been pushed to that topic branch in our repo so you can deploy it on cdev if you need to.

Thanks for your work on this, Dom. Looks like you've gotten a good handle on using the WP_List_Table class.

As it stands, there are some implementation issues with your plugin.
a) The goal is to change non-supersub users to supersub in bulk. Currently, your plugin shows 25 group memberships per page, mixing supersubs with non-supersubs. As a result, most pages have only a few non-supersub items on them. And there are thousands of pages. Making the actual changes will take a very long time.
b) The value of your user checkboxes is the user id. But the meaningful unit of measurement here is the group membership, which is a one-to-one relationship between a group and a user. So, you should either be setting the value of the checkboxes to something that contains the group and user ids (eg, value="10:35", for user 10 and group 35), or you should be sending the group membership ID (though that'd require an additional lookup after submitting).

For (b), assuming you took something like my first suggestion, your handler would look something like this:

    $members = $_REQUEST['member'];
    foreach ( $members as $member ) {
        $m_array = explode( ':', $member );
        $user_id = intval( $m_array[0] );
        $group_id = intval( $m_array[1] );
        $group_subs = groups_get_groupmeta( $group_id, 'ass_subscribed_users' );
        $group_subs[ $user_id ] = 'supersub';
        groups_update_groupmeta( $group_id, 'ass_subscribed_users', $group_subs );
    }

For (a), it would be ideal to sort/filter by email status. But, because of the way the subscription data is stored - as serialized arrays, keyed by group_id - it's not going to be possible to write the right kind of query (and you could only do it in PHP by loading up every single group and its subscription statuses into memory, which is probably not viable). What this likely means (alas) is that your chosen UI is not going to work for our purposes - there'll be no way to make it scale correctly. Moreover, when you key by user rather than by group, it means exponentially more database activity: for each user in a given group, the group subscription data will have to be retrieved, modified, and resaved.

So, I think we're going to need a different approach. Let's forget about the UI for the time being. What we need is logic that will:

$group_ids = a list of groups that have subscribed users (or a subset of them, if we're going to do it in batches)
foreach ( $group_ids as $group_id ) {
    $subs = groups_get_groupmeta( $group_id, 'ass_subscribed_users' );
    foreach ( $subs as $user_id => $group_status ) {
        if ( it looks like this user needs his subscription status changed ) {
            $subs[ $user_id ] = 'supersub';
        }
    }
    groups_update_groupmeta( $group_id, 'ass_subscribed_users', $subs );
}

When I need to write these sorts of scripts, where I don't need a UI (and in fact the UI details get in the way of writing a working implementation, as is the case of your plugin), I often do something like this in an mu-plugins file:

function bbg_subscription_fixer() {
    if ( is_super_admin() && ! empty( $_GET['bbg_subscription_fixer'] ) ) {
        do your stuff
    }
}
add_action( 'admin_init', 'bbg_subscription_fixer' );

Run it by visiting wp-admin?bbg_subscription_fixer=1. If you need to debug or show results, just echo/var_dump() them to the screen.

Thanks again for your work on this - the plugin is really slick, but I think it's not a good fit for the specific kinds of data that we're working with in this case.

I love it when you answer in this much detail. Thank you for taking the time.

I know you mentioned it previously - that an mu-plugin as opposed to an admin plugin UI - would probably be the best bet. I incorrectly thought that I knew better. :-)

Wrapping my head around WP_List_Table was made an order of magnitude easier once I found this plugin

It's a great example of the best practices for using that class. I hope I get to use it for something useful in the future. I'll eventually be creating a plugin for Everybit, maybe I'll need it for that project.

I'll get started on the mu version you suggested by taking any relevant code I've already written and combining it with your suggestions above. I'll update as soon as I've got something to report.

Your development suggestion of echoing/var_dump'ing to the screen is one of my preferred methods anyways. Which reminds me of a question I didn't get a chance to ask at our last meeting. You were coding during the first few minutes and I noticed you were refreshing a page as you were coding. Is this the technique you were using? Writing code and then monitoring the output? Or were you working on something completely different?

Thank you for taking the time.

My pleasure :)

noticed you were refreshing a page as you were coding. Is this the technique you were using? Writing code and then monitoring the output? Or were you working on something completely different?

I don't remember what I was doing. When I'm working on code that generates markup, yes, it is necessary to view it in a browser. When I'm writing something on the server (especially something that'll accept some sort of automated testing) then I use the browser less during development.

OK Boone,

I think I've got an mu-plugin that is either much closer to or almost exactly what you outlined in comment 28

I've only coded it up to the point of echoing out the info that it has found so we can run a test on CDEV. As soon as you let me know if this is finally the right direction for me to be going I will add the code that sets the subscription status instead of just echoing it.

If you don't even want to test on cdev and would rather I just finish the code so we can merge and deploy the topic branch let me know.

Branch/Commit: https://github.com/castiron/cac/commit/0d9f0c7a13d15e3cb5ccd60da322b61f50e4c6ef

Is it possible that the Assignee was changed in error? This is not Michael's normal area of responsibility.

Hey Boone,

Just wanted to remind you that I haven't completed this yet. I went in so many wrong directions with this ticket that I took your advice and made it a super simple single use mu-plugin. Right now it doesn't actually change the email status. It only loads up and gathers all the info. I echoed that info out to the screen per your suggestion so I could verify that it does indeed gather up the proper email information.

Would you like me to go ahead and finish writing the code that will actually update the DB? I pushed it up incomplete so you could have a look to make sure I wasn't yet again going in the wrong direction.

Boone,

I've removed my local email-status topic branch because this issue is marked as resolved. There is still a remote copy of that topic branch up in our repo. I don't want to remove it if you still need it for something. Would you like me to go ahead and get rid of it?

Thanks, Dom. Please go ahead and remove the remote branch too.

Done.