Users API: Filter by email

[OmarIthawi]

This change makes it possible to know whether a specific user is registered in a site or not.

GET /tahoe/api/v1/users/?email_exact=someone@example.com

Returning either a list of one user, or no one at all:

{
    "count": 0,
    "next": null,
    "previous": null,
    "results": []
}

RED-594: Enrollment API update epic.

[johnbaldwin]

@OmarIthawi Once we release this api update, we may have to live with it for a long time.

It seems like a very small addition, but I'm concerned on the context that we are using a list context to check on a single user. Do you see where we want a single email address to be shared among multiple users?

To fix this, how about using icontains instead of exact? Now we have the proper list context

Then the API consumer can check for specific matches in the response set. This would also be helpful when filtering for users from a specific domain.

[OmarIthawi]

@OmarIthawi Once we release this api update, we may have to live with it for a long time.

It seems like a very small addition, but I'm concerned on the context that we are using a list context to check on a single user. Do you see where we want a single email address to be shared among multiple users?

Not really. At least not within a single site.

To fix this, how about using icontains instead of exact? Now we have the proper list context
Then the API consumer can check for specific matches in the response set. This would also be helpful when filtering for users from a specific domain.

Thanks @johnbaldwin. I wanted the email parameter to have an exact match to give API users a strong and reliable way to see whether such learner exists.

During our discussion with Maxi (although not written anywhere) we thought of an additional more generic search or search_term parameter which could do a icontains match, perhaps on both username and email fields. We can go further and make it for the user.profile.name as well if needed.

class UserIndexFilter(FilterSet):
    search_term = CharFilter()
    email = ...

    class Meta:
        model = User
        fields = [
            'search',
            'email',
        ]

    def filter_search_term(self, queryset, name, value):
        return queryset.filter(
            Q(username__icontains=value) | Q(email__icontains=value) | Q(profile__name__icontains=value)
        )

I didn't want to add this feature now because no one requested it yet and we have other features that are being requested.

[johnbaldwin]

@OmarIthawi My caution again, adding this effectively will become a contractual obligation and we will have to live with for the duration that 'v1' of this API lives. I see it as an anti-pattern because we are using a list view filter to report the state of a single record.

However, this is also a rather benign example of an anti-pattern, so I'll approve the PR and hopefully we won't regret it

[johnbaldwin]

@OmarIthawi Can you at least use a different query param name instead of email or qualify the query param like email_exact? This way we don't lose the ability to have email=X as an icontains filter

[OmarIthawi]

@OmarIthawi Can you at least use a different query param name instead of email or qualify the query param like email_exact? This way we don't lose the ability to have email=X as an icontains filter

Great suggestion. Thanks @johnbaldwin! I've updated the PR accordingly.

[grozdanowski]

I can only comment on the general idea (not the details of implementation) - looks according to spec required. I can see some reasoning for a list:

• ?email=XXXX search should return a list (since it is not exact)
  therefore:
• ?email_exact=XXXX should also return a list for consistency, but can only return one record of course.

[OmarIthawi]

I can only comment on the general idea (not the details of implementation) - looks according to spec required. I can see some reasoning for a list:

• ?email=XXXX search should return a list (since it is not exact)
  therefore:
• ?email_exact=XXXX should also return a list for consistency, but can only return one record of course.

Thanks @grozdanowski. Yes, the other (now rejected) alternatives were one of the following:

A. Overload the single User API to accept email as a primary key: GET /tahoe/api/v1/users/someone@example.com/ just like GET /tahoe/api/v1/users/25425/: Overloading is not really a good idea and the implementation might not be straightforward.

B. Create a new API endpoint just for that. Not really RESTful since this is something definitely the /tahoe/api/v1/users/ API should handle.

C. Maxi and I ended up choosing the list-but-not-really-list although it's not a prefect approach. The main motivator it was very simple to test and implement besides being 100% fulling the immediate needs.

@johnbaldwin ☝️