Ana/send claim rewards amount to extension #3736

Bitcoinera · 2020-03-19T12:49:40Z

Closes #ISSUE

Description:

The TransactionData being returned from ModalWithdrawRewards was incomplete and the getter for stakingDenom there was broken too.

Includes a hack I'll need to refactor to include a last message in Claim rewards with the amount (for extension).

This is a work in progress.

Thank you! 🚀

For contributor:

Added changes entries. Run yarn changelog for a guided process.
Reviewed Files changed in the github PR explorer
Attach screenshots of the UI components on the PR description (if applicable)
Scope of work approved for big PRs

For reviewer:

Manually tested the changes on the UI

Bitcoinera · 2020-03-19T14:14:32Z

Another possibility would be, instead of adding amount as the last message in the signMessage for extension, to add it in every message together with validators, like this:

(lines 103-109)

          txMessage = {
            ...txMessage,
            value: {
              ...txMessage.value,
              amount: transactionProperties.amounts
            }
          }

But there is not much difference and I think it is better to add it as the last one

codecov · 2020-03-19T14:23:01Z

Codecov Report

Merging #3736 into develop will increase coverage by 0.19%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #3736      +/-   ##
===========================================
+ Coverage    86.88%   87.08%   +0.19%     
===========================================
  Files          125      125              
  Lines         2669     2671       +2     
  Branches       395      396       +1     
===========================================
+ Hits          2319     2326       +7     
+ Misses         330      325       -5     
  Partials        20       20

Impacted Files	Coverage Δ
src/scripts/extension-utils.js	`96.55% <ø> (ø)`
src/ActionModal/components/ActionModal.vue	`96.05% <100.00%> (+0.01%)`	⬆️
...rc/ActionModal/components/ModalWithdrawRewards.vue	`97.29% <100.00%> (+13.96%)`	⬆️
src/ActionModal/utils/ActionManager.js	`100.00% <100.00%> (ø)`
src/ActionModal/utils/signer.js	`94.59% <100.00%> (ø)`

Bitcoinera · 2020-03-19T14:23:13Z

It is actually working quite well

faboweb · 2020-03-19T16:19:43Z

src/ActionModal/utils/ActionManager.js

+        type: "/".concat(type),
+        value: { amount: transactionProperties.amounts }
+      }
+      txMessages.push(claimAmountMessage)


lol I get it now. very hacky... ^^
you remember that it is a bad idea to mix data types. same goes for adding pseudo messages to an actual message array.

Proposal:

extend getSigner in signer.js to return functions of the format (signMessage, metaData) => ... where metaData = { claimableRewards } then pass metaData to signWithExtension and send it to the extension. The extension now reads this prop and uses it to display this information.

OK, so then I don't throw this into the messages... it feels weird to call this a metaData though. Especially when there is already a txMetaData already dancing around in ActionManager (can lead to confusion)

yeah this whole thing needs to be heavily refactored. you can call it anything else. how about displayingData.

Well, I will added it right there then. To txMetadata

What about #3736 (comment)?

EDIT: doesn't work either

Bitcoinera · 2020-03-19T18:34:00Z

Oops, you were right. The blockchain is not liking my message xD

I was wrong. It never worked

faboweb · 2020-03-19T19:51:18Z

Looks better! Why are e2e failing?

Bitcoinera · 2020-03-19T21:45:07Z

Looks better! Why are e2e failing?

Some problem with fetching from browserstack

EDIT: Let's merge 💃 !

src/ActionModal/components/ActionModal.vue

jbibla · 2020-03-20T13:18:29Z

src/ActionModal/utils/ActionManager.js

+      submitType,
+      password,
+      displayedProperties
+    } = txMetaData


Co-Authored-By: Jordan Bibla <[email protected]>

faboweb · 2020-03-20T13:41:20Z

src/ActionModal/components/ActionModal.vue

+        displayedProperties:
+          this.title === "Claim Rewards" ? properties.amounts : null
+      }


Suggested change

displayedProperties:

this.title === "Claim Rewards" ? properties.amounts : null

}

displayedProperties: {

amounts: properties.amounts

}

}

…github.com/luniehq/lunie into ana/send-claim-rewards-amount-to-extension

Bitcoinera · 2020-03-20T13:58:16Z

Give me a bit and I also implement this

faboweb · 2020-03-20T19:43:55Z

src/ActionModal/components/ModalWithdrawRewards.vue

