ensure bounty is open when advanced payout is done using tips

[thelostone-mc]

Description

As of now -> if you use advanced payout using wallet ,
the UI renders the status as DONE which is confusing despite the bounty being open.

• This happens on the explorer
• Bounty Page (preventing users from performing contributor/funder actions which should be possible on the original bounty )

AFTER THIS CHANGE

[codecov]

Codecov Report

Merging #5447 into master will increase coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #5447      +/-   ##
==========================================
+ Coverage   29.13%   29.16%   +0.02%     
==========================================
  Files         242      242              
  Lines       20900    20900              
  Branches     3021     3021              
==========================================
+ Hits         6090     6096       +6     
+ Misses      14565    14559       -6     
  Partials      245      245

Impacted Files	Coverage Δ
app/dashboard/models.py	50.77% <0%> (ø)	⬆️
app/dashboard/embed.py	31.6% <0%> (+3.44%)	⬆️

---

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 2be3d75...d3285b7. Read the comment docs.

[danlipert]

Seems good to me - I think the previous logic, while it may have worked, was not very clear in regards to the intent. Is there any edge case we can think of where a bounty has a tip sent and we want to mark it done, where advanced payout was not used? Otherwise I think we're good - nice fix!

[owocki]

this will create a circular logic where the bounty.status is toggled back and forth between done and cancelled, wont it? the bounty.status will be marked done then cancelled and back and forth forever.

[owocki]

note dashboard.models:psave_bounty where

instance.idx_status = instance.status

is set

[thelostone-mc]

this will create a circular logic where the bounty.status is toggled back and forth between done and cancelled, won't it ?

Could you give an example?

bounty.status depends on the bounty.idx_status + business logic to decide the value.
If the idx_status is done -> the status would be set to done via https://github.com/gitcoinco/web/pull/5447/files#diff-0d2d86c824ef942c5a3a01c83ddfe758R728 right ?

Another solution which we could do make it even better:

and self.idx_status in ['cancelled'] and not self.is_open

[owocki]

im not sure how to give you an example; except to say this: instance.idx_status is set by instance.status, and then instance.idx_status is set by instance.idx_status. (circularly)

see line 1669 of dashboard/models.py to see where idx_status is set to status

this is circular dependancy and will cause the idx_status to toggle back and forth between the two statuses each time idx_status is computed.

[owocki]

here

In [11]: from dashboard.models import Bounty
    ...: bounty = Bounty.objects.last()
    ...: bounty.idx_status = 'cancelled'
    ...: bounty.save()
    ...: print(bounty.idx_status)
    ...: bounty.save()
    ...: print(bounty.idx_status)
    ...:
done
open

In [12]: from dashboard.models import Bounty
    ...: bounty = Bounty.objects.last()
    ...: bounty.idx_status = 'cancelled'
    ...: bounty.save()
    ...: print(bounty.idx_status)
    ...: bounty.save()
    ...: print(bounty.idx_status)
    ...:
done
open

[owocki]

what happened to the previous architecture we talked about on zoom on friday? where you just add a param called .has_staked_funds and toggle showing the payout based upon that, not based upon status..

[thelostone-mc]

@owocki So thinking over it adding a new has_staked_funds
( wouldn't this again depend on the idx_status ? )
I felt would make our codebase confusing.
Our logic as it stands for bounty.status is broken which is what I attempted to fix

I wanted to write up the logic to keep things uniform as we use status to render bounty status in UI

===

and toggle showing the payout based upon that

Just to clarify It would not be just payout but instead al funder/contributor actions

---

Is there any edge case we can think of where a bounty has a tip sent and we want to mark it done, where advanced payout was not used

@danlipert I could not think of one 🤔
The only way to use tip against a bounty on gitcoin would be via advanced payout right?
If funder uses tips independently -> I don't think it even shows up in the activity (not sure though )

[owocki]

hey guys;

1. see my comment about the circular logic in the save field. i think thats objectively a problem.
2. @thelostone-mc i typed this up this comment just now and i almost threw it away because i fundamentally believe that i am moving away from coding this part of the app, and that we should build a culture where i can argue from authority in code review.. but then i thought "nah; i think i have something to add here as the former maintainer of this part of the code, and i trust the team to have the freedom to not agree with me"

anyway, that aside... here are my thoughts on this:

> I felt would make our codebase confusing.
> Our logic as it stands for bounty.status is broken which is what I attempted to fix

IMHO - it's not *broken* its just being used for too many things that it was never intended to be used for.  thats why we should break the logic of the app that depends on status into the component atomic parts of status to solve this; not duct tape in more special business logic into one field IMHO.

i dont think breaking out the status field from one computed field into *many inputs* (and exposing them) makes the codebase more confusing..  i think its more logically transparent than relying on the status field for everyone.  the only drawback is that you have to take the care to thread it all the way from the model to the API to the JS (and possible wrangle some fugly JS along the way)

another way to solve this would be to break the status computation workflow from

instance.status

to 

instance.onchain_status
=> 
instance.computed_status (and maybe if were feeling super explcit, instance.computed_status_reason)
=>
instance.final_status