HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.
Please upgrade here. These earlier versions are no longer being updated and have security issues.

not compatible with vanilla 2.3b

This discussion is related to the Mail Checker addon.

Hi there, we just realized how important such a plugin is!
But sadly it allow sign ups anyway in vanilla 2.3b. :(

Comments

  • R_JR_J Ex-Fanboy Munich Admin

    Thanks for the feedback! I've got it fixed and will upload a new version soon. While updating it I'll add a setting screen to update the list from Francois-Guillaume Ribreaus GitHub repo, since this is where the most important part of this plugin comes from.

    I'll drop a note if I've uploaded the new version.

    P.S.: This will also influence RegistrationRestrictLogger, @vrijvlinder
    The EventArgument['User'] in the event UserModel_BeforeRegister is no more. It has been replaced with EventArgument['RegisteringUser']


    River
  • tchncstchncs Germany New

    Thank you <3

  • R_JR_J Ex-Fanboy Munich Admin

    If you need the solution ASAP, you would have to change the mailchecker by yourself. You would have to replace this (line 2025):

            $mail = explode('@', $args['User']['Email']);
    

    by that

            if (isset($args['RegisteringUser'])) {
                $email = $args['RegisteringUser']['Email'];
            } else {
                $email = $args['User']['Email'];
            }
            $mail = explode('@', $email);
    

    But I would advice to make changes only if you know php and really have an urge to do so. In other cases, just wait for a fixed version.


    tchncs
  • tchncstchncs Germany New

    Jey, wonderful, it's working, thank you so much!

  • R_JR_J Ex-Fanboy Munich Admin

    New version is available =)


    tchncs
  • tchncstchncs Germany New

    Nice B)

  • RiverRiver MVP
    edited August 2016

    @R_J said:
    Thanks for the feedback! I've got it fixed and will upload a new version soon. While updating it I'll add a setting screen to update the list from Francois-Guillaume Ribreaus GitHub repo, since this is where the most important part of this plugin comes from.

    I'll drop a note if I've uploaded the new version.

    P.S.: This will also influence RegistrationRestrictLogger, @vrijvlinder
    The EventArgument['User'] in the event UserModel_BeforeRegister is no more. It has been replaced with EventArgument['RegisteringUser']

    regarding RegistrationRestrictLogger @vrijvlinder
    The EventArgument['User'] in the event UserModel_BeforeRegister is no more.

    some changes to make RegistrationRestrictLogger it functional in 2.3b1 and still allow it to work in 2.2.1

    • 1

    class RegistrationRestrictLogger extends Gdn_Plugin {

    to

    class RegistrationRestrictLoggerPlugin extends Gdn_Plugin {

    • 2

    public function UserModel_BeforeRegister_Handler($Sender) {

    to // add args as shortcut

    public function UserModel_BeforeRegister_Handler($Sender,$args) {

    • 3

    $applicantname = $Sender->EventArguments['User']['Name'];
    $DisText = $Sender->EventArguments['User']['DiscoveryText'];

    to

               if (isset($args['RegisteringUser'])) {
                $applicantname = $args['RegisteringUser']['Name'];
                $DisText = $args['RegisteringUser']['DiscoveryText'];
                } else {
                $applicantname = $args['User']['Name'];
                $DisText = $args['User']['DiscoveryText'];
                }
    
    • 4

    $Sender->EventArguments['Valid'] = FALSE;

    to

    $args['Valid'] = false;

    you could change cases to all names and variables (e.g. $Sender to $sender) if desired and match "coding standards" if desired.. the above is primarily functional changes.

    Pragmatism is all I have to offer. Avoiding the sidelines and providing centerline pro-tips.

    vrijvlinder
  • RiverRiver MVP
    edited August 2016

    @r_j

    'Disposable mail addresses are not allowed.'

    to

    t( 'Disposable mail addresses are not allowed.')

    the spammers may not read english to understand the message.

    Pragmatism is all I have to offer. Avoiding the sidelines and providing centerline pro-tips.

  • R_JR_J Ex-Fanboy Munich Admin

    Thanks for checking my code!

    The part you are referencing is

                $sender->Validation->addValidationResult(
                    'Email',
                    'Disposable mail addresses are not allowed.'
                );
    

    But the method is defined like that:

        /**
         * Add a validation result (error) to the validation.
         *
         * @param string $FieldName The name of the form field that has the error.
         * @param string $ErrorCode The translation code of the error.
         *    Codes that begin with an '@' symbol are treated as literals and not translated.
         */
        public function addValidationResult($FieldName, $ErrorCode = '') {
    

    So no need to enclose validation results in t()


  • R_JR_J Ex-Fanboy Munich Admin

    But doing a quick look up on Vanillas sources, shows there are several usages of t() inside an addValidationResult call. I guess I will make a PR soon! Thanks for the inspiration!


  • RiverRiver MVP
    edited August 2016

    @R_J said:
    But doing a quick look up on Vanillas sources, shows there are several usages of t() inside an addValidationResult call. I guess I will make a PR soon! Thanks for the inspiration!

    public static function resultsAsArray($results) {
            $Errors = array();
            foreach ($results as $Name => $Value) {
                if (is_array($Value)) {
                    foreach ($Value as $Code) {
                        $Errors[] = trim(sprintf(T($Code), T($Name)), '.').'.';
                    }
                } else {
                    $Errors[] = trim(sprintf(T($Value), T($Name)), '.').'.';
                }
            }
            return $Errors;
        }
    

    To me it is much easier to follow code that you add the t everywhere, easier for users to locate definitions (since they are not documented in most cases) then to guess if the method you are using decided to use a t or not. Much harder to follow inconsistent usage. Done here, but not there except when here and there.

    Pragmatism is all I have to offer. Avoiding the sidelines and providing centerline pro-tips.

  • R_JR_J Ex-Fanboy Munich Admin

    I've used addValidationResult() and the documentation for that method reads "The translation code of the error.". What could be more straight forward?

    Complaining seems to be some kind of strange passion of yours...


  • RiverRiver MVP
    edited August 2016

    @R_J said:
    I've used addValidationResult() and the documentation for that method reads "The translation code of the error.". What could be more straight forward?

    .

    What could be more straight forward?

    a t used everywhere not hidden sometimes, would be more straightforward to the new coder, since you asked.

    My mistake for suggesting something easier for new users to understand and core inconsistencies in the core in

    applications/dashboard/models/class.importmodel.php

    if (!$Result) {
    $this->Validation->addValidationResult('Email', t('ErrorCredentials'));

    vs.
    applications/vanilla/models/class.categorymodel.php
    $this->Validation->addValidationResult('UrlCode', 'The specified url code is already in use by another category.');

    vs.

    library/core/class.pluginmanager.php
    $validation->addValidationResult('addon', '@'.$ex->getMessage());

    which is easier to notice a translation without digging deep. no different than your nitpicking on uPperLoWercase.

    bottom line your plugin is functional, and that is the most important.

    so you don't have to chose to take suggestion (even though it would process translate twice) but be easier to read.
    just wanted to present opinion my way not have you project what you think I think. And I will not be anal-retentive about it, just wanted to point it out.

    it has been said before here ....https://vanillaforums.org/discussion/26793/finding-translation-candidates-configs-and-fired-events

    The remaining function calls contain translatable data in a way that is more complicate to deal with. Those 119 functions have 82 unique parameters and only 40 of them contain strings (e.g. T($Discussion->Sink == '1' ? 'Unsink' : 'Sink')).
    The rest is untranslatable by just looking at the code that way: eg T($this->Data['Title']) makes a string translatable but it is not obvious how to do it...

    Pragmatism is all I have to offer. Avoiding the sidelines and providing centerline pro-tips.

Sign In or Register to comment.