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

(Feature) Reorder contract executors: owners are first #239

Merged
merged 12 commits into from
Dec 29, 2018

Conversation

vbaranov
Copy link
Collaborator

@vbaranov vbaranov commented Dec 25, 2018

If an executed contract has owner or getOwners getters, and addresses from the resulted array are in the list of wallet accounts, they will be first in the list in the Choose contract executor screen.

@dennis00010011b
Copy link

dennis00010011b commented Dec 26, 2018

@vbaranov
Uncaught error in Chrome's console when attempt to execute method transfer from contract 0x215b2ab35749e5a9f3efe890de602fb9844e842f, Sokol

Input data:

  • address 0xd7b7AFeCa35e32594e29504771aC847E2a803742
  • value 1

screen shot 2018-12-26 at 05 54 43

As a result NW can't execute method transfer

@vbaranov
Copy link
Collaborator Author

@dennis00010011b

Uncaught error in Chrome's console when attempt to execute method transfer from contract 0x215b2ab35749e5a9f3efe890de602fb9844e842f, Sokol

Fixed 291089d

@vbaranov
Copy link
Collaborator Author

@dennis00010011b could you please resolve merging conflicts in e2e tests?

@@ -113,6 +152,47 @@ class ChooseContractExecutor extends Component {
})
}

getAllOwners = () => {
this.props.showLoadingIndication()

Choose a reason for hiding this comment

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

May it be possible to add a catch to the promise so you can call hideLoadingIndicator if for any reason the Promise.all fails?

or you can refactor to use async/awaits? whatever fits best.

Copy link
Collaborator Author

@vbaranov vbaranov Dec 28, 2018

Choose a reason for hiding this comment

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

Thanks @fernandomg! Catch was added.

@vbaranov vbaranov merged commit 9359e12 into develop Dec 29, 2018
@ghost ghost removed the awaiting for review label Dec 29, 2018
@vbaranov vbaranov deleted the contract-executors-reorder branch December 29, 2018 11:24
@vbaranov vbaranov mentioned this pull request Dec 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants