Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#51379 closed feature request (fixed)

Add filter to function email_exists()

Reported by: apermo's profile apermo Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.6 Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch commit has-dev-note
Focuses: Cc:

Description

At a current project I'm running WordPress together with a third party tool that is the leading system for the user data, but I'm also able to register new users, obviously I want to throw an error if a username or an e-mail address is already taken.

In case of the function username_exists a filter has been added in 4.9 that I can use just for this.

With this addition the same functionality would be available for email_exists() as well

<?php
/**
 * Determines whether the given email exists.
 *
 * For more information on this and similar theme functions, check out
 * the {@link https://developer.wordpress.org/themes/basics/conditional-tags/
 * Conditional Tags} article in the Theme Developer Handbook.
 *
 * @since 2.1.0
 *
 * @param string $email Email.
 * @return int|false The user's ID on success, and false on failure.
 */
function email_exists( $email ) {
        $user = get_user_by( 'email', $email );
        if ( $user ) {
                $user_id = $user->ID;
        } else {
                $user_id = false;
        }

        /**
         * Filters whether the given email exists or not.
         *
         * @since TBD
         *
         * @param int|false $user_id The user's ID on success, and false on failure.
         * @param string    $email   Email.
         */
        return apply_filters( 'email_exists', $user_id, $email );
}

The filter name is in line with what is used in username_exists and is checked to be unused within WordPress Core.

Attachments (6)

email_exists.patch (807 bytes) - added by apermo 4 years ago.
Patch for the given example
51379.patch (691 bytes) - added by mukesh27 4 years ago.
Updated patch.
51379.2.diff (686 bytes) - added by garrett-eclipse 4 years ago.
Updated filter documentation to be more explicit than just 'Email.'
51379.3.diff (1.5 KB) - added by garrett-eclipse 4 years ago.
Further phpdoc enhancement bringing more parity between the email_exists and username_exists functions and filters
51379.4.diff (2.2 KB) - added by garrett-eclipse 4 years ago.
One further minor docblock update remove or not as when refering to exists and valid the or not is already inferred.
51379.5.diff (884 bytes) - added by garrett-eclipse 4 years ago.
Docblock improvement I missed getting into my patches.

Download all attachments as: .zip

Change History (32)

@apermo
4 years ago

Patch for the given example

#1 @apermo
4 years ago

The function and obviously also the patch is found in /wp-includes/user.php

This ticket was mentioned in PR #547 on WordPress/wordpress-develop by apermo.


4 years ago
#2

As described in the trac ticket, I'd like to have a filter in email_exists() in order to have more freedom when working with a third party system that is the leading system for my user-data.

The filter is working the same way as the filter in username_exists in the lines above which was added in 4.9.0.

Trac ticket: https://core.trac.wordpress.org/ticket/51379

#3 @mukesh27
4 years ago

  • Component changed from General to Users
  • Keywords dev-feedback added
  • Version trunk deleted

Looks good to me.

Let's wait for other dev feedback.

#4 follow-up: @swissspidy
4 years ago

Previously / Duplicate: #35509

#5 in reply to: ↑ 4 @apermo
4 years ago

Replying to swissspidy:

Previously / Duplicate: #35509

Basically I have the same use-case as mentioned in the ticket by @mikelopez.

The only way I can currently do this is by using the filter pre_user_email and replacing the given e-mail address with one of a dummy account where I know that this will never be changed, so that the function email_exists will answer with false.

I would actually prefer a completely different system similar to what is available with the login authorization.

So some way to fire a custom error within wp_insert_user() would even be better. For examlple like this.

<?php
        /**
         * Checks the given userdata and allows to inject a custom error during registration.
         *
         * This filter is called before the user is created or updated.
         *
         * @since TBD
         *
         * @param false|WP_Error
         * @param string $userdata Array with Userdata.
         */
        $custom_checks = apply_filters( 'wp_insert_user_custom_checks', false, $userdata );
        if ( $custom_checks instanceof WP_Error ) {
                return $custom_checks;
        }

Added just before $pre_user_login = apply_filters( 'pre_user_login', $sanitized_user_login );

That would likely be a new ticket, but would be a even more convenient way to solve my problem, as I would be able to throw a custom message.

mikaykun commented on PR #547:


4 years ago
#6

Looks good to me, apart from some indenting issues (not using 4 spaces).

The indents are correct, see https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/#indentation

The checks failed and should be corrected if possible.

TimothyBJacobs commented on PR #547:


4 years ago
#7

Looks good to me, apart from some indenting issues (not using 4 spaces).

The indents are correct, see https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/#indentation

The checks failed and should be corrected if possible.

The indentations are not correct. The patch is using spaces where it should be using tabs.

https://i0.wp.com/user-images.githubusercontent.com/3460448/94350334-4cf25900-001b-11eb-8a6f-c58ce55ef1f7.png

#8 follow-up: @Mista-Flo
4 years ago

Hi there. Well if username_exists function has the same filter, then I don't see any reason to not add the same kind of filter for email_exists as well. Patch looks good.

@mukesh27
4 years ago

Updated patch.

#9 @mukesh27
4 years ago

51379.patch patch updated with correct text indent and @since for 5.6

Patch ready for 5.6

apermo commented on PR #547:


4 years ago
#10

Looks good to me, apart from some indenting issues (not using 4 spaces).

