MetacoSA / NBitpayClient

A non official library for using bitpay's API
MIT License
19 stars 18 forks source link

allow custom invoice type for fetch #19

Closed Kukks closed 5 years ago

Kukks commented 5 years ago

Ran into a scenario where I needed to serialize info to json but the ShouldSerialize properties made it ignore some key elements such as Status.

NicolasDorier commented 5 years ago

I don't like those Should methods, can I just replace it with , DefaultValueHandling = DefaultValueHandling.Ignore on the fields?

NicolasDorier commented 5 years ago

Use InvoiceResponse in BTCpay

Kukks commented 5 years ago

Issue is not in Btcpay, using it in the external fiat system. As long as there is a way to override this behavior I am good. Here is how I managed with this PR:

var invoices = await client.GetInvoicesAsync<BtcPayInvoice>(data.PairedDate);

public class BtcPayInvoice : Invoice
    {
        public override bool ShouldSerializeId()
        {
            return true;
        }

        public override bool ShouldSerializeUrl()
        {
            return true;
        }

        public override bool ShouldSerializeStatus()
        {
            return true;
        }

        public override bool ShouldSerializeBtcPrice()
        {
            return true;
        }

        public override bool ShouldSerializeInvoiceTime()
        {
            return true;
        }

        public override bool ShouldSerializeExpirationTime()
        {
            return true;
        }

        public override bool ShouldSerializeBtcPaid()
        {
            return true;
        }

        public override bool ShouldSerializeBtcDue()
        {
            return true;
        }

        public override bool ShouldSerializeTransactions()
        {
            return true;
        }

        public override bool ShouldSerializeExRates()
        {
            return true;
        }

        public override bool ShouldSerializeExceptionStatus()
        {
            return true;
        }

        public override bool ShouldSerializePaymentUrls()
        {
            return true;
        }

        public override bool ShouldSerializeAddresses()
        {
            return true;
        }
    }
NicolasDorier commented 5 years ago

Should we just replace the Invoice type here with the InvoiceResponse one from BTCPay (without renaming the Invoice type here though)

Kukks commented 5 years ago

I looked into it & Invoice Response misses some properties (such as fullnotifications, buyerXXX fields, etc)

NicolasDorier commented 5 years ago

In BTCPay today I just created https://github.com/btcpayserver/btcpayserver/blob/master/BTCPayServer/Models/CreateInvoiceRequest.cs

The problem is that this lib is using the same type for both, the request and the response.

Kukks commented 5 years ago

So we should move both formats to this lib and do a breaking change? It should be fairly minor and good in the long run

NicolasDorier commented 5 years ago

I think this convention driven stuff is ugly. But well, will merge.

Kukks commented 5 years ago

I agree, but it's what's in there right now :)