[FEAT] cleanup GET /wallet endpoint, add wallet api routes (#1932)

* [FEAT] cleanup GET /wallet endpoint, add wallet api route
this removes the functionalitiy to create accounts and wallets via
the GET /wallet endpoint in generic.py

it add endpoints inside the api.py for it and the frontend is modified to use the api endpoints

this also simplifies for the `feat/login` for the route.

* remove stale generic tests and add api tests

* bug wrong endpoint create account

* vlad nitpick

* added checkif deleted is 404

* reload after renaming wallet

* another iteration with vlad

* create new wallet if it none exist

* fix delete refresh

* formatting

lnbits/core/crud.py

@@ -77,7 +77,7 @@ async def get_user(user_id: str, conn: Optional[Connection] = None) -> Optional[
                 SELECT balance FROM balances WHERE wallet = wallets.id
             ), 0) AS balance_msat
             FROM wallets
-            WHERE "user" = ?
+            WHERE "user" = ? and wallets.deleted = false
             """,
             (user_id,),
         )

lnbits/core/models.py

@@ -339,6 +339,10 @@ class CreateLnurlAuth(BaseModel):
     callback: str
 
 
+class CreateWallet(BaseModel):
+    name: Optional[str] = None
+
+
 class CreateWebPushSubscription(BaseModel):
     subscription: str
 

lnbits/core/static/js/index.js

@@ -12,7 +12,9 @@ new Vue({
   },
   methods: {
     createWallet: function () {
-      LNbits.href.createWallet(this.walletName)
+      LNbits.api.createAccount(this.walletName).then(res => {
+        window.location = '/wallet?usr=' + res.data.user + '&wal=' + res.data.id
+      })
     },
     processing: function () {
       this.$q.notify({

lnbits/core/static/js/wallet.js

@@ -702,7 +702,7 @@ new Vue({
 
       LNbits.api
         .authLnurl(this.g.wallet, this.parse.lnurlauth.callback)
-        .then(response => {
+        .then(_ => {
           dismissAuthMsg()
           this.$q.notify({
             message: `Authentication successful.`,
@@ -728,27 +728,35 @@ new Vue({
     updateWallet: function (data) {
       LNbits.api
         .request('PATCH', '/api/v1/wallet', this.g.wallet.adminkey, data)
-        .then(res => {
+        .then(_ => {
           this.$q.notify({
             message: `Wallet updated.`,
             type: 'positive',
             timeout: 3500
           })
-          LNbits.href.updateWallet(
+          window.location.reload()
-            res.data.name,
-            this.user.id,
-            this.g.wallet.id
-          )
         })
         .catch(err => {
           LNbits.utils.notifyApiError(err)
         })
     },
-    deleteWallet: function (walletId, user) {
+    deleteWallet: function () {
       LNbits.utils
         .confirmDialog('Are you sure you want to delete this wallet?')
         .onOk(() => {
-          LNbits.href.deleteWallet(walletId, user)
+          LNbits.api
+            .deleteWallet(this.g.wallet)
+            .then(_ => {
+              this.$q.notify({
+                timeout: 3000,
+                message: `Wallet deleted!`,
+                spinner: true
+              })
+            })
+            .catch(err => {
+              this.paymentsTable.loading = false
+              LNbits.utils.notifyApiError(err)
+            })
         })
     },
     fetchPayments: function (props) {

lnbits/core/templates/core/wallet.html

@@ -428,7 +428,7 @@
                       <q-btn
                         unelevated
                         color="red-10"
-                        @click="deleteWallet('{{ wallet.id }}', '{{ user.id }}')"
+                        @click="deleteWallet()"
                         :label="$t('delete_wallet')"
                       ></q-btn>
                     </q-card-section>

lnbits/core/views/api.py

@@ -36,6 +36,7 @@ from lnbits.core.models import (
     CreateInvoice,
     CreateLnurl,
     CreateLnurlAuth,
+    CreateWallet,
     CreateWebPushSubscription,
     DecodePayment,
     Payment,
@@ -75,11 +76,14 @@ from lnbits.utils.exchange_rates import (
 from ..crud import (
     DateTrunc,
     add_installed_extension,
+    create_account,
     create_tinyurl,
+    create_wallet,
     create_webpush_subscription,
     delete_dbversion,
     delete_installed_extension,
     delete_tinyurl,
+    delete_wallet,
     delete_webpush_subscription,
     drop_extension_db,
     get_dbversions,
@@ -148,6 +152,30 @@ async def api_update_wallet(
     return await update_wallet(wallet.wallet.id, name, currency)
 
 
+@api_router.delete("/api/v1/wallet")
+async def api_delete_wallet(
+    wallet: WalletTypeInfo = Depends(require_admin_key),
+) -> None:
+    await delete_wallet(
+        user_id=wallet.wallet.user,
+        wallet_id=wallet.wallet.id,
+    )
+
+
+@api_router.post("/api/v1/wallet", response_model=Wallet)
+async def api_create_wallet(
+    data: CreateWallet,
+    wallet: WalletTypeInfo = Depends(require_admin_key),
+) -> Wallet:
+    return await create_wallet(user_id=wallet.wallet.user, wallet_name=data.name)
+
+
+@api_router.post("/api/v1/account", response_model=Wallet)
+async def api_create_account(data: CreateWallet) -> Wallet:
+    account = await create_account()
+    return await create_wallet(user_id=account.id, wallet_name=data.name)
+
+
 @api_router.get(
     "/api/v1/payments",
     name="Payment List",

lnbits/core/views/generic.py

@@ -23,7 +23,6 @@ from ...utils.exchange_rates import currencies
 from ..crud import (
     create_account,
     create_wallet,
-    delete_wallet,
     get_balance_check,
     get_dbversions,
     get_inactive_extensions,
@@ -161,37 +160,35 @@ async def extensions_install(
 @generic_router.get(
     "/wallet",
     response_class=HTMLResponse,
-    description="""
+    description="show wallet page",
-just **wallet_name**: create a new user, then create a new wallet
-                      for user with wallet_name
-just **user_id**: return the first user wallet or create one if none found
-                  (with default wallet_name)
-**user_id** and **wallet_name**: create a new wallet for user with wallet_name
-**user_id** and **wallet_id**: return that wallet if user is the owner
-nothing: create everything
-""",
 )
 async def wallet(
     request: Request,
-    nme: Optional[str] = Query(None),
+    usr: UUID4 = Query(...),
-    usr: Optional[UUID4] = Query(None),
     wal: Optional[UUID4] = Query(None),
 ):
-    user_id = usr.hex if usr else None
+    user_id = usr.hex
-    wallet_id = wal.hex if wal else None
-    wallet_name = nme
-
-    if not user_id:
-        new_user = await create_account()
-        user = await get_user(new_user.id)
-        assert user, "Newly created user has to exist."
-        logger.info(f"Create user {user.id}")
-    else:
     user = await get_user(user_id)
+
     if not user:
         return template_renderer().TemplateResponse(
             "error.html", {"request": request, "err": "User does not exist."}
         )
+
+    if not wal:
+        if len(user.wallets) == 0:
+            wallet = await create_wallet(user_id=user.id)
+            return RedirectResponse(url=f"/wallet?usr={user_id}&wal={wallet.id}")
+        return RedirectResponse(url=f"/wallet?usr={user_id}&wal={user.wallets[0].id}")
+    else:
+        wallet_id = wal.hex
+
+    userwallet = user.get_wallet(wallet_id)
+    if not userwallet or userwallet.deleted:
+        return template_renderer().TemplateResponse(
+            "error.html", {"request": request, "err": "Wallet not found"}
+        )
+
     if (
         len(settings.lnbits_allowed_users) > 0
         and user_id not in settings.lnbits_allowed_users
@@ -201,36 +198,13 @@ async def wallet(
         return template_renderer().TemplateResponse(
             "error.html", {"request": request, "err": "User not authorized."}
         )
+
     if user_id == settings.super_user or user_id in settings.lnbits_admin_users:
         user.admin = True
     if user_id == settings.super_user:
         user.super_user = True
 
-    if not wallet_id:
+    logger.debug(f"Access user {user.id} wallet {userwallet.name}")
-        if user.wallets and not wallet_name:
-            wallet = user.wallets[0]
-        else:
-            wallet = await create_wallet(user_id=user.id, wallet_name=wallet_name)
-            logger.info(
-                f"Created new wallet {wallet_name if wallet_name else '(no name)'} for"
-                f" user {user.id}"
-            )
-
-        return RedirectResponse(
-            f"/wallet?usr={user.id}&wal={wallet.id}",
-            status_code=status.HTTP_307_TEMPORARY_REDIRECT,
-        )
-
-    logger.debug(
-        "Access "
-        f"{'user '+ user.id + ' ' if user else ''} "
-        f"{'wallet ' + wallet_name if wallet_name else ''}"
-    )
-    userwallet = user.get_wallet(wallet_id)
-    if not userwallet:
-        return template_renderer().TemplateResponse(
-            "error.html", {"request": request, "err": "Wallet not found"}
-        )
 
     return template_renderer().TemplateResponse(
         "core/wallet.html",
@@ -312,32 +286,6 @@ async def lnurl_full_withdraw_callback(request: Request):
     return {"status": "OK"}
 
 
-@generic_router.get("/deletewallet", response_class=RedirectResponse)
-async def deletewallet(wal: str = Query(...), usr: str = Query(...)):
-    user = await get_user(usr)
-    if not user:
-        raise HTTPException(HTTPStatus.FORBIDDEN, "User not found.")
-
-    user_wallet_ids = [u.id for u in user.wallets]
-
-    if wal not in user_wallet_ids:
-        raise HTTPException(HTTPStatus.FORBIDDEN, "Not your wallet.")
-    else:
-        await delete_wallet(user_id=user.id, wallet_id=wal)
-        user_wallet_ids.remove(wal)
-        logger.debug("Deleted wallet {wal} of user {user.id}")
-
-    if user_wallet_ids:
-        return RedirectResponse(
-            url_for("/wallet", usr=user.id, wal=user_wallet_ids[0]),
-            status_code=status.HTTP_307_TEMPORARY_REDIRECT,
-        )
-
-    return RedirectResponse(
-        url_for("/"), status_code=status.HTTP_307_TEMPORARY_REDIRECT
-    )
-
-
 @generic_router.get("/withdraw/notify/{service}")
 async def lnurl_balance_notify(request: Request, service: str):
     wal_param = request.query_params.get("wal")

lnbits/static/js/base.js

@@ -64,9 +64,35 @@ window.LNbits = {
         callback
       })
     },
+    createAccount: function (name) {
+      return this.request('post', '/api/v1/account', null, {
+        name: name
+      })
+    },
     getWallet: function (wallet) {
       return this.request('get', '/api/v1/wallet', wallet.inkey)
     },
+    createWallet: function (wallet, name) {
+      return this.request('post', '/api/v1/wallet', wallet.adminkey, {
+        name: name
+      }).then(res => {
+        window.location = '/wallet?usr=' + res.data.user + '&wal=' + res.data.id
+      })
+    },
+    updateWallet: function (name, wallet) {
+      return this.request('patch', '/api/v1/wallet', wallet.adminkey, {
+        name: name
+      })
+    },
+    deleteWallet: function (wallet) {
+      return this.request('delete', '/api/v1/wallet', wallet.adminkey).then(
+        _ => {
+          let url = new URL(window.location.href)
+          url.searchParams.delete('wal')
+          window.location = url
+        }
+      )
+    },
     getPayments: function (wallet, query) {
       const params = new URLSearchParams(query)
       return this.request(
@@ -118,18 +144,6 @@ window.LNbits = {
       }
     }
   },
-  href: {
-    createWallet: function (walletName, userId) {
-      window.location.href =
-        '/wallet?' + (userId ? 'usr=' + userId + '&' : '') + 'nme=' + walletName
-    },
-    updateWallet: function (walletName, userId, walletId) {
-      window.location.href = `/wallet?usr=${userId}&wal=${walletId}&nme=${walletName}`
-    },
-    deleteWallet: function (walletId, userId) {
-      window.location.href = '/deletewallet?usr=' + userId + '&wal=' + walletId
-    }
-  },
   map: {
     extension: function (data) {
       var obj = _.object(

lnbits/static/js/components.js

@@ -86,7 +86,7 @@ Vue.component('lnbits-wallet-list', {
   },
   methods: {
     createWallet: function () {
-      LNbits.href.createWallet(this.walletName, this.user.id)
+      LNbits.api.createWallet(this.user.wallets[0], this.walletName)
     },
     updateWalletBalance: function (payload) {
       this.activeBalance = payload

tests/core/views/test_api.py

@@ -23,11 +23,51 @@ from ...helpers import (
 WALLET = get_wallet_class()
 
 
-# check if the client is working
+# create account POST /api/v1/account
 @pytest.mark.asyncio
-async def test_core_views_generic(client):
+async def test_create_account(client):
-    response = await client.get("/")
+    response = await client.post("/api/v1/account", json={"name": "test"})
     assert response.status_code == 200
+    result = response.json()
+    assert "name" in result
+    assert result["name"] == "test"
+    assert "balance_msat" in result
+    assert "id" in result
+    assert "user" in result
+
+
+# check POST and DELETE /api/v1/wallet with adminkey:
+# create additional wallet and delete it
+@pytest.mark.asyncio
+async def test_create_wallet_and_delete(client, adminkey_headers_to):
+    response = await client.post(
+        "/api/v1/wallet", json={"name": "test"}, headers=adminkey_headers_to
+    )
+    assert response.status_code == 200
+    result = response.json()
+    assert "name" in result
+    assert result["name"] == "test"
+    assert "balance_msat" in result
+    assert "id" in result
+    assert "adminkey" in result
+    response = await client.delete(
+        "/api/v1/wallet",
+        headers={
+            "X-Api-Key": result["adminkey"],
+            "Content-type": "application/json",
+        },
+    )
+    assert response.status_code == 200
+
+    # get deleted wallet
+    response = await client.get(
+        "/api/v1/wallet",
+        headers={
+            "X-Api-Key": result["adminkey"],
+            "Content-type": "application/json",
+        },
+    )
+    assert response.status_code == 404
 
 
 # check GET /api/v1/wallet with inkey: wallet info, no balance

tests/core/views/test_generic.py

@@ -7,35 +7,6 @@ async def test_core_views_generic(client):
     assert response.status_code == 200, f"{response.url} {response.status_code}"
 
 
-# check GET /wallet: wallet info
-@pytest.mark.asyncio
-async def test_get_wallet(client):
-    response = await client.get("wallet")
-    # redirect not modified
-    assert response.status_code == 307, f"{response.url} {response.status_code}"
-
-
-# check GET /wallet: do not allow redirects, expect code 307
-@pytest.mark.asyncio
-async def test_get_wallet_no_redirect(client):
-    response = await client.get("wallet", follow_redirects=False)
-    assert response.status_code == 307, f"{response.url} {response.status_code}"
-
-    # determine the next redirect location
-    request = client.build_request("GET", "wallet")
-    i = 0
-    while request is not None:
-        response = await client.send(request)
-        request = response.next_request
-        if i == 0:
-            # first redirect
-            assert response.status_code == 307, f"{response.url} {response.status_code}"
-        elif i == 1:
-            # then get the actual page
-            assert response.status_code == 200, f"{response.url} {response.status_code}"
-        i += 1
-
-
 # check GET /wallet: wrong user, expect 400
 @pytest.mark.asyncio
 async def test_get_wallet_with_nonexistent_user(client):
@@ -43,27 +14,6 @@ async def test_get_wallet_with_nonexistent_user(client):
     assert response.status_code == 400, f"{response.url} {response.status_code}"
 
 
-# check GET /wallet: with user
-@pytest.mark.asyncio
-async def test_get_wallet_with_user(client, to_user):
-    response = await client.get("wallet", params={"usr": to_user.id})
-    assert response.status_code == 307, f"{response.url} {response.status_code}"
-
-    # determine the next redirect location
-    request = client.build_request("GET", "wallet", params={"usr": to_user.id})
-    i = 0
-    while request is not None:
-        response = await client.send(request)
-        request = response.next_request
-        if i == 0:
-            # first redirect
-            assert response.status_code == 307, f"{response.url} {response.status_code}"
-        elif i == 1:
-            # then get the actual page
-            assert response.status_code == 200, f"{response.url} {response.status_code}"
-        i += 1
-
-
 # check GET /wallet: wallet and user
 @pytest.mark.asyncio
 async def test_get_wallet_with_user_and_wallet(client, to_user, to_wallet):
@@ -89,7 +39,7 @@ async def test_get_extensions(client, to_user):
 
 # check GET /extensions: extensions list wrong user, expect 400
 @pytest.mark.asyncio
-async def test_get_extensions_wrong_user(client, to_user):
+async def test_get_extensions_wrong_user(client):
     response = await client.get("extensions", params={"usr": "1"})
     assert response.status_code == 400, f"{response.url} {response.status_code}"