Plume/Plume
9
18
Fork 24
Code Issues 180 Pull requests 8 Releases 8 Wiki Activity

Fix moderation issues #1156 #1157

Merged
trinity-1686a merged 5 commits from gro2bl/Plume:fix_moderation_issues into main 2024-07-08 23:54:36 +00:00
Conversation 0 Commits 5 Files changed 9 +53 -47
9 changed files with 53 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
plume-models/src/admin.rs
View file

@ -5,7 +5,7 @@ use rocket::{
Outcome,
};
/// Wrapper around User to use as a request guard on pages reserved to admins.
/// Wrapper around User to use as a request guard on pages exclusively reserved to admins.
pub struct Admin(pub User);
impl<'a, 'r> FromRequest<'a, 'r> for Admin {
@ -21,6 +21,23 @@ impl<'a, 'r> FromRequest<'a, 'r> for Admin {
}
}
/// Same as `Admin` but it forwards to next guard if the user is not an admin.
/// It's useful when there are multiple implementations of routes for admin and moderator.
pub struct InclusiveAdmin(pub User);
impl<'a, 'r> FromRequest<'a, 'r> for InclusiveAdmin {
type Error = ();
fn from_request(request: &'a Request<'r>) -> request::Outcome<InclusiveAdmin, ()> {
let user = request.guard::<User>()?;
if user.is_admin() {
Outcome::Success(InclusiveAdmin(user))
} else {
Outcome::Forward(())
}
}
}
/// Same as `Admin` but for moderators.
pub struct Moderator(pub User);

2
src/routes/instance.rs
View file

