Refactorings

- fix typo in nav.html and template functions
- implement template cache and render function
- refactor motion list methods to reduce cyclomatic complexity
This commit is contained in:
Jan Dittberner 2022-05-21 20:49:35 +02:00
parent ec7623a51a
commit ff93acb65c
4 changed files with 249 additions and 141 deletions

View file

@ -18,17 +18,112 @@ limitations under the License.
package main
import (
"fmt"
"html/template"
"net/http"
"path/filepath"
"strings"
"time"
"git.cacert.org/cacert-boardvoting/internal/models"
"github.com/Masterminds/sprig/v3"
"github.com/gorilla/csrf"
"git.cacert.org/cacert-boardvoting/internal/models"
)
const motionsPerPage = 10
func newTemplateCache() (map[string]*template.Template, error) {
cache := map[string]*template.Template{}
pages, err := filepath.Glob("./ui/html/pages/*.html")
if err != nil {
return nil, fmt.Errorf("could not find page templates: %w", err)
}
for _, page := range pages {
name := filepath.Base(page)
files := []string{
"./ui/html/base.html",
"./ui/html/partials/motion_actions.html",
"./ui/html/partials/motion_display.html",
"./ui/html/partials/motion_status_class.html",
"./ui/html/partials/nav.html",
"./ui/html/partials/pagination.html",
page,
}
funcMaps := sprig.FuncMap()
funcMaps["nl2br"] = func(text string) template.HTML {
// #nosec G203 input is sanitized
return template.HTML(strings.ReplaceAll(template.HTMLEscapeString(text), "\n", "<br>"))
}
funcMaps["canManageUsers"] = func(*models.Voter) bool {
return false
}
funcMaps[csrf.TemplateTag] = csrf.TemplateField
ts, err := template.New("").Funcs(funcMaps).ParseFiles(files...)
if err != nil {
return nil, fmt.Errorf("could not parse templates: %w", err)
}
cache[name] = ts
}
return cache, nil
}
func (app *application) render(w http.ResponseWriter, status int, page string, data interface{}) {
ts, ok := app.templateCache[page]
if !ok {
app.serverError(w, fmt.Errorf("the template %s does not exist", page))
return
}
w.WriteHeader(status)
err := ts.ExecuteTemplate(w, "base", data)
if err != nil {
app.serverError(w, err)
}
}
type motionListTemplateData struct {
Voter *models.Voter
Flashes []string
Params struct {
Flags struct {
Unvoted bool
}
}
PrevPage, NextPage string
Motions []*models.MotionForDisplay
}
func (m *motionListTemplateData) setPaginationParameters(first, last *time.Time) error {
motions := m.Motions
if len(motions) > 0 && first.Before(motions[len(motions)-1].Proposed) {
marshalled, err := motions[len(motions)-1].Proposed.MarshalText()
if err != nil {
return fmt.Errorf("could not serialize timestamp: %w", err)
}
m.NextPage = string(marshalled)
}
if len(motions) > 0 && last.After(motions[0].Proposed) {
marshalled, err := motions[0].Proposed.MarshalText()
if err != nil {
return fmt.Errorf("could not serialize timestamp: %w", err)
}
m.PrevPage = string(marshalled)
}
return nil
}
func (app *application) motionList(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/motions/" {
@ -37,39 +132,20 @@ func (app *application) motionList(w http.ResponseWriter, r *http.Request) {
return
}
var err error
var (
listOptions *models.MotionListOptions
err error
)
listOptions, err = calculateMotionListOptions(r)
if err != nil {
app.clientError(w, http.StatusBadRequest)
return
}
ctx := r.Context()
listOptions := &models.MotionListOptions{Limit: motionsPerPage}
const queryParamBefore = "before"
const queryParamAfter = "after"
if r.URL.Query().Has(queryParamAfter) {
var after time.Time
err := after.UnmarshalText([]byte(r.URL.Query().Get(queryParamAfter)))
if err != nil {
app.clientError(w, http.StatusBadRequest)
return
}
listOptions.After = &after
} else if r.URL.Query().Has(queryParamBefore) {
var before time.Time
err := before.UnmarshalText([]byte(r.URL.Query().Get(queryParamBefore)))
if err != nil {
app.clientError(w, http.StatusBadRequest)
return
}
listOptions.Before = &before
}
motions, err := app.motions.GetMotions(ctx, listOptions)
if err != nil {
app.serverError(w, err)
@ -84,72 +160,48 @@ func (app *application) motionList(w http.ResponseWriter, r *http.Request) {
return
}
files := []string{
"./ui/html/base.html",
"./ui/html/partials/nav.html",
"./ui/html/partials/pagination.html",
"./ui/html/partials/motion_actions.html",
"./ui/html/partials/motion_display.html",
"./ui/html/partials/motion_status_class.html",
"./ui/html/pages/motions.html",
}
templateData := &motionListTemplateData{Motions: motions}
funcMaps := sprig.FuncMap()
funcMaps["nl2br"] = func(text string) template.HTML {
// #nosec G203 input is sanitized
return template.HTML(strings.ReplaceAll(template.HTMLEscapeString(text), "\n", "<br>"))
}
funcMaps["canMangageUsers"] = func(*models.Voter) bool {
return false
}
funcMaps[csrf.TemplateTag] = func() template.HTML {
return csrf.TemplateField(r)
}
ts, err := template.New("").Funcs(funcMaps).ParseFiles(files...)
err = templateData.setPaginationParameters(first, last)
if err != nil {
app.serverError(w, err)
return
}
templateCtx := struct {
Voter *models.Voter
Flashes []string
Params struct {
Flags struct {
Unvoted bool
}
}
PrevPage, NextPage string
Motions []*models.MotionForDisplay
}{Motions: motions}
app.render(w, http.StatusOK, "motions.html", &templateData)
}
if len(motions) > 0 && first.Before(motions[len(motions)-1].Proposed) {
marshalled, err := motions[len(motions)-1].Proposed.MarshalText()
func calculateMotionListOptions(r *http.Request) (*models.MotionListOptions, error) {
const (
queryParamBefore = "before"
queryParamAfter = "after"
motionsPerPage = 10
)
listOptions := &models.MotionListOptions{Limit: motionsPerPage}
if r.URL.Query().Has(queryParamAfter) {
var after time.Time
err := after.UnmarshalText([]byte(r.URL.Query().Get(queryParamAfter)))
if err != nil {
app.serverError(w, err)
return
return nil, fmt.Errorf("could not unmarshal timestamp: %w", err)
}
templateCtx.NextPage = string(marshalled)
}
if len(motions) > 0 && last.After(motions[0].Proposed) {
marshalled, err := motions[0].Proposed.MarshalText()
listOptions.After = &after
} else if r.URL.Query().Has(queryParamBefore) {
var before time.Time
err := before.UnmarshalText([]byte(r.URL.Query().Get(queryParamBefore)))
if err != nil {
app.serverError(w, err)
return
return nil, fmt.Errorf("could not unmarshal timestamp: %w", err)
}
templateCtx.PrevPage = string(marshalled)
listOptions.Before = &before
}
err = ts.ExecuteTemplate(w, "base", templateCtx)
if err != nil {
app.serverError(w, err)
}
return listOptions, nil
}
func (app *application) home(w http.ResponseWriter, r *http.Request) {

View file

@ -23,6 +23,7 @@ import (
"database/sql"
"flag"
"fmt"
"html/template"
"log"
"net/http"
"os"
@ -48,6 +49,7 @@ type application struct {
mailNotifier *MailNotifier
mailConfig *mailConfig
baseURL string
templateCache map[string]*template.Template
}
func main() {
@ -82,13 +84,19 @@ func main() {
errorLog.Fatalf("could not setup decision model: %v", err)
}
templateCache, err := newTemplateCache()
if err != nil {
errorLog.Fatal(err)
}
app := &application{
errorLog: errorLog,
infoLog: infoLog,
motions: &models.MotionModel{DB: db, InfoLog: infoLog},
voters: &models.VoterModel{DB: db},
mailConfig: config.MailConfig,
baseURL: config.BaseURL,
errorLog: errorLog,
infoLog: infoLog,
motions: &models.MotionModel{DB: db, InfoLog: infoLog},
voters: &models.VoterModel{DB: db},
mailConfig: config.MailConfig,
baseURL: config.BaseURL,
templateCache: templateCache,
}
app.NewMailNotifier()

View file

@ -436,62 +436,22 @@ func (m *MotionModel) TimestampRange(ctx context.Context) (*time.Time, *time.Tim
}
func (m *MotionModel) GetMotions(ctx context.Context, options *MotionListOptions) ([]*MotionForDisplay, error) {
var rows *sqlx.Rows
var err error
var (
rows *sqlx.Rows
err error
)
if options.Before != nil {
rows, err = m.DB.QueryxContext(
ctx,
`SELECT decisions.id, decisions.tag, decisions.proponent, voters.name AS proposer, decisions.proposed, decisions.title,
decisions.content, decisions.votetype, decisions.status, decisions.due, decisions.modified
FROM decisions
JOIN voters ON decisions.proponent=voters.id
WHERE decisions.proposed < $1
ORDER BY proposed DESC
LIMIT $2`,
options.Before,
options.Limit,
)
} else if options.After != nil {
rows, err = m.DB.QueryxContext(
ctx,
`WITH display_decision AS (SELECT decisions.id,
decisions.tag,
decisions.proponent,
voters.name AS proposer,
decisions.proposed,
decisions.title,
decisions.content,
decisions.votetype,
decisions.status,
decisions.due,
decisions.modified
FROM decisions
JOIN voters ON decisions.proponent = voters.id
WHERE decisions.proposed > $1
ORDER BY proposed
LIMIT $2)
SELECT *
FROM display_decision
ORDER BY proposed DESC`,
options.After,
options.Limit,
)
} else {
rows, err = m.DB.QueryxContext(
ctx,
`SELECT decisions.id, decisions.tag, decisions.proponent, voters.name AS proposer, decisions.proposed, decisions.title,
decisions.content, decisions.votetype, decisions.status, decisions.due, decisions.modified
FROM decisions
JOIN voters ON decisions.proponent=voters.id
ORDER BY proposed DESC
LIMIT $1`,
options.Limit,
)
switch {
case options.Before != nil:
rows, err = m.GetMotionRowsBefore(ctx, options)
case options.After != nil:
rows, err = m.GetMotionRowsAfter(ctx, options)
default:
rows, err = m.GetFirstMotionRows(ctx, options)
}
if err != nil {
return nil, fmt.Errorf("could not execute query: %w", err)
return nil, err
}
defer func(rows *sqlx.Rows) {
@ -578,3 +538,91 @@ func (m *MotionModel) FillVoteSums(ctx context.Context, decisions []*MotionForDi
return nil
}
func (m *MotionModel) GetMotionRowsBefore(ctx context.Context, options *MotionListOptions) (*sqlx.Rows, error) {
rows, err := m.DB.QueryxContext(
ctx,
`SELECT decisions.id,
decisions.tag,
decisions.proponent,
voters.name AS proposer,
decisions.proposed,
decisions.title,
decisions.content,
decisions.votetype,
decisions.status,
decisions.due,
decisions.modified
FROM decisions
JOIN voters ON decisions.proponent = voters.id
WHERE decisions.proposed < $1
ORDER BY proposed DESC
LIMIT $2`,
options.Before,
options.Limit,
)
if err != nil {
return nil, fmt.Errorf("could not query motions before %s: %w", options.Before, err)
}
return rows, nil
}
func (m *MotionModel) GetMotionRowsAfter(ctx context.Context, options *MotionListOptions) (*sqlx.Rows, error) {
rows, err := m.DB.QueryxContext(
ctx,
`WITH display_decision AS (SELECT decisions.id,
decisions.tag,
decisions.proponent,
voters.name AS proposer,
decisions.proposed,
decisions.title,
decisions.content,
decisions.votetype,
decisions.status,
decisions.due,
decisions.modified
FROM decisions
JOIN voters ON decisions.proponent = voters.id
WHERE decisions.proposed > $1
ORDER BY proposed
LIMIT $2)
SELECT *
FROM display_decision
ORDER BY proposed DESC`,
options.After,
options.Limit,
)
if err != nil {
return nil, fmt.Errorf("could not query motions after %s: %w", options.After, err)
}
return rows, nil
}
func (m *MotionModel) GetFirstMotionRows(ctx context.Context, options *MotionListOptions) (*sqlx.Rows, error) {
rows, err := m.DB.QueryxContext(
ctx,
`SELECT decisions.id,
decisions.tag,
decisions.proponent,
voters.name AS proposer,
decisions.proposed,
decisions.title,
decisions.content,
decisions.votetype,
decisions.status,
decisions.due,
decisions.modified
FROM decisions
JOIN voters ON decisions.proponent = voters.id
ORDER BY proposed DESC
LIMIT $1`,
options.Limit,
)
if err != nil {
return nil, fmt.Errorf("could not query motions: %w", err)
}
return rows, nil
}

View file

@ -1,5 +1,5 @@
{{ define "nav" }}
{{ if .Voter | canMangageUsers }}
{{ if .Voter | canManageUsers }}
<nav class="ui top attached tabular menu">
<a class="active item" href="/motions/">Motions</a>
<a class="item" href="/users/">User management</a>