Refactor subsbscription status option on the import page.

- Refactor subimporter New*() funcs to take opt structs.
- Refactor and simplify Vue code.
- Remove redundant i18n entries and use existing ones.
- Remove redundant subimporter constants and use existing ones.

- Consider 'overwrite' option for subscription status as well.
- Write Cypress integration tests for the new feature.
This commit is contained in:
Kailash Nadh 2021-06-06 17:31:29 +05:30
parent 7ca08f0a36
commit 868fae6ac2
16 changed files with 579 additions and 534 deletions

View file

@ -8,18 +8,10 @@ import (
"strings"
"github.com/knadh/listmonk/internal/subimporter"
"github.com/knadh/listmonk/models"
"github.com/labstack/echo"
)
// reqImport represents file upload import params.
type reqImport struct {
Mode string `json:"mode"`
SubscriptionStatus string `json:"subscriptionStatus"`
Overwrite bool `json:"overwrite"`
Delim string `json:"delim"`
ListIDs []int `json:"lists"`
}
// handleImportSubscribers handles the uploading and bulk importing of
// a ZIP file of one or more CSV files.
func handleImportSubscribers(c echo.Context) error {
@ -31,21 +23,34 @@ func handleImportSubscribers(c echo.Context) error {
}
// Unmarsal the JSON params.
var r reqImport
if err := json.Unmarshal([]byte(c.FormValue("params")), &r); err != nil {
var opt subimporter.SessionOpt
if err := json.Unmarshal([]byte(c.FormValue("params")), &opt); err != nil {
return echo.NewHTTPError(http.StatusBadRequest,
app.i18n.Ts("import.invalidParams", "error", err.Error()))
}
if r.Mode != subimporter.ModeSubscribe && r.Mode != subimporter.ModeBlocklist {
// Validate mode.
if opt.Mode != subimporter.ModeSubscribe && opt.Mode != subimporter.ModeBlocklist {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("import.invalidMode"))
}
if r.SubscriptionStatus != subimporter.SubscriptionStatusUnconfirmed && r.SubscriptionStatus != subimporter.SubscriptionStatusConfirmed {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("import.invalidSubscriptionStatus"))
// If no status is specified, pick a default one.
if opt.SubStatus == "" {
switch opt.Mode {
case subimporter.ModeSubscribe:
opt.SubStatus = models.SubscriptionStatusUnconfirmed
case subimporter.ModeBlocklist:
opt.SubStatus = models.SubscriptionStatusUnsubscribed
}
}
if len(r.Delim) != 1 {
if opt.SubStatus != models.SubscriptionStatusUnconfirmed &&
opt.SubStatus != models.SubscriptionStatusConfirmed &&
opt.SubStatus != models.SubscriptionStatusUnsubscribed {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("import.invalidSubStatus"))
}
if len(opt.Delim) != 1 {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("import.invalidDelim"))
}
@ -74,7 +79,8 @@ func handleImportSubscribers(c echo.Context) error {
}
// Start the importer session.
impSess, err := app.importer.NewSession(file.Filename, r.Mode, r.SubscriptionStatus, r.Overwrite, r.ListIDs)
opt.Filename = file.Filename
impSess, err := app.importer.NewSession(opt)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError,
app.i18n.Ts("import.errorStarting", "error", err.Error()))
@ -82,7 +88,7 @@ func handleImportSubscribers(c echo.Context) error {
go impSess.Start()
if strings.HasSuffix(strings.ToLower(file.Filename), ".csv") {
go impSess.LoadCSV(out.Name(), rune(r.Delim[0]))
go impSess.LoadCSV(out.Name(), rune(opt.Delim[0]))
} else {
// Only 1 CSV from the ZIP is considered. If multiple files have
// to be processed, counting the net number of lines (to track progress),
@ -95,7 +101,7 @@ func handleImportSubscribers(c echo.Context) error {
return echo.NewHTTPError(http.StatusInternalServerError,
app.i18n.Ts("import.errorProcessingZIP", "error", err.Error()))
}
go impSess.LoadCSV(dir+"/"+files[0], rune(r.Delim[0]))
go impSess.LoadCSV(dir+"/"+files[0], rune(opt.Delim[0]))
}
return c.JSON(http.StatusOK, okResp{app.importer.GetStats()})

View file

@ -7,12 +7,19 @@ describe('Import', () => {
it('Imports subscribers', () => {
const cases = [
{ mode: 'check-subscribe', status: 'enabled', count: 102 },
{ mode: 'check-blocklist', status: 'blocklisted', count: 102 },
{ chkMode: 'subscribe', status: 'enabled', chkSubStatus: 'unconfirmed', subStatus: 'unconfirmed', overwrite: true, count: 102 },
{ chkMode: 'subscribe', status: 'enabled', chkSubStatus: 'confirmed', subStatus: 'confirmed', overwrite: true, count: 102 },
{ chkMode: 'subscribe', status: 'enabled', chkSubStatus: 'unconfirmed', subStatus: 'confirmed', overwrite: false, count: 102 },
{ chkMode: 'blocklist', status: 'blocklisted', chkSubStatus: 'unsubscribed', subStatus: 'unsubscribed', overwrite: true, count: 102 },
];
cases.forEach((c) => {
cy.get(`[data-cy=${c.mode}] .check`).click();
cy.get(`[data-cy=check-${c.chkMode}] .check`).click();
cy.get(`[data-cy=check-${c.chkSubStatus}] .check`).click();
if (!c.overwrite) {
cy.get(`[data-cy=overwrite]`).click();
}
if (c.status === 'enabled') {
cy.get('.list-selector input').click();
@ -39,12 +46,39 @@ describe('Import', () => {
cy.expect(parseInt($el.text().trim())).to.equal(c.count);
});
// Subscriber status.
cy.get('tbody td[data-label=Status]').each(($el) => {
cy.wrap($el).find(`.tag.${c.status}`);
});
// Subscription status.
cy.get('tbody td[data-label=E-mail]').each(($el) => {
cy.wrap($el).find(`.tag.${c.subStatus}`);
});
cy.loginAndVisit('/subscribers/import');
cy.wait(100);
});
});
it('Imports subscribers incorrectly', () => {
cy.resetDB();
cy.loginAndVisit('/subscribers/import');
cy.get('.list-selector input').click();
cy.get('.list-selector .autocomplete a').first().click();
cy.get('input[name=delim]').clear().type('|');
cy.fixture('subs.csv').then((data) => {
cy.get('input[type="file"]').attachFile({
fileContent: data.toString(),
fileName: 'subs.csv',
mimeType: 'text/csv',
});
});
cy.get('button.is-primary').click();
cy.wait(250);
cy.get('section.wrap .has-text-danger');
});
});

View file

@ -8,7 +8,7 @@
<div>
<div class="columns">
<div class="column">
<b-field :label="$t('import.mode')">
<b-field :label="$t('import.mode')" :addons="false">
<div>
<b-radio v-model="form.mode" name="mode"
native-value="subscribe"
@ -21,52 +21,47 @@
</b-field>
</div>
<div class="column">
<b-field :label="$t('import.subscriptionStatus')">
<div>
<div v-if="form.mode === 'subscribe'" style="display:block">
<b-field :label="$t('globals.fields.status')" :addons="false">
<template v-if="form.mode === 'subscribe'">
<b-radio
v-model="form.subscriptionStatus"
name="subscriptionStatus"
v-model="form.subStatus"
name="subStatus"
native-value="unconfirmed"
data-cy="check-unconfirmed">
{{ $t('import.unconfirmed') }}
{{ $t('subscribers.status.unconfirmed') }}
</b-radio>
</div>
<div v-if="form.mode === 'subscribe'" style="display:block">
<b-radio
v-model="form.subscriptionStatus"
name="subscriptionStatus"
v-model="form.subStatus"
name="subStatus"
native-value="confirmed"
data-cy="check-confirmed">
{{ $t('import.confirmed') }}
{{ $t('subscribers.status.confirmed') }}
</b-radio>
</div>
<div v-if="form.mode === 'blocklist'" style="display:block">
<b-radio
v-model="form.subscriptionStatus"
name="subscriptionStatus"
</template>
<b-radio v-else
v-model="form.subStatus"
name="subStatus"
native-value="unsubscribed"
data-cy="check-unsubscribed">
{{ $t('import.unsubscribed') }}
{{ $t('subscribers.status.unsubscribed') }}
</b-radio>
</div>
</div>
</b-field>
</div>
<div class="column">
<b-field v-if="form.mode === 'subscribe'"
:label="$t('import.overwrite')"
:message="$t('import.overwriteHelp')">
<div>
<b-switch v-model="form.overwrite" name="overwrite" />
<b-switch v-model="form.overwrite" name="overwrite" data-cy="overwrite" />
</div>
</b-field>
</div>
<div class="column">
<b-field :label="$t('import.csvDelim')" :message="$t('import.csvDelimHelp')"
class="delimiter">
<b-input v-model="form.delim" name="delim"
placeholder="," maxlength="1" required />
<b-field :label="$t('import.csvDelim')" :message="$t('import.csvDelimHelp')" class="delimiter">
<b-input v-model="form.delim" name="delim" placeholder="," maxlength="1" required />
</b-field>
</div>
</div>
@ -187,7 +182,7 @@ export default Vue.extend({
return {
form: {
mode: 'subscribe',
subscriptionStatus: 'unconfirmed',
subStatus: 'unconfirmed',
delim: ',',
lists: [],
overwrite: true,
@ -206,15 +201,15 @@ export default Vue.extend({
},
watch: {
form: {
handler(val) {
if (val.mode === 'subscribe') {
this.form.subscriptionStatus = 'unconfirmed';
} else if (val.mode === 'blocklist') {
this.form.subscriptionStatus = 'unsubscribed';
'form.mode': function formMode() {
// Select the appropriate status radio whenever mode changes.
this.$nextTick(() => {
if (this.form.mode === 'subscribe') {
this.form.subStatus = 'unconfirmed';
} else {
this.form.subStatus = 'unsubscribed';
}
},
deep: true,
});
},
},
@ -317,7 +312,7 @@ export default Vue.extend({
const params = new FormData();
params.set('params', JSON.stringify({
mode: this.form.mode,
subscriptionStatus: this.form.subscriptionStatus,
subscription_status: this.form.subStatus,
delim: this.form.delim,
lists: this.form.lists.map((l) => l.id),
overwrite: this.form.overwrite,

View file

@ -186,6 +186,7 @@
"import.invalidFile": "Ungültige Datei: {error}",
"import.invalidMode": "Ungültiger Modus",
"import.invalidParams": "Ungültiger Parameter: {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "Listen die abonniert werden.",
"import.mode": "Modus",
"import.overwrite": "Überschreiben?",

View file

@ -185,16 +185,12 @@
"import.invalidDelim": "Delimiter should be a single character.",
"import.invalidFile": "Invalid file: {error}",
"import.invalidMode": "Invalid mode",
"import.invalidSubscriptionStatus": "Invalid subscription status",
"import.invalidParams": "Invalid params: {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "Lists to subscribe to.",
"import.mode": "Mode",
"import.subscriptionStatus": "Subscription Status",
"import.confirmed": "Confirmed",
"import.unconfirmed": "Unconfirmed",
"import.unsubscribed": "Unsubscribed",
"import.overwrite": "Overwrite?",
"import.overwriteHelp": "Overwrite name and attribs of existing subscribers?",
"import.overwriteHelp": "Overwrite name, attribs, subscription status of existing subscribers?",
"import.recordsCount": "{num} / {total} records",
"import.stopImport": "Stop import",
"import.subscribe": "Subscribe",

View file

@ -186,6 +186,7 @@
"import.invalidFile": "Archivo inválido: {error}",
"import.invalidMode": "Modo inválido",
"import.invalidParams": "Paramétros inválidos: {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "Listas a subscribir",
"import.mode": "Modo",
"import.overwrite": "¿Sobre-escribir?",

View file

@ -186,6 +186,7 @@
"import.invalidFile": "Fichier non valide : {error}",
"import.invalidMode": "Mode invalide",
"import.invalidParams": "Paramètres non valides : {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "Abonner aux listes",
"import.mode": "Mode",
"import.overwrite": "Écraser ?",

View file

@ -186,6 +186,7 @@
"import.invalidFile": "File non valido: {error}",
"import.invalidMode": "Modalità non valida",
"import.invalidParams": "Parametri non validi: {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "Liste a cui iscriversi.",
"import.mode": "Modalità",
"import.overwrite": "Sovrascrivere?",

View file

@ -186,6 +186,7 @@
"import.invalidFile": " ഫയൽ അസാധുവാണ് : {error}",
"import.invalidMode": "ശൈലി അസാധുവാണ്",
"import.invalidParams": "പരാമുകൾ അസാധുവാണ്: {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "വരിക്കാരനാകാനുള്ള ലിസ്റ്റുകൾ.",
"import.mode": "ശൈലി",
"import.overwrite": "തിരുത്തിയെഴുതട്ടേ?",

View file

@ -186,6 +186,7 @@
"import.invalidFile": "Nieprawidłowy plik: {error}",
"import.invalidMode": "Nieprawidłowy tryp",
"import.invalidParams": "Nieprawidłowe parametry: {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "Listy do subskrybowania.",
"import.mode": "Tryb",
"import.overwrite": "Nadpisać?",

View file

@ -186,6 +186,7 @@
"import.invalidFile": "Arquivo inválido: {error}",
"import.invalidMode": "Modo inválido",
"import.invalidParams": "Parâmetros inválidos: {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "Listas para inscrever.",
"import.mode": "Modo",
"import.overwrite": "Sobrescrever?",

View file

@ -186,6 +186,7 @@
"import.invalidFile": "Ficheiro inválido: {error}",
"import.invalidMode": "Modo inválido",
"import.invalidParams": "Parâmetros inválidos: {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "Listas a subscrever.",
"import.mode": "Modo",
"import.overwrite": "Sobrescrever?",

View file

@ -186,6 +186,7 @@
"import.invalidFile": "Неверный файл: {error}",
"import.invalidMode": "Неверный режим",
"import.invalidParams": "Неверные параметры: {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "Списки для подписки.",
"import.mode": "Режим",
"import.overwrite": "Перезаписать?",

View file

@ -25,6 +25,7 @@
"campaigns.fromAddress": "Gelen adres",
"campaigns.fromAddressPlaceholder": "isminiz <cevap-verme@siteniz.com>",
"campaigns.invalid": "Yanlış tanımlı kapmanya",
"campaigns.markdown": "Markdown",
"campaigns.needsSendAt": "Kampanya için tanımlanmış bir tarih gerekli.",
"campaigns.newCampaign": "Yeni kampanya",
"campaigns.noKnownSubsToTest": "Test için bilinen üye yok.",
@ -185,6 +186,7 @@
"import.invalidFile": "Hatalı dosya: {error}",
"import.invalidMode": "Hatalı mod",
"import.invalidParams": "Hatalı parametre: {error}",
"import.invalidSubStatus": "Invalid subscription status",
"import.listSubHelp": "Üye olunacak listeler.",
"import.mode": "Mod",
"import.overwrite": "Üzerine yaz?",
@ -259,10 +261,10 @@
"public.privacyWipeHelp": "Tüm üyeliklerinizi ve ilişkili verilerinizi veritabanından silin.",
"public.sub": "Üyelik",
"public.subConfirmed": "Başarıyla üye olundu.",
"public.subOptinPending": "Üyelik doğrulaması için bir e-posta gönderilmiştir.",
"public.subConfirmedTitle": "Doğrulanmıştır",
"public.subName": "İsim (opsiyonel)",
"public.subNotFound": "Üyelik bulunamadı.",
"public.subOptinPending": "Üyelik doğrulaması için bir e-posta gönderilmiştir.",
"public.subPrivateList": "Kişisel liste",
"public.subTitle": "Üye ol",
"public.unsub": "Üyelikten ayrıl",
@ -272,15 +274,14 @@
"public.unsubbedInfo": "Başarı ile üyeliğinizi bitirdiniz.",
"public.unsubbedTitle": "Üyelik bitirildi.",
"public.unsubscribeTitle": "e-posta listesi üyeliğini bitir",
"settings.needsRestart": "Ayarlar değişti. Çalışan tüm kampanyaları durdur ve uygulamayı yeniden başlat.",
"settings.confirmRestart": "Çalışan kampanyaların duraklatıldığından emin ol. Yeniden başlat?",
"settings.updateAvailable": "Yeni bir güncel sürüm {version} mevcuttur.",
"settings.restart": "Yeniden başlat",
"settings.duplicateMessengerName": "Çoklanmış messenger ismi: {name}",
"settings.errorEncoding": "Hatalı kodlama ayarları: {error}",
"settings.errorNoSMTP": "En azından bir SMTP bloğu etkin olmalı",
"settings.general.adminNotifEmails": "Yönetici e-posta bildirimleri",
"settings.general.adminNotifEmailsHelp": "İçe aktarma güncellemeleri, kampanya tamamlama, başarısızlık gibi yönetici bildirimlerinin gönderilmesi gereken e-posta adreslerinin virgülle ayrılmış listesi.",
"settings.general.checkUpdates": "Check for updates",
"settings.general.checkUpdatesHelp": "Periodically check for new app releases and notify.",
"settings.general.enablePublicSubPage": "Erişime açık üyelik sayfasını etkinleştir",
"settings.general.enablePublicSubPageHelp": "Kişilerin abone olması için tüm genel listeleri içeren genel bir abonelik sayfası gösterin.",
"settings.general.faviconURL": "Favicon URL",
@ -326,6 +327,7 @@
"settings.messengers.url": "URL",
"settings.messengers.urlHelp": "Postback sunusucu için kök URL.",
"settings.messengers.username": "Kullanıcı adı",
"settings.needsRestart": "Ayarlar değişti. Çalışan tüm kampanyaları durdur ve uygulamayı yeniden başlat.",
"settings.performance.batchSize": "Batch büyüklüğü",
"settings.performance.batchSizeHelp": "Veritabanından tek bir yinelemede çekilecek abone sayısı. Her yineleme, aboneleri veritabanından çeker, onlara mesajlar gönderir ve ardından bir sonraki grubu çekmek için bir sonraki yinelemeye geçer. Bu, ideal olarak elde edilebilecek maksimum iş hacminden (eşzamanlılık * ileti_ hızı) daha yüksek olmalıdır.",
"settings.performance.concurrency": "Çoklu bağlantı",
@ -352,6 +354,7 @@
"settings.privacy.listUnsubHeader": " `List-Unsubscribe` Başlık bilgisini ekle",
"settings.privacy.listUnsubHeaderHelp": "E-posta istemcilerinin kullanıcıların tek bir tıklamayla abonelikten çıkmalarına olanak tanıyan abonelik iptal başlıklarını ekleyin.",
"settings.privacy.name": "Gizlilik",
"settings.restart": "Yeniden başlat",
"settings.smtp.authProtocol": "Protokol",
"settings.smtp.customHeaders": "Özel başlık bilgisi",
"settings.smtp.customHeadersHelp": "Bu sunucudan gönderilen tüm iletilere eklenecek isteğe bağlı e-posta başlıkları dizisi. Örnek: [{\"X-Custom\": \"value\"}, {\"X-Custom2\": \"value\"}]",
@ -380,6 +383,7 @@
"settings.smtp.waitTimeout": "Bekleme süresi aşımı",
"settings.smtp.waitTimeoutHelp": "Bir bağlantıdaki yeni etkinliği kapatmadan ve havuzdan kaldırmadan önce bekleme süresi (saniye için s, dakika için m). ",
"settings.title": "Ayarlar",
"settings.updateAvailable": "Yeni bir güncel sürüm {version} mevcuttur.",
"subscribers.advancedQuery": "İleri düzey",
"subscribers.advancedQueryHelp": "Üye attributes verisini görüntülemek için SQL verisi",
"subscribers.attribs": "Attributes",

View file

@ -46,9 +46,6 @@ const (
ModeSubscribe = "subscribe"
ModeBlocklist = "blocklist"
SubscriptionStatusUnconfirmed = "unconfirmed"
SubscriptionStatusConfirmed = "confirmed"
)
// Importer represents the bulk CSV subscriber import system.
@ -75,10 +72,17 @@ type Session struct {
subQueue chan SubReq
log *log.Logger
mode string
subscriptionStatus string
overwrite bool
listIDs []int
opt SessionOpt
}
// SessionOpt represents the options for an importer session.
type SessionOpt struct {
Filename string `json:"filename"`
Mode string `json:"mode"`
SubStatus string `json:"subscription_status"`
Overwrite bool `json:"overwrite"`
Delim string `json:"delim"`
ListIDs []int `json:"lists"`
}
// Status reporesents statistics from an ongoing import session.
@ -131,14 +135,14 @@ func New(opt Options, db *sql.DB) *Importer {
// NewSession returns an new instance of Session. It takes the name
// of the uploaded file, but doesn't do anything with it but retains it for stats.
func (im *Importer) NewSession(fName, mode string, subscriptionStatus string, overWrite bool, listIDs []int) (*Session, error) {
func (im *Importer) NewSession(opt SessionOpt) (*Session, error) {
if im.getStatus() != StatusNone {
return nil, errors.New("an import is already running")
}
im.Lock()
im.status = Status{Status: StatusImporting,
Name: fName,
Name: opt.Filename,
logBuf: bytes.NewBuffer(nil)}
im.Unlock()
@ -146,13 +150,10 @@ func (im *Importer) NewSession(fName, mode string, subscriptionStatus string, ov
im: im,
log: log.New(im.status.logBuf, "", log.Ldate|log.Ltime|log.Lshortfile),
subQueue: make(chan SubReq, commitBatchSize),
mode: mode,
subscriptionStatus: subscriptionStatus,
overwrite: overWrite,
listIDs: listIDs,
opt: opt,
}
s.log.Printf("processing '%s'", fName)
s.log.Printf("processing '%s'", opt.Filename)
return s, nil
}
@ -240,10 +241,10 @@ func (s *Session) Start() {
total = 0
cur = 0
listIDs = make(pq.Int64Array, len(s.listIDs))
listIDs = make(pq.Int64Array, len(s.opt.ListIDs))
)
for i, v := range s.listIDs {
for i, v := range s.opt.ListIDs {
listIDs[i] = int64(v)
}
@ -256,7 +257,7 @@ func (s *Session) Start() {
continue
}
if s.mode == ModeSubscribe {
if s.opt.Mode == ModeSubscribe {
stmt = tx.Stmt(s.im.opt.UpsertStmt)
} else {
stmt = tx.Stmt(s.im.opt.BlocklistStmt)
@ -270,9 +271,9 @@ func (s *Session) Start() {
break
}
if s.mode == ModeSubscribe {
_, err = stmt.Exec(uu, sub.Email, sub.Name, sub.Attribs, listIDs, s.subscriptionStatus, s.overwrite)
} else if s.mode == ModeBlocklist {
if s.opt.Mode == ModeSubscribe {
_, err = stmt.Exec(uu, sub.Email, sub.Name, sub.Attribs, listIDs, s.opt.SubStatus, s.opt.Overwrite)
} else if s.opt.Mode == ModeBlocklist {
_, err = stmt.Exec(uu, sub.Email, sub.Name, sub.Attribs)
}
if err != nil {

View file

@ -95,7 +95,7 @@ subs AS (
INSERT INTO subscriber_lists (subscriber_id, list_id, status)
VALUES((SELECT id FROM sub), UNNEST($5::INT[]), $6)
ON CONFLICT (subscriber_id, list_id) DO UPDATE
SET updated_at=NOW()
SET updated_at=NOW(), status=(CASE WHEN $7 THEN $6 ELSE subscriber_lists.status END)
)
SELECT uuid, id from sub;