CUNY Academic Commons

@boone, I know I've been quiet here on Redmine for a while. Class load got a little heavy. I've been working on this whenever I could find time and so far I've been able to get the clear link appended to the latest update in member-header using a filter. I'm now looking into everything you mentioned on our call about the URL parsing functions. With this coming week being my spring break I plan on getting as many of my current issues closed out as I can.

Will be back with more updates (and questions) soon...

Boone,

I've pushed a branch (cac-latest-updates) up to the repo:

https://github.com/castiron/cac/tree/clear-latest-update

So far I've managed to get a clear link to show up in the member header and to only show that link if the current user is logged in and ALSO is the same user as the "displayed user."

I'm getting stuck on the whole "catching the click" process, the function that needs to look at the URL for the "clear link" and use it to remove the latest update from the wp_usermeta table.

I'd appreciate any input that you feel will speed me along so I can close this one out. I've been trying different ways of using the components/actions/items functions but haven't found the combination that will allow me to grab the info and clear the latest update.

One major part I need help with is sending the visitor back to their activity / member page after they click "Clear." It's a simple plugin, there is one function that adds the clear link and a second that will clear the latest update.

I am available to talk today if you think a phone call would be easier.

Hey Dom -

Thanks for the update (no pun intended). Overall, your strategy in this plugin is quite good. I made some improvements in https://github.com/castiron/cac/commit/a818fb541b106cee4246b60b29dcadf77fe0aaae to respond to some of your questions, and to bring it in line with some WP standards:

- Note how I bail out of cac_clear_latest_update() if you're not actually requesting a 'clear' action
- You had originally written in the docblock that permissions checking isn't necessary, because it's handled in the display function. But if you only check in cac_append_clear_link(), it's going to allow people to manually enter the URL for another user, and clear their status. This is not secure, so the check should be done in both places.
- You were using a direct SQL query to delete the usermeta. Use delete_user_meta() instead - it's more secure (you weren't doing any sanitization, which leaves us open to injection attacks of the form
http://commons.gc.cuny.edu/clear/DROP TABLE ....
Theoretically, you should be protected against this kind of attack by the use of the %d parameterization, but you forgot to use the $wpdb->prepare() wrapper. In any case, using delete_usermeta() also interfaces better with caching, etc.
- After delete, the user is now sent back to the page that they came from.
- I switched from the 'init' hook to 'bp_actions'. This is mainly so that the function doesn't fire in cases where BP isn't present, or hasn't loaded yet (since the bp_actions hook will only exist when BP is up and running). Strictly speaking, you should only be loading your BP-dependent functions when you know for sure that BP is running (PHP has a habit of parsing greedily at runtime, and it'll throw fatal errors on things like bp_get_user_meta()): http://codex.buddypress.org/plugin-development/checking-buddypress-is-active/ Doesn't matter for our installation on the Commons, but for distribution plugins, it's very important.

I hope that's helpful! Pull down my changes and have another look. If it looks good to you, and you don't have any further questions, then it's OK by me if you want to merge this into the master branch and mark it resolved. Thanks, Dom!

It is hard to describe how much I love Git!!

I did so much reading up on plugin development and best practices over the past few weeks that it means a lot to hear you say that my strategy is "quite good." :-)

I'm going to try and respond in the order you commented:

- the early bail out makes total sense
- I always forget about direct URL entries (dammit) :-(
- I knew there were potential (injection) vulnerabilities with the direct call to $wpdb. It was more or less there to remind me what I needed to do "inside" the DB while I was looking for the proper WordPress wrapper function. I however did not know that delete_usermeta() worked better with caching.
- I just found both bp_core_redirect() and wp_get_referer() last night. The line you replaced was the preceding attempt. I really wanted to get something to you since both you and Matt emailed me because I had been out of contact for so long. I wanted to make sure you both knew I was doing something! :-)
- I suspected there was a better hook. Hooking into a BP action makes complete sense to me, for all the reasons you listed.

I've pulled your changes and verified everything works the way I've been trying to get it to work on my end. I'm going to commit now, but since there's been a lot of development since my last commit, I want to verify my git steps with you just to make sure I don't fudge anything up in the repo.

This is what I plan on doing...

1.) git co master
2.) git pull origin master
3.) git merge clear-latest-update
4.) git push origin master

...and then delete my local and the remote clear-latest-update branches...

1.) git br d clear-latest-update (< local branch)
2.) git push origin :clear-latest-update (<- remote branch)

Does that look right to you?

Hi Dom. Yes, that git process sounds correct. Report back in this ticket about how it goes!