Fix broken sorting in list query. Closes #1076.

- Replace "%s %s" sprintf substitution in some raw SQL queries with named
  string replace params. Eg: `%order%`.
This commit is contained in:
Kailash Nadh 2022-12-25 14:04:43 +05:30
parent 4dee2e9a1b
commit 49f20f33db
4 changed files with 15 additions and 9 deletions

View file

@ -3,6 +3,7 @@ package core
import ( import (
"fmt" "fmt"
"net/http" "net/http"
"strings"
"github.com/knadh/listmonk/models" "github.com/knadh/listmonk/models"
"github.com/labstack/echo/v4" "github.com/labstack/echo/v4"
@ -22,7 +23,7 @@ func (c *Core) QueryBounces(campID, subID int, source, orderBy, order string, of
} }
out := []models.Bounce{} out := []models.Bounce{}
stmt := fmt.Sprintf(c.q.QueryBounces, orderBy, order) stmt := strings.ReplaceAll(c.q.QueryBounces, "%order%", orderBy+" "+order)
if err := c.db.Select(&out, stmt, 0, campID, subID, source, offset, limit); err != nil { if err := c.db.Select(&out, stmt, 0, campID, subID, source, offset, limit); err != nil {
c.log.Printf("error fetching bounces: %v", err) c.log.Printf("error fetching bounces: %v", err)
return nil, 0, echo.NewHTTPError(http.StatusInternalServerError, return nil, 0, echo.NewHTTPError(http.StatusInternalServerError,

View file

@ -99,7 +99,9 @@ func makeSearchQuery(searchStr, orderBy, order, query string) (string, string) {
order = SortDesc order = SortDesc
} }
return searchStr, fmt.Sprintf(query, orderBy, order) query = strings.ReplaceAll(query, "%order%", orderBy+" "+order)
return searchStr, query
} }
// strSliceContains checks if a string is present in the string slice. // strSliceContains checks if a string is present in the string slice.

View file

@ -113,7 +113,8 @@ func (c *Core) QuerySubscribers(query string, listIDs []int, order, orderBy stri
// Run the query again and fetch the actual data. stmt is the raw SQL query. // Run the query again and fetch the actual data. stmt is the raw SQL query.
var out models.Subscribers var out models.Subscribers
stmt = fmt.Sprintf(c.q.QuerySubscribers, cond, orderBy, order) stmt = strings.ReplaceAll(c.q.QuerySubscribers, "%query%", cond)
stmt = strings.ReplaceAll(stmt, "%order%", orderBy+" "+order)
if err := tx.Select(&out, stmt, pq.Array(listIDs), offset, limit); err != nil { if err := tx.Select(&out, stmt, pq.Array(listIDs), offset, limit); err != nil {
return nil, 0, echo.NewHTTPError(http.StatusInternalServerError, return nil, 0, echo.NewHTTPError(http.StatusInternalServerError,
c.i18n.Ts("globals.messages.errorFetching", "name", "{globals.terms.subscribers}", "error", pqErrMsg(err))) c.i18n.Ts("globals.messages.errorFetching", "name", "{globals.terms.subscribers}", "error", pqErrMsg(err)))
@ -186,6 +187,7 @@ func (c *Core) ExportSubscribers(query string, subIDs, listIDs []int, batchSize
} }
stmt := fmt.Sprintf(c.q.QuerySubscribersForExport, cond) stmt := fmt.Sprintf(c.q.QuerySubscribersForExport, cond)
stmt = strings.ReplaceAll(c.q.QuerySubscribersForExport, "%query%", cond)
// Verify that the arbitrary SQL search expression is read only. // Verify that the arbitrary SQL search expression is read only.
if cond != "" { if cond != "" {

View file

@ -295,8 +295,8 @@ SELECT subscribers.* FROM subscribers
AND subscriber_lists.subscriber_id = subscribers.id AND subscriber_lists.subscriber_id = subscribers.id
) )
WHERE (CARDINALITY($1) = 0 OR subscriber_lists.list_id = ANY($1::INT[])) WHERE (CARDINALITY($1) = 0 OR subscriber_lists.list_id = ANY($1::INT[]))
%s %query%
ORDER BY %s %s OFFSET $2 LIMIT (CASE WHEN $3 < 1 THEN NULL ELSE $3 END); ORDER BY %order% OFFSET $2 LIMIT (CASE WHEN $3 < 1 THEN NULL ELSE $3 END);
-- name: query-subscribers-count -- name: query-subscribers-count
-- Replica of query-subscribers for obtaining the results count. -- Replica of query-subscribers for obtaining the results count.
@ -331,7 +331,7 @@ SELECT subscribers.id,
) )
WHERE sl.list_id = ALL($1::INT[]) AND id > $2 WHERE sl.list_id = ALL($1::INT[]) AND id > $2
AND (CASE WHEN CARDINALITY($3::INT[]) > 0 THEN id=ANY($3) ELSE true END) AND (CASE WHEN CARDINALITY($3::INT[]) > 0 THEN id=ANY($3) ELSE true END)
%s %query%
ORDER BY subscribers.id ASC LIMIT (CASE WHEN $4 < 1 THEN NULL ELSE $4 END); ORDER BY subscribers.id ASC LIMIT (CASE WHEN $4 < 1 THEN NULL ELSE $4 END);
-- name: query-subscribers-template -- name: query-subscribers-template
@ -403,6 +403,7 @@ WITH ls AS (
WHEN $3 != '' THEN to_tsvector(name) @@ to_tsquery ($3) WHEN $3 != '' THEN to_tsvector(name) @@ to_tsquery ($3)
ELSE true ELSE true
END END
ORDER BY %order%
OFFSET $4 LIMIT (CASE WHEN $5 < 1 THEN NULL ELSE $5 END) OFFSET $4 LIMIT (CASE WHEN $5 < 1 THEN NULL ELSE $5 END)
), ),
counts AS ( counts AS (
@ -413,7 +414,7 @@ counts AS (
) row GROUP BY list_id ) row GROUP BY list_id
) )
SELECT ls.*, subscriber_statuses FROM ls SELECT ls.*, subscriber_statuses FROM ls
LEFT JOIN counts ON (counts.list_id = ls.id) ORDER BY %s %s; LEFT JOIN counts ON (counts.list_id = ls.id) ORDER BY %order%;
-- name: get-lists-by-optin -- name: get-lists-by-optin
@ -504,7 +505,7 @@ FROM campaigns c
WHERE ($1 = 0 OR id = $1) WHERE ($1 = 0 OR id = $1)
AND status=ANY(CASE WHEN CARDINALITY($2::campaign_status[]) != 0 THEN $2::campaign_status[] ELSE ARRAY[status] END) AND status=ANY(CASE WHEN CARDINALITY($2::campaign_status[]) != 0 THEN $2::campaign_status[] ELSE ARRAY[status] END)
AND ($3 = '' OR TO_TSVECTOR(CONCAT(name, ' ', subject)) @@ TO_TSQUERY($3)) AND ($3 = '' OR TO_TSVECTOR(CONCAT(name, ' ', subject)) @@ TO_TSQUERY($3))
ORDER BY %s %s OFFSET $4 LIMIT (CASE WHEN $5 < 1 THEN NULL ELSE $5 END); ORDER BY %order% OFFSET $4 LIMIT (CASE WHEN $5 < 1 THEN NULL ELSE $5 END);
-- name: get-campaign -- name: get-campaign
SELECT campaigns.*, SELECT campaigns.*,
@ -1018,7 +1019,7 @@ WHERE ($1 = 0 OR bounces.id = $1)
AND ($2 = 0 OR bounces.campaign_id = $2) AND ($2 = 0 OR bounces.campaign_id = $2)
AND ($3 = 0 OR bounces.subscriber_id = $3) AND ($3 = 0 OR bounces.subscriber_id = $3)
AND ($4 = '' OR bounces.source = $4) AND ($4 = '' OR bounces.source = $4)
ORDER BY %s %s OFFSET $5 LIMIT $6; ORDER BY %order% OFFSET $5 LIMIT $6;
-- name: delete-bounces -- name: delete-bounces
DELETE FROM bounces WHERE CARDINALITY($1::INT[]) = 0 OR id = ANY($1); DELETE FROM bounces WHERE CARDINALITY($1::INT[]) = 0 OR id = ANY($1);