Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

#5633 closed defect (bug) (fixed)

Group Administration - Add new members metabox & changing opinion

Reported by: imath Owned by: boonebgorges
Milestone: 2.1 Priority: normal
Severity: normal Version: 2.0
Component: Groups Keywords: has-patch
Cc:

Description

While on a particular Group Administration, it's possible to use the "Add new members" metabox to add a user to the current group using the autocomplete feature.

If i select a member, a new li is created in the ul#bp-groups-new-members-list and an hidden text field containing the user id is created. This hidden field ( new_members[] ) is the one used to actually get the members to add to the group.

The problem is when before updating the group i decide to change my mind about one of the users i want to add. If i click on the x link, it removes the li but not the corresponding hidden field new_members[]. As a result this member will "wrongly" be added when updating the group.

I suggest the attached patch to make sure it's not the case.

Attachments (2)

5633.patch (1.2 KB) - added by imath 5 years ago.
5633.02.patch (2.7 KB) - added by imath 5 years ago.

Download all attachments as: .zip

Change History (6)

@imath
5 years ago

#1 @DJPaul
5 years ago

Short of re-working all the JS to not have use a hidden field(s), patch looks OK, but there's some small improvements:

  • Use === instead of ==. You might have to typecast .val() for this.
  • Since you use $(this) and $(e.target) more than once, you should store these in variables and use those.

#2 follow-up: @boonebgorges
5 years ago

Short of re-working all the JS to not have use a hidden field(s), patch looks OK

I guess I probably originally wrote this, but I don't know why I would have done it this way. First thought was no-js support, but this technique certainly does not do *that*.

imath, your patch is fine, but if you felt like rewriting this section so that the hidden input is not required, feel free. (Maybe put data- fields into the #bp-groups-new-members-list items containing the user IDs, and then pre-submit, grab all of those IDs and put them into a single hidden input. Or even into .bp-suggest-user as comma-separated user_login values, which would have the bonus of providing true no-js support.)

#3 in reply to: ↑ 2 @imath
5 years ago

Replying to boonebgorges:

... Or even into .bp-suggest-user as comma-separated user_login values, which would have the bonus of providing true no-js support.)

Thanks DJPaul & boonebgorges for your feedbacks. I like the "true no-js support" option. 5633.02.patch is trying to achieve it.

@imath
5 years ago

#4 @boonebgorges
5 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 8417:

Improve logic for adding a user to a group in the Dashboard

On submit, the JS converts the dynamically created <li> items to a proper
comma-separated list of user logins. This creates greater consistency with the
noscript workflow, and also fixes a bug where users who were found via
autocomplete but then removed from the list of users to add would be added
anyway.

Fixes #5633

Props imath

Note: See TracTickets for help on using tickets.