8
16
Fork 22

Email blocklisting #718

Zusammengeführt
epsilon-phase hat 9 Commits von email-blocklisting nach master vor 4 Jahren zusammengeführt
epsilon-phase hat vor 4 Jahren kommentiert (Migriert von github.com)

This should address #587 and any concerns for the terminology hopefully.

This should address #587 and any concerns for the terminology hopefully.
codecov[bot] hat vor 4 Jahren kommentiert (Migriert von github.com)

Codecov Report

Merging #718 into master will decrease coverage by 0.18%.
The diff coverage is 27.89%.

@@            Coverage Diff             @@
##           master     #718      +/-   ##
==========================================
- Coverage    39.3%   39.11%   -0.19%     
==========================================
  Files          72       73       +1     
  Lines        9534     9676     +142     
  Branches     2275     2313      +38     
==========================================
+ Hits         3747     3785      +38     
- Misses       4714     4807      +93     
- Partials     1073     1084      +11
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/718?src=pr&el=h1) Report > Merging [#718](https://codecov.io/gh/Plume-org/Plume/pull/718?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/458baf5f78daec1b823e0bbd17eaa96c23ae92f8?src=pr&el=desc) will **decrease** coverage by `0.18%`. > The diff coverage is `27.89%`. ```diff @@ Coverage Diff @@ ## master #718 +/- ## ========================================== - Coverage 39.3% 39.11% -0.19% ========================================== Files 72 73 +1 Lines 9534 9676 +142 Branches 2275 2313 +38 ========================================== + Hits 3747 3785 +38 - Misses 4714 4807 +93 - Partials 1073 1084 +11 ```
igalic (Migriert von github.com) hat vor 4 Jahren überprüft
igalic (Migriert von github.com) hat einen Kommentar hinterlassen

very very tired 👀

very very tired 👀
igalic (Migriert von github.com) hat vor 4 Jahren kommentiert

I'm a bit confused as to why there are no spaces after , s in function calls, starting here

I'm a bit confused as to why there are no spaces after `, `s in function calls, starting here
epsilon-phase (Migriert von github.com) hat vor 4 Jahren überprüft
epsilon-phase (Migriert von github.com) hat vor 4 Jahren kommentiert

Oh, that's because rustfmt can't touch the templates :)

Fixed.

Oh, that's because rustfmt can't touch the templates :) Fixed.
elegaanz (Migriert von github.com) hat vor 4 Jahren überprüft
elegaanz (Migriert von github.com) hat einen Kommentar hinterlassen

Globally it works fine, great job! I left comments about "details", but after that it should be good!

Globally it works fine, great job! I left comments about "details", but after that it should be good!
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

Maybe make this column unique, to avoid duplicated blocks?

Maybe make this column unique, to avoid duplicated blocks?
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

Little detail, so if you don't feel like doing it, don't change, but: keywords in SQL usually are in all caps.

Little detail, so if you don't feel like doing it, don't change, but: keywords in SQL usually are in all caps.
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert
You could use https://doc.rust-lang.org/std/result/enum.Result.html#method.err
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

Maybe add a @ so that in only matches one domain, and not all domains with the same ending (I doubt this case will ever be encountered actually, be just to be sure).

Maybe add a `@` so that in only matches one domain, and not all domains with the same ending (I doubt this case will ever be encountered actually, be just to be sure).
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

It should probably redirect to /admin/emails instead.

It should probably redirect to `/admin/emails` instead.
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

It should probably redirect to /admin/emails.

It should probably redirect to `/admin/emails`.
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

Maybe call it /admin/emails/delete if it is what it does?

