Skip to content
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

Run WordCamp Participation Notifier when post is published #964

Open
wants to merge 3 commits into
base: production
Choose a base branch
from

Conversation

timiwahalahti
Copy link
Collaborator

WordCamp Participation Notifier sends profile activity log messages and hands out the badges for organisers and speakers, in the future possibly for volunteers and attendees as well.

The notifier relied on add_post_meta and update_post_meta hooks to detect when the notifications for profiles.wordpress.org should be sent. This has proven to cause problems, as many WordCamps do fill out the WordPress.org username for the post when they draft it. That has caused the filters to run, but bail because the post isn't public yet. In the end, people haven't received their badges or organisers have had to empty the username field, save and fill in the username again to send notifications.

This PR tries to fix that by hooking into wp_after_insert_post action that is run after the post itself and related meta have been saved. If all conditions are met (not revision, published, notifiable post type) then notifications are sent.

The username_meta_update function was left because it is used to notify about a mentor assigned to a WordCamp, which has a bit different logic.

Someone with sandbox needs to test this and see that activity log and badges do in fact get assigned.

Fixes #809

Props @vertizio, @CdrMarks

How to test the changes in this Pull Request:

  1. Navigate to WordCamp site
  2. Create new organiser post, fill in the WordPress.org username field and save as a draft
  3. Check that activity log didn't get message and badge was not assigned
  4. Publish the organiser post
  5. Check that activity log did get message and badge was assigned

timiwahalahti and others added 3 commits September 28, 2023 16:45
`is_post_notifiable()` was returning `false` because the passed `id` wan't a `WP_Post`
@iandunn
Copy link
Member

iandunn commented Sep 29, 2023

I pushed 02ae763 to fix a problem that was preventing my tests from working.

I'm seeing another problem now, but it doesn't seem to be in the PR. maybe_remove_badge() is called when clearing the username field and saving, and it makes the HTTP request to remove the badge, and that gets a successful response, but the badge isn't actually removed. It still shows up on the front end, and in wp bp group list --user-id=iandunn-test3. groups_is_user_member(() returns false, so it never calls groups_leave_group().

https://github.com/WordPress/wordpress.org/blob/e9cc598fb59062f5ec3a8f75a8c91204e458f6d5/profiles.wordpress.org/public_html/wp-content/plugins/wporg-profiles-association-handler/wporg-profiles-association-handler.php#L257-L260

The CLI command also doesn't remove it either, though:

> wp  bp group member remove --group-id=wordcamp-organizer --user-id=13904193
Error: Could not remove member from the group.

( list will work with the username, but remove seems to need the ID )

The SQL query in BP_Groups_Member::remove() fails because there isn't anything for the user: SELECT * FROM groups_members WHERE user_id = 13904193 shows 0 results, despite it showing up li the group list command.

I'm a bit stuck there, so I'm gonna move on to other tests and try again next week.

@iandunn
Copy link
Member

iandunn commented Oct 4, 2023

🤔 it seems like the reason why wp bp group list was showing a badge when there wasn't one in the DB was because the group was in memcache:

https://github.com/buddypress/buddypress/blob/0bba3f74ba55db8aa71a1708550ec25f2bb6103f/src/bp-groups/classes/class-bp-groups-group.php#L1376

https://github.com/buddypress/buddypress/blob/0bba3f74ba55db8aa71a1708550ec25f2bb6103f/src/bp-groups/classes/class-bp-groups-group.php#L1445

After setting $cached = false to force a fresh value, the list command correctly shows that there aren't any assigned groups.

After removing that code, though, I can still reproduce the error:

  1. > wp bp group member add --group-id=2 --user-id=13904193

    Success: Added user 13904193 to group 2 as member.

  2. > wp bp group list --user-id=13904193

    +----+--------------------+--------------------+---------+---------------------+
    | id | name | slug | status | date_created |
    +----+--------------------+--------------------+---------+---------------------+
    | 2 | WordCamp Organizer | wordcamp-organizer | private | 2014-02-25 20:04:56 |
    +----+--------------------+--------------------+---------+---------------------+

  3. > wp bp group member remove --group-id=2 --user-id=13904193

    Success: Member 13904193 removed from the group 2.

  4. > wp bp group list --user-id=13904193

    +----+--------------------+--------------------+---------+---------------------+
    | id | name | slug | status | date_created |
    +----+--------------------+--------------------+---------+---------------------+
    | 2 | WordCamp Organizer | wordcamp-organizer | private | 2014-02-25 20:04:56 |
    +----+--------------------+--------------------+---------+---------------------+

I'm gonna look and see if it's a bug in BuddyPress, or some conflict with our custom code/environment.

@iandunn
Copy link
Member

iandunn commented Oct 5, 2023

It looks like it was a BuddyPress bug, so I filed a report.

I'll see if there's a good workaround. If not we may be need to wait until it's fixed.

@pkevan
Copy link
Contributor

pkevan commented Feb 8, 2024

It looks like it was a BuddyPress bug, so I filed a report.

I'll see if there's a good workaround. If not we may be need to wait until it's fixed.

Looks like this was released in https://buddypress.trac.wordpress.org/changeset/13592, so we can test this again.

@renintw renintw added [Type] Bug [Type] Good First Issue Straightforward, self-contained, doesn't require deep knowledge of codebase [Priority] High and removed [Priority] 1 labels Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Participation Notifier [Priority] High [Type] Bug [Type] Good First Issue Straightforward, self-contained, doesn't require deep knowledge of codebase
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

WordCamp Participation Notifier should run when an eligible CPT is published
4 participants