knadh/listmonk
1
1
Fork 0
mirror of https://github.com/knadh/listmonk.git synced 2024-09-19 23:06:32 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix incorrect bulk blocklisting behaviour (#2041). Fixes #1841

Browse source
This commit is contained in:
Bowrna 2024-09-19 10:56:56 +05:30 committed by GitHub
parent 550cd3e1f8
commit 16f4dfd3e9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 34 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
cmd/subscribers.go
View file

@ -22,12 +22,13 @@ const (
// subQueryReq is a "catch all" struct for reading various
// subscriber related requests.
type subQueryReq struct {
Query string `json:"query"`
ListIDs []int `json:"list_ids"`
TargetListIDs []int `json:"target_list_ids"`
SubscriberIDs []int `json:"ids"`
Action string `json:"action"`
Status string `json:"status"`
Query string `json:"query"`
ListIDs []int `json:"list_ids"`
TargetListIDs []int `json:"target_list_ids"`
SubscriberIDs []int `json:"ids"`
Action string `json:"action"`
Status string `json:"status"`
SubscriptionStatus string `json:"subscription_status"`
}
// subProfileData represents a subscriber's collated data in JSON
@ -415,7 +416,7 @@ func handleDeleteSubscribersByQuery(c echo.Context) error {
return err
}
if err := app.core.DeleteSubscribersByQuery(req.Query, req.ListIDs); err != nil {
if err := app.core.DeleteSubscribersByQuery(req.Query, req.ListIDs, req.SubscriptionStatus); err != nil {
return err
}
@ -434,7 +435,7 @@ func handleBlocklistSubscribersByQuery(c echo.Context) error {
return err
}
if err := app.core.BlocklistSubscribersByQuery(req.Query, req.ListIDs); err != nil {
if err := app.core.BlocklistSubscribersByQuery(req.Query, req.ListIDs, req.SubscriptionStatus); err != nil {
return err
}
@ -461,11 +462,11 @@ func handleManageSubscriberListsByQuery(c echo.Context) error {
var err error
switch req.Action {
case "add":
err = app.core.AddSubscriptionsByQuery(req.Query, req.ListIDs, req.TargetListIDs, req.Status)
err = app.core.AddSubscriptionsByQuery(req.Query, req.ListIDs, req.TargetListIDs, req.Status, req.SubscriptionStatus)
case "remove":
err = app.core.DeleteSubscriptionsByQuery(req.Query, req.ListIDs, req.TargetListIDs)
err = app.core.DeleteSubscriptionsByQuery(req.Query, req.ListIDs, req.TargetListIDs, req.SubscriptionStatus)
case "unsubscribe":
err = app.core.UnsubscribeListsByQuery(req.Query, req.ListIDs, req.TargetListIDs)
err = app.core.UnsubscribeListsByQuery(req.Query, req.ListIDs, req.TargetListIDs, req.SubscriptionStatus)
default:
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.invalidAction"))
}

4
frontend/src/views/Subscribers.vue
View file

@ -375,6 +375,7 @@ export default Vue.extend({
this.$api.blocklistSubscribersByQuery({
query: this.queryParams.queryExp,
list_ids: this.queryParams.listID ? [this.queryParams.listID] : null,
subscription_status: this.queryParams.subStatus,
}).then(() => this.querySubscribers());
};
}
@ -426,6 +427,7 @@ export default Vue.extend({
this.$api.deleteSubscribersByQuery({
query: this.queryParams.queryExp,
list_ids: this.queryParams.listID ? [this.queryParams.listID] : null,
subscription_status: this.queryParams.subStatus,
}).then(() => {
this.querySubscribers();
@ -460,6 +462,7 @@ export default Vue.extend({
} else {
// 'All' is selected, perform by query.
data.query = this.queryParams.queryExp;
data.subscription_status = this.queryParams.subStatus;
fn = this.$api.addSubscribersToListsByQuery;
}
@ -503,7 +506,6 @@ export default Vue.extend({
// Get subscribers on load.
this.querySubscribers();
}
if (this.$route.query.subscription_status) {
this.queryParams.subStatus = this.$route.query.subscription_status;
}

8
internal/core/subscribers.go
View file

@ -394,8 +394,8 @@ func (c *Core) BlocklistSubscribers(subIDs []int) error {
}
// BlocklistSubscribersByQuery blocklists the given list of subscribers.
func (c *Core) BlocklistSubscribersByQuery(query string, listIDs []int) error {
if err := c.q.ExecSubQueryTpl(sanitizeSQLExp(query), c.q.BlocklistSubscribersByQuery, listIDs, c.db); err != nil {
func (c *Core) BlocklistSubscribersByQuery(query string, listIDs []int, subStatus string) error {
if err := c.q.ExecSubQueryTpl(sanitizeSQLExp(query), c.q.BlocklistSubscribersByQuery, listIDs, c.db, subStatus); err != nil {
c.log.Printf("error blocklisting subscribers: %v", err)
return echo.NewHTTPError(http.StatusInternalServerError,
c.i18n.Ts("subscribers.errorBlocklisting", "error", pqErrMsg(err)))
@ -423,8 +423,8 @@ func (c *Core) DeleteSubscribers(subIDs []int, subUUIDs []string) error {
}
// DeleteSubscribersByQuery deletes subscribers by a given arbitrary query expression.
func (c *Core) DeleteSubscribersByQuery(query string, listIDs []int) error {
err := c.q.ExecSubQueryTpl(sanitizeSQLExp(query), c.q.DeleteSubscribersByQuery, listIDs, c.db)
func (c *Core) DeleteSubscribersByQuery(query string, listIDs []int, subStatus string) error {
err := c.q.ExecSubQueryTpl(sanitizeSQLExp(query), c.q.DeleteSubscribersByQuery, listIDs, c.db, subStatus)
if err != nil {
c.log.Printf("error deleting subscribers: %v", err)
return echo.NewHTTPError(http.StatusInternalServerError,

12
internal/core/subscriptions.go
View file

@ -35,12 +35,12 @@ func (c *Core) AddSubscriptions(subIDs, listIDs []int, status string) error {
// AddSubscriptionsByQuery adds list subscriptions to subscribers by a given arbitrary query expression.
// sourceListIDs is the list of list IDs to filter the subscriber query with.
func (c *Core) AddSubscriptionsByQuery(query string, sourceListIDs, targetListIDs []int, status string) error {
func (c *Core) AddSubscriptionsByQuery(query string, sourceListIDs, targetListIDs []int, status string, subStatus string) error {
if sourceListIDs == nil {
sourceListIDs = []int{}
}
err := c.q.ExecSubQueryTpl(sanitizeSQLExp(query), c.q.AddSubscribersToListsByQuery, sourceListIDs, c.db, pq.Array(targetListIDs), status)
err := c.q.ExecSubQueryTpl(sanitizeSQLExp(query), c.q.AddSubscribersToListsByQuery, sourceListIDs, c.db, subStatus, pq.Array(targetListIDs), status)
if err != nil {
c.log.Printf("error adding subscriptions by query: %v", err)
return echo.NewHTTPError(http.StatusInternalServerError,
@ -64,12 +64,12 @@ func (c *Core) DeleteSubscriptions(subIDs, listIDs []int) error {
// DeleteSubscriptionsByQuery deletes list subscriptions from subscribers by a given arbitrary query expression.
// sourceListIDs is the list of list IDs to filter the subscriber query with.
func (c *Core) DeleteSubscriptionsByQuery(query string, sourceListIDs, targetListIDs []int) error {
func (c *Core) DeleteSubscriptionsByQuery(query string, sourceListIDs, targetListIDs []int, subStatus string) error {
if sourceListIDs == nil {
sourceListIDs = []int{}
}
err := c.q.ExecSubQueryTpl(sanitizeSQLExp(query), c.q.DeleteSubscriptionsByQuery, sourceListIDs, c.db, pq.Array(targetListIDs))
err := c.q.ExecSubQueryTpl(sanitizeSQLExp(query), c.q.DeleteSubscriptionsByQuery, sourceListIDs, c.db, subStatus, pq.Array(targetListIDs))
if err != nil {
c.log.Printf("error deleting subscriptions by query: %v", err)
return echo.NewHTTPError(http.StatusInternalServerError,
@ -92,12 +92,12 @@ func (c *Core) UnsubscribeLists(subIDs, listIDs []int, listUUIDs []string) error
// UnsubscribeListsByQuery sets list subscriptions to 'unsubscribed' by a given arbitrary query expression.
// sourceListIDs is the list of list IDs to filter the subscriber query with.
func (c *Core) UnsubscribeListsByQuery(query string, sourceListIDs, targetListIDs []int) error {
func (c *Core) UnsubscribeListsByQuery(query string, sourceListIDs, targetListIDs []int, subStatus string) error {
if sourceListIDs == nil {
sourceListIDs = []int{}
}
err := c.q.ExecSubQueryTpl(sanitizeSQLExp(query), c.q.UnsubscribeSubscribersFromListsByQuery, sourceListIDs, c.db, pq.Array(targetListIDs))
err := c.q.ExecSubQueryTpl(sanitizeSQLExp(query), c.q.UnsubscribeSubscribersFromListsByQuery, sourceListIDs, c.db, subStatus, pq.Array(targetListIDs))
if err != nil {
c.log.Printf("error unsubscribing from lists by query: %v", err)
return echo.NewHTTPError(http.StatusInternalServerError,

12
models/queries.go
View file

@ -114,7 +114,7 @@ type Queries struct {
// out of it using the raw `query-subscribers-template` query template.
// While doing this, a readonly transaction is created and the query is
// dry run on it to ensure that it is indeed readonly.
func (q *Queries) CompileSubscriberQueryTpl(exp string, db *sqlx.DB) (string, error) {
func (q *Queries) CompileSubscriberQueryTpl(exp string, db *sqlx.DB, subStatus string) (string, error) {
tx, err := db.BeginTxx(context.Background(), &sql.TxOptions{ReadOnly: true})
if err != nil {
return "", err
@ -126,19 +126,18 @@ func (q *Queries) CompileSubscriberQueryTpl(exp string, db *sqlx.DB) (string, er
exp = " AND " + exp
}
stmt := fmt.Sprintf(q.QuerySubscribersTpl, exp)
if _, err := tx.Exec(stmt, true, pq.Int64Array{}); err != nil {
if _, err := tx.Exec(stmt, true, pq.Int64Array{}, subStatus); err != nil {
return "", err
}
return stmt, nil
}
// compileSubscriberQueryTpl takes an arbitrary WHERE expressions and a subscriber
// query template that depends on the filter (eg: delete by query, blocklist by query etc.)
// combines and executes them.
func (q *Queries) ExecSubQueryTpl(exp, tpl string, listIDs []int, db *sqlx.DB, args ...interface{}) error {
func (q *Queries) ExecSubQueryTpl(exp, tpl string, listIDs []int, db *sqlx.DB, subStatus string, args ...interface{}) error {
// Perform a dry run.
filterExp, err := q.CompileSubscriberQueryTpl(exp, db)
filterExp, err := q.CompileSubscriberQueryTpl(exp, db, subStatus)
if err != nil {
return err
}
@ -148,10 +147,9 @@ func (q *Queries) ExecSubQueryTpl(exp, tpl string, listIDs []int, db *sqlx.DB, a
}
// First argument is the boolean indicating if the query is a dry run.
a := append([]interface{}{false, pq.Array(listIDs)}, args...)
a := append([]interface{}{false, pq.Array(listIDs), subStatus}, args...)
if _, err := db.Exec(fmt.Sprintf(tpl, filterExp), a...); err != nil {
return err
}
return nil
}

7
queries.sql
View file

@ -370,6 +370,7 @@ ON (
-- Optional list filtering.
(CASE WHEN CARDINALITY($2::INT[]) > 0 THEN true ELSE false END)
AND subscriber_lists.subscriber_id = subscribers.id
AND ($3 = '' OR subscriber_lists.status = $3::subscription_status)
)
WHERE subscriber_lists.list_id = ALL($2::INT[]) %s
LIMIT (CASE WHEN $1 THEN 1 END)
@ -393,20 +394,20 @@ UPDATE subscriber_lists SET status='unsubscribed', updated_at=NOW()
-- raw: true
WITH subs AS (%s)
INSERT INTO subscriber_lists (subscriber_id, list_id, status)
(SELECT a, b, (CASE WHEN $4 != '' THEN $4::subscription_status ELSE 'unconfirmed' END) FROM UNNEST(ARRAY(SELECT id FROM subs)) a, UNNEST($3::INT[]) b)
(SELECT a, b, (CASE WHEN $5 != '' THEN $5::subscription_status ELSE 'unconfirmed' END) FROM UNNEST(ARRAY(SELECT id FROM subs)) a, UNNEST($4::INT[]) b)
ON CONFLICT (subscriber_id, list_id) DO NOTHING;
-- name: delete-subscriptions-by-query
-- raw: true
WITH subs AS (%s)
DELETE FROM subscriber_lists
WHERE (subscriber_id, list_id) = ANY(SELECT a, b FROM UNNEST(ARRAY(SELECT id FROM subs)) a, UNNEST($3::INT[]) b);
WHERE (subscriber_id, list_id) = ANY(SELECT a, b FROM UNNEST(ARRAY(SELECT id FROM subs)) a, UNNEST($4::INT[]) b);
-- name: unsubscribe-subscribers-from-lists-by-query
-- raw: true
WITH subs AS (%s)
UPDATE subscriber_lists SET status='unsubscribed', updated_at=NOW()
WHERE (subscriber_id, list_id) = ANY(SELECT a, b FROM UNNEST(ARRAY(SELECT id FROM subs)) a, UNNEST($3::INT[]) b);
WHERE (subscriber_id, list_id) = ANY(SELECT a, b FROM UNNEST(ARRAY(SELECT id FROM subs)) a, UNNEST($4::INT[]) b);
-- lists