Web: bounty details page: Don't use pk= as a reference to the bounty in requests.

Created on 7 Sep 2018  ·  32Comments  ·  Source: gitcoinco/web

Describe the bug
Let's say you are on the bounty details page, you take a look at the issue and decide to submitting a 'start work' request. In the meantime, the funder increased the bounty payout or any action which forces an old_bounty > new_bounty transition / (new pk) on the backend.

You hit submit, everything is fine - gitcoinbot comments go out and all other notifications.
Now you refresh the page and see nothing in the activity list nor did your 'start work' request somehow show up. 💣 😕

Fix:
Update the frontend & backend to refer to the standard bounty id and network for the requests.
OR
Check for current_bounty in the request handlers and either automatically use the newest bounty object or throw an error.

@mbeacom

For Contributor Gitcoin Issue Detail

All 32 comments

@PixelantDesign Knows what happened 😄 .

Issue with this specific bug in action:
https://github.com/gitcoinco/web/issues/2164

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


__This issue now has a funding of 0.2 ETH (46.18 USD @ $230.92/ETH) attached to it.__

Hi @mul1sh and @Naggertooth....
Could you be more specific in your technical approach?

@mbeacom thoughts on this one?

@PixelantDesign sure.

My approach is this, after a user has expressed interest in a bounty by clicking [Start Work] then before updating the bounty with the users interest and enabling them to start work, i'll first check if another bounty exists in the database with the same issue url as the current bounty and if it does exist i'll then check the created timestamp to see which bounty is the newest and then use its issue url and created timestamp as the reference for the bounty.

Because these 2 fields grouped together are unique, this means the bounty in the issue details will always point to the newest updated version instead of the old version.

I believe this will be a better reference than the primary key.

@PixelantDesign, ofcourse.

As I said, I'll choose best one from 2 provided ideas for keep code uniformity in project.

Update the frontend & backend to refer to the standard bounty id and network for the requests. OR Check for current_bounty in the request handlers and either automatically use the newest bounty object or throw an error.

I belive that when you hit submit - there would be a request with all data for an issue, and all I have to do is choose the best solution.
If more effective would be - changing the data on backend, when funder increased the bounty payout or any action which forces an oldbounty > newbounty transition / (new pk) on the backend. I'll choose my solution.

I suggest using standard_bounties_id and network for the api requests/endpoints.
Also, only modify bounty objects where .current_bounty == True

@pinkiebell Thank you for suggestion :)
It'll help a lot!

@Naggertooth please work with @pinkiebell to push to completion!

@PixelantDesign okay, thank you!
I'm expecting to start work on Friday.

