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

"Send funds back" feature needs to pass "From" account in the route state #451

Open
keerifox opened this issue Jul 23, 2021 · 13 comments
Open
Labels
enhancement New feature or request

Comments

@keerifox
Copy link
Member

at the moment there is a very slight chance that the selected FROM account is different so it'd be good to make sure it's the one that the funds were originally sent to

@keerifox keerifox added the enhancement New feature or request label Jul 23, 2021
@andrew-m-higgs
Copy link
Contributor

Hi there

Where is the 'Send funds back' feature? I can't seem to find it when viewing a transaction from the accounts detail page which is where I would expect to see it.

Thanks

@keerifox
Copy link
Member Author

keerifox commented Oct 4, 2021

Where is the 'Send funds back' feature? I can't seem to find it when viewing a transaction from the accounts detail page which is where I would expect to see it.

it's in the narrow (mobile) layout when clicking on a Receive type transaction

@andrew-m-higgs
Copy link
Contributor

Where is the 'Send funds back' feature? I can't seem to find it when viewing a transaction from the accounts detail page which is where I would expect to see it.

it's in the narrow (mobile) layout when clicking on a Receive type transaction

Is there a way to up the odds of it being incorrect? All the transactions I tried showed the correct return account.

@keerifox
Copy link
Member Author

Is there a way to up the odds of it being incorrect? All the transactions I tried showed the correct return account.

by changing active account in the sidebar

@andrew-m-higgs
Copy link
Contributor

Okay. I see it now.
Two questions:

  1. Why is this only available to small screens?
  2. Is there a reason why Send does not GET the from address and just uses the selected account? I see if no account is selected (ie Total Balance) the correct address is used.

@keerifox
Copy link
Member Author

  1. because it's not a feature implemented in the desktop view layout
  2. i'm not quite sure what you're referring to, the title of this issue has everything that needs to be done

@andrew-m-higgs
Copy link
Contributor

2. i'm not quite sure what you're referring to, the title of this issue has everything that needs to be done

I understand what needs to be done. I am trying to understand what would be the best way to achieve it.

I mean that the method used to pass the amount and the to address is GET (ie in the link). But the 'Send' page does not support receiving the from address in this way. The 'Send' page uses the from address only when an account is not already selected (Total Balance). If an account is selected it uses that account for the from address.

So, the way I see it is, we need to include the from address in the GET request so it can be used on the send page? Otherwise we need another way to know that this is a 'Return Funds' send.

@andrew-m-higgs
Copy link
Contributor

Sorry. I have just re-read the title and for some or other reason understand it differently now. Not sure why I didn't understand it the first time.

I think we are talking about the same thing. We need to add the from address to the URL taking us to the 'Send' page? Is that correct?

@keerifox
Copy link
Member Author

not sure how up-to-date this article is, but route state works something like this: https://netbasal.com/set-state-object-when-navigating-in-angular-7-2-b87c5b977bb

it's not something that user can set in the url but it can contain data from a different part of the application

@keerifox
Copy link
Member Author

keerifox commented Oct 15, 2021

here's an example in existing nault code

this.router.navigate(['sweeper'], { state: { seed: seed } });

if (this.route.getCurrentNavigation().extras.state && this.route.getCurrentNavigation().extras.state.seed) {
this.sourceWallet = this.route.getCurrentNavigation().extras.state.seed;
this.validSeed = true;
}

@andrew-m-higgs
Copy link
Contributor

Can we not just add the from to the URL as the to and amount are:

if (params && params.to) {
this.toAccountID = params.to;
this.validateDestination();
this.sendDestinationType = 'external-address';
}

Might make it easier for anyone wanting to create their own links to make payments from a certain account?

@keerifox
Copy link
Member Author

i don't remember what were the reasons for preferring state over query params, i guess it might be undesired to give a third party an ability to override default selection of "from" account if they know one of the addresses in your wallet? it makes sense to support "to" and "amount" for the payment details, but "from" address likely should not be settable via url

@andrew-m-higgs
Copy link
Contributor

If I were trying to steal funds from people I would be much happier to be able to set the TO address rather than the FROM address. Or am I missing something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants