Refactor default payment method setting (#803)

* Refactor setting of default providers

* fixed warning about component update while rendering another component
* individual providers no longer need to know if they are the default or not
* default setting is now handled by WebLNContext -- the same context that returns the provider. this makes a lot more sense and is a lot easier to read
* default payment checkbox is now also disabled if there is only one enabled provider or if it is the default provider

* Fix order lost on page reload

On page reload, the providers were synced in the order they were loaded.

This means that the default payment provider setting was lost.

Fixed this by syncing order to local storage and on page reload, only syncing providers when they were initialized (else the order would have been lost again).

components/webln/index.js

@@ -1,45 +1,76 @@
-import { createContext, useContext, useEffect, useState } from 'react'
+import { createContext, useCallback, useContext, useEffect, useState } from 'react'
 import { LNbitsProvider, useLNbits } from './lnbits'
 import { NWCProvider, useNWC } from './nwc'
 import { useToast } from '../toast'
 import { gql, useMutation } from '@apollo/client'
 
 const WebLNContext = createContext({})
+
+const syncProvider = (array, provider) => {
+  const idx = array.findIndex(({ name }) => provider.name === name)
+  if (idx === -1) {
+    // add provider to end if enabled
+    return provider.enabled ? [...array, provider] : array
+  }
+  return [
+    ...array.slice(0, idx),
+    // remove provider if not enabled
+    ...provider.enabled ? [provider] : [],
+    ...array.slice(idx + 1)
+  ]
+}
+
 const storageKey = 'webln:providers'
 
-const paymentMethodHook = (methods, { name, enabled }) => {
-  let newMethods
-  if (enabled) {
-    newMethods = methods.includes(name) ? methods : [...methods, name]
-  } else {
-    newMethods = methods.filter(m => m !== name)
-  }
-  savePaymentMethods(newMethods)
-  return newMethods
-}
-
-const savePaymentMethods = (methods) => {
-  window.localStorage.setItem(storageKey, JSON.stringify(methods))
-}
-
 function RawWebLNProvider ({ children }) {
   const lnbits = useLNbits()
   const nwc = useNWC()
-  const providers = [lnbits, nwc]
+  const availableProviders = [lnbits, nwc]
+  const [enabledProviders, setEnabledProviders] = useState([])
 
-  // TODO: Order of payment methods depends on user preference.
+  // restore order on page reload
-  // Payment method at index 0 should be default,
+  useEffect(() => {
-  // if that one fails we try the remaining ones in order as fallbacks.
+    const storedOrder = window.localStorage.getItem(storageKey)
-  // We should be able to implement this via dragging of cards.
+    if (!storedOrder) return
-  // This list should then match the order in which the (payment) cards are rendered.
+    const providerNames = JSON.parse(storedOrder)
-  // eslint-disable-next-line no-unused-vars
+    setEnabledProviders(providers => {
-  const [paymentMethods, setPaymentMethods] = useState([])
+      return providerNames.map(name => {
-  const loadPaymentMethods = () => {
+        for (const p of availableProviders) {
-    const methods = window.localStorage.getItem(storageKey)
+          if (p.name === name) return p
-    if (!methods) return
-    setPaymentMethods(JSON.parse(methods))
         }
-  useEffect(loadPaymentMethods, [])
+        console.warn(`Stored provider with name ${name} not available`)
+        return null
+      })
+    })
+  }, [])
+
+  // keep list in sync with underlying providers
+  useEffect(() => {
+    setEnabledProviders(providers => {
+      // Sync existing provider state with new provider state
+      // in the list while keeping the order they are in.
+      // If provider does not exist but is enabled, it is just added to the end of the list.
+      // This can be the case if we're syncing from a page reload
+      // where the providers are initially not enabled.
+      // If provider is no longer enabled, it is removed from the list.
+      const isInitialized = p => p.initialized
+      const newProviders = availableProviders.filter(isInitialized).reduce(syncProvider, providers)
+      const newOrder = newProviders.map(({ name }) => name)
+      window.localStorage.setItem(storageKey, JSON.stringify(newOrder))
+      return newProviders
+    })
+  }, [lnbits, nwc])
+
+  // sanity check
+  for (const p of enabledProviders) {
+    if (!p.enabled && p.initialized) {
+      console.warn('Expected provider to be enabled but is not:', p.name)
+    }
+  }
+
+  // first provider in list is the default provider
+  // TODO: implement fallbacks via provider priority
+  const provider = enabledProviders[0]
 
   const toaster = useToast()
   const [cancelInvoice] = useMutation(gql`
@@ -50,43 +81,6 @@ function RawWebLNProvider ({ children }) {
     }
   `)
 
-  useEffect(() => {
-    setPaymentMethods(methods => paymentMethodHook(methods, nwc))
-    if (!nwc.enabled) nwc.setIsDefault(false)
-  }, [nwc.enabled])
-
-  useEffect(() => {
-    setPaymentMethods(methods => paymentMethodHook(methods, lnbits))
-    if (!lnbits.enabled) lnbits.setIsDefault(false)
-  }, [lnbits.enabled])
-
-  const setDefaultPaymentMethod = (provider) => {
-    for (const p of providers) {
-      if (p.name !== provider.name) {
-        p.setIsDefault(false)
-      }
-    }
-  }
-
-  useEffect(() => {
-    if (nwc.isDefault) setDefaultPaymentMethod(nwc)
-  }, [nwc.isDefault])
-
-  useEffect(() => {
-    if (lnbits.isDefault) setDefaultPaymentMethod(lnbits)
-  }, [lnbits.isDefault])
-
-  // TODO: implement numeric provider priority using paymentMethods list
-  // when we have more than two providers for sending
-  let provider = providers.filter(p => p.enabled && p.isDefault)[0]
-  if (!provider && providers.length > 0) {
-    // if no provider is the default, pick the first one and use that one as the default
-    provider = providers.filter(p => p.enabled)[0]
-    if (provider) {
-      provider.setIsDefault(true)
-    }
-  }
-
   const sendPaymentWithToast = function ({ bolt11, hash, hmac }) {
     let canceled = false
     let removeToast = toaster.warning('payment pending', {
@@ -116,8 +110,21 @@ function RawWebLNProvider ({ children }) {
       })
   }
 
+  const setProvider = useCallback((defaultProvider) => {
+    // move provider to the start to set it as default
+    setEnabledProviders(providers => {
+      const idx = providers.findIndex(({ name }) => defaultProvider.name === name)
+      if (idx === -1) {
+        console.warn(`tried to set unenabled provider ${defaultProvider.name} as default`)
+        return providers
+      }
+      return [defaultProvider, ...providers.slice(0, idx), ...providers.slice(idx + 1)]
+    })
+  }, [setEnabledProviders])
+
+  const value = { provider: { ...provider, sendPayment: sendPaymentWithToast }, enabledProviders, setProvider }
   return (
-    <WebLNContext.Provider value={{ ...provider, sendPayment: sendPaymentWithToast }}>
+    <WebLNContext.Provider value={value}>
       {children}
     </WebLNContext.Provider>
   )
@@ -136,5 +143,10 @@ export function WebLNProvider ({ children }) {
 }
 
 export function useWebLN () {
+  const { provider } = useContext(WebLNContext)
+  return provider
+}
+
+export function useWebLNConfigurator () {
   return useContext(WebLNContext)
 }

components/webln/lnbits.js

@@ -64,7 +64,7 @@ export function LNbitsProvider ({ children }) {
   const [url, setUrl] = useState('')
   const [adminKey, setAdminKey] = useState('')
   const [enabled, setEnabled] = useState()
-  const [isDefault, setIsDefault] = useState()
+  const [initialized, setInitialized] = useState(false)
 
   const name = 'LNbits'
   const storageKey = 'webln:provider:lnbits'
@@ -104,10 +104,9 @@ export function LNbitsProvider ({ children }) {
 
     const config = JSON.parse(configStr)
 
-    const { url, adminKey, isDefault } = config
+    const { url, adminKey } = config
     setUrl(url)
     setAdminKey(adminKey)
-    setIsDefault(isDefault)
 
     try {
       // validate config by trying to fetch wallet
@@ -117,6 +116,8 @@ export function LNbitsProvider ({ children }) {
       console.error('invalid LNbits config:', err)
       setEnabled(false)
       throw err
+    } finally {
+      setInitialized(true)
     }
   }, [])
 
@@ -124,7 +125,6 @@ export function LNbitsProvider ({ children }) {
     // immediately store config so it's not lost even if config is invalid
     setUrl(config.url)
     setAdminKey(config.adminKey)
-    setIsDefault(config.isDefault)
 
     // XXX This is insecure, XSS vulns could lead to loss of funds!
     //   -> check how mutiny encrypts their wallet and/or check if we can leverage web workers
@@ -153,7 +153,7 @@ export function LNbitsProvider ({ children }) {
     loadConfig().catch(console.error)
   }, [])
 
-  const value = { name, url, adminKey, saveConfig, clearConfig, enabled, isDefault, setIsDefault, getInfo, sendPayment }
+  const value = { name, url, adminKey, initialized, enabled, saveConfig, clearConfig, getInfo, sendPayment }
   return (
     <LNbitsContext.Provider value={value}>
       {children}

components/webln/nwc.js

@@ -11,7 +11,7 @@ export function NWCProvider ({ children }) {
   const [relayUrl, setRelayUrl] = useState()
   const [secret, setSecret] = useState()
   const [enabled, setEnabled] = useState()
-  const [isDefault, setIsDefault] = useState()
+  const [initialized, setInitialized] = useState(false)
   const [relay, setRelay] = useState()
 
   const name = 'NWC'
@@ -26,9 +26,8 @@ export function NWCProvider ({ children }) {
 
     const config = JSON.parse(configStr)
 
-    const { nwcUrl, isDefault } = config
+    const { nwcUrl } = config
     setNwcUrl(nwcUrl)
-    setIsDefault(isDefault)
 
     const params = parseWalletConnectUrl(nwcUrl)
     setRelayUrl(params.relayUrl)
@@ -42,14 +41,15 @@ export function NWCProvider ({ children }) {
       console.error('invalid NWC config:', err)
       setEnabled(false)
       throw err
+    } finally {
+      setInitialized(true)
     }
   }, [])
 
   const saveConfig = useCallback(async (config) => {
     // immediately store config so it's not lost even if config is invalid
-    const { nwcUrl, isDefault } = config
+    const { nwcUrl } = config
     setNwcUrl(nwcUrl)
-    setIsDefault(isDefault)
     if (!nwcUrl) {
       setEnabled(undefined)
       return
@@ -174,7 +174,7 @@ export function NWCProvider ({ children }) {
     loadConfig().catch(console.error)
   }, [])
 
-  const value = { name, nwcUrl, relayUrl, walletPubkey, secret, saveConfig, clearConfig, enabled, isDefault, setIsDefault, getInfo, sendPayment }
+  const value = { name, nwcUrl, relayUrl, walletPubkey, secret, initialized, enabled, saveConfig, clearConfig, getInfo, sendPayment }
   return (
     <NWCContext.Provider value={value}>
       {children}

pages/settings/wallets/lnbits.js

@@ -7,11 +7,15 @@ import { useToast } from '../../../components/toast'
 import { useRouter } from 'next/router'
 import { useLNbits } from '../../../components/webln/lnbits'
 import { WalletSecurityBanner } from '../../../components/banners'
+import { useWebLNConfigurator } from '../../../components/webln'
 
 export const getServerSideProps = getGetServerSideProps({ authRequired: true })
 
 export default function LNbits () {
-  const { url, adminKey, saveConfig, clearConfig, enabled, isDefault } = useLNbits()
+  const { provider, enabledProviders, setProvider } = useWebLNConfigurator()
+  const lnbits = useLNbits()
+  const { name, url, adminKey, saveConfig, clearConfig, enabled } = lnbits
+  const isDefault = provider?.name === name
   const toaster = useToast()
   const router = useRouter()
 
@@ -27,9 +31,10 @@ export default function LNbits () {
           isDefault: isDefault || false
         }}
         schema={lnbitsSchema}
-        onSubmit={async (values) => {
+        onSubmit={async ({ isDefault, ...values }) => {
           try {
             await saveConfig(values)
+            if (isDefault) setProvider(lnbits)
             toaster.success('saved settings')
             router.push('/settings/wallets')
           } catch (err) {
@@ -53,7 +58,7 @@ export default function LNbits () {
           name='adminKey'
         />
         <ClientCheckbox
-          disabled={!enabled}
+          disabled={!enabled || isDefault || enabledProviders.length === 1}
           initialValue={isDefault}
           label='default payment method'
           name='isDefault'

pages/settings/wallets/nwc.js

@@ -7,11 +7,15 @@ import { useToast } from '../../../components/toast'
 import { useRouter } from 'next/router'
 import { useNWC } from '../../../components/webln/nwc'
 import { WalletSecurityBanner } from '../../../components/banners'
+import { useWebLNConfigurator } from '../../../components/webln'
 
 export const getServerSideProps = getGetServerSideProps({ authRequired: true })
 
 export default function NWC () {
-  const { nwcUrl, saveConfig, clearConfig, enabled, isDefault } = useNWC()
+  const { provider, enabledProviders, setProvider } = useWebLNConfigurator()
+  const nwc = useNWC()
+  const { name, nwcUrl, saveConfig, clearConfig, enabled } = nwc
+  const isDefault = provider?.name === name
   const toaster = useToast()
   const router = useRouter()
 
@@ -26,9 +30,10 @@ export default function NWC () {
           isDefault: isDefault || false
         }}
         schema={nwcSchema}
-        onSubmit={async (values) => {
+        onSubmit={async ({ isDefault, ...values }) => {
           try {
             await saveConfig(values)
+            if (isDefault) setProvider(nwc)
             toaster.success('saved settings')
             router.push('/settings/wallets')
           } catch (err) {
@@ -45,7 +50,7 @@ export default function NWC () {
           autoFocus
         />
         <ClientCheckbox
-          disabled={!enabled}
+          disabled={!enabled || isDefault || enabledProviders.length === 1}
           initialValue={isDefault}
           label='default payment method'
           name='isDefault'