Fix/cleanup create app screen #118
Conversation
|
This looks a bit wrong now. I wonder if we should remove the top section? if now you can create an app without permission to do payments in some cases (for creating invoices in the future) |
|
@stackingsaunter we probably need some hirachy in the permissions here now?
|
| for _, appPerm := range appPermissions { | ||
| if appPerm.RequestMethod == NIP_47_PAY_INVOICE_METHOD { | ||
| //find the pay_invoice-specific permissions |
There was a problem hiding this comment.
What if there is no pay permission for this app? won't this break? (Edit: for example the expiry will not show on the show screen currently)
There was a problem hiding this comment.
Could it be renamed to appPayInvoicePermission?
There was a problem hiding this comment.
I would name it paySpecificPermission or something like that then.
There was a problem hiding this comment.
Maybe the AppPermission is the wrong place for the expiry, since it should be the same across all permissions for an app?
There was a problem hiding this comment.
I don't see any problems with it. Yes it will be the same across all permissions.
There was a problem hiding this comment.
Could we get the first permission of the app if it exists then to display the expiry in show.html? right now it will not show the expiry for apps that don't have a pay permission
There was a problem hiding this comment.
If we really don't wish to give payInvoice special treatment then ig we shouldn't also send the paySpecificPermission to the frontend separately. Let's send ExpiresAt, MaxAmount and BudgetRenewal just like we send BudgetUsage
renewsIn := ""
budgetRenewal := ""
budgetUsage := int64(0)
maxAmount := 0
expiresAt := time.Time{}
for _, appPerm := range appPermissions {
if expiresAt.isZero() && !appPerm.ExpiresAt.isZero() {
expiresAt = appPerm.ExpiresAt
}
if appPerm.RequestMethod == NIP_47_PAY_INVOICE_METHOD {
//find the pay_invoice-specific permissions
if appPerm.MaxAmount > 0 {
budgetUsage = svc.GetBudgetUsage(&appPerm)
budgetRenewal = appPerm.BudgetRenewal
endOfBudget := GetEndOfBudget(budgetRenewal, app.CreatedAt)
renewsIn = getEndOfBudgetString(endOfBudget)
}
}
requestMethods = append(requestMethods, nip47MethodDescriptions[appPerm.RequestMethod])
}
// and then send all of them to frontend
We can probably put the bottom section on top and drop the top section, yes. |
|
All permissions can have an expiry. And I would just leave the budget like it is and have it be meaningless when set for non-paying requests. |
|
echo_handlers.go
Outdated
| renewsIn = getEndOfBudgetString(endOfBudget) | ||
| } | ||
|
|
||
| return c.Render(http.StatusOK, "apps/show.html", map[string]interface{}{ | ||
| "App": app, | ||
| "AppPermission": appPermission, | ||
| "AppPermission": paySpecificPermission, |
There was a problem hiding this comment.
Could "AppPermission" also be renamed?
| {{end}} | ||
| {{end}} | ||
| {{ if not .AppPermission.ExpiresAt.IsZero}} | ||
| {{ if not .ExpiresAt.IsZero}} |
There was a problem hiding this comment.
A few lines down it uses the wrong value {{.AppPermission.ExpiresAt}} should be {{.ExpiresAt}}
There is currently an issue because there are several places where the
pay_invoicepermission is handled in a special way. In this PR this is mostly removed. This causes a bug in the current master where an app that has bothget_balanceandpay_invoicepermissions is not in fact able to pay invoices.