Not sure if this is the right place to file, given how CVS content is mixed up, but here it goes. Apparently recent changes to Buddylist API broke Buddylist UI, in multiple places. This is probably incomplete list:

#1) Incorrect buddy_api_get_buddies calls

UI module calls the parameters in the wrong order. Fixed everywhere in the patch.

#2) Recursive loading of user objects in buddylist_ui_user

When op = load, buddies should not be loaded as full objects, as this is likely to cause infinite recursion. Changed to UID loading.

#3) Requester / requestee confusion in buddylist_ui_pending_requested_accept_submit

I found this problem at least once - at the step where friend request is confirmed. There is a possibility of a similar problem in other request code / form submits, but I am not familiar enough with the code to be sure.

#4) Various errors with Workflow tokens

When friend request is created, even though it is sent out fine, this message appears: "Workflow-ng: Applying token replacements failed. To fix this try editing and saving all conditions and actions."

It may be possible that #3/#4 are related because of more mis-matches is request API. Please help with reviewing this.

CommentFileSizeAuthor
#4 buddylist.patch8.31 KBdkruglyak
#1 buddylist_ui.module.patch8.57 KBdkruglyak
buddylist_ui.module.patch2.52 KBdkruglyak

Comments

dkruglyak’s picture

dkruglyak commented
StatusFileSize
new buddylist_ui.module.patch8.57 KB

OK, I dug deeper into the module and made a few more fixes - all in consolidated patch. Summary here:

A) Incorrect buddy_api_get_requestees calls
Parameters of buddy_api_get_requestees are called in wrong order, just like buddy_api_get_buddies. Rearranged

B) Requestee status actions were not themeable
This is a problem and I changed them into themeable functions (called by buddylist_ui_get_buddy_actions)

C) Requester / requestee confusion in buddylist_ui_pending_requested_deny_submit
Problem identical to buddylist_ui_pending_requested_accept_submit. Rearranged params the same way.

D) Menus not generated correctly
The module does not generate a proper menu for navigation block and does not use translations. Fixed.

E) Central page should be themeable, just like request page
Added theming function and updated the menu accordlingly

F) Theme function naming confusing
Standardized, renaming existing functions wherever appropriate

The patch was tested pretty well and should be ready to go. I also verified that "Cancel request" feature works fine.

nodestroy’s picture

nodestroy commented

Hi,

thx for your great work!

Due to the little cvs mess, i allready fixed the most of your issues from your first patch. (Maybe we will have a buddylist2 project page soon)

I will quickly review and apply your second patch!

regards,
dominik

nodestroy’s picture

nodestroy commented
Status: Needs review » Fixed
dkruglyak’s picture

dkruglyak commented
Status: Fixed » Needs review
StatusFileSize
new buddylist.patch8.31 KB

Dominic, thanks for commiting! With more testing I found and fixed a few more problems:

FIX #1:

Made persistent menu items cached, using user.module as a model. This is required to allow admins to customize / move them. Note the change from MENU_SUGGESTED_ITEM to MENU_NORMAL_ITEM to show up in navigation by default.

FIX #2:

Streamlined buddylist_ui_user to generate output through themeable functions. In buddylist_ui_get_buddy_actions added keys to the action list to allow further theming customization by name. E.g. to make "Remove Friend" harder to find, like on Facebook.

FIX #3:

Removed buddylist_ui_buddy_api function, which is never used and is unnecessary

TBD:

We have buddylist_ui_translation and buddylist_api_translation duplicating one another adding to confusion. I suggest merging them into one, probably buddylist_api_translation. Not included in my patch.

Attached patch can be applied against latest CVS.

nodestroy’s picture

nodestroy commented

thx for this great patch!

@translation functions:
i think buddy_api_translation() would be best, because other UI´s can use it too. i will apply this soon.

dkruglyak’s picture

dkruglyak commented

I agree that buddy_api_translation is the preferred function to keep. Look forward to seeing the patches applied, this one and the fixed one-way delete function here: http://drupal.org/node/203500

As you are working on the code, would love to hear your thoughts on activities / requests per this: http://drupal.org/node/203335

nodestroy’s picture

nodestroy commented

patch and translation function changes applied & commited

dkruglyak’s picture

dkruglyak commented
Status: Needs review » Fixed

All fixes from this issue look good, marking fixed.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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