Skip to:
Content

Opened 9 months ago

Closed 7 weeks ago

Last modified 7 weeks ago

#7536 closed enhancement (fixed)

Use bp_get_admin_url() in XProfile instead of hard-coded links

Reported by: JohnPBloch Owned by: johnjamesjacoby
Milestone: 3.0 Priority: normal
Severity: normal Version: 1.2
Component: Extended Profile Keywords: needs-testing
Cc:

Description

There were several areas in XProfile that used hard-coded URLs instead of the BuddyPress helper function. Replacing hard-coded addresses with addresses generated by the BP helper will improve overall stability and add flexibility to the plugin overall.

Attachments (1)

7536.1.patch (7.0 KB) - added by JohnPBloch 9 months ago.

Download all attachments as: .zip

Change History (8)

@JohnPBloch
9 months ago

#1 @johnjamesjacoby
9 months ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 2.9
  • Owner set to johnjamesjacoby
  • Status changed from new to reviewing
  • Version set to 1.2

I'll test this out in the multitude of environment setups that we support.

I think relying on bp_get_admin_url() is OK, but there may be weirdness with single-site activations on multisite setups, where site-admins wouldn't normally have the option to edit user settings.

This ticket was mentioned in Slack in #buddypress by hnla. View the logs.


9 months ago

#3 @hnla
8 months ago

@johnjamesjacoby does the patch still need testing or do you want to squeeze it in or punt to 3.0?

#4 @hnla
8 months ago

  • Milestone changed from 2.9 to 3.0

#5 @DJPaul
7 weeks ago

@johnjamesjacoby I'm inclined to merge this patch -- are you able to spend a little bit of time testing on the scenario you mentioned previously, or at least clarify that configuration so I can test it?

#6 @johnjamesjacoby
7 weeks ago

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

In 11808:

XProfile: use bp_get_admin_url() where appropriate.

This change updates all inline and querystring-based URL references to users.php to instead use the built-in supplied wrapper function. This comes with a few benefits:

  • URLs will be consistent across BuddyPress installation configurations (rather than relative to the current admin dashboard area)
  • Code is cleaner and easier to understand
  • Variable IDs are now cast to integers as needed (where they were not all previously)

Bonus: also updates a few bp_get_admin_url() calls in the about page to use the second parameter of add_query_arg() rather than wrapping it entirely, to match the expected usage format of all add_query_arg() calls.

Fixes #7536. Props JohnPBloch.

#7 @johnjamesjacoby
7 weeks ago

Related: we may want to consider adding protected methods to the field and field-group classes, so they can self-generate these URLs. It would reduce code duplication, remove some logic from the local scope of the functions we use to output the admin pages, and make these URLs testable so we can ensure their stability for future enhancements.

Imagine something like:

$group->get_admin_edit_url()

...or...

$group->get_url( 'edit', 'admin' )
Last edited 7 weeks ago by johnjamesjacoby (previous) (diff)
Note: See TracTickets for help on using tickets.