@Naggertooth Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • [x] warning (3 days)
  • [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

Hi, guys
I'm finally home and starting work on this
Sorry for delay
I was forced to go to Crimea

@pinkiebell can you tell me please: where should I find any action which forces an old_bounty > new_bounty transition / (new pk) on the backend?
I can't find it in the UI on issue editor page and any saving in administrator panel don't imitate new pk.
So I can't even repeat the problem :)

@Naggertooth
For example in: https://github.com/gitcoinco/web/blob/master/app/dashboard/helpers.py#L653 .

Our endpoints in question are in app/dashboard/views.py.
Also check app/app/urls.py.

You could use `handle_bounty_views' to lookup the bounty.

In get_bounty_view_kwargs:

standard_bounties_id = request.GET.get('sb_id') or request.GET.get('standard_bounties_id')
...
network = request.GET.get('network', 'mainnet')

Instead of passing pk= in the requests to the backend, use that above ^ and change the endpoints to use handle_bounty_views to lookup the bounty from the query string.
Also you need to change all js code in app/assets/v2/js/* to use the standard_bounties_id and network instead of bounty's pk.
👍

@pinkiebell thank you!
But do you think that I don't need to repeat the problem locally at all?

I mean you didn't answer my question.
If I can test it somehow - I need to know how.

@Naggertooth
Follow these steps:

  1. Fund a new bounty on rinkeby
  2. Use sync_geth command to sync the bounties on rinkeby
  3. In another private browser window, login with a different account on your local instance and visit the bounty details page.
  4. Now increase the funding of your bounty.
  5. Use sync_geth again, make sure it picked up these changes.
  6. Apply to start work on the browser window from step 3

And now we have that problem I mentioned, the private window from step 3 has the old pk of the bounty and our endpoint(s) will update the old bounty revision not the new one.

@pinkiebell
I'm asking about step 4.
Where should I find any action which forces an old_bounty > new_bounty transition / (new pk) on the backend?

@Naggertooth
If you mean the frontend part: On the issue detail page, the contribute button.
You can also just grep through the codebase for old_bounty

@pinkiebell
Oh
I finally can see the bug! :)
Thank you

@Naggertooth Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • [x] warning (3 days)
  • [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@Naggertooth Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • [x] warning (3 days)
  • [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

Hi, @pinkiebell
While I was working on it I found that:

Gitcoin creates new issue and pk in database with the same id
after funder deside to give additional bounty to the issue.
And there is no field for performer in bounty database before submiting work,
so when he trying to submit work - he doesn't bound to new issue.

New issue refers to same standart_bounty_id and it has current_bounty = true, any old issues has current_bounty = false and error 401 occurs before using new bounty object.

web_1      | ERROR:dashboard.helpers:401 {'message': 'Bad credentials', 'documentation_url': 'https://developer.github.com/v3'}
web_1      | - processing changes, 31 => 32
web_1      | - increased_bounty event; diff => {'balance': 50000000000000000, 'fulfillmentAmount': 50000000000000000}
web_1      | ============ posting ==============
web_1      | ============ done posting ==============

I see this or similar error in debugger very often

web_1      | ERROR:dashboard.helpers:401 {'message': 'Bad credentials', 'documentation_url': 'https://developer.github.com/v3'}

I want to ask: Is it right that contributing creates new pk in database?
And should I change this functional for mutating the old issue?

Because if I shoudn't - I can get why I have 401 error from git and it's much bigger problem.


PS
When I tap "Approve worker" button - I getting POST part in my url string.
And reloading this page creates another approve action.

@Naggertooth
The errors might be from a missing/wrong github access token (https://github.com/gitcoinco/web/blob/master/docs/ENVIRONMENT_VARIABLES.md).

I want to ask: Is it right that contributing creates new pk in database?

Yes

And should I change this functional for mutating the old issue?

Nope

When I tap "Approve worker" button - I getting POST part in my url string.
And reloading this page creates another approve action.

Thats a task for another time 👍

@pinkiebell
Thank you.

The errors might be from a missing/wrong github access token
I know. I was watching around this and I thought to make an exception for this call.
But there is a lot of similar errors and it can make errors afterwards. So I stopped.

I mean: When I fund an issue by a different account -
gitcoin tryes to access to performer account too. That's strange.

@Naggertooth Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • [x] warning (3 days)
  • [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@Naggertooth Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • [x] warning (3 days)
  • [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@Naggertooth
What's your status?

@pinkiebell I stopped.

@Naggertooth
Oh, I see. I totally missed that ^>^

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


__Work has been started__.

These users each claimed they can complete the work by 2 years, 3 months ago.
Please review their action plans below:

1) pinkiebell has been approved to start work.

I will refactor some endpoints + fronted and get this done today :)

Learn more on the Gitcoin Issue Details page.

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


__Work for 0.2 ETH (40.94 USD @ $204.68/ETH) has been submitted by__:

  1. @pinkiebell

@PixelantDesign please take a look at the submitted work:

  • PR by @pinkiebell

@PixelantDesign can we pay out pinkie at least a bit? since he did much work on this and it never got merged. :-(

cc @thelostone-mc

Was this page helpful?
0 / 5 - 0 ratings

Related issues

thelostone-mc picture thelostone-mc  ·  4Comments

kziemianek picture kziemianek  ·  3Comments

abitrolly picture abitrolly  ·  4Comments

sethmcleod picture sethmcleod  ·  4Comments

uluhonolulu picture uluhonolulu  ·  3Comments