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

Funder and hunter personas in user profile, sync personas to individual mailchimp lists #4481

Merged
merged 3 commits into from
Jun 24, 2019

Conversation

danlipert
Copy link
Contributor

@danlipert danlipert commented May 24, 2019

Description

This PR adds two additional boolean values to user profiles, whether a user is a bounty hunter, and/or a bounty funder. In addition, a migration is added that sets these flags for each user. The SQL query is used directly to maintain compatibility between the query used here and the query used on Metabase created by @frankchen07 . Also, two endpoints are created to export these lists easily for mailing list purposes. The hunter and funder flags are set daily via cronjob for all users.

The hunter and funder flags are only set to true for a given user if the SQL query lists the user as active.

The different personas are then synced to 3 new mailchimp audiences on the new mailchimp account: one for funders, one for hunters, and one for everyone.

Refers/Fixes

#4415

Testing

Tested persona assignment locally by applying migration manually and checking the export endpoints. Mailchimp integration tested via unit test.

@codecov
Copy link

codecov bot commented May 29, 2019

Codecov Report

Merging #4481 into master will decrease coverage by 0.01%.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4481      +/-   ##
==========================================
- Coverage   30.08%   30.07%   -0.02%     
==========================================
  Files         209      211       +2     
  Lines       16850    16873      +23     
  Branches     2267     2270       +3     
==========================================
+ Hits         5070     5075       +5     
- Misses      11582    11600      +18     
  Partials      198      198
Impacted Files Coverage Δ
...dashboard/management/commands/set_user_personas.py 0% <0%> (ø)
app/dashboard/sql/persona.py 100% <100%> (ø)
app/dashboard/models.py 56.29% <31.25%> (-0.28%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d59ae5c...ba33bbc. Read the comment docs.

@codecov
Copy link

codecov bot commented May 29, 2019

Codecov Report

Merging #4481 into master will increase coverage by 0.13%.
The diff coverage is 50.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4481      +/-   ##
==========================================
+ Coverage    30.3%   30.44%   +0.13%     
==========================================
  Files         214      216       +2     
  Lines       17142    17442     +300     
  Branches     2319     2362      +43     
==========================================
+ Hits         5195     5310     +115     
- Misses      11740    11926     +186     
+ Partials      207      206       -1
Impacted Files Coverage Δ
app/app/urls.py 89.36% <ø> (ø) ⬆️
...dashboard/management/commands/set_user_personas.py 0% <0%> (ø)
app/app/settings.py 78.81% <100%> (+0.15%) ⬆️
app/dashboard/sql/persona.py 100% <100%> (ø)
app/dashboard/models.py 56.39% <26.31%> (+0.13%) ⬆️
app/dashboard/views.py 14.43% <33.33%> (+0.11%) ⬆️
app/marketing/management/commands/sync_mail.py 29.16% <94.44%> (+29.16%) ⬆️
app/grants/templatetags/grants_extra.py 71.42% <0%> (-28.58%) ⬇️
app/grants/models.py 49.67% <0%> (-8.89%) ⬇️
app/economy/models.py 56.34% <0%> (-2.07%) ⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0955aff...4263b6c. Read the comment docs.

@danlipert danlipert marked this pull request as ready for review May 29, 2019 12:55
@danlipert danlipert changed the title Funder vs hunter personas in user profile [DO NOT MERGE] Funder vs hunter personas in user profile May 29, 2019
@danlipert danlipert force-pushed the funder-vs-hunter branch 3 times, most recently from 8b58493 to 0f8db6d Compare June 19, 2019 09:54
@danlipert danlipert requested review from frankchen07 and removed request for SaptakS June 19, 2019 10:17
@danlipert danlipert changed the title [DO NOT MERGE] Funder vs hunter personas in user profile Funder and hunter personas in user profile, sync personas to individual mailchimp lists Jun 19, 2019
Copy link
Contributor

@frankchen07 frankchen07 left a comment

Choose a reason for hiding this comment

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

Thanks for the hard work on this @danlipert.

  1. This is the statement that I would revise: The hunter and funder flags are only set to true for a given user if the SQL query lists the user as active.

I would modify this with the flags are set to true if a funder has ever funded an issue / performed a funder action, and a bounty hunter has ever done work on an issue / performed a bounty hunter action. If we set them to true only if they're active, we miss out on emailing bounty hunters and/or funders who have done something but not in the last 3 months. I'm assuming with how it stands now, "not active funder/bounty hunters" are labeled as users?

I've coded up that broader definition of designation here: https://metabase.gitcoin.co/question/423

  1. Because this runs on a cron job, I'm seeing the possible path that a user can only become bounty hunter or funder, but never can revert back to just "user" (if we use the logic in the metabase question above). That's also a point of contention if we use "active" set to true - that means bounty hunters and funders could potentially become "users" again if they fall inactive, which we don't want.

Lemme know if these make sense, and if you're able to access that Metabase question.

@danlipert
Copy link
Contributor Author

@frankchen07 Correct, this would only classify you as a given persona if you were active in the last three months. I will change this to use the updated metabase question so that bounty_hunter has persona_is_hunter set to True, and funder has persona_is_funder set to True. Once a given user is marked as a hunter or funder, then they won't later be have these persona flags removed.

@frankchen07
Copy link
Contributor

@danlipert - also, Alisa made a good point to make sure that the github handles are also in mailchimp, since when we send it out, we'll be using their handle to address them. I'm not sure if it's already in there, but the updated SQL query also has a handle listed.

@frankchen07
Copy link
Contributor

frankchen07 commented Jun 21, 2019

@danlipert - sorry caught one small change on metabase yesterday: https://metabase.gitcoin.co/question/423

line 120 - changed to db.web3_created created_on, instead of db.created_on,

Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

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

codewise lgtm but could you shed some info on the query ?
might be worth to document it in the file itself ?

@danlipert
Copy link
Contributor Author

@danlipert - also, Alisa made a good point to make sure that the github handles are also in mailchimp, since when we send it out, we'll be using their handle to address them. I'm not sure if it's already in there, but the updated SQL query also has a handle listed.

Isn't the github handle out of scope for this ticket? It can be added at some point (as a first name, or something else) but I want to make sure this change is in sync with the other related tickets, such as @octavioamu 's. We haven't been capturing this previously in mailchimp. @PixelantDesign @frankchen07

thelostone-mc
thelostone-mc previously approved these changes Jun 23, 2019
octavioamu
octavioamu previously approved these changes Jun 24, 2019
Copy link
Contributor

@octavioamu octavioamu left a comment

Choose a reason for hiding this comment

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

Tested locally working fine!!

thelostone-mc
thelostone-mc previously approved these changes Jun 24, 2019
@thelostone-mc thelostone-mc dismissed stale reviews from octavioamu and themself via 4263b6c June 24, 2019 16:57
@thelostone-mc thelostone-mc dismissed frankchen07’s stale review June 24, 2019 16:58

will get this in as separate PR if needed

@thelostone-mc thelostone-mc merged commit 3649843 into master Jun 24, 2019
@thelostone-mc thelostone-mc deleted the funder-vs-hunter branch July 4, 2019 14:53
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.

4 participants