Skip to:

Opened 8 years ago

Last modified 7 years ago

#6959 assigned enhancement

Use is_default_field() everywhere instead of comparing to 1

Reported by: slaffik's profile slaFFik Owned by: slaffik's profile slaFFik
Milestone: Awaiting Contributions Priority: normal
Severity: normal Version:
Component: Extended Profile Keywords: needs-patch needs-refresh


xProfile field Name is default, has an id=1 and in different places we can see both:

  1. comparison to 1
  2. is_default_field() check

I propose to switch to using the is_default_field() and add a filter there as well.

Related and inspired by #6955.

Attachments (1)

6959.patch (2.9 KB) - added by slaFFik 8 years ago.

Download all attachments as: .zip

Change History (9)

#1 @slaFFik
8 years ago

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

#2 @slaFFik
8 years ago

  • Milestone changed from Awaiting Review to 2.6

I will prepare a patch for this task.

#3 @slaFFik
8 years ago

  • Component changed from Component - Activity to Component - XProfile
  • Keywords dev-feedback has-patch 2nd-opinion added

There are some places, like xprofile_clear_fullname_cache_on_profile_field_edit(), where we need to stuck with 1, because there is a hard-coded reference to a fullname field value. So it's not a general use default field.
Other proposed changes are in a patch.

But after creating a patch I got the idea, that we actually have can_delete & is_required separately, so it's already possible to fine-tune fields behaviour. So right now I'm not 100% sure that this patch change is required.

Please, advise.

8 years ago

#4 @boonebgorges
8 years ago

  • Keywords needs-patch added; dev-feedback has-patch 2nd-opinion removed

I agree that we should not be hardcoding this, and I think it's an OK idea to have a wrapper function for this purpose.

But there is a long and awful history here, so I want to be careful not to cause more problems. See #3725 (especially [5778]) and [7812]. We should probably be consolidating on the use of bp_xprofile_fullname_field_id() rather than 1.

@slaFFik Can you chew on those tickets and changesets and see what you think?

#5 @DJPaul
8 years ago

  • Milestone changed from 2.6 to Future Release

#6 @r-a-y
8 years ago

Related: #7109.

#7 @DJPaul
8 years ago

  • Type changed from task to enhancement

#8 @slaFFik
7 years ago

  • Keywords needs-refresh added
Note: See TracTickets for help on using tickets.