Skip to content
This repository has been archived by the owner on Jun 29, 2020. It is now read-only.

Improve error handling in frontend #60

Merged
merged 6 commits into from
Aug 3, 2018
Merged

Improve error handling in frontend #60

merged 6 commits into from
Aug 3, 2018

Conversation

garatortiz
Copy link
Contributor

Closes #36

@garatortiz garatortiz added the WIP Please, don't merge label Jul 26, 2018
@ghost ghost assigned garatortiz Jul 26, 2018
@ghost ghost added the in progress label Jul 26, 2018
@garatortiz
Copy link
Contributor Author

@fvictorio @phahulin Please, if you can review it, and comment if you have any ideas.

@coveralls
Copy link

coveralls commented Jul 26, 2018

Pull Request Test Coverage Report for Build 158

  • 0 of 4 (0.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.5%) to 54.441%

Changes Missing Coverage Covered Lines Changed/Added Lines %
frontend/src/alerts.js 0 1 0.0%
frontend/src/PoBA.js 0 3 0.0%
Totals Coverage Status
Change from base Build 151: -0.5%
Covered Lines: 162
Relevant Lines: 266

💛 - Coveralls

@fvictorio
Copy link
Contributor

We should also emit a proper error if the contract is not deployed in the current network. I think this error will be thrown by the PobaContract.deployed() function.

@garatortiz garatortiz removed the WIP Please, don't merge label Jul 30, 2018
@garatortiz garatortiz removed the request for review from fvictorio July 31, 2018 14:38
this.setState({ registeredAccounts })
this.setState({ registeredAccounts })
} catch (e) {
console.error('There was a problem deploying the contract', e)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's change the error message to something like "Contract is not deployed on this network".


const registeredAcountsCount = await this.pobaContract.accountsLength.call(account)
const registeredAcountsCount = await this.pobaContract.accountsLength.call(account)
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a typo in variable name, should be registeredAccountsCount

@phahulin phahulin self-requested a review August 3, 2018 13:40
@garatortiz garatortiz merged commit 2a6b6f5 into master Aug 3, 2018
@ghost ghost removed the in progress label Aug 3, 2018
@garatortiz garatortiz deleted the issue-#36 branch August 3, 2018 18:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants