-
-
Notifications
You must be signed in to change notification settings - Fork 771
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
Add twitter verification for grants #7367
Add twitter verification for grants #7367
Conversation
Codecov Report
@@ Coverage Diff @@
## round-7-integration #7367 +/- ##
====================================================
Coverage 26.32% 26.32%
====================================================
Files 306 306
Lines 30317 30373 +56
Branches 4477 4486 +9
====================================================
+ Hits 7981 7997 +16
- Misses 22061 22106 +45
+ Partials 275 270 -5
Continue to review full report at Codecov.
|
app/grants/models.py
Outdated
@@ -364,6 +364,9 @@ class Meta: | |||
|
|||
# Grant Query Set used as manager. | |||
objects = GrantQuerySet.as_manager() | |||
verified = models.BooleanField(default=False) | |||
verified_by = models.ForeignKey('dashboard.Profile', null=True, blank=True, on_delete=models.SET_NULL) | |||
verified_at = models.DateTimeField(blank=True, null=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ Is this meant only for twitter ! If so rename all by adding prefix twitter
Also also help_text attribute explaining what it is (even if it's obvious)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -16,7 +16,7 @@ | |||
{% endcomment %} | |||
{% load static humanize i18n grants_extra %} | |||
<!DOCTYPE html> | |||
<html lang="en"> | |||
<html lang="en" xmlns="http://www.w3.org/1999/html"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a few comments
|
||
.verification__warning__icon { | ||
font-size: 2.8rem; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't this be shifted to the css file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
data-size="small" | ||
data-text="I am verifying my ownership of the {{ grant.title }}. \n{{ user_code }}\n" | ||
data-url="https://gitcoin.co{{ grant.get_absolute_url }}" | ||
data-hashtags="grants,round7"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's not hardcode round7! make it dynamic by looking at active_clr column
|
||
</div> | ||
<div class="modal-footer" style="border-top: none"> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nop, i removed it
</div> | ||
</div> | ||
</div> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ could we reduce the overall size of this to make it look neater ? It looks very big on the screenshot
app/grants/views.py
Outdated
'msg': 'Grant doesn\'t exists' | ||
}) | ||
|
||
grant = grants.first() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ could we do a grants.object.get ? every grant is unique
If we have duplicate entries -> something is wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
app/grants/views.py
Outdated
}) | ||
|
||
user_code = get_user_code(request.user.profile.id, emoji_codes) | ||
text = f"I am verifying my ownership of the { grant.title }" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
text = f"I am verifying my ownership of {grant.title } on Gitcoin Grants at {URL}"
pls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
has_code = user_code in last_tweet.full_text | ||
has_text = text in last_tweet.full_text | ||
|
||
if has_code and has_text: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noob question, this checks the username of the twitter account + grant id, so that i cant do a URL param attack on the endpoint.. right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, it verifies for twitter account + grant id + is member of this grant + the generated emoji code corresponds to the given grant and users that is verifying.
todo |
Description
https://www.loom.com/share/96b2918f42b4414b84dd86d79f20a9d3
Refers/Fixes
#7126
Testing