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

[WOC] PR@2764: Update corner case canceling started issue #2811

Merged
merged 5 commits into from
Jun 28, 2019
Merged

[WOC] PR@2764: Update corner case canceling started issue #2811

merged 5 commits into from
Jun 28, 2019

Conversation

kuhnchris
Copy link
Contributor

Description

Additional check for open == false but other things being true to show additional info message.

Checklist
  • test
Affected core subsystem(s)

UI

Testing

Cannot test due to broken pyvips :-(

Refers/Fixes

Fixes: #2764

Copy link
Contributor

@mbeacom mbeacom left a comment

Choose a reason for hiding this comment

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

Should we update the BE to not allow this as well..?

app/assets/v2/js/pages/kill_bounty.js Outdated Show resolved Hide resolved
@mbeacom mbeacom added waiting on contributor frontend This needs frontend expertise. labels Nov 14, 2018
@kuhnchris
Copy link
Contributor Author

tbh I think we gotta have to do a backend audit in the near future anyways, as the platform grows we are not allowed to get screwed over by beginner errors, I think. Prolly gotta talk to Kevin and set up something so we can put that into scope.

@codecov
Copy link

codecov bot commented Nov 14, 2018

Codecov Report

Merging #2811 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2811      +/-   ##
==========================================
- Coverage   30.41%   30.38%   -0.04%     
==========================================
  Files         216      216              
  Lines       17223    17223              
  Branches     2332     2332              
==========================================
- Hits         5239     5233       -6     
- Misses      11776    11782       +6     
  Partials      208      208
Impacted Files Coverage Δ
app/dashboard/embed.py 28.16% <0%> (-3.45%) ⬇️

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 3067cbf...ef9626a. Read the comment docs.

@mbeacom
Copy link
Contributor

mbeacom commented Nov 19, 2018

@kuhnchris I was hoping to include the restriction on the BE as well, but if you're opposed to adding it right now, I guess we can merge it and I'll try to tackle it at some point, time permitting.

@kuhnchris
Copy link
Contributor Author

Your decision, that PR at least fixes the front-end side (for now).

@thelostone-mc
Copy link
Member

@kuhnchris would be super neat if we could throw it into the BE if you are up for it ? ^_^

@mbeacom mbeacom requested a review from octavioamu November 28, 2018 19:57
@mbeacom mbeacom changed the title PR@2764: Update corner case canceling started issue [WOC] PR@2764: Update corner case canceling started issue Nov 28, 2018
@mbeacom
Copy link
Contributor

mbeacom commented Nov 28, 2018

Anything you need from us here? @kuhnchris

@kuhnchris
Copy link
Contributor Author

ah pardon me, didn't see the reply. want me to implement that check on backend side aswell (just that check) or want me to create a build-ticket to check this in general?

@thelostone-mc
Copy link
Member

@kuhnchris would be ideal :P or do you just wanna resolve the conflicts and roll with this ?

@kuhnchris
Copy link
Contributor Author

I do not mind either way. If the official statement is "fix be aswell" then I'll fix it, we just do not want to let this fall through the roof.

@kuhnchris
Copy link
Contributor Author

@thelostone-mc you still know if this is still out there and causing issues? else I'd suggest we throw this away! Thanks!

@thelostone-mc thelostone-mc requested review from danlipert and removed request for SaptakS June 23, 2019 19:02
@danlipert danlipert merged commit 302f61a into gitcoinco:master Jun 28, 2019
if (fromAddress != web3.eth.coinbase) {
errormsg =
gettext('Only the address that submitted this funded issue may kill the bounty.');
if (bountyAmount > 0 && !is_open) {
Copy link
Contributor

Choose a reason for hiding this comment

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

parseInt(0.09, 10)>0 will be false
0.09 > 0 will be true (what we want)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend This needs frontend expertise.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Canceling "started" bounties shows wrong error message
5 participants