-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
new profile data methods #5159
new profile data methods #5159
Conversation
…les to give them activity data
Codecov Report
@@ Coverage Diff @@
## kevin/profile-v2 #5159 +/- ##
====================================================
+ Coverage 30.9% 31.15% +0.24%
====================================================
Files 217 219 +2
Lines 17622 17774 +152
Branches 2430 2460 +30
====================================================
+ Hits 5446 5537 +91
- Misses 11953 11998 +45
- Partials 223 239 +16
Continue to review full report at Codecov.
|
Super dope |
@@ -2105,6 +2129,12 @@ class Profile(SuperModel): | |||
help_text=_('override profile avatar'), | |||
) | |||
dominant_persona = models.CharField(max_length=25, choices=PERSONAS, blank=True) | |||
longest_streak = models.IntegerField(default=0) | |||
activity_level = models.CharField(max_length=10, blank=True, help_text=_('the users activity level (high, low, new)')) |
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.
did you forget medium
? (I guess it's only the help 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.
in the help text only yes
app/dashboard/models.py
Outdated
return "High" | ||
if visits_last_month > med_threshold: | ||
return "Medium" | ||
if visits_last_month > med_threshold: |
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.
should this be visits_last_month
< med_threshold
?
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.
no; d1574cc
|
||
if visits_last_month > high_threshold: | ||
return "High" | ||
if visits_last_month > med_threshold: |
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.
med_threshold
is the same as high_threshold
, does that make sense?
also - should there be an upper bound as well? (visits_last_month
> med_threshold
and <= high_threshold
?)
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.
yeah.... it is.
alos i just made this up... you guys tell me what the thresholds should be pls. in particular, @frankchen07 curious your opionion
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.
I think it's fine where it is - 15 stands to be a visit every other day, medium is 2 visit per week, and lower than that is low.
if bounty.is_funder(self.handle): | ||
for handle in fulfiller_handles: | ||
relationships.append(handle) | ||
if self.handle in fulfiller_handles: |
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.
nice this handles both funder -> fulfiller and and vice versa
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.
yar
eligible_bounties = bounties.filter(idx_status__in=['done', 'expired', 'cancelled']) | ||
|
||
if eligible_bounties.count() == 0: | ||
return -1 |
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.
hm why -1?
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.
its a way of saying "this user hasnt done any bounties; pls dont display on frontend". as opposed to 0 which is for when the user has made $0/hr for many bounties
iterdate += timezone.timedelta(days=1) | ||
|
||
is_weekday = iterdate.weekday() < 5 | ||
if not is_weekday: |
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.
shouldn't weekends be counted in the streak as well?
also is the if not is_weekday
grabbing weekends here? if its 5 or greater is_weekday
is false and the if not
will render true
, and it'll continue, no?
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.
the if not weekday
thing is an if statement that calls a continue
which effectivbely skips weekend days
i dont think weekends should count in the streak mechanic.. we dont want to build a culture that encourages burnout
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.
would <3 a review from @gitcoinco/engineers
if bounty.is_funder(self.handle): | ||
for handle in fulfiller_handles: | ||
relationships.append(handle) | ||
if self.handle in fulfiller_handles: |
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.
yar
eligible_bounties = bounties.filter(idx_status__in=['done', 'expired', 'cancelled']) | ||
|
||
if eligible_bounties.count() == 0: | ||
return -1 |
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.
its a way of saying "this user hasnt done any bounties; pls dont display on frontend". as opposed to 0 which is for when the user has made $0/hr for many bounties
@@ -2105,6 +2129,12 @@ class Profile(SuperModel): | |||
help_text=_('override profile avatar'), | |||
) | |||
dominant_persona = models.CharField(max_length=25, choices=PERSONAS, blank=True) | |||
longest_streak = models.IntegerField(default=0) | |||
activity_level = models.CharField(max_length=10, blank=True, help_text=_('the users activity level (high, low, new)')) |
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.
in the help text only yes
|
||
if visits_last_month > high_threshold: | ||
return "High" | ||
if visits_last_month > med_threshold: |
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.
yeah.... it is.
alos i just made this up... you guys tell me what the thresholds should be pls. in particular, @frankchen07 curious your opionion
Description
this commit adds methods for calculating all the info needed on profiles to give them activity data
Refers/Fixes
#4908
Testing
locally
https://gitcoincore.slack.com/archives/CMGLVLSJX/p1568078390010700g