The indents are correct, see https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/#indentation

The checks failed and should be corrected if possible.

The indentations are not correct. The patch is using spaces where it should be using tabs.

https://i0.wp.com/user-images.githubusercontent.com/3460448/94350334-4cf25900-001b-11eb-8a6f-c58ce55ef1f7.png

That's probably due to me using the GitHub Web Editor instead of actually pushing the patch from PHPStorm. The Patch has already been updated on trac to honor the correct indentation.

#11 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.6
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#12 @hellofromTonya
4 years ago

  • Keywords needs-dev-note added

A mention in the Misc Dev note for the new filter.

@garrett-eclipse
4 years ago

Updated filter documentation to be more explicit than just 'Email.'

#13 in reply to: ↑ 8 ; follow-up: @garrett-eclipse
4 years ago

  • Keywords commit added; dev-feedback removed

The filter itself looks good and applied via trunk. I've updated the patch in 51379.2.diff to expand upon what 'Email.' in both the filter and the function signature. As well introduced a @since description to the email_exists function to introduce the new filter to that documentation.

Concerning the dev-feedback I'm going to remove as I concur with the comment by @Mista-Flo;
Replying to Mista-Flo:

Hi there. Well if username_exists function has the same filter, then I don't see any reason to not add the same kind of filter for email_exists as well. Patch looks good.

Marking for commit as everything looks good and my latest patch is just a final improvement to the docblock.

@garrett-eclipse
4 years ago

Further phpdoc enhancement bringing more parity between the email_exists and username_exists functions and filters

@garrett-eclipse
4 years ago

One further minor docblock update remove or not as when refering to exists and valid the or not is already inferred.

#14 @SergeyBiryukov
4 years ago

#35509 was marked as a duplicate.

#15 @SergeyBiryukov
4 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 49148:

Users: Introduce email_exists filter, to complement username_exists.

Props garrett-eclipse, apermo, mukesh27, Mista-Flo, sebastian.pisula, mikelopez.
Fixes #51379. See #35509.

@garrett-eclipse
4 years ago

Docblock improvement I missed getting into my patches.

#16 in reply to: ↑ 13 ; follow-up: @garrett-eclipse
4 years ago

Thanks for the commit @SergeyBiryukov looking at it I've realized somehow my changes didn't get saved in the second patch, I must have made them in /build/ accidentally... referring to this comment;
Replying to garrett-eclipse:

The filter itself looks good and applied via trunk. I've updated the patch in 51379.2.diff to expand upon what 'Email.' in both the filter and the function signature. As well introduced a @since description to the email_exists function to introduce the new filter to that documentation.

Would you mind following up with the docblock improvement in 51379.5.diff.

#17 in reply to: ↑ 16 @SergeyBiryukov
4 years ago

Replying to garrett-eclipse:

Would you mind following up with the docblock improvement in 51379.5.diff.

Sure, also noticed this myself after the commit :)

I don't think the function needs the @since note though, as we already have it on the filter itself.

Per the documentation standards, @since tags are generally added for significant changes like:

  • Adding new arguments or parameters
  • Required arguments becoming optional
  • Changing default/expected behaviors
  • Functions or methods becoming wrappers for new APIs

#18 follow-up: @garrett-eclipse
4 years ago

Thanks @SergeyBiryukov, if we remove the @since on the function please do so on the username_exists as well since we added it there.

I was going that way to expose the new filter to the function's phpdoc pages that get generated so users looking up the known email_exists function will be introduced to the change of behaviour that they can control as the function now wraps a filter.

I'm ok with it off, just see it as a way to link the two together in doc spaces and indicate the function can now behave differently than expected if anything has hooked into that filter.

#19 in reply to: ↑ 18 ; follow-up: @SergeyBiryukov
4 years ago

Replying to garrett-eclipse:

I'm ok with it off, just see it as a way to link the two together in doc spaces and indicate the function can now behave differently than expected if anything has hooked into that filter.

I think they would still be linked the same way that username_exists() currently links to the username_exists filter in the "Uses" section.

I'm not strongly opposed to adding it to the function, it just seems like we don't generally do that for new hooks, as that partially duplicates the hook documentation.

#20 @SergeyBiryukov
4 years ago

In 49153:

Docs: Improve description of the $email parameter in email_exists().

Follow-up to [49148].

Props garrett-eclipse.
See #51379.

#21 in reply to: ↑ 19 @garrett-eclipse
4 years ago

Replying to SergeyBiryukov:

Replying to garrett-eclipse:

I'm ok with it off, just see it as a way to link the two together in doc spaces and indicate the function can now behave differently than expected if anything has hooked into that filter.

I think they would still be linked the same way that username_exists() currently links to the username_exists filter in the "Uses" section.

I'm not strongly opposed to adding it to the function, it just seems like we don't generally do that for new hooks, as that partially duplicates the hook documentation.

Ah thanks @SergeyBiryukov, I didn't realize there was already some magic linking in there. Let's drop it then and drop it off the username_exists function as we added in via this ticket.

#22 @SergeyBiryukov
4 years ago

Already done, [49148] didn't include it for username_exists() :)

#23 @garrett-eclipse
4 years ago

Niice, you rock @SergeyBiryukov

This ticket was mentioned in Slack in #core by sncoker. View the logs.


4 years ago

Note: See TracTickets for help on using tickets.