Add Value Criteria For Payment Method #1871

xpayserver · 2020-08-27T19:02:00Z

I upgrade lightning max/bitcoin min to support better control in store. Now can have setting only enable specific payment method only if value high/low I think make code simple more too and backward compatible with old feature

Kukks

LGTM, only small nits from my end and glad to be getting rid of that extra method on the payment method handler implementations!

Kukks · 2020-09-10T13:00:17Z

BTCPayServer/Controllers/InvoiceController.cs

-                    currencyPairsToFetch.Add(new CurrencyPair(network.CryptoCode, storeBlob.OnChainMinValue.Currency));
+                foreach (var paymentMethodCriteria in store.GetPaymentMethodCriteria(_NetworkProvider, storeBlob))
+                {
+                    if (paymentMethodCriteria.Value != null)


better to put in foreach with a .Where

BTCPayServer/Controllers/InvoiceController.cs

NicolasDorier · 2020-09-11T14:26:02Z

LGTM, @pavlenex you can test?

pavlenex · 2020-09-11T16:35:14Z

To me, this looks a bit odd way to solve this UX-wise. But I am not nacking it since I can't think of a different way to present this.

Maybe @dstrukt has an idea on how this can be solved in a better way

What we have now

What is proposed in this PR

dstrukt · 2020-09-12T23:49:41Z

@pavlenex will take a look today

dstrukt · 2020-09-14T00:14:37Z

Few clarifications, but will use the mocks for reference:

We can clarify that the payment will be enabled if a there a minimum reached via invoice total, however, wouldn't we need the ability to disable it? Conversely, the ability to add another payment method easily? (Unless there are other methods for doing this more easily / clearly?)

Option B might solve our problem at the moment, but thinking ahead, option A could address a few potential solutions as well, but might not be needed at this time.

Stepping back from that specific feature, I think a few things on this screen need clarification and should broken out into respective sections... but I realize this pushes the scope of this ticket, so regardless of what we choose for Creating CNAME file for domain #1 (which can be updated and reflected in the follow), here are some other ideas for discussion....

Explicitly some of the sections need better descriptors, and even in this mock they could be pushed a bit more, but overall, this is an initiative I'd like to push across the entire product.

Feedback, need for clarification, etc.. welcome as always!

@pavlenex

dstrukt · 2020-09-15T07:35:20Z

@Kukks and I chatted about the feature in MM. I had a few misunderstandings..made some changes.

An idea proposed by Kukks that gives the user the ability to select greater/less than..

The wording for the label could use a second pair of eyes, but I think it's clear enough.

Apologize for any delay in release.

pavlenex · 2020-09-15T07:39:35Z

IMO ^ is a perfect solution and quite clear. Good idea @dstrukt and @Kukks.

@xpayserver could you please make the change according to the proposal so that it's clearer on what's going on?

I upgrade lightning max/bitcoin min to support better control in store. Now can have setting only enable specific payment method only if value high/low I think make code simple more too and backward compatible

pavlenex · 2020-09-22T11:03:54Z

tACK with a minor cosmetic change.

Just to have everyone on board here's a PR preview with e276443

Looks great @xpayserver. I've tested it and here are a few minor notes (some of them may not be related to this PR, but perhaps are good to discuss them so we can improve stuff in the future).

I just suggest we change Enable payment methods only when amount is.. to Enable payment method when the invoice total is as originally suggested.

Unrelated to this PR, but an interesting edge-case.

Have 1 payment method, on-chain BTC for example.
Do not enable payment method if value is lower than 10 USD
Create an invoice at 9 USD
Infinite loop invoce creation called

Exception thrown: 'BTCPayServer.BitpayHttpException' in BTCPayServer.dll
Exception thrown: 'BTCPayServer.BitpayHttpException' in System.Private.CoreLib.dll
Exception thrown: 'BTCPayServer.BitpayHttpException' in System.Private.CoreLib.dll
Loaded 'csmx3w15.vlp'. Cannot find or open the PDB file.
Exception thrown: 'BTCPayServer.BitpayHttpException' in System.Private.CoreLib.dll
Exception thrown: 'NBXplorer.Models.NBXplorerException' in System.Private.CoreLib.dll
Exception thrown: 'NBXplorer.Models.NBXplorerException' in System.Private.CoreLib.dll
Loaded 'c5vyykp5.rgn'. Cannot find or open the PDB file.
Loaded 'oqculfgf.d3s'. Cannot find or open the PDB file.
The thread 319301 has exited with code 0 (0x0).
Exception thrown: 'Microsoft.EntityFrameworkCore.DbUpdateConcurrencyException' in System.Private.CoreLib.dll

NicolasDorier · 2020-09-23T03:41:44Z

thinking merging that for next release.

Infinite loop invoce creation called

This should not happen. It should say no payment method available.

NicolasDorier · 2020-09-23T03:42:47Z

BTCPayServer/Controllers/InvoiceController.cs

+                        }
+                        if (amount > limitValueCrypto && !criteria.Above)
+                        {
+                            logs.Write($"{logPrefix} invoice amount above accepted value for payment method", InvoiceEventData.EventSeverity.Error);


Should not be errors.

Looking at the diff, this was an error before

pavlenex · 2020-09-23T06:06:16Z

thinking merging that for next release.

Infinite loop invoce creation called

This should not happen. It should say no payment method available.

@NicolasDorier Should I create a separate issue for this?

NicolasDorier · 2020-09-23T06:33:47Z

no leave it here

NicolasDorier · 2020-09-24T09:47:54Z

@pavlenex I think your "loops" came from your weird PC. Can you replicate again?

Kukks

Looks ok to me

Kukks · 2020-09-30T08:44:03Z

BTCPayServer/Controllers/InvoiceController.cs

+                        }
+                        if (amount > limitValueCrypto && !criteria.Above)
+                        {
+                            logs.Write($"{logPrefix} invoice amount above accepted value for payment method", InvoiceEventData.EventSeverity.Error);


Looking at the diff, this was an error before

xpayserver force-pushed the feat/payment-method-criteria branch from dad6566 to 6cb7110 Compare September 9, 2020 06:23

Kukks approved these changes Sep 10, 2020

View reviewed changes

xpayserver force-pushed the feat/payment-method-criteria branch 2 times, most recently from 2b6f7a8 to a125ade Compare September 17, 2020 13:37

Add Value Criteria For Payment Method

e276443

I upgrade lightning max/bitcoin min to support better control in store. Now can have setting only enable specific payment method only if value high/low I think make code simple more too and backward compatible

xpayserver force-pushed the feat/payment-method-criteria branch from a125ade to e276443 Compare September 22, 2020 09:20

pavlenex added this to In Progress in 0.09 Miles-stone via automation Sep 22, 2020

NicolasDorier reviewed Sep 23, 2020

View reviewed changes

pavlenex moved this from In Progress to Pending Review in 0.09 Miles-stone Sep 24, 2020

0.09 Miles-stone automation moved this from Pending Review to Review complete Sep 30, 2020

Kukks approved these changes Sep 30, 2020

View reviewed changes

NicolasDorier merged commit a66578c into btcpayserver:master Sep 30, 2020

0.09 Miles-stone automation moved this from Review complete to Done Sep 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Value Criteria For Payment Method #1871

Add Value Criteria For Payment Method #1871

xpayserver commented Aug 27, 2020

Kukks left a comment

Kukks Sep 10, 2020

NicolasDorier commented Sep 11, 2020

pavlenex commented Sep 11, 2020 •

edited

dstrukt commented Sep 12, 2020

dstrukt commented Sep 14, 2020 •

edited

dstrukt commented Sep 15, 2020 •

edited

pavlenex commented Sep 15, 2020

pavlenex commented Sep 22, 2020 •

edited

NicolasDorier commented Sep 23, 2020

NicolasDorier Sep 23, 2020

Kukks Sep 30, 2020

pavlenex commented Sep 23, 2020

NicolasDorier commented Sep 23, 2020

NicolasDorier commented Sep 24, 2020

Kukks left a comment

Kukks Sep 30, 2020

Add Value Criteria For Payment Method #1871

Add Value Criteria For Payment Method #1871

Conversation

xpayserver commented Aug 27, 2020

Kukks left a comment

Choose a reason for hiding this comment

Kukks Sep 10, 2020

Choose a reason for hiding this comment

NicolasDorier commented Sep 11, 2020

pavlenex commented Sep 11, 2020 • edited

dstrukt commented Sep 12, 2020

dstrukt commented Sep 14, 2020 • edited

dstrukt commented Sep 15, 2020 • edited

pavlenex commented Sep 15, 2020

pavlenex commented Sep 22, 2020 • edited

NicolasDorier commented Sep 23, 2020

NicolasDorier Sep 23, 2020

Choose a reason for hiding this comment

Kukks Sep 30, 2020

Choose a reason for hiding this comment

pavlenex commented Sep 23, 2020

NicolasDorier commented Sep 23, 2020

NicolasDorier commented Sep 24, 2020

Kukks left a comment

Choose a reason for hiding this comment

Kukks Sep 30, 2020

Choose a reason for hiding this comment

pavlenex commented Sep 11, 2020 •

edited

dstrukt commented Sep 14, 2020 •

edited

dstrukt commented Sep 15, 2020 •

edited

pavlenex commented Sep 22, 2020 •

edited