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

Is can_request_send method correct? #86

Closed
hisyatokaku opened this issue Feb 9, 2019 · 8 comments
Closed

Is can_request_send method correct? #86

hisyatokaku opened this issue Feb 9, 2019 · 8 comments

Comments

@hisyatokaku
Copy link

hisyatokaku commented Feb 9, 2019

The code below seems to check if request can be sent from 'from_user' to 'to_user'

    def can_request_send(self, from_user, to_user):
        """ Checks if a request was sent """
        if from_user == to_user or FriendshipRequest.objects.filter(
            from_user=from_user,
            to_user=to_user,
            ).exists() is False:
            return False
        else: 
            return True

The method returns False when the request from 'from_user' to 'to_user' has NOT been made yet, which seems to be opposite.
Should is False be changed to is True ?

https://github.com/revsys/django-friendship/blob/master/friendship/models.py

@uri-rodberg
Copy link
Contributor

    if from_user == to_user or FriendshipRequest.objects.filter(
        from_user=from_user,
        to_user=to_user,
        ).exists() is False:

I recommend to use parentheses because it's not clear what is False refers to.

Also, maybe not is better than is False.

@uri-rodberg
Copy link
Contributor

In this case maybe you should just remove is False?

@uri-rodberg
Copy link
Contributor

        if self.can_request_send(from_user, to_user):
            raise AlreadyExistsError("Friendship already requested")

I'm also not sure if this code is correct.

@umbertov
Copy link
Contributor

It should be

    if from_user == to_user or not FriendshipRequest.objects.filter(
        from_user=from_user,
        to_user=to_user,
        ).exists():

And

if not self.can_request_send:
    raise  AlreadyExistsError (...)

Pardon formatting errors I'm on my phone

If somebody with write rights sees this maybe he can do the commit

uri-rodberg added a commit to uri-rodberg/django-friendship that referenced this issue Apr 25, 2019
@uri-rodberg
Copy link
Contributor

#88

@umbertov
Copy link
Contributor

Is this library still maintained?

@uri-rodberg
Copy link
Contributor

Is this library still maintained?

Last commit more than 3 months ago. But I'm not sure.

@frankwiles
Copy link
Member

@hisyatokaku yes the method is correct. I just refactored it a bit to make it slightly more clear.

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

No branches or pull requests

4 participants