Removing the site creator, adding leave_admin. Fixes #1808 (#2052)

* Removing the site creator, adding leave_admin. Fixes #1808

* Making sure there's at least one admin. Fixing unit tests
This commit is contained in:
Dessalines 2022-01-26 12:57:16 -05:00 committed by GitHub
parent 1372827b41
commit e36ad9d984
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 63 additions and 116 deletions

View file

@ -44,7 +44,6 @@ use lemmy_db_schema::{
}, },
person::Person, person::Person,
post::Post, post::Post,
site::Site,
}, },
traits::{Bannable, Blockable, Crud, Followable, Joinable}, traits::{Bannable, Blockable, Crud, Followable, Joinable},
}; };
@ -457,20 +456,7 @@ impl Perform for TransferCommunity {
let local_user_view = let local_user_view =
get_local_user_view_from_jwt(&data.auth, context.pool(), context.secret()).await?; get_local_user_view_from_jwt(&data.auth, context.pool(), context.secret()).await?;
let site_creator_id = blocking(context.pool(), move |conn| { let admins = blocking(context.pool(), PersonViewSafe::admins).await??;
Site::read(conn, 1).map(|s| s.creator_id)
})
.await??;
let mut admins = blocking(context.pool(), PersonViewSafe::admins).await??;
// Making sure the site creator, if an admin, is at the top
let creator_index = admins
.iter()
.position(|r| r.person.id == site_creator_id)
.context(location_info!())?;
let creator_person = admins.remove(creator_index);
admins.insert(0, creator_person);
// Fetch the community mods // Fetch the community mods
let community_id = data.community_id; let community_id = data.community_id;

View file

@ -111,9 +111,7 @@ pub async fn match_websocket_operation(
UserOperation::TransferCommunity => { UserOperation::TransferCommunity => {
do_websocket_operation::<TransferCommunity>(context, id, op, data).await do_websocket_operation::<TransferCommunity>(context, id, op, data).await
} }
UserOperation::TransferSite => { UserOperation::LeaveAdmin => do_websocket_operation::<LeaveAdmin>(context, id, op, data).await,
do_websocket_operation::<TransferSite>(context, id, op, data).await
}
// Community ops // Community ops
UserOperation::FollowCommunity => { UserOperation::FollowCommunity => {

View file

@ -1,6 +1,5 @@
use crate::{captcha_as_wav_base64, Perform}; use crate::{captcha_as_wav_base64, Perform};
use actix_web::web::Data; use actix_web::web::Data;
use anyhow::Context;
use bcrypt::verify; use bcrypt::verify;
use captcha::{gen, Difficulty}; use captcha::{gen, Difficulty};
use chrono::Duration; use chrono::Duration;
@ -51,7 +50,6 @@ use lemmy_db_views_actor::{
}; };
use lemmy_utils::{ use lemmy_utils::{
claims::Claims, claims::Claims,
location_info,
utils::{is_valid_display_name, is_valid_matrix_id, naive_from_unix}, utils::{is_valid_display_name, is_valid_matrix_id, naive_from_unix},
ConnectionId, ConnectionId,
LemmyError, LemmyError,
@ -409,18 +407,7 @@ impl Perform for AddAdmin {
blocking(context.pool(), move |conn| ModAdd::create(conn, &form)).await??; blocking(context.pool(), move |conn| ModAdd::create(conn, &form)).await??;
let site_creator_id = blocking(context.pool(), move |conn| { let admins = blocking(context.pool(), PersonViewSafe::admins).await??;
Site::read(conn, 1).map(|s| s.creator_id)
})
.await??;
let mut admins = blocking(context.pool(), PersonViewSafe::admins).await??;
let creator_index = admins
.iter()
.position(|r| r.person.id == site_creator_id)
.context(location_info!())?;
let creator_person = admins.remove(creator_index);
admins.insert(0, creator_person);
let res = AddAdminResponse { admins }; let res = AddAdminResponse { admins };

View file

@ -1,6 +1,5 @@
use crate::Perform; use crate::Perform;
use actix_web::web::Data; use actix_web::web::Data;
use anyhow::Context;
use diesel::NotFound; use diesel::NotFound;
use lemmy_api_common::{ use lemmy_api_common::{
blocking, blocking,
@ -27,6 +26,7 @@ use lemmy_db_schema::{
source::{ source::{
local_user::{LocalUser, LocalUserForm}, local_user::{LocalUser, LocalUserForm},
moderator::*, moderator::*,
person::Person,
registration_application::{RegistrationApplication, RegistrationApplicationForm}, registration_application::{RegistrationApplication, RegistrationApplicationForm},
site::Site, site::Site,
}, },
@ -62,7 +62,7 @@ use lemmy_db_views_moderator::{
mod_sticky_post_view::ModStickyPostView, mod_sticky_post_view::ModStickyPostView,
mod_transfer_community_view::ModTransferCommunityView, mod_transfer_community_view::ModTransferCommunityView,
}; };
use lemmy_utils::{location_info, settings::structs::Settings, version, ConnectionId, LemmyError}; use lemmy_utils::{settings::structs::Settings, version, ConnectionId, LemmyError};
use lemmy_websocket::LemmyContext; use lemmy_websocket::LemmyContext;
#[async_trait::async_trait(?Send)] #[async_trait::async_trait(?Send)]
@ -462,7 +462,7 @@ async fn convert_response(
} }
#[async_trait::async_trait(?Send)] #[async_trait::async_trait(?Send)]
impl Perform for TransferSite { impl Perform for LeaveAdmin {
type Response = GetSiteResponse; type Response = GetSiteResponse;
#[tracing::instrument(skip(context, _websocket_id))] #[tracing::instrument(skip(context, _websocket_id))]
@ -471,44 +471,36 @@ impl Perform for TransferSite {
context: &Data<LemmyContext>, context: &Data<LemmyContext>,
_websocket_id: Option<ConnectionId>, _websocket_id: Option<ConnectionId>,
) -> Result<GetSiteResponse, LemmyError> { ) -> Result<GetSiteResponse, LemmyError> {
let data: &TransferSite = self; let data: &LeaveAdmin = self;
let local_user_view = let local_user_view =
get_local_user_view_from_jwt(&data.auth, context.pool(), context.secret()).await?; get_local_user_view_from_jwt(&data.auth, context.pool(), context.secret()).await?;
is_admin(&local_user_view)?; is_admin(&local_user_view)?;
let read_site = blocking(context.pool(), Site::read_simple).await??; // Make sure there isn't just one admin (so if one leaves, there will still be one left)
let admins = blocking(context.pool(), PersonViewSafe::admins).await??;
// Make sure user is the creator if admins.len() == 1 {
if read_site.creator_id != local_user_view.person.id { return Err(LemmyError::from_message("cannot_leave_admin"));
return Err(LemmyError::from_message("not_an_admin"));
} }
let new_creator_id = data.person_id; let person_id = local_user_view.person.id;
let transfer_site = move |conn: &'_ _| Site::transfer(conn, new_creator_id); blocking(context.pool(), move |conn| {
blocking(context.pool(), transfer_site) Person::leave_admin(conn, person_id)
.await? })
.map_err(LemmyError::from) .await??;
.map_err(|e| e.with_message("couldnt_update_site"))?;
// Mod tables // Mod tables
let form = ModAddForm { let form = ModAddForm {
mod_person_id: local_user_view.person.id, mod_person_id: person_id,
other_person_id: data.person_id, other_person_id: person_id,
removed: Some(false), removed: Some(true),
}; };
blocking(context.pool(), move |conn| ModAdd::create(conn, &form)).await??; blocking(context.pool(), move |conn| ModAdd::create(conn, &form)).await??;
// Reread site and admins
let site_view = blocking(context.pool(), SiteView::read).await??; let site_view = blocking(context.pool(), SiteView::read).await??;
let admins = blocking(context.pool(), PersonViewSafe::admins).await??;
let mut admins = blocking(context.pool(), PersonViewSafe::admins).await??;
let creator_index = admins
.iter()
.position(|r| r.person.id == site_view.creator.id)
.context(location_info!())?;
let creator_person = admins.remove(creator_index);
admins.insert(0, creator_person);
let federated_instances = build_federated_instances( let federated_instances = build_federated_instances(
context.pool(), context.pool(),

View file

@ -155,8 +155,7 @@ pub struct MyUserInfo {
} }
#[derive(Debug, Serialize, Deserialize)] #[derive(Debug, Serialize, Deserialize)]
pub struct TransferSite { pub struct LeaveAdmin {
pub person_id: PersonId,
pub auth: Sensitive<String>, pub auth: Sensitive<String>,
} }

View file

@ -62,7 +62,6 @@ impl PerformCrud for CreateSite {
description, description,
icon, icon,
banner, banner,
creator_id: local_user_view.person.id,
enable_downvotes: data.enable_downvotes, enable_downvotes: data.enable_downvotes,
open_registration: data.open_registration, open_registration: data.open_registration,
enable_nsfw: data.enable_nsfw, enable_nsfw: data.enable_nsfw,

View file

@ -79,18 +79,7 @@ impl PerformCrud for GetSite {
} }
}; };
let mut admins = blocking(context.pool(), PersonViewSafe::admins).await??; let admins = blocking(context.pool(), PersonViewSafe::admins).await??;
// Make sure the site creator is the top admin
if let Some(site_view) = site_view.to_owned() {
let site_creator_id = site_view.creator.id;
// TODO investigate why this is sometimes coming back null
// Maybe user_.admin isn't being set to true?
if let Some(creator_index) = admins.iter().position(|r| r.person.id == site_creator_id) {
let creator_person = admins.remove(creator_index);
admins.insert(0, creator_person);
}
}
let online = context let online = context
.chat_server() .chat_server()

View file

@ -54,7 +54,6 @@ impl PerformCrud for EditSite {
} }
let site_form = SiteForm { let site_form = SiteForm {
creator_id: found_site.creator_id,
name: data.name.to_owned().unwrap_or(found_site.name), name: data.name.to_owned().unwrap_or(found_site.name),
sidebar, sidebar,
description, description,

View file

@ -55,7 +55,6 @@ mod tests {
let site_form = SiteForm { let site_form = SiteForm {
name: "test_site".into(), name: "test_site".into(),
creator_id: inserted_person.id,
sidebar: None, sidebar: None,
description: None, description: None,
icon: None, icon: None,
@ -133,7 +132,12 @@ mod tests {
let community_num_deleted = Community::delete(&conn, inserted_community.id).unwrap(); let community_num_deleted = Community::delete(&conn, inserted_community.id).unwrap();
assert_eq!(1, community_num_deleted); assert_eq!(1, community_num_deleted);
let after_delete = SiteAggregates::read(&conn); // Site should still exist, it can without a site creator.
assert!(after_delete.is_err()); let after_delete_creator = SiteAggregates::read(&conn);
assert!(after_delete_creator.is_ok());
Site::delete(&conn, 1).unwrap();
let after_delete_site = SiteAggregates::read(&conn);
assert!(after_delete_site.is_err());
} }
} }

View file

@ -274,6 +274,12 @@ impl Person {
pub fn is_banned(&self) -> bool { pub fn is_banned(&self) -> bool {
is_banned(self.banned, self.ban_expires) is_banned(self.banned, self.ban_expires)
} }
pub fn leave_admin(conn: &PgConnection, person_id: PersonId) -> Result<Self, Error> {
diesel::update(person.find(person_id))
.set(admin.eq(false))
.get_result::<Self>(conn)
}
} }
impl PersonSafe { impl PersonSafe {

View file

@ -1,4 +1,4 @@
use crate::{naive_now, newtypes::PersonId, source::site::*, traits::Crud}; use crate::{source::site::*, traits::Crud};
use diesel::{dsl::*, result::Error, *}; use diesel::{dsl::*, result::Error, *};
impl Crud for Site { impl Crud for Site {
@ -27,13 +27,6 @@ impl Crud for Site {
} }
impl Site { impl Site {
pub fn transfer(conn: &PgConnection, new_creator_id: PersonId) -> Result<Site, Error> {
use crate::schema::site::dsl::*;
diesel::update(site.find(1))
.set((creator_id.eq(new_creator_id), updated.eq(naive_now())))
.get_result::<Self>(conn)
}
pub fn read_simple(conn: &PgConnection) -> Result<Self, Error> { pub fn read_simple(conn: &PgConnection) -> Result<Self, Error> {
use crate::schema::site::dsl::*; use crate::schema::site::dsl::*;
site.first::<Self>(conn) site.first::<Self>(conn)

View file

@ -441,7 +441,6 @@ table! {
id -> Int4, id -> Int4,
name -> Varchar, name -> Varchar,
sidebar -> Nullable<Text>, sidebar -> Nullable<Text>,
creator_id -> Int4,
published -> Timestamp, published -> Timestamp,
updated -> Nullable<Timestamp>, updated -> Nullable<Timestamp>,
enable_downvotes -> Bool, enable_downvotes -> Bool,
@ -648,7 +647,6 @@ joinable!(post_read -> post (post_id));
joinable!(post_report -> post (post_id)); joinable!(post_report -> post (post_id));
joinable!(post_saved -> person (person_id)); joinable!(post_saved -> person (person_id));
joinable!(post_saved -> post (post_id)); joinable!(post_saved -> post (post_id));
joinable!(site -> person (creator_id));
joinable!(site_aggregates -> site (site_id)); joinable!(site_aggregates -> site (site_id));
joinable!(email_verification -> local_user (local_user_id)); joinable!(email_verification -> local_user (local_user_id));
joinable!(registration_application -> local_user (local_user_id)); joinable!(registration_application -> local_user (local_user_id));

View file

@ -1,7 +1,4 @@
use crate::{ use crate::{newtypes::DbUrl, schema::site};
newtypes::{DbUrl, PersonId},
schema::site,
};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
#[derive(Queryable, Identifiable, PartialEq, Debug, Clone, Serialize, Deserialize)] #[derive(Queryable, Identifiable, PartialEq, Debug, Clone, Serialize, Deserialize)]
@ -10,7 +7,6 @@ pub struct Site {
pub id: i32, pub id: i32,
pub name: String, pub name: String,
pub sidebar: Option<String>, pub sidebar: Option<String>,
pub creator_id: PersonId,
pub published: chrono::NaiveDateTime, pub published: chrono::NaiveDateTime,
pub updated: Option<chrono::NaiveDateTime>, pub updated: Option<chrono::NaiveDateTime>,
pub enable_downvotes: bool, pub enable_downvotes: bool,
@ -30,7 +26,6 @@ pub struct Site {
#[table_name = "site"] #[table_name = "site"]
pub struct SiteForm { pub struct SiteForm {
pub name: String, pub name: String,
pub creator_id: PersonId,
pub sidebar: Option<Option<String>>, pub sidebar: Option<Option<String>>,
pub updated: Option<chrono::NaiveDateTime>, pub updated: Option<chrono::NaiveDateTime>,
pub enable_downvotes: Option<bool>, pub enable_downvotes: Option<bool>,

View file

@ -1,38 +1,24 @@
use diesel::{result::Error, *}; use diesel::{result::Error, *};
use lemmy_db_schema::{ use lemmy_db_schema::{
aggregates::site_aggregates::SiteAggregates, aggregates::site_aggregates::SiteAggregates,
schema::{person, site, site_aggregates}, schema::{site, site_aggregates},
source::{ source::site::Site,
person::{Person, PersonSafe},
site::Site,
},
traits::ToSafe,
}; };
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
#[derive(Debug, Serialize, Deserialize, Clone)] #[derive(Debug, Serialize, Deserialize, Clone)]
pub struct SiteView { pub struct SiteView {
pub site: Site, pub site: Site,
pub creator: PersonSafe,
pub counts: SiteAggregates, pub counts: SiteAggregates,
} }
impl SiteView { impl SiteView {
pub fn read(conn: &PgConnection) -> Result<Self, Error> { pub fn read(conn: &PgConnection) -> Result<Self, Error> {
let (site, creator, counts) = site::table let (site, counts) = site::table
.inner_join(person::table)
.inner_join(site_aggregates::table) .inner_join(site_aggregates::table)
.select(( .select((site::all_columns, site_aggregates::all_columns))
site::all_columns, .first::<(Site, SiteAggregates)>(conn)?;
Person::safe_columns_tuple(),
site_aggregates::all_columns,
))
.first::<(Site, PersonSafe, SiteAggregates)>(conn)?;
Ok(SiteView { Ok(SiteView { site, counts })
site,
creator,
counts,
})
} }
} }

View file

@ -136,7 +136,7 @@ pub enum UserOperation {
MarkAllAsRead, MarkAllAsRead,
SaveUserSettings, SaveUserSettings,
TransferCommunity, TransferCommunity,
TransferSite, LeaveAdmin,
PasswordReset, PasswordReset,
PasswordChange, PasswordChange,
MarkPrivateMessageAsRead, MarkPrivateMessageAsRead,

View file

@ -0,0 +1,14 @@
-- Add the column back
alter table site add column creator_id int references person on update cascade on delete cascade;
-- Add the data, selecting the highest admin
update site
set creator_id = sub.id
from (
select id from person
where admin = true
limit 1
) as sub;
-- Set to not null
alter table site alter column creator_id set not null;

View file

@ -0,0 +1,2 @@
-- Drop the column
alter table site drop column creator_id;

View file

@ -19,7 +19,6 @@ pub fn config(cfg: &mut web::ServiceConfig, rate_limit: &RateLimit) {
// Admin Actions // Admin Actions
.route("", web::post().to(route_post_crud::<CreateSite>)) .route("", web::post().to(route_post_crud::<CreateSite>))
.route("", web::put().to(route_post_crud::<EditSite>)) .route("", web::put().to(route_post_crud::<EditSite>))
.route("/transfer", web::post().to(route_post::<TransferSite>))
.route("/config", web::get().to(route_get::<GetSiteConfig>)) .route("/config", web::get().to(route_get::<GetSiteConfig>))
.route("/config", web::put().to(route_post::<SaveSiteConfig>)), .route("/config", web::put().to(route_post::<SaveSiteConfig>)),
) )
@ -212,7 +211,8 @@ pub fn config(cfg: &mut web::ServiceConfig, rate_limit: &RateLimit) {
) )
.route("/report_count", web::get().to(route_get::<GetReportCount>)) .route("/report_count", web::get().to(route_get::<GetReportCount>))
.route("/unread_count", web::get().to(route_get::<GetUnreadCount>)) .route("/unread_count", web::get().to(route_get::<GetUnreadCount>))
.route("/verify_email", web::post().to(route_post::<VerifyEmail>)), .route("/verify_email", web::post().to(route_post::<VerifyEmail>))
.route("/leave_admin", web::post().to(route_post::<LeaveAdmin>)),
) )
// Admin Actions // Admin Actions
.service( .service(