Opened 9 years ago
Last modified 8 years ago
#6959 assigned enhancement
Use is_default_field() everywhere instead of comparing to 1
Reported by: | slaFFik | Owned by: | slaFFik |
---|---|---|---|
Milestone: | Awaiting Contributions | Priority: | normal |
Severity: | normal | Version: | |
Component: | Extended Profile | Keywords: | needs-patch needs-refresh |
Cc: |
Description
xProfile field Name is default, has an id=1 and in different places we can see both:
- comparison to
1
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)
Change History (9)
#3
@
9 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.
#4
@
9 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?
I will prepare a patch for this task.