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

Parsed Err msg #1094

Merged
merged 18 commits into from
Aug 14, 2018
Merged

Parsed Err msg #1094

merged 18 commits into from
Aug 14, 2018

Conversation

fedekunze
Copy link
Contributor

Closes #1071

Description:

Now errors are well parsed and shows correctly on delegation attempt.

❤️ Thank you!

@fedekunze fedekunze self-assigned this Aug 8, 2018
@fedekunze fedekunze requested review from okwme and faboweb August 8, 2018 11:20
@codecov
Copy link

codecov bot commented Aug 8, 2018

Codecov Report

Merging #1094 into develop will increase coverage by 0.01%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #1094      +/-   ##
===========================================
+ Coverage    96.05%   96.06%   +0.01%     
===========================================
  Files           81       81              
  Lines         1595     1600       +5     
  Branches        79       80       +1     
===========================================
+ Hits          1532     1537       +5     
  Misses          56       56              
  Partials         7        7
Impacted Files Coverage Δ
app/src/renderer/vuex/modules/send.js 100% <ø> (ø) ⬆️
app/src/renderer/vuex/modules/delegation.js 100% <ø> (ø) ⬆️
app/src/renderer/components/staking/PageBond.vue 100% <100%> (ø) ⬆️
app/src/renderer/connectors/lcdClientMock.js 97.45% <100%> (+0.02%) ⬆️

title: "Error While Bonding Atoms",
body: err.message
})
let errData = err.message.split("\n")[5]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will probably be used more often (like with sending coins). Can we implement this somehow in send.js like changing the error message there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to figure out which types of errors are encountered while transferring tokens

@faboweb faboweb changed the title Parsed Err msg and updated config file for 7005 Parsed Err msg Aug 8, 2018
voting_power: 20000,
shares: 75000,
description: "descriptionZ",
country: "Chile",
Copy link
Collaborator

Choose a reason for hiding this comment

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

🇨🇱

@okwme
Copy link
Contributor

okwme commented Aug 9, 2018

i updated the snapshots because the added lines looked like what i expect a revoked validator should look like. there might have also been some errors from the e2e tests yesterday that seem to be circle's fault

@fedekunze
Copy link
Contributor Author

@okwme Le'ts take a look into them today

@@ -270,7 +270,7 @@ describe("LCD Client Mock", () => {
it("executes a delegate tx", async () => {
let stake = await client.queryDelegation(
lcdClientMock.addresses[0],
lcdClientMock.validators[2]
lcdClientMock.validators[1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this changed?

Copy link
Contributor

Choose a reason for hiding this comment

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

because the validators[2] is the one with status revoked, so all the other tests get the revoked error before anything else happens

revoked: true
})

let candidates = await node.candidates()
Copy link
Collaborator

Choose a reason for hiding this comment

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

candidates are directly exported from lcdClientMock as validators, no need to query them like this. we can leave it like this, though.

wrapper.findAll("#btn-bond").trigger("click")
expect(wrapper.vm.$el.querySelector(".tm-form-msg--error")).not.toBeNull()
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't there appear an inline error rather then a notification?

@okwme okwme removed the need help label Aug 13, 2018
@@ -324,7 +324,9 @@ module.exports = {
candidate.delegator_shares = (
parseInt(candidate.delegator_shares) + amount
).toString()
storeTx("cosmos-sdk/MsgDelegate", tx)

// storeTx("cosmos-sdk/MsgDelegate", tx) // do we need this anymore?
Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, this stores the txs in the txs list to be able to show them in the transaction history

@okwme
Copy link
Contributor

okwme commented Aug 13, 2018 via email

@okwme okwme added the blocked ✋ issues blocked by other implementations/issues label Aug 14, 2018
@faboweb faboweb merged commit 4fec2aa into develop Aug 14, 2018
@faboweb faboweb deleted the fedekunze/1071-err-msgs branch August 14, 2018 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked ✋ issues blocked by other implementations/issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parse error msgs on delegation
4 participants