BP_Feed class

[r-a-y]

In the current codebase (BP 1.7), creating an activity RSS feed requires loading a separate file and hardcoding a bunch of parameters that get reused over and over again.

What would be swell is if we had a dedicated class where we can pass some parameters into the class constructor and it will generate the RSS feed for us.

[r-a-y]

Attached patch (5020.01.patch​) is a first pass at what is described above.

This builds upon enhancements that were originally included in ticket:4564#comment:18 (adding slash and syndication module support) as well as streamlining the RSS spec in a few areas (implement <ttl>, do not use <description> and CDATA together, use <content:encoded> instead).

Feedback welcome.

[r-a-y]

Updated patch to fix mentions feed

[boonebgorges]

+10000

I've only had a few minutes to read over the patch, but my preliminary judgment is that it totally kicks ass, and we should've done it years ago.

r-a-y - I'll test some more in the upcoming days, but r-a-y, what kind of testing have you done for backward compatibility? I see that you're supporting some legacy hooks with some wrapper methods. It looks like some of your changes may alter feed output somewhat (like the CDATA stuff), but my impression is that this will increase compliance with the spec, correct? Have you tested that the feeds validate, etc?

[ubernaut]

+1000000

;)

i'll get to that testing asap!

[ubernaut]

hmm just realized this patch is rather more extensive then the one from the other ticket i'm maybe a little more concerned about running this on a live site. the glitch i was reporting pretty much requires a fairly active live site in order to test. if anyone else can confirm this patch doesn't do anything terrible i can try running it on our live site after that.

[johnjamesjacoby]

This is really great. A few things:

• Rename BP_Feed to BP_Activity_Feed (other components might have other kinds of feeds)
• Change namespace from 'bp_feed' to 'bp_activity_feed' per above, in actions/filters/etc...
• Rather than hook to 'bp_actions' does it make sense to have a new hook for 'bp_feeds' -- Haven't thought this through, but wondering if all feeds should be hooked to the same action, to make them easy to unhook later?
• Do members have activity feeds that need doing?

Let's fix up these bits, and get this in for 1.8.

[boonebgorges]

Marked #4564 as duplicate

[r-a-y]

02.patch addresses some of JJJ's notes and:

• Fixes the bp_activity_enable_feeds filter. It wasn't working properly when returning false for the filter.
• The item guid was not generating a unique value (an old BP RSS bug)
• Add a new hook - bp_activity_feed_postfetch
• Changed the logic in groups_action_group_feed() to use bp_is_group_visible()

---

Regarding Boone's notes:

It looks like some of your changes may alter feed output somewhat (like the CDATA stuff), but my impression is that this will increase compliance with the spec, correct?

I followed Mozilla's recommendation about using only <content:encoded> instead of abusing the <description> element with CDATA:
​https://developer.mozilla.org/en-US/docs/RSS/Article/Why_RSS_Content_Module_is_Popular_-_Including_HTML_Contents

However, the RSS spec is kind of lenient about this:
​http://www.rssboard.org/rss-profile#element-channel-item-description

Have you tested that the feeds validate, etc?

Just tested. Fixed an old BP RSS bug where the item guid wasn't unique. Now it validates against W3C's feed validator.

---

Regarding JJJ's notes:

Rename BP_Feed to BP_Activity_Feed (other components might have other kinds of feeds)

Change namespace from 'bp_feed' to 'bp_activity_feed' per above, in actions/filters/etc...

Done

Rather than hook to 'bp_actions' does it make sense to have a new hook for 'bp_feeds' -- Haven't thought this through, but wondering if all feeds should be hooked to the same action, to make them easy to unhook later?

I've already added a filter called 'bp_activity_enable_feeds', which allows plugin devs to disable all feeds at once. I'm kind of agnostic about the 'bp_feeds' hook.

Do members have activity feeds that need doing?

I think we've covered all instances.

[boonebgorges]

Whoop!

[ubernaut]

awesome hopefully i helped a little still need to get the hang of these patches i think.

[boonebgorges]

In 7207:

Introduces BP_Activity_Feed class, and migrates existing feeds

RSS feeds for BP activity have historically been generated on a one-by-one
basis, with separate template files used for each theme. This resulted in a
huge amount of unnecessarily duplicated code. Moreover, it made the process
of customizing feeds, or providing new feeds, unnecessarily cumbersome.

BP_Activity_Feed abstracts the central parts of RSS feed building, allowing for
easier customization, and fewer points of failure.

This changeset also fixes our RSS implementation to match best practices with
respect to encoding content inside of the <description> element.

Fixes #5020

Props r-a-y

[boonebgorges]

In 7335:

Implement BP_Activity_Feed for activity mentions feed

This particular feed was missed during the original implementation of
BP_Activity_Feed. See #5020, r7207. As a result, the mentions feed was
attempting to load from a separate feed template file that no longer
exists. This broke the feed and filled the error logs with warnings.

Fixes #5124

Props r-a-y

[boonebgorges]

In 7336:

Implement BP_Activity_Feed for activity mentions feed

This particular feed was missed during the original implementation of
BP_Activity_Feed. See #5020, r7207. As a result, the mentions feed was
attempting to load from a separate feed template file that no longer
exists. This broke the feed and filled the error logs with warnings.

Fixes #5124

Props r-a-y

[ubernaut]

i was going to mention that i was still seeing the original out-of-sync issue reported in #4564 under 1.8 but forgot at the last devchat not sure if this is the cause but thought i would add that info.