Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate wplf_{$form->ID}_* and wplf_{$forms->post_name}_* actions and filters #169

Open
k1sul1 opened this issue Jun 2, 2019 · 2 comments

Comments

@k1sul1
Copy link
Member

k1sul1 commented Jun 2, 2019

While it was a good idea at the time (#52), it's cumbersome to add new filters and actions. I think I implemented it the way I did, because I heavily dislike this, probably because I've been bitten by it more times than I can count:

add_action(
  'x', 
  function($x, $y, $z) {

  },
  10,
  3
);

I've no clue why WordPress chose to add the $accepted_args parameter, when they could've just passed all available arguments to the function like a normal person, but what is done is done and that ain't changing anytime soon. Probably some prehistoric reasons that I don't know of. I looked at call_user_func and call_user_func_array, and there's zero indication of having to know the amount of parameters.

Nevertheless, I propose that we start doing this instead of creating 3 filters / actions every time we add a new one.

$data = do_action('wplf_form_something', $data, $form); 

I don't know if we have access to full $form post object everywhere where we have these kind of filters, if we don't, then we should just get the full post object. I don't want to do this.

$data = do_action('wplf_form_something', $data, $form->ID, $form->post_name); 

I suggest that we use _deprecated_function (or create our own equivalent, because you know...) and deprecate all filters and actions that are like this. I don't want to remove them in 2.0, but I certainly don't want any new projects using them either.

Although naming things is hard, it should be pretty easy to create equivalent filters and actions that follow the new style. I wouldn't just do them without thought though, but instead make a list of all of our available actions and filters and figure out a consistent naming scheme for all of them.

v3.0 would remove the deprecated things completely. 3.0 doesn't have to include huge amounts of new features, it's just to indicate breaking changes. Practise what you preach, use semantic versioning and so on.

Pinging @rask @luizbills @ironland @libreform/maintainers and everyone else that might care.

@k1sul1 k1sul1 added this to the 2.0 milestone Jun 2, 2019
@k1sul1
Copy link
Member Author

k1sul1 commented Jun 2, 2019

All filters from #167:

classes/class-cpt-wplf-submission.php
3:if (! class_exists('CPT_WPLF_Submission')) :
5:  class CPT_WPLF_Submission
14:        self::$instance = new CPT_WPLF_Submission($wplf);
36:      add_action('admin_notices', array( $this, 'wplf_submission_bulk_action_admin_notice' ));
37:      add_filter('bulk_actions-edit-wplf-submission', array( $this, 'register_wplf_submission_bulk_actions' ));
38:      add_filter('handle_bulk_actions-edit-wplf-submission', array( $this, 'wplf_submission_bulk_action_handler' ), 10, 3);
174:    public function register_wplf_submission_bulk_actions($bulk_actions) {
179:    public function wplf_submission_bulk_action_handler($redirect_to, $doaction, $post_ids) {
195:    public function wplf_submission_bulk_action_admin_notice() {

wp-libre-form.php
64:      CPT_WPLF_Submission::init($this);
105:      CPT_WPLF_Submission::register_cpt();

classes/class-cpt-wplf-form.php
602:      $default = '%name% <%email%>'; // default submission title format

classes/class-wplf-dynamic-values.php
63:      return apply_filters('wplf_dynamic_values', array_merge(

classes/class-cpt-wplf-form.php
652:      $template_content = apply_filters('wplf_import_html_template', null, $form_id);
822:        $success = apply_filters('wplf_save_success_message', $success, $post_id);
942:        $content = apply_filters("wplf_{$form->post_name}_form", $content);
943:        $content = apply_filters("wplf_{$form->ID}_form", $content);
946:        $content = apply_filters('wplf_form', $content, $id, $xclass, $attributes);
1029:          apply_filters('wplf_frontend_script_dependencies', array()),
1037:      wp_localize_script('wplf-form-js', 'WPLF_DATA', apply_filters('wplf_ajax_object', array(
1038:      'ajax_url' => apply_filters('wplf_ajax_endpoint', "$admin_url?action=wplf_submit"),
1039:      'ajax_credentials' => apply_filters('wplf_ajax_fetch_credentials_mode', 'same-origin'),
1040:      'request_headers' => (object) apply_filters('wplf_ajax_request_headers', []),
1132:      return apply_filters('wplf-form-publicly-visible', false, $id);

classes/class-wplf-plugins.php
222:    return apply_filters('wplf_enabled_plugins', $this->plugins);
259:    return apply_filters('wplf_recommended_plugins', $list);

classes/class-cpt-wplf-submission.php
122:      $allowed = apply_filters('wplf-dropdown-filter', $allowed);

inc/wplf-form-actions.php
37:    $content = apply_filters('wplf_email_copy_content_start', $content, $form_title, $form_id) . "\n\n";
66:    $to = apply_filters('wplf_email_copy_to', $to, $return, $submission_id);
67:    $subject = apply_filters('wplf_email_copy_subject', $subject, $return, $submission_id);
68:    $content = apply_filters('wplf_email_copy_content', $content, $return, $submission_id);
69:    $headers = apply_filters('wplf_email_copy_headers', $headers, $return, $submission_id);
70:    $attachments = apply_filters('wplf_email_copy_attachments', $attachments, $return, $submission_id);
73:    $to = apply_filters("wplf_{$form->post_name}_email_copy_to", $to, $return, $submission_id);
74:    $subject = apply_filters("wplf_{$form->post_name}_email_copy_subject", $subject, $return, $submission_id);
75:    $content = apply_filters("wplf_{$form->post_name}_email_copy_content", $content, $return, $submission_id);
76:    $headers = apply_filters("wplf_{$form->post_name}_email_copy_headers", $headers, $return, $submission_id);
77:    $attachments = apply_filters("wplf_{$form->post_name}_email_copy_attachments", $attachments, $return, $submission_id);
80:    $to = apply_filters("wplf_{$form->ID}_email_copy_to", $to, $return, $submission_id);
81:    $subject = apply_filters("wplf_{$form->ID}_email_copy_subject", $subject, $return, $submission_id);
82:    $content = apply_filters("wplf_{$form->ID}_email_copy_content", $content, $return, $submission_id);
83:    $headers = apply_filters("wplf_{$form->ID}_email_copy_headers", $headers, $return, $submission_id);
84:    $attachments = apply_filters("wplf_{$form->ID}_email_copy_attachments", $attachments, $return, $submission_id);
168:    $implode_glue = apply_filters('wplf_email_array_field_implode_glue', ', ');
169:    $implode_glue = apply_filters("wplf_{$form_name}_email_array_field_implode_glue", $implode_glue);
170:    $implode_glue = apply_filters("wplf_{$form_id}_email_array_field_implode_glue", $implode_glue);

wp-libre-form.php
91:      wp_localize_script('wplf-form-edit-js', 'WPLF_DATA', apply_filters('wplf_admin_ajax_object', [
131:      if (apply_filters('wplf_load_polylang', true) && class_exists('Polylang')) {

inc/wplf-ajax.php
18:  $return = apply_filters('wplf_validate_submission', $return);
27:    $return = apply_filters("wplf_{$form->post_name}_validate_submission", $return);
28:    $return = apply_filters("wplf_{$form->ID}_validate_submission", $return);
46:    $_POST = apply_filters('wplf_submission_post_data', $_POST);
102:        $file_name = sanitize_file_name(apply_filters('wplf_uploaded_file_name', $default_file_name, $file, $post_id));
105:        $file_path = apply_filters('wplf_uploaded_file_path', $file_path, $file, $post_id);
116:    add_post_meta($post_id, '_wplf_email_copy_to', apply_filters('wplf_email_copy_to', $to));
123:    $success = apply_filters("wplf_{$form->post_name}_success_message", $success);
124:    $success = apply_filters("wplf_{$form->ID}_success_message", $success);
125:    $success = apply_filters('wplf_success_message', $success);

inc/wplf-form-validation.php
82:  $disable_validation = apply_filters('wplf_disable_validate_additional_fields', $disable_validation, $form);
85:  $disable_validation = apply_filters("wplf_{$form->ID}_disable_validate_additional_fields", $disable_validation, $form);
88:  $disable_validation = apply_filters("wplf_{$form->post_name}_disable_validate_additional_fields", $disable_validation, $form);
105:  $custom_fields = apply_filters('wplf_allowed_additional_form_fields', $custom_fields, $form);
106:  $custom_fields = apply_filters("wplf_{$form->ID}_allowed_additional_form_fields", $custom_fields, $form);
107:  $custom_fields = apply_filters("wplf_{$form->post_name}_allowed_additional_form_fields", $custom_fields, $form);

All actions:

classes/class-cpt-wplf-form.php
772:        do_action("wplf_delete_form", $post);
773:        do_action("wplf_{$post->slug}_delete_form", $post);
774:        do_action("wplf_{$post->slug}_delete_form", $post);

inc/wplf-ajax.php
14:  do_action('wplf_pre_validate_submission');
132:    do_action('wplf_post_validate_submission', $return);
133:    do_action("wplf_{$form->post_name}_post_validate_submission", $return);
134:    do_action("wplf_{$form->ID}_post_validate_submission", $return);

Gathered with ag.

@rask
Copy link
Contributor

rask commented Jun 4, 2019

All in for getting rid of hook IDs that rely on variables. Rather have the form identifiers available as callback arguments instead. The following is a sensible approach and I think it is quite common in WP core as well:

$data = do_action('wplf_form_something', $data, $form); 

If we cannot pass a full form object 100% of the time, I would consider using the form ID and providing a helper function similar to wplf_get_form_by_id(int $id) : \WP_Post. Then again we need to validate whether we have forms that have no ID, e.g. drafts and similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants