Skip to:
Content

BuddyPress.org

Opened 14 years ago

Closed 13 years ago

#2002 closed defect (bug) (fixed)

When deleting activity in permalink view page should redirect back to stream

Reported by: modemlooper's profile modemlooper Owned by: cnorris23's profile cnorris23
Milestone: 1.5 Priority: major
Severity: Version:
Component: Activity Keywords: has-patch needs-testing
Cc:

Description

If you delete a view the page just stays static and blank. It would be better if the page went back to the activity stream.

Attachments (5)

2002.001.diff (2.0 KB) - added by cnorris23 14 years ago.
2002.002.diff (5.5 KB) - added by cnorris23 14 years ago.
redirect via query arg
2002.003.diff (10.5 KB) - added by cnorris23 13 years ago.
2002.004.diff (10.0 KB) - added by DJPaul 13 years ago.
2002.005.diff (10.1 KB) - added by cnorris23 13 years ago.

Download all attachments as: .zip

Change History (29)

#1 @r-a-y
14 years ago

  • Priority changed from trivial to major
  • Type changed from enhancement to defect

Changing type to defect and priority to major as this is a big usability issue.

#2 @r-a-y
14 years ago

  • Milestone set to 1.2.1

#3 @cnorris23
14 years ago

  • Keywords needs-patch added

#4 @apeatling
14 years ago

  • Milestone changed from 1.2.3 to 1.2.2

#5 @apeatling
14 years ago

  • Milestone changed from 1.2.2 to 1.2.3

#6 @cnorris23
14 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

This is a first pass. It gets the basic functionality down, but the redirect variable probably needs to be sanitized. Sanitizing will need to be done in global.js, and javascript is not my strong suite. I know WP has the function wp_sanitize_redirect(), but I'm not sure how to implement it in js.

@cnorris23
14 years ago

#7 @cnorris23
14 years ago

  • Component set to Activity

#8 @paulhastings0
14 years ago

  • Summary changed from When deleting activity in permalink view page should redirct back to stream to [patch] When deleting activity in permalink view page should redirct back to stream

#9 @paulhastings0
14 years ago

  • Summary changed from [patch] When deleting activity in permalink view page should redirct back to stream to [patch] When deleting activity in permalink view page should redirect back to stream

#10 @boonebgorges
14 years ago

The patch is a good start. If the redirect is going to happen with JS (something that doesn't appear to happen anywhere else in BP, fwiw), it does need to be sanitized. (JS is not my strong suit either.) Or we could just remove the ajax delete from single activity items, so that the redirect happens in PHP. That way we don't have to worry about sanitization.

There's also a little funkiness with using wp_get_referer() for the redirect address, which I discovered as I was refreshing the page over and over again, and so the bp_redirect_url was set to the single activity item :)

#11 @DJPaul
14 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 1.3 to Future Release
  • Summary changed from [patch] When deleting activity in permalink view page should redirect back to stream to When deleting activity in permalink view page should redirect back to stream

@cnorris23
14 years ago

redirect via query arg

#12 @cnorris23
14 years ago

Updated patch to redirect single activity items in PHP rather than javascript

edit: Also adds some PHPdoc

Last edited 14 years ago by cnorris23 (previous) (diff)

#13 @cnorris23
14 years ago

  • Keywords has-patch added; needs-patch removed

#14 @DJPaul
13 years ago

  • Milestone changed from Future Release to 1.3

I don't think the attached patch is the best approach, but this could do with getting fixed for 1.3 as it's not ideal right now.

#15 @cnorris23
13 years ago

Do you have any direction on how you think it should be done?

#16 @DJPaul
13 years ago

I think I must have taken a quick look at the 001 patch, sorry. 002 is good. For 1.3, the patch could now use bp_get_root_domain(), bp_is_single_activity(), and perhaps bp_activity_get_permalink() as that will take care of the root_slug changes, too :)

#17 @cnorris23
13 years ago

Alrighty. I'll whip it up :)

#18 @cnorris23
13 years ago

  • Owner set to cnorris23
  • Status changed from new to assigned

Updating patch

@cnorris23
13 years ago

#19 @DJPaul
13 years ago

Going to put this patch in shortly -- just working on a bit of tidy up of nearby code, too

@DJPaul
13 years ago

#20 @DJPaul
13 years ago

Updated patch attached; 90% the same as cnorris23's 2002.003.diff.

It works fine with no AJAX, but it doesn't redirect with AJAX. Might need to take the approach as in 2002.001.diff for that. However, it also seemed inelegant that a lot of bp_activity_action_delete_activity() is duplicated in bp_dtheme_delete_activity(), so I started trying to add an optional argument into bp_activity_action_delete_activity(), and manually set $bp->action = 'delete' in bp_dtheme_delete_activity(), but it didn't seem to work. I've left that in the patch, and ideas welcome

@cnorris23
13 years ago

#21 @cnorris23
13 years ago

As Boone suggested (comment:10) we could just avoid the ajax altogether and let the redirect happen in php only. We don't really need the ajax delete on a single activity page anyway. Paul, I've updated your updated patch ;) to include an easy fix. Not great on back-compat, but the impact should be minimal. Since, jquery is listening for clicks with the 'delete-activity', we can just set things to 'delete-activity-single' to avoid the click event. A little hackish maybe, but it doesn't cause any design/layout issues in bp-default.

#22 @boonebgorges
13 years ago

I think 2002.005.diff is pretty close. I definitely prefer removing the AJAX delete, which is silly in the case of single items.

I thought maybe it would be possible to change the JS so it wouldn't be listening for the click, but since it's done by delegation on the parent, changing the CSS class is probably the only easy way to fix it. I think the backpat considerations are probably pretty minimal. DJPaul, what do you think?

#23 @DJPaul
13 years ago

Patch seems to work fine, and it's good and tasty.

#24 @boonebgorges
13 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [4337]) Redirects user back to activity stream after single activity item delete. Removes AJAX from delete link on activity permalink page. Fixes #2002. Props cnorris23

Note: See TracTickets for help on using tickets.