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

fix for https://github.com/gitcoinco/web/issues/1661 #1662

Merged
merged 6 commits into from
Jul 9, 2018

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Jul 9, 2018

fix for #1661

  1. clears ALL activity if force_refresh, not just for current_bounty
  2. fixes the attribution of worker_approved, work_applied, etc to be for the funder, not the bounty hunter.

TODOS:

  1. why are we making two requests

http://localhost:8000/actions/api/v0.1/bounties/?github_url=https://github.com/XLNT/gnarly/issues/11&network=mainnet&standard_bounties_id=677&not_current=1
and
http://localhost:8000/actions/api/v0.1/bounties/?github_url=https://github.com/XLNT/gnarly/issues/11&network=mainnet&standard_bounties_id=677

??

i think we should only care about activites related to teh current bounty and we should migrate the actions to the current_bounty=1 when we create a new current_bounty

@ghost ghost assigned owocki Jul 9, 2018
@ghost ghost added the in progress label Jul 9, 2018
@owocki owocki mentioned this pull request Jul 9, 2018
@@ -71,9 +71,9 @@ def add_arguments(self, parser):

def handle(self, *args, **options):
force_refresh = options['force_refresh']
if force_refresh:
Activity.objects.all().delete()
Copy link
Member

Choose a reason for hiding this comment

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

👍

user_profile = Profile.objects.filter(handle__iexact=fulfillment.fulfiller_github_username).first()
if not user_profile:
user_profile = sync_profile(fulfillment.fulfiller_github_username)
funder_actions = ['new_bounty', 'worker_approved', 'killed_bounty', 'increased_bounty', 'worker_rejected']
Copy link
Member

@thelostone-mc thelostone-mc Jul 9, 2018

Choose a reason for hiding this comment

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

should we also cover start / stop work here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

those are not funder actions.. those are fulfiller actions, no?

Copy link
Member

Choose a reason for hiding this comment

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

Derp! 👍

@codecov
Copy link

codecov bot commented Jul 9, 2018

Codecov Report

Merging #1662 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1662      +/-   ##
==========================================
- Coverage   29.16%   29.15%   -0.02%     
==========================================
  Files         134      134              
  Lines       10056    10060       +4     
  Branches     1317     1319       +2     
==========================================
  Hits         2933     2933              
- Misses       7018     7022       +4     
  Partials      105      105
Impacted Files Coverage Δ
app/dashboard/models.py 56.13% <ø> (ø) ⬆️
app/dashboard/helpers.py 25.79% <0%> (-0.31%) ⬇️
...ard/management/commands/create_activity_records.py 0% <0%> (ø) ⬆️

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 e79f961...c019aca. Read the comment docs.

for interest in latest_old_bounty.interested.all():
new_bounty.interested.add(interest)

# pull the activities off the last old bounty

Choose a reason for hiding this comment

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

W291 trailing whitespace

# pull the activities off the last old bounty
for activity in latest_old_bounty.activities.all():
new_bounty.activities.add(activity)

Choose a reason for hiding this comment

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

W293 blank line contains whitespace

@owocki
Copy link
Contributor Author

owocki commented Jul 9, 2018

did some local testing.. merging soon

thelostone-mc
thelostone-mc previously approved these changes Jul 9, 2018
@owocki owocki merged commit c019aca into master Jul 9, 2018
@ghost ghost removed the in progress label Jul 9, 2018
@SaptakS
Copy link
Contributor

SaptakS commented Jul 9, 2018

Looked good to me. :)

@thelostone-mc thelostone-mc deleted the kevin/fix_hotfix branch July 9, 2018 17:15
@owocki
Copy link
Contributor Author

owocki commented Jul 9, 2018

sorry to jump in front of you for triage @SaptakS @thelostone-mc -- i just couldnt let this stand in prod :P

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