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

Feature: Show Recent People I've Tipped #7830

Merged
merged 5 commits into from
Nov 10, 2020

Conversation

sebastiantf
Copy link
Contributor

@sebastiantf sebastiantf commented Nov 6, 2020

Description

This PR adds a new feature that shows the list of users (max of 7) that you've recently tipped, when you visit http://gitcoin.co/tip

Refers/Fixes

Fixes #7681

Screenshots
  1. If user has not tipped anyone yet:

GIF:

Video: https://www.loom.com/share/bdd437d9ea5245df9005173b90f0e9ab

  1. If user has tipped before:

Video: https://www.loom.com/share/cffb44d8cb8345fbb051b6edb50011bc

Screenshot:
FireShot Capture 314 - Send Tip - Gitcoin - Gitcoin - localhost

Done:

  • Display icons of the last 7 people tipped below the search (max of 7).
  • On hover, show the persons gitcoin username
  • Clicking on the name takes user to profile
  • If only one person has been tipped, left align the entire row

@@ -462,6 +462,15 @@ def send_tip_2(request):
user['avatar_url'] = profile.avatar_baseavatar_related.filter(active=True).first().avatar_url
user['preferred_payout_address'] = profile.preferred_payout_address

recent_tips = Tip.objects.filter(sender_profile=request.user.profile).order_by('-created_on')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recent_tips_profiles = Profile.objects.filter(received_tips__in=Tip.objects.filter(sender_profile__id=7766)).order_by('-received_tips__created_on')

Unfortunately you can't use distinct here or it could be a one-liner! https://docs.djangoproject.com/en/dev/ref/models/querysets/#distinct

Anyway, no need to change the code, but thought it was an interesting way to access the profiles directly..

Copy link
Contributor Author

@sebastiantf sebastiantf Nov 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I spent 1.5+ days only on trying to make this one query better and efficient. But couldn't find a way to make it a single query/one-liner. Tried several several several ways, but no use. That's a bummer ☹️

Another good one may look something like:

recent_tips_profiles = Tip.objects.filter(sender_profile=request.user.profile).distinct('recipient_profile').order_by('-created_on')

But it produces this error on Postgres:
ERROR: SELECT DISTINCT ON expressions must match initial ORDER BY expressions

I did find this: https://code.djangoproject.com/ticket/24218. But it's still new.

Tried several other ways too. But had to settle with this in the end. 🤢

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it really exposes where the ORM falls short, basically since you are joining tables then the rows are no longer distinct

@thelostone-mc thelostone-mc merged commit 419a967 into gitcoinco:master Nov 10, 2020
@PixelantDesign
Copy link
Contributor

yay!

@owocki
Copy link
Contributor

owocki commented Nov 24, 2020

'AnonymousUser' object has no attribute 'profile'

@sebastiantf this PR breaks the gitcoin.co/tip experience for a non logged in user. can u pls put in a follow up PR for that?

@sebastiantf
Copy link
Contributor Author

'AnonymousUser' object has no attribute 'profile'

@sebastiantf this PR breaks the gitcoin.co/tip experience for a non logged in user. can u pls put in a follow up PR for that?

@owocki I have a follow-up PR at #7938

Apologies for the issue

@PixelantDesign
Copy link
Contributor

Thanks Sebastian!

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

Successfully merging this pull request may close these issues.

Show Recent People I've Tipped
5 participants