Update btcdirect payment methods with fees #3171
Conversation
thisconnect
force-pushed
the
frontend-btcdirect-disclaimers
branch
from
451b028 to
f98d242
Compare
thisconnect
changed the title
thisconnect
force-pushed
the
frontend-btcdirect-disclaimers
branch
from
61d2afa to
24ce8e8
Compare
| const formattedCardFee = cardFee && cardFee * 100; | ||
| const formattedBankTransferFee = bankTransferFee && bankTransferFee * 100; | ||
| const formattedSofortFee = sofortFee && sofortFee * 100; | ||
| const formattedBancontactFee = bancontactFee && bancontactFee * 100; |
There was a problem hiding this comment.
this is not ideal, with 0.036 fee I get 3.5999999999999996%
probably better we do such math stuff in the backend 😇
There was a problem hiding this comment.
I think that we could store the fees in the backend deals as 3.6 instead of 0.036 so that we don't have to convert in the frontend. Would that be good enough?
There was a problem hiding this comment.
yes that makes sense. Looks to me that we do not use fee other than isBest calculation here
bitbox-wallet-app/backend/exchanges/exchanges.go
Lines 214 to 223 in bbdc3b8
change in last commit
There was a problem hiding this comment.
changed to percentages in last commit.
thisconnect
force-pushed
the
frontend-btcdirect-disclaimers
branch
2 times, most recently
from
11f91a8 to
2737571
Compare
|
@Beerosagos PTAL |
| @@ -45,21 +44,6 @@ type TProps = { | |||
| setInfo: (into: TInfoContentProps) => void; | |||
There was a problem hiding this comment.
done, fixedup into this commit
| case 'sofort': | ||
| case 'bancontact': | ||
| // hide these payment methods in the overview | ||
| return ''; |
There was a problem hiding this comment.
This is not really robust, imo. If the only available deals in a specific situation were the hidden ones, this would show a component without available options. It would be better to have a "hidden" field in the deal struct that can be set in the backend and properly checked in the frontend.
There was a problem hiding this comment.
I don't know sofort or bancontact (not available in CH), aren't these some kind of fiat banktransfers anyway?
Hidden field sounds more robust, but I am not 100% confident about how to do the backend changes... I'll dm you later.
There was a problem hiding this comment.
as discussed I'll leave in ths PR, and we can add isHidden in a new PR
thisconnect
force-pushed
the
frontend-btcdirect-disclaimers
branch
from
2737571 to
468a5ce
Compare
backend/exchanges/exchanges.go
Outdated
| // - Payment is the payment method offered in the deal (usually different payment methods bring different fees). | ||
| // - IsFast is usually associated with card payments. It is used by the frontend to display the `fast` tag in deals list. | ||
| type ExchangeDeal struct { | ||
| Fee float32 `json:"fee"` | ||
| Payment PaymentMethod `json:"payment"` | ||
| IsFast bool `json:"isFast"` | ||
| IsBest bool `json:"isBest"` | ||
| IsLocal bool `json:"isLocal"` |
There was a problem hiding this comment.
as discussed in dm, we keep the badges we have right now, but skip "Local payments" for now and rethink what badges it should show.
There was a problem hiding this comment.
done, @Beerosagos PTAL
thisconnect
force-pushed
the
frontend-btcdirect-disclaimers
branch
from
468a5ce to
941332d
Compare
| Payment: CardPayment, | ||
| IsFast: true, |
There was a problem hiding this comment.
I think we should keep the isFast here like for the other card payments (e.g. moonpay)
There was a problem hiding this comment.
ack. fixedup and force pushed.
There was a problem hiding this comment.
fixed, @Beerosagos PTAL should be all good now to merge this
| const cardFee = exchange.deals.find(feeDetail => feeDetail.payment === 'card')?.fee; | ||
| const bankTransferFee = exchange.deals.find(feeDetail => feeDetail.payment === 'bank-transfer')?.fee; | ||
| const bancontactFee = exchange.deals.find(feeDetail => feeDetail.payment === 'bancontact')?.fee; | ||
| const sofortFee = exchange.deals.find(feeDetail => feeDetail.payment === 'sofort')?.fee; |
There was a problem hiding this comment.
Another one, sorry: I think that this could scale better by refactoring the payment methods fees as a map.
There was a problem hiding this comment.
Not a blocker, but I think it could simplify the code a bit
There was a problem hiding this comment.
yeah this function just got a bit more ugly :)
thisconnect
force-pushed
the
frontend-btcdirect-disclaimers
branch
from
941332d to
fa3ae6b
Compare
Added a BTC Direct infobox that is shown when user clicks (i) button. Added an exchange option btcdirect-otc for the old OTC infobox that is shown for the BTC Direct OTC option. Also moved getBTCDirectLink to infocontent so we do not have circular dependencies/imports.
Added SOFORT and Bancontact payment options for BTC Direct. Also changed exchange fee to percentage: So that the frontend does not have to convert this number to percentage which can lead to JavaScript rounding errors such as displaying 3.5999999999999996% instead of 3.6%.
Adjusted fontsize ans spacing so the BTC Direct does not break onto 2 lines.
thisconnect
force-pushed
the
frontend-btcdirect-disclaimers
branch
from
fa3ae6b to
b24c9f9
Compare
No description provided.