Maybe call it `/admin/emails/delete` if it is what it does?
@ -177,0 +205,4 @@
}
#[post("/admin/emails/new", data = "<form>")]
pub fn add_email_blocklist(
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

I'm almost sure I did something similar for user management, couldn't the code be shared between the two?

I'm almost sure I did something similar for user management, couldn't the code be shared between the two?
@ -14,6 +14,7 @@
(&uri!(instance::admin).to_string(), i18n!(ctx.1, "Configuration"), true),
(&uri!(instance::admin_instances: page = _).to_string(), i18n!(ctx.1, "Instances"), false),
(&uri!(instance::admin_users: page = _).to_string(), i18n!(ctx.1, "Users"), false),
(&uri!(instance::admin_email_blocklist: page=_).to_string(), i18n!(ctx.1, "Email blocklist"), false)
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

This link is absent from the "Instances" page.

This link is absent from the "Instances" page.
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

This button should probably be red (class="destructive" IIRC), and explicitly say that it deletes all blocklisted emails if it is what it does.

This button should probably be red (`class="destructive"` IIRC), and explicitly say that it deletes all blocklisted emails if it is what it does.
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

This <input> should be before the label IMO, as it is usually done with checkboxes.

This `<input>` should be before the label IMO, as it is usually done with checkboxes.
@ -0,0 +6,4 @@
@(ctx:BaseContext, emails: Vec<BlocklistedEmail>, page:i32, n_pages:i32)
@:base(ctx, i18n!(ctx.1, "Blocklisted Emails"), {}, {}, {
<h1>@i18n!(ctx.1,"Blocklisted Emails")</h1>
@tabs(&[
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

There should probably be a <h1> here, like on other administration pages.

There should probably be a `<h1>` here, like on other administration pages.
epsilon-phase (Migriert von github.com) hat vor 4 Jahren überprüft
epsilon-phase (Migriert von github.com) hat vor 4 Jahren kommentiert

It deletes only the ones selected.

But yeah, that's a good idea

It deletes only the ones selected. But yeah, that's a good idea
epsilon-phase (Migriert von github.com) hat vor 4 Jahren überprüft
@ -177,0 +205,4 @@
}
#[post("/admin/emails/new", data = "<form>")]
pub fn add_email_blocklist(
epsilon-phase (Migriert von github.com) hat vor 4 Jahren kommentiert

We couldn't see how to do that cleanly, since that's significantly more complicated.

We couldn't see how to do that cleanly, since that's significantly more complicated.
epsilon-phase (Migriert von github.com) hat vor 4 Jahren überprüft
epsilon-phase (Migriert von github.com) hat vor 4 Jahren kommentiert

That makes sense.

That makes sense.
epsilon-phase (Migriert von github.com) hat vor 4 Jahren überprüft
epsilon-phase (Migriert von github.com) hat vor 4 Jahren kommentiert

Might as well follow the conventions :)

Might as well follow the conventions :)
epsilon-phase (Migriert von github.com) hat vor 4 Jahren überprüft
epsilon-phase (Migriert von github.com) hat vor 4 Jahren kommentiert

That's much nicer :)

That's much nicer :)
elegaanz (Migriert von github.com) hat vor 4 Jahren überprüft
@ -177,0 +205,4 @@
}
#[post("/admin/emails/new", data = "<form>")]
pub fn add_email_blocklist(
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

OK, let's do it this way then, we can always refactor later if needed.

OK, let's do it this way then, we can always refactor later if needed.
elegaanz (Migriert von github.com) hat vor 4 Jahren überprüft
elegaanz (Migriert von github.com) hat einen Kommentar hinterlassen

Sorry, actually there is more… 😕 (but it should be the last this time!)

Sorry, actually there is more… :confused: (but it should be the last this time!)
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

Actually, this button should probably be hidden if there is currently no blocked addresses, and a placeholder message should be shown (<p class="center"> is made for that).

Actually, this button should probably be hidden if there is currently no blocked addresses, and a placeholder message should be shown (`<p class="center">` is made for that).
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

Moving it inside of the <label> tag would be better, otherwise they are not on the same line, and it is hard to understand they are linked.

Moving it inside of the `<label>` tag would be better, otherwise they are not on the same line, and it is hard to understand they are linked.
elegaanz (Migriert von github.com) hat vor 4 Jahren kommentiert

There is a i missing in i18n here, it doesn't show correctly (it shouldn't even compile, but for some reason, it does).

There is a `i` missing in `i18n` here, it doesn't show correctly (it shouldn't even compile, but for some reason, it does).
epsilon-phase (Migriert von github.com) hat vor 4 Jahren überprüft
epsilon-phase (Migriert von github.com) hat vor 4 Jahren kommentiert

What an odd mystery

What an odd mystery
epsilon-phase (Migriert von github.com) hat vor 4 Jahren überprüft
epsilon-phase (Migriert von github.com) hat vor 4 Jahren kommentiert

That makes sense

That makes sense
elegaanz (Migriert von github.com) hat die Änderungen vor 4 Jahren genehmigt
elegaanz (Migriert von github.com) hat einen Kommentar hinterlassen

Perfect! Thank you!

Perfect! Thank you!

Reviewer

Der Pull Request wurde als f3c05dae62 gemergt.

Schritt 1:

Wechsle auf einen neuen Branch in deinem lokalen Repository und teste die Änderungen.
git checkout -b email-blocklisting master
git pull origin email-blocklisting

Schritt 2:

Führe die Änderungen zusammen und aktualisiere den Stand online auf Forgejo.
git checkout master
git merge --no-ff email-blocklisting
git push origin master
Anmelden, um an der Diskussion teilzunehmen.
Keine Reviewer
Kein Meilenstein
Niemand zuständig
1 Beteiligte
Nachrichten
Fällig am
Das Fälligkeitsdatum ist ungültig oder außerhalb des zulässigen Bereichs. Bitte verwende das Format „jjjj-mm-tt“.

Kein Fälligkeitsdatum gesetzt.

Abhängigkeiten

Keine Abhängigkeiten gesetzt.

Referenz: Plume/Plume#718
Laden…
Hier gibt es bis jetzt noch keinen Inhalt.