From c59825f3a5f4c4ccaac25e2d57b19a4e3084d1df Mon Sep 17 00:00:00 2001 From: Kailash Nadh Date: Sat, 11 Nov 2023 22:26:25 +0530 Subject: [PATCH] Fix broken sorting (lists -> subcount, subscribers -> status) in queries. Closes #1076. --- frontend/cypress/e2e/subscribers.cy.js | 21 +++++++++++++++++---- internal/core/campaigns.go | 2 +- internal/core/core.go | 10 ++++++---- internal/core/lists.go | 4 ++-- internal/core/subscribers.go | 4 ---- queries.sql | 9 +++++---- 6 files changed, 31 insertions(+), 19 deletions(-) diff --git a/frontend/cypress/e2e/subscribers.cy.js b/frontend/cypress/e2e/subscribers.cy.js index 63a8717..338bcea 100644 --- a/frontend/cypress/e2e/subscribers.cy.js +++ b/frontend/cypress/e2e/subscribers.cy.js @@ -234,9 +234,9 @@ describe('Subscribers', () => { }); it('Sorts subscribers', () => { - const asc = [3, 4, 5, 6, 7, 8]; - const desc = [8, 7, 6, 5, 4, 3]; - const cases = ['cy-status', 'cy-email', 'cy-name', 'cy-created_at', 'cy-updated_at']; + let asc = [3, 4, 5, 6, 7, 8]; + let desc = [8, 7, 6, 5, 4, 3]; + let cases = ['cy-email', 'cy-name', 'cy-created_at', 'cy-updated_at']; cases.forEach((c) => { cy.sortTable(`thead th.${c}`, asc); @@ -244,6 +244,19 @@ describe('Subscribers', () => { cy.sortTable(`thead th.${c}`, desc); cy.wait(250); }); + + + asc = [4, 6, 8, 3, 5, 7]; + desc = [7, 5, 3, 8, 6, 4]; + cases = ['cy-status']; + + cases.forEach((c) => { + cy.sortTable(`thead th.${c}`, asc); + cy.wait(250); + cy.sortTable(`thead th.${c}`, desc); + cy.wait(250); + }); + }); }); @@ -339,7 +352,7 @@ describe('Domain blocklist', () => { cy.get('.b-tabs nav a').eq(2).click(); cy.get('textarea[name="privacy.domain_blocklist"]').clear(); cy.get('[data-cy=btn-save]').click(); - cy.wait(1000); + cy.wait(3000); // Add banned domain. cy.request({ diff --git a/internal/core/campaigns.go b/internal/core/campaigns.go index 6194aa4..9733c08 100644 --- a/internal/core/campaigns.go +++ b/internal/core/campaigns.go @@ -24,7 +24,7 @@ const ( // QueryCampaigns retrieves paginated campaigns optionally filtering them by the given arbitrary // query expression. It also returns the total number of records in the DB. func (c *Core) QueryCampaigns(searchStr string, statuses []string, orderBy, order string, offset, limit int) (models.Campaigns, int, error) { - queryStr, stmt := makeSearchQuery(searchStr, orderBy, order, c.q.QueryCampaigns) + queryStr, stmt := makeSearchQuery(searchStr, orderBy, order, c.q.QueryCampaigns, campQuerySortFields) if statuses == nil { statuses = []string{} diff --git a/internal/core/core.go b/internal/core/core.go index 54a4677..0dd993c 100644 --- a/internal/core/core.go +++ b/internal/core/core.go @@ -57,9 +57,11 @@ type Opt struct { } var ( - regexFullTextQuery = regexp.MustCompile(`\s+`) - regexpSpaces = regexp.MustCompile(`[\s]+`) - querySortFields = []string{"name", "status", "created_at", "updated_at"} + regexFullTextQuery = regexp.MustCompile(`\s+`) + regexpSpaces = regexp.MustCompile(`[\s]+`) + campQuerySortFields = []string{"name", "status", "created_at", "updated_at"} + subQuerySortFields = []string{"email", "status", "name", "created_at", "updated_at"} + listQuerySortFields = []string{"name", "status", "created_at", "updated_at", "subscriber_count"} ) // New returns a new instance of the core. @@ -88,7 +90,7 @@ func pqErrMsg(err error) string { // makeSearchQuery cleans an optional search string and prepares the // query SQL statement (string interpolated) and returns the // search query string along with the SQL expression. -func makeSearchQuery(searchStr, orderBy, order, query string) (string, string) { +func makeSearchQuery(searchStr, orderBy, order, query string, querySortFields []string) (string, string) { if searchStr != "" { searchStr = `%` + string(regexFullTextQuery.ReplaceAll([]byte(searchStr), []byte("&"))) + `%` } diff --git a/internal/core/lists.go b/internal/core/lists.go index 28ce7ed..e66eba0 100644 --- a/internal/core/lists.go +++ b/internal/core/lists.go @@ -39,7 +39,7 @@ func (c *Core) GetLists(typ string) ([]models.List, error) { func (c *Core) QueryLists(searchStr, orderBy, order string, offset, limit int) ([]models.List, int, error) { out := []models.List{} - queryStr, stmt := makeSearchQuery(searchStr, orderBy, order, c.q.QueryLists) + queryStr, stmt := makeSearchQuery(searchStr, orderBy, order, c.q.QueryLists, listQuerySortFields) if err := c.db.Select(&out, stmt, 0, "", queryStr, offset, limit); err != nil { c.log.Printf("error fetching lists: %v", err) @@ -75,7 +75,7 @@ func (c *Core) GetList(id int, uuid string) (models.List, error) { } var res []models.List - queryStr, stmt := makeSearchQuery("", "", "", c.q.QueryLists) + queryStr, stmt := makeSearchQuery("", "", "", c.q.QueryLists, nil) if err := c.db.Select(&res, stmt, id, uu, queryStr, 0, 1); err != nil { c.log.Printf("error fetching lists: %v", err) return models.List{}, echo.NewHTTPError(http.StatusInternalServerError, diff --git a/internal/core/subscribers.go b/internal/core/subscribers.go index b31c352..3b0920b 100644 --- a/internal/core/subscribers.go +++ b/internal/core/subscribers.go @@ -14,10 +14,6 @@ import ( "github.com/lib/pq" ) -var ( - subQuerySortFields = []string{"email", "name", "created_at", "updated_at"} -) - // GetSubscriber fetches a subscriber by one of the given params. func (c *Core) GetSubscriber(id int, uuid, email string) (models.Subscriber, error) { var uu interface{} diff --git a/queries.sql b/queries.sql index d6412c4..92972b3 100644 --- a/queries.sql +++ b/queries.sql @@ -416,15 +416,16 @@ WITH ls AS ( WHEN $3 != '' THEN to_tsvector(name) @@ to_tsquery ($3) ELSE true END - ORDER BY %order% OFFSET $4 LIMIT (CASE WHEN $5 < 1 THEN NULL ELSE $5 END) ), counts AS ( - SELECT list_id, JSON_OBJECT_AGG(status, subscriber_count) AS subscriber_statuses FROM ( - SELECT COUNT(*) as subscriber_count, list_id, status FROM subscriber_lists + SELECT list_id, JSON_OBJECT_AGG(status, num) AS subscriber_statuses, SUM(num) AS subscriber_count + FROM ( + SELECT list_id, status, COUNT(*) as num + FROM subscriber_lists WHERE ($1 = 0 OR list_id = $1) GROUP BY list_id, status - ) row GROUP BY list_id + ) AS subquery GROUP BY list_id ) SELECT ls.*, subscriber_statuses FROM ls LEFT JOIN counts ON (counts.list_id = ls.id) ORDER BY %order%;