Do not deploy testmode value from payment gateways

[MichaelNielsenDK]

So I've just had an issue, where I've created a Development environment on Cloud to Upgrade Umbraco. I then put the gateway I'm using (Nets Easy) into testmode, and tested that everything worked, I then pushed the upgrade out to Live.

I made sure everything worked on Live, I did not go as far as the payment gateway, which in retrospect, I should have, because I later found out that the change to testmode, had been deployed with the upgrade, and all payments was now test payments, because that's how Nets Easy testmode works. 🤔🔫

So my request is, that the testmode parameter is omitted from deployment. I see this setting as something you would change individually on environments, and not deploy between environments.

[mattbrailsford]

Hmm, good point.

Unfortunately I don't think we can specifically target the test mode prop as this is an option provided by the payment provider implementation so we are at the whim of what the implementors want to name it.

Maybe the right thing to do here is to only restore the full payment provider if it doesn't exist at all on the other end. If it does exist, then I don't think we should modify anything 🤔

[MichaelNielsenDK]

Oh yeah you're right, it's not given that the property is named 'testmode' or even something with 'test'. 🙄

Well then there might not be anything that can be done, but I thought I'd ask 🤷‍♂️🙂

In my opinion, the payment provider should still be deployed, as there can be changes you do want deployed, and I think it will confuse some, if subsequent changes are not deployed as they might not understand why that is.

I think the better option there, would be to handle it manually, by adding/removing the .uda file to .gitgnore, depending on whether you want it deployed or not.

But thanks for looking at it. 👍

[mattbrailsford]

No problem.

The best I can think atm is maybe if if we introduced some special attribute to add to the settings model that says "this field is sensitive" or something like that, and then maybe we do something fancy to parse the JSON and ignore it, but we'd also then have to do something special to merge it with the existing value at the other end (instead of just outright replacing it).

I'll give it a bit if thought as I don't tend to like introducing secondary package specific settings in the core product, but it might be doable in a more generic way 🤔

[FransdeJong]

Shouldn't this be a feature request in Umbraco Deploy? If we have a general way to ignore properties in deploy this would be a lot easier to implement right?

I can add a feature request if you like?

[mattbrailsford]

I'm looking at the possibility of a DeployIgnore attribute for payment provider devs to add to their settings model to dictate properties that should not be deployed, but I can't decide if it's right to give this responsibility to provider devs, as what would we say should be deployable. For example, lets take the Mollie settings

public class MollieOneTimeSettings
{
    [PaymentProviderSetting(Name = "Continue URL",
        Description = "The URL to continue to after this provider has done processing. eg: /continue/",
        SortOrder = 100)]
    public string ContinueUrl { get; set; }

    [PaymentProviderSetting(Name = "Cancel URL",
        Description = "The URL to return to if the payment attempt is canceled. eg: /cancel/",
        SortOrder = 200)]
    public string CancelUrl { get; set; }

    [PaymentProviderSetting(Name = "Error URL",
        Description = "The URL to return to if the payment attempt errors. eg: /error/",
        SortOrder = 300)]
    public string ErrorUrl { get; set; }

    [PaymentProviderSetting(Name = "Billing Address (Line 1) Property Alias",
        Description = "The order property alias containing line 1 of the billing address",
        SortOrder = 400)]
    public string BillingAddressLine1PropertyAlias { get; set; }

    [PaymentProviderSetting(Name = "Billing Address City Property Alias",
        Description = "The order property alias containing the city of the billing address",
        SortOrder = 600)]
    public string BillingAddressCityPropertyAlias { get; set; }

    [PaymentProviderSetting(Name = "Billing Address State Property Alias",
        Description = "The order property alias containing the state of the billing address",
        SortOrder = 700)]
    public string BillingAddressStatePropertyAlias { get; set; }

    [PaymentProviderSetting(Name = "Billing Address ZipCode Property Alias",
        Description = "The order property alias containing the zip code of the billing address",
        SortOrder = 800)]
    public string BillingAddressZipCodePropertyAlias { get; set; }

