Overriding the code validation and using a custom validation is a feature that may be useful for a number of people or cases.

In our case, we had the following requirement

We needed to create custom registration codes (custom format and specific data within).
The format of the registration code had to be in plain text in the backend (registration code text field), but sent to the user encoded so that it would be unreadable.
So upon registration the user would enter the encoded version of the code.
For that case we had to create our own code validation function in order to decrypt the code and validate it against the saved one.
In order for the custom validation function to work we had to actually disable (comment out) the validation callback set in the regcode_simple.module.

Based on the above little hack of the module, i created a patch which does the following
1. Provide a 'Bypass Validation' checkbox in the regcode_simple configuration form
2. If checked, the validation callback defined in the module is not set. This means that no validation will be done.
3. A new, custom validation function must be provided by the developer in order to actually validate the registration code.

CommentFileSizeAuthor
#2 regcode_simple-bypass_validation-2860862-D8.patch3.28 KB_dcre_

Comments

_dcre_ created an issue. See original summary.

_dcre_’s picture

_dcre_ commented
StatusFileSize
new regcode_simple-bypass_validation-2860862-D8.patch3.28 KB
rpsu’s picture

rpsu
he/him
Primary language Finnish
commented
Status: Active » Needs work

Thanks for the patch! Even though overriding the validation is somewhat overriding the core of this module, I can see the need to have custom validation done elsewhere. However, there are some things to consider and some code issues to fix.

  1. +++ b/regcode_simple.module
@@ -56,13 +56,14 @@ function regcode_simple_form_user_admin_settings_alter(&$form, \Drupal\Core\Form
+    'code_salt_email_domain' => t('Salt for email domain')

    No need to remove the trailing comma. 

  2. +++ b/regcode_simple.module
@@ -104,6 +105,12 @@ function regcode_simple_form_user_admin_settings_alter(&$form, \Drupal\Core\Form
+    '#description' => t("This setting only applies to developers. Select it if you want to bypass default registration-code validation function 'regcode_simple_form_user_register_form_validate' and use a custom validation function. To do that you need to create a custom module and use hook_form_user_register_form_alter and add your custom validation callback."),

    In this description I'd WARN users much more clearly about the fact, that code won't be validated. Also; checking this would disable all text fields (hashes etc.).

    One thing to consider would be if user should provide a validation hook/function name here, if they want to override existing validation rules (ang that function existence would be verified on form submit). I think it might make this selection safer. 

  3. +++ b/regcode_simple.module
@@ -284,6 +292,10 @@ function regcode_simple_form_user_admin_settings_submit(&$form, \Drupal\Core\For
+    \Drupal::configFactory()->getEditable('regcode_simple.settings')
+    ->set('bypass_validation', $regcode_bypass_validation)
+    ->save();
+

    This must be defined also in
    config/schema/regcode_simple.schema.yml, or tests will fail.

_dcre_’s picture

_dcre_ commented

Hi rpsu, thanx for the quick feedback.

I had a look at your comments and would propose the following.

  • To have a 'Bypass Validation' checkbox, which will only be available when 'Plain text' is selected as registration code type.
  • A fully descriptive text is going to be added.
  • To have another field by the name e.g. 'Bypass Validation Callback'
  • If 'Bypass Validation' is checked then then 'Bypass Validation Callback' is required.
  • Module could try to check if ' 'Bypass Validation Callback' exists, upon save. Fail to save if it does not exist.
  • In the code, another check of the existence of 'Bypass Validation Callback' will be performed, and default validation is going to be done in case it does not exist.

With the above implementation, i think it will be quite safe to include this feature.

One more thing, about the tests.
I tried to run the tests on a vanilla d8 (latest version) and i am getting the following error
Fatal error: Cannot declare class Drupal\regcode_simple\Tests\RegcodeSimpleCodePlainTextTest, because the name is already in use in /var/www/html/web/modules/regcode_simple/tests/src/Functional/RegcodeSimpleCodePlainTextTest.php on line 90

rpsu’s picture

rpsu
he/him
Primary language Finnish
commented

I've been pondering if this module should actually provide API and use it for anything else but for the very basic plain txt regcodes.

It would mean all current methods would be moved to separate module(s). However the current structure does not really support that very well, so it would mean quite a lot of work (for the benefit IMHO, regarding the current use base).

Any thoughts? Another way is the one you proposed in comment #4 added with some error handling mechanism in the case where the external module/method would disappear/get uninstalled. Maybe hook_module_preuninstall() would be helpful in that?

megachriz’s picture

megachriz
Primary language Dutch
commented

@_dcre_
To bypass the validation callback of this module, the module doesn't need a setting for that. In a custom module you can just alter the user_register form and remove "regcode_simple_form_user_register_form_validate" from $form['#validate']. You do need to ensure that your custom module's form alter is called later than regcode_simple. You can do that by increasing the module weight:

mymodule.install:

/**
 * Implements hook_install().
 */
function mymodule_install() {
  module_set_weight('mymodule', 1);
}

@rpsu
An alternative from the current implementation is to use the plugin system. So each registration code type would then be its own plugin. This way, custom modules can add their own implementation.