@@ -60,8 +60,15 @@ export default {
  computed: {
    ...mapGetters([`address`, `network`, `stakingDenom`]),
    transactionData() {
+      if (!this.claimedReward) return {}


what is this for? will this break if claimedReward is not there?

I just copied from the other components 🤷‍♀

Yes, it would break but it would in any case. I can just get rid of it

By the way, we should merge this now that luniehq/lunie-browser-extension#179 is in, since otherwise it will break sowieso with the first claim rewards transaction

* changelog * preload network capabilities * tests fixed * add slug to store and mutations * lunie version changed * Aleksei/fixing path on extension (#173) * changelog * fixing redirect path after account creation in extension * refactor isExtension getter using lunie config Co-authored-by: Bitcoinera <[email protected]> * correct submodule version * added missing webpack resolution * force checkout * change to working commit * make updates async * remove package lock again * use working commit * first steps to parsing to v2 txs * clone parsing from API * linted * move parsetx to store * delete consolelog * fix tests * fix actions tests * lint * just in case add ledger_app like in fe * working with #3736 in fe * stay clean * fix validators names in claim rewards * fix tests * lint * working with #3736 now for real * add claimable rewards * lint. kill now unused functions * changelog * refactor property names * linted Co-authored-by: Aleksei Rudometov <[email protected]> Co-authored-by: Bitcoinera <[email protected]>

send claim rewards amount to extension

7aace73

Bitcoinera requested review from faboweb and jbibla as code owners March 19, 2020 12:49

Bitcoinera added 2 commits March 19, 2020 15:19

fix test

2316662

changelog

558b7fc

Bitcoinera changed the title ~~WIP Ana/send claim rewards amount to extension~~ Ana/send claim rewards amount to extension Mar 19, 2020

faboweb reviewed Mar 19, 2020

View reviewed changes

send claim amount as metadata displayed properties

315be90

jbibla reviewed Mar 20, 2020

View reviewed changes

src/ActionModal/components/ActionModal.vue Outdated Show resolved Hide resolved

jbibla reviewed Mar 20, 2020

View reviewed changes

src/ActionModal/components/ActionModal.vue Outdated Show resolved Hide resolved

jbibla reviewed Mar 20, 2020

View reviewed changes

src/ActionModal/utils/ActionManager.js

submitType,

password,

displayedProperties

} = txMetaData

Copy link

Collaborator

jbibla Mar 20, 2020

Choose a reason for hiding this comment

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

😉

Apply suggestions from code review

7384134

Co-Authored-By: Jordan Bibla <[email protected]>

faboweb reviewed Mar 20, 2020

View reviewed changes

Bitcoinera added 2 commits March 20, 2020 14:56

apply suggestion for displayed properties

aed75fd

Merge branch 'ana/send-claim-rewards-amount-to-extension' of https://…

5d4d43f

…github.com/luniehq/lunie into ana/send-claim-rewards-amount-to-extension

apply suggestion add claimablerewards to displayed

3dc2c00

faboweb reviewed Mar 20, 2020

View reviewed changes

faboweb merged commit 509a0a1 into develop Mar 20, 2020

faboweb deleted the ana/send-claim-rewards-amount-to-extension branch March 20, 2020 23:39

faboweb mentioned this pull request Mar 20, 2020

[Simsala] automatic release created for v1.0.178 #3746

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ana/send claim rewards amount to extension #3736

Ana/send claim rewards amount to extension #3736

Bitcoinera commented Mar 19, 2020

Bitcoinera commented Mar 19, 2020

codecov bot commented Mar 19, 2020 •

edited

Loading

Bitcoinera commented Mar 19, 2020

faboweb Mar 19, 2020

faboweb Mar 19, 2020

Bitcoinera Mar 19, 2020

faboweb Mar 19, 2020

Bitcoinera Mar 19, 2020

Bitcoinera Mar 19, 2020 •

edited

Loading

Bitcoinera commented Mar 19, 2020

faboweb commented Mar 19, 2020

Bitcoinera commented Mar 19, 2020 •

edited

Loading

jbibla Mar 20, 2020

faboweb Mar 20, 2020

Bitcoinera commented Mar 20, 2020

faboweb Mar 20, 2020

Bitcoinera Mar 20, 2020

Ana/send claim rewards amount to extension #3736

Ana/send claim rewards amount to extension #3736

Conversation

Bitcoinera commented Mar 19, 2020

Bitcoinera commented Mar 19, 2020

codecov bot commented Mar 19, 2020 • edited Loading

Codecov Report

Bitcoinera commented Mar 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bitcoinera Mar 19, 2020 • edited Loading

Choose a reason for hiding this comment

Bitcoinera commented Mar 19, 2020

faboweb commented Mar 19, 2020

Bitcoinera commented Mar 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bitcoinera commented Mar 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 19, 2020 •

edited

Loading

Bitcoinera Mar 19, 2020 •

edited

Loading

Bitcoinera commented Mar 19, 2020 •

edited

Loading