    [PaymentProviderSetting(Name = "Test API Key",
        Description = "Your test Mollie API key",
        SortOrder = 900)]
    public string TestApiKey { get; set; }

    [PaymentProviderSetting(Name = "Live API Key",
        Description = "Your live Mollie API key",
        SortOrder = 1000)]
    public string LiveApiKey { get; set; }

    [PaymentProviderSetting(Name = "Test Mode",
        Description = "Set whether to process payments in test mode.",
        SortOrder = 10000)]
    public bool TestMode { get; set; }

    // Advanced settings

    [PaymentProviderSetting(Name = "Locale",
        Description = "The locale to display the payment provider portal in.",
        IsAdvanced = true,
        SortOrder = 1000100)]
    public string Locale { get; set; }

    [PaymentProviderSetting(Name = "Payment Methods",
        Description = "A comma separated list of payment methods to limit the payment method selection screen by. Can be 'applepay', 'bancontact', 'banktransfer', 'belfius', 'creditcard', 'directdebit', 'eps', 'giftcard', 'giropay', 'ideal', 'kbc', 'klarnapaylater', 'klarnasliceit', 'mybank', 'paypal', 'paysafecard', 'przelewy24', 'sofort' or 'voucher'.",
        IsAdvanced = true,
        SortOrder = 1000200)]
    public string PaymentMethods { get; set; }

    [PaymentProviderSetting(Name = "Order Line Product Type Property Alias",
        Description = "The order line property alias containing a Mollie product type for the order line. Can be either 'physical' or 'digital'.",
        IsAdvanced = true,
        SortOrder = 1000300)]
    public string OrderLineProductTypePropertyAlias { get; set; }

    [PaymentProviderSetting(Name = "Order Line Product Category Property Alias",
        Description = "The order line property alias containing a Mollie product category for the order line. Can be 'meal', 'eco' or 'gift'.",
        IsAdvanced = true,
        SortOrder = 1000300)]
    public string OrderLineProductCategoryPropertyAlias { get; set; }
}

I could imagine that we might want to not deploy LiveApiKey and TestMode? And maybe even the PaymentMethods advanced property, but that later one at leas feels subjective. Maybe even LiveApiKey is subjective?

This also means that every payment provider would need to be updated to add this DeployIgnore attribute to their settings models to make this all work, which it would be pretty easy for someone to forget.

Only other thing I could think to do is potentially introduce some known Test Mode toggle, but again this would have a big effect on payment provider implementations.

Maybe this control needs to be given to the implementor of the site, rather than the payment provider dev with something being added to the properties UI (maybe only if Vendr Deploy or Vendr uSync is installed) to say don't deploy this settings or maybe even just some app setting to say "Don't deploy any settings with the given aliases". The latter might be the easiest to implement, but less visible.

[FransdeJong]

If we can set a setting somwhere with wat we can prevent the deploy that would be super. Yesterday we came in a situation where there were 10 orders on live that were payed with the test molly before we identified the error. That's not great.

[mattbrailsford]

@FransdeJong oof, indeed not 😥

[mattbrailsford]

I think maybe I'll look at the app settings route for now as this would at least mean it can be added as a backwards compatible feature.

[mattbrailsford]

Ok, so I'm just compiling on our unstable feed a new v2.0.2-beta0003 release which now supports a config to disable specific properties from being deployed.

So in Umbraco v8 you'd need to create an app settings

<add key="Vendr.Deploy:PaymentMethods:IgnoreSettings" value="testMode,sandboxMode,liveApiKey" />

or in v9, you'll need an app settings like

{
   ...
   "Vendr.Deploy": {
        "PaymentMethods": {
            "IgnoreSettings": [ "testMode", "sanboxMode", "liveApiKey" ]
        }
    }
}

Obviously this will only work for Vendr v2 instances. If this is needed for v1 then I'll have to back port this. If someone is able to test this though this would be hugely appreciated as I don't currently have a test cloud instance setup.

I guess I'm also going to need to implement this same thing for Vendr uSync 🤦‍♂️

UPDATE the beta build is now available on our unstable feed https://nuget.outfield.digital/unstable/vendr/v3/index.json