[buddypress-trac] [BuddyPress Trac] #5738: Required xprofile fields errors management in members/single/profile/edit.php
noreply at wordpress.org
Fri Jul 25 01:15:43 UTC 2014
#5738: Required xprofile fields errors management in
Reporter: WCUADD | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: Future Release
Component: XProfile | Version:
Severity: normal | Resolution:
Keywords: needs-patch |
Comment (by WCUADD):
Unfornately, I was not able to factorize the function which prints the
nature of the field error, because it is a parametrized function (the
parameter is the string `$error_message`). This concerns
`bp_core_screen_signup()` function in `bp_members_screens.php`.
As far as I know, using an `add_action` hook only permits to specify the
number of arguments of the function that will be applied to the hook (here
`'bp_' . $fieldname . '_errors'` hook). The arguments themselves are given
when executing the hook `do_action('bp_' . $fieldname . '_errors')` that
is to say in the templates `register.php` (for the non-xprofile fields)
and then in the `edit_field()` function (for the xprofile fields).
It seems to me that it would be more consistent if the two processes
(registration and profile update) could operate from an unique basis with
advantages for further maintenance, evolution, and flexibility. But it
also seems more difficult than I thought at first glance.
Just to be sure of a few things, I tried to point the differences between
the two processes (registration and profile update) with regard to
required xprofile fields checks and the way these errors are rendered.
The differences concern:
- the fact that it has to be extended to other fields than xprofile fields
(in the registration case)
- the fact that the nature of the error is only stored in the registration
process (in a global object `$bp->signup->errors['fieldname']` that could
potentially be accessed further on)
- the use of a function which aims to print the html error message
relating to the specific field which is associated to a hook which name is
parametrized with the field name `'bp_' . $fieldname . '_errors'`. The
`create_function` mecanism inside the hook seems an elegant way to
parametrize the function applied to the hook and to give the value of the
parameter at the same time (bypassing the regular `add_action/do_action`
way of working)? This will allow to render the error attached to the
specific field. I'm not sure that this function was necessary because of
the previous point, as the nature of the error is globally stored?
- a local boolean variable `$errors` vs. a global defined steps variable
`$bp->signup->step` to test if the form validation has succeded
- as a consequence, the fact that profile update process doesn't allow to
print a localized error message to a specific field, but only a global
error message for the form thanks to the `bp_core_message` mecanism.
- (the errors are catched for profile update only if the form has been
submitted. Why are the required xprofile checks not performed when first
editing the profile?)
A few assomptions:
- It seems to me that rendering errors process should be factorized either
if it is registration process or profile update process but also whatever
type of checks is performed on the xprofile field. As a consequence, the
rendering errors process should not only relate to the required xprofile
fields checks, but to all types of checks?
- if I correctly understand #5731 discussion, all types of checks should
be gathered in the `BP_XProfile_Field_Type` classes. @boonebgorges, as you
stated in ticket:5731#comment:14
> it's worth noting that each of these types of checks should be handled
on a field-type basis. That is, all of these checks belong in the
BP_XProfile_Field_Type classes, and *not* in the form handlers.
and a few consequences:
- Does that mean that required xprofile fields controls should be
performed inside the `BP_XProfile_Field_Type` classes? If so, a
`BP_XProfile_Field_Type` method should check the required character of the
field? This is not included for the moment or I did not see it? Going this
direction tends to say that the required field check should be like a
separate function called by `is_valid()` check?
- Then, are the tests `if ( $is_required[$field_id] && empty(
$_POST['field_' . $field_id] ) )` still needed in the two screen functions
(even with `empty()` replaced with `!isset()`)? Perhaps the whole first
part of the screen validation should not exist (the part leading to `save-
details` step and the corresponding one in profile update)?
- if all types of checks are performed somehow within the `is_valid()`
method, it would say that they are performed when trying to set the field
data in `xprofile_set_field_data()` which is relatively late to detect
If I all misunderstood and if the purpose is not to gather all types of
checks within the `BP_XProfile_Field_Type` classes, if the purpose is to
keep the required xprofile checks within the screen functions, then
replacing `empty()` by `!isset()` will not be sufficient for the single
fields cases as `$_POST['field_'.$field_id]` is always defined as a
`string '' (length=0)` for them.
I know that I'm missing not something but a lot of things ;) I'd be
grateful if someone explains them to me, but I'd truly understand if not
possible. I'm aware that it is already an effort to read my english and I
apologize for it.
Nevertheless, I finish with developing all this, as I've spent some time
trying to understand and to think about it.
So what about the rendering errors process?
- Am I wrong thinking that it should belong the `BP_XProfile_Field_Type`
classes in a similar way as the `is_valid()` method? As the nature of the
error should be catched somewhere within the `is_valid()` method, it has
to be transmitted from this point, either the nature of the error being
stored in a global object, either calling the `'bp_' . $fieldname .
'_errors'` hook directly from here, keeping the `create_function` mecanism
to parametrize the error message?
- If not keeping the `create_function` mecanism, it amounts to store the
nature of the error in a global object in order to give it in the
`do_action()`. But in this case, is there still a need to use the `'bp_' .
$fieldname . '_errors'` hook? Perhaps because of the register template
that can not be changed due to compatibility constraints?
- If each error is not stored in a global object, then how to reconstruct
the notion of success or failure at the global form validation level? One
failing field validation causes the form validation failing => So it could
be with a local variable?
Another detail I don't understand, in the `xprofile_screen_edit_profile()`
screen function, in `bp-xprofile-screens.php:L143` :
// Redirect back to the edit screen to display the updates and message
bp_core_redirect( trailingslashit( bp_displayed_user_domain() .
$bp->profile->slug . '/edit/group/' . bp_action_variable( 1 ) ) );
This is executed when there is no errors, but it leads to execute twice
the screen function, second time with no changes. It seems unnecessary,
but there's surely one good reason I wasn't able to find.
Thanks for reading if coming till here.
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5738#comment:3>
BuddyPress Trac <http://buddypress.org/>
More information about the buddypress-trac