@ -51,7 +51,7 @@ pub fn index(conn: DbConn, rockets: PlumeRocket) -> Result<Ructe, ErrorPage> {
}
#[get("/admin")]
pub fn admin(_admin: Admin, conn: DbConn, rockets: PlumeRocket) -> Result<Ructe, ErrorPage> {
pub fn admin(_admin: InclusiveAdmin, conn: DbConn, rockets: PlumeRocket) -> Result<Ructe, ErrorPage> {
let local_inst = Instance::get_local()?;
Ok(render!(instance::admin(
&(&conn, &rockets).to_context(),

2
templates/base.rs.html
View file

@ -87,6 +87,8 @@
<a href="@uri!(instance::privacy)">@i18n!(ctx.1, "Privacy policy")</a>
@if ctx.2.clone().map(|u| u.is_admin()).unwrap_or(false) {
<a href="@uri!(instance::admin)">@i18n!(ctx.1, "Administration")</a>
} else if ctx.2.clone().map(|u| u.is_moderator()).unwrap_or(false) {
<a href="@uri!(instance::admin_mod)">@i18n!(ctx.1, "Moderation")</a>
}
</div>
<div>

11
templates/instance/admin.rs.html
View file

@ -1,6 +1,6 @@
@use plume_models::instance::Instance;
@use validator::ValidationErrors;
@use crate::templates::base;
@use crate::templates::{base, instance::admin_header};
@use crate::template_utils::*;
@use crate::routes::instance::InstanceSettingsForm;
@use crate::routes::*;
@ -8,14 +8,7 @@
@(ctx: BaseContext, instance: Instance, form: InstanceSettingsForm, errors: ValidationErrors)
@:base(ctx, i18n!(ctx.1, "Administration of {0}"; instance.name.clone()), {}, {}, {
<h1>@i18n!(ctx.1, "Administration")</h1>
@tabs(&[
(&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)
])
@:admin_header(ctx, "Administration", 1)
<form method="post" action="@uri!(instance::update_settings)">
@(Input::new("name", i18n!(ctx.1, "Name"))

21
templates/instance/admin_header.rs.html Normal file
View file

@ -0,0 +1,21 @@
@use crate::template_utils::*;
@use crate::routes::*;
@(ctx: BaseContext, title: &str, selected_tab: u8)
<h1>@i18n!(ctx.1, title)</h1>
@if ctx.2.clone().map(|u| u.is_admin()).unwrap_or(false) {
@tabs(&[
(&uri!(instance::admin).to_string(), i18n!(ctx.1, "Configuration"), selected_tab == 1),
(&uri!(instance::admin_instances: page = _).to_string(), i18n!(ctx.1, "Instances"), selected_tab == 2),
(&uri!(instance::admin_users: page = _).to_string(), i18n!(ctx.1, "Users"), selected_tab == 3),
(&uri!(instance::admin_email_blocklist: page=_).to_string(), i18n!(ctx.1, "Email blocklist"), selected_tab == 4)
])
} else {
@tabs(&[
(&uri!(instance::admin_instances: page = _).to_string(), i18n!(ctx.1, "Instances"), selected_tab == 2),
(&uri!(instance::admin_users: page = _).to_string(), i18n!(ctx.1, "Users"), selected_tab == 3),
(&uri!(instance::admin_email_blocklist: page=_).to_string(), i18n!(ctx.1, "Email blocklist"), selected_tab == 4)
])
}

11
templates/instance/admin_mod.rs.html
View file

@ -1,15 +1,8 @@
@use crate::templates::base;
@use crate::templates::{base, instance::admin_header};
@use crate::template_utils::*;
@use crate::routes::*;
@(ctx: BaseContext)
@:base(ctx, i18n!(ctx.1, "Moderation"), {}, {}, {
<h1>@i18n!(ctx.1, "Moderation")</h1>
@tabs(&[
(&uri!(instance::admin).to_string(), i18n!(ctx.1, "Home"), 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),
])
@:admin_header(ctx, "Moderation", 0)
trinity-1686a commented 2024-06-29 09:55:19 +00:00
Review
Copy link

shouldn't it be 1 instead of 0?

shouldn't it be 1 instead of 0?
gro2bl commented 2024-07-04 18:51:54 +00:00
Review
Copy link

There is an inconsistency problem on how admin.rs.html handles default tab and how admin_mod.rs.html does it. Actually the Configuration tab has been hardcoded inside admin.rs.html which is not available in admin_mod.rs.html. Also other tabs have their own layout (i.e. they are not a child template) so it's not feasible to include the first moderation tab (Instances) by default to be loaded in /admin when the user is a moderator. I just left it as is for you to make a decision:

  1. We can just redirect the /admin to their first tab route, both for admin and moderator, this needs the creation of a separate template for Configuration tab and then both admin.rs.html and admin_mod.rs.html would be useless.
  2. We can leave it as is, and later show some dashboard like stuff as the default content of the /admin and instead of tabs on the top, use some other type of UX component to show different pages.
  3. For now we can just show a welcome message to admins and moderators as the default content of the /admin route and do the #1.

If you want I can go for one of these or any other idea you have but the 0, 1 or even 2 doesn't make any difference here, the result would be an empty admin page with some tabs.

There is an inconsistency problem on how `admin.rs.html` handles default tab and how `admin_mod.rs.html` does it. Actually the Configuration tab has been hardcoded inside `admin.rs.html` which is not available in `admin_mod.rs.html`. Also other tabs have their own layout (i.e. they are not a child template) so it's not feasible to include the first moderation tab (Instances) by default to be loaded in `/admin` when the user is a moderator. I just left it as is for you to make a decision: 1. We can just redirect the `/admin` to their first tab route, both for admin and moderator, this needs the creation of a separate template for Configuration tab and then both `admin.rs.html` and `admin_mod.rs.html` would be useless. 2. We can leave it as is, and later show some dashboard like stuff as the default content of the `/admin` and instead of tabs on the top, use some other type of UX component to show different pages. 3. For now we can just show a welcome message to admins and moderators as the default content of the `/admin` route and do the #1. If you want I can go for one of these or any other idea you have but the `0`, `1` or even `2` doesn't make any difference here, the result would be an empty admin page with some tabs.
trinity-1686a commented 2024-07-07 14:41:23 +00:00
Review
Copy link

I agree that 0 or 1 changes nothing, i just find it more coherent. We are in a way on the configuration tab which refused to load. But yeah, the html generated is the same either way (2 would change slightly the result, showing Instances as the selected tab i believe)

I agree that 0 or 1 changes nothing, i just find it more coherent. We are in a way on the configuration tab which refused to load. But yeah, the html generated is the same either way (2 would change slightly the result, showing `Instances` as the selected tab i believe)
gro2bl commented 2024-07-08 11:01:35 +00:00
Review
Copy link

2 It shows Instances as selected tab but without the instances page template because "Instances" template is a whole page template not a partial template and we cannot include it as a child inside admin_mod. that's why we might need to do one of those 3 solutions I mentioned above.

2 It shows Instances as selected tab but without the instances page template because "Instances" template is a whole page template not a partial template and we cannot include it as a child inside admin_mod. that's why we might need to do one of those 3 solutions I mentioned above.
})

12
templates/instance/emailblocklist.rs.html
View file

@ -1,17 +1,11 @@
@use plume_models::blocklisted_emails::BlocklistedEmail;
@use crate::templates::base;
@use crate::templates::{base, instance::admin_header};
@use crate::template_utils::*;
@use crate::routes::*;
@(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(&[
(&uri!(instance::admin).to_string(), i18n!(ctx.1, "Configuration"), false),
(&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"), true),
])
@:base(ctx, i18n!(ctx.1, "Blocklisted Emails"), {}, {}, {
@:admin_header(ctx, "Blocklisted Emails", 4)
<form method="post" action="@uri!(instance::add_email_blocklist)">
@(Input::new("email_address", i18n!(ctx.1, "Email address"))
.details(i18n!(ctx.1, "The email address you wish to block. In order to block domains, you can use globbing syntax, for example '*@example.com' blocks all addresses from example.com"))

11
templates/instance/list.rs.html
View file

@ -1,19 +1,12 @@
@use plume_models::instance::Instance;
@use crate::templates::base;
@use crate::templates::{base, instance::admin_header};
@use crate::template_utils::*;
@use crate::routes::*;
@(ctx: BaseContext, instance: Instance, instances: Vec<Instance>, page: i32, n_pages: i32)
@:base(ctx, i18n!(ctx.1, "Administration of {0}"; instance.name), {}, {}, {
<h1>@i18n!(ctx.1, "Instances")</h1>
@tabs(&[
(&uri!(instance::admin).to_string(), i18n!(ctx.1, "Configuration"), false),
(&uri!(instance::admin_instances: page = _).to_string(), i18n!(ctx.1, "Instances"), true),
(&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),
])
@:admin_header(ctx, "Instances", 2))
<div class="list">
@for instance in instances {

11
templates/instance/users.rs.html
View file

@ -1,19 +1,12 @@
@use plume_models::users::User;
@use crate::templates::base;
@use crate::templates::{base, instance::admin_header};
@use crate::template_utils::*;
@use crate::routes::*;
@(ctx: BaseContext, users: Vec<User>, user: Option<&str>, page: i32, n_pages: i32)
@:base(ctx, i18n!(ctx.1, "Users"), {}, {}, {
<h1>@i18n!(ctx.1, "Users")</h1>
@tabs(&[
(&uri!(instance::admin).to_string(), i18n!(ctx.1, "Configuration"), false),
(&uri!(instance::admin_instances: page = _).to_string(), i18n!(ctx.1, "Instances"), false),
(&uri!(instance::admin_users: page = _).to_string(), i18n!(ctx.1, "Users"), true),
(&uri!(instance::admin_email_blocklist: page=_).to_string(), i18n!(ctx.1, "Email blocklist"), false)
])
@:admin_header(ctx, "Users", 3))
<form method="get" action="@uri!(instance::admin_search_users: page = _, user = user.unwrap_or_default())">
<header>