Skip to content

Commit

Permalink
Apply fix suggestions to spaces pallet (#79)
Browse files Browse the repository at this point in the history 
* Refactor spaces pallet code

* Fix code formatting in spaces pallet

* Remove handles and scores from spaces

* Rename some variables

* Fix spaces pallet in a runtime

* Fix formatting manually

* Fix `init_pallet` in `pallet_spaces`
  • Loading branch information
@F3Joule
F3Joule committed Jul 8, 2022
1 parent 8dececb commit 04e70c8
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 164 deletions.
2 changes: 1 addition & 1 deletion pallets/spaces/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ authors = ['DappForce <dappforce@pm.me>']
edition = '2021'
license = 'GPL-3.0-only'
homepage = 'https://subsocial.network'
repository = 'https://github.com/dappforce/dappforce-subsocial-node'
repository = 'https://github.com/dappforce/subsocial-parachain'
description = 'Space management pallet'
keywords = ['blockchain', 'cryptocurrency', 'social-network', 'news-feed', 'marketplace']
categories = ['cryptography::cryptocurrencies']
Expand Down
132 changes: 34 additions & 98 deletions pallets/spaces/src/lib.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! # Spaces Module
//!
//! Spaces are the primary components of Subsocial. This module allows you to create a Space
//! and customize it by updating its' owner(s), content, unique handle, and permissions.
//! and customize it by updating its' owner(s), content, and permissions.
//!
//! To understand how Spaces fit into the Subsocial ecosystem, you can think of how
//! folders and files work in a file system. Spaces are similar to folders, that can contain Posts,
Expand Down Expand Up @@ -41,7 +41,7 @@ pub mod pallet {
use frame_system::pallet_prelude::*;

use pallet_permissions::{
Pallet as Permissions, SpacePermissionsContext, PermissionChecker, SpacePermissionsInfoOf,
Pallet as Permissions, PermissionChecker, SpacePermissionsContext, SpacePermissionsInfoOf,
};
use subsocial_support::{
ensure_content_is_valid,
Expand All @@ -68,9 +68,6 @@ pub mod pallet {

type IsContentBlocked: IsContentBlocked;

#[pallet::constant]
type MaxHandleLen: Get<u32>;

#[pallet::constant]
type MaxSpacesPerAccount: Get<u32>;
}
Expand All @@ -88,19 +85,10 @@ pub mod pallet {
SpaceDeleted(T::AccountId, SpaceId),
}

// TODO_REMOVE_IF_NO_EVENT
/// Old name generated by `decl_event`.
#[deprecated(note = "use `Event` instead")]
pub type RawEvent<T> = Event<T>;

#[pallet::error]
pub enum Error<T> {
/// Space was not found by id.
SpaceNotFound,
/// Space handle is not unique.
SpaceHandleIsNotUnique,
/// Handles are disabled in `PalletSettings`.
HandlesAreDisabled,
/// Nothing to update in this space.
NoUpdatesForSpace,
/// Only space owners can manage this space.
Expand Down Expand Up @@ -132,21 +120,11 @@ pub mod pallet {
#[pallet::getter(fn space_by_id)]
pub type SpaceById<T: Config> = StorageMap<_, Twox64Concat, SpaceId, Space<T>>;

/// Find a given space id by its' unique handle.
/// If a handle is not registered, nothing will be returned (`None`).
#[pallet::storage]
#[pallet::getter(fn space_id_by_handle)]
pub type SpaceIdByHandle<T: Config> = StorageMap<_, Blake2_128Concat, Handle<T>, SpaceId>;

/// Find the ids of all spaces owned, by a given account.
#[pallet::storage]
#[pallet::getter(fn space_ids_by_owner)]
pub type SpaceIdsByOwner<T: Config> =
StorageMap<_, Twox64Concat, T::AccountId, SpacesByAccount<T>, ValueQuery>;

#[pallet::storage]
#[pallet::getter(fn settings)]
pub type PalletSettings<T: Config> = StorageValue<_, SpacesSettings, ValueQuery>;
StorageMap<_, Twox64Concat, T::AccountId, SpacesByAccount<T>, ValueQuery>;

#[pallet::genesis_config]
pub struct GenesisConfig<T: Config> {
Expand All @@ -156,28 +134,14 @@ pub mod pallet {
#[cfg(feature = "std")]
impl<T: Config> Default for GenesisConfig<T> {
fn default() -> Self {
Self {
endowed_account: None,
}
Self { endowed_account: None }
}
}

#[pallet::genesis_build]
impl<T: Config> GenesisBuild<T> for GenesisConfig<T> {
fn build(&self) {
if let Some(endowed_account) = self.endowed_account.clone() {
let mut spaces = Vec::new();

for id in FIRST_SPACE_ID..=RESERVED_SPACE_COUNT {
spaces.push((
id,
Space::<T>::new(id, None, endowed_account.clone(), Content::None, None),
));
}
spaces.iter().for_each(|(k, v)| {
SpaceById::<T>::insert(k, v);
});
}
Pallet::<T>::init_pallet(self.endowed_account.as_ref());
}
}

Expand All @@ -187,15 +151,11 @@ pub mod pallet {
pub fn create_space(
origin: OriginFor<T>,
parent_id_opt: Option<SpaceId>,
// FIXME: unused since domains release
handle_opt: Option<Vec<u8>>,
content: Content,
permissions_opt: Option<SpacePermissions>,
) -> DispatchResultWithPostInfo {
let owner = ensure_signed(origin)?;

ensure!(handle_opt.is_none(), Error::<T>::HandlesAreDisabled);

ensure_content_is_valid(content.clone())?;

Self::ensure_space_limit_not_reached(&owner)?;
Expand Down Expand Up @@ -225,23 +185,15 @@ pub mod pallet {
permissions_opt.map(|perms| Permissions::<T>::override_permissions(perms));

let space_id = Self::next_space_id();
let new_space = &mut Space::new(
space_id,
parent_id_opt,
owner.clone(),
content,
permissions,
);
let new_space =
&mut Space::new(space_id, parent_id_opt, owner.clone(), content, permissions);

// FIXME: What's about handle reservation if this fails?
T::BeforeSpaceCreated::before_space_created(owner.clone(), new_space)?;

SpaceById::<T>::insert(space_id, new_space);
SpaceIdsByOwner::<T>::mutate(
&owner, |ids| {
ids.try_push(space_id).expect("qed; too many spaces per account")
}
);
SpaceIdsByOwner::<T>::mutate(&owner, |ids| {
ids.try_push(space_id).expect("qed; too many spaces per account")
});
NextSpaceId::<T>::mutate(|n| *n += 1);

Self::deposit_event(Event::SpaceCreated(owner, space_id));
Expand All @@ -256,15 +208,14 @@ pub mod pallet {
) -> DispatchResult {
let owner = ensure_signed(origin)?;

let has_updates = update.parent_id.is_some()
|| update.content.is_some()
|| update.hidden.is_some()
|| update.permissions.is_some();
let has_updates =
update.parent_id.is_some() ||
update.content.is_some() ||
update.hidden.is_some() ||
update.permissions.is_some();

ensure!(has_updates, Error::<T>::NoUpdatesForSpace);

ensure!(update.handle.is_none(), Error::<T>::HandlesAreDisabled);

let mut space = Self::require_space(space_id)?;

ensure!(
Expand Down Expand Up @@ -356,34 +307,30 @@ pub mod pallet {
}
Ok(())
}
}

#[pallet::weight(10_000 + T::DbWeight::get().reads_writes(1, 1))]
pub fn update_settings(
origin: OriginFor<T>,
new_settings: SpacesSettings,
) -> DispatchResultWithPostInfo {
ensure_root(origin)?;

let space_settings = Self::settings();
ensure!(
space_settings != new_settings,
Error::<T>::NoUpdatesForSpacesSettings
);

PalletSettings::<T>::mutate(|settings| *settings = new_settings);
impl<T: Config> Pallet<T> {
/// Create reserved spaces either on genesis build or when pallet is added to a runtime.
pub fn init_pallet(endowed_account_opt: Option<&T::AccountId>) {
if let Some(endowed_account) = endowed_account_opt {
let mut spaces = Vec::new();

Ok(().into())
for id in FIRST_SPACE_ID..=RESERVED_SPACE_COUNT {
spaces.push((
id,
Space::<T>::new(id, None, endowed_account.clone(), Content::None, None),
));
}
spaces.iter().for_each(|(space_id, space)| {
SpaceById::<T>::insert(space_id, space);
});
}
}
}

impl<T: Config> Pallet<T> {
/// Check that there is a `Space` with such `space_id` in the storage
/// or return`SpaceNotFound` error.
pub fn ensure_space_exists(space_id: SpaceId) -> DispatchResult {
ensure!(
<SpaceById<T>>::contains_key(space_id),
Error::<T>::SpaceNotFound
);
ensure!(<SpaceById<T>>::contains_key(space_id), Error::<T>::SpaceNotFound);
Ok(())
}

Expand Down Expand Up @@ -411,14 +358,6 @@ pub mod pallet {
T::Roles::ensure_account_has_space_permission(account, ctx, permission, error)
}

pub fn ensure_handles_enabled() -> DispatchResult {
ensure!(
Self::settings().handles_enabled,
Error::<T>::HandlesAreDisabled
);
Ok(())
}

pub fn try_move_space_to_root(space_id: SpaceId) -> DispatchResult {
let mut space = Self::require_space(space_id)?;
space.parent_id = None;
Expand All @@ -436,7 +375,7 @@ pub mod pallet {
f(space);
*space_opt = Some(space.clone());

return Ok(space.clone());
return Ok(space.clone())
}

Err(Error::<T>::SpaceNotFound.into())
Expand All @@ -458,10 +397,7 @@ pub mod pallet {
) -> Result<SpacePermissionsInfoOf<T>, DispatchError> {
let space = Pallet::<T>::require_space(id)?;

Ok(SpacePermissionsInfo {
owner: space.owner,
permissions: space.permissions,
})
Ok(SpacePermissionsInfo { owner: space.owner, permissions: space.permissions })
}
}

Expand Down
35 changes: 0 additions & 35 deletions pallets/spaces/src/migrations.rs
View file

This file was deleted.

30 changes: 2 additions & 28 deletions pallets/spaces/src/types.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
use frame_support::pallet_prelude::*;

use subsocial_support::{WhoAndWhenOf, new_who_and_when};
use subsocial_support::{new_who_and_when, WhoAndWhenOf};

use super::*;
use sp_std::vec::Vec;

pub const FIRST_SPACE_ID: u64 = 1;
pub const RESERVED_SPACE_COUNT: u64 = 1000;
pub const DEFAULT_MAX_HANDLE_LEN: u32 = 50;

pub(crate) type Handle<T> = BoundedVec<u8, <T as Config>::MaxHandleLen>;
pub(crate) type SpacesByAccount<T> = BoundedVec<SpaceId, <T as Config>::MaxSpacesPerAccount>;

/// Information about a space's owner, its' content, visibility and custom permissions.
Expand All @@ -28,10 +25,6 @@ pub struct Space<T: Config> {
// The next fields can be updated by the owner:
pub(super) parent_id: Option<SpaceId>,

/// Unique alpha-numeric identifier that can be used in a space's URL.
/// Handle can only contain numbers, letter and underscore: `0`-`9`, `a`-`z`, `_`.
pub handle: Option<Handle<T>>,

pub content: Content,

/// Hidden field is used to recommend to end clients (web and mobile apps) that a particular
Expand All @@ -47,8 +40,6 @@ pub struct Space<T: Config> {
/// The number of account following a given space.
pub followers_count: u32,

pub(super) score: i32,

/// This allows you to override Subsocial's default permissions by enabling or disabling role
/// permissions.
pub permissions: Option<SpacePermissions>,
Expand All @@ -57,25 +48,11 @@ pub struct Space<T: Config> {
#[derive(Encode, Decode, Clone, Eq, PartialEq, Default, RuntimeDebug, TypeInfo)]
pub struct SpaceUpdate {
pub parent_id: Option<Option<SpaceId>>,
pub handle: Option<Option<Vec<u8>>>,
pub content: Option<Content>,
pub hidden: Option<bool>,
pub permissions: Option<Option<SpacePermissions>>,
}

#[derive(Encode, Decode, Clone, Eq, PartialEq, RuntimeDebug, TypeInfo)]
pub struct SpacesSettings {
pub handles_enabled: bool,
}

impl Default for SpacesSettings {
fn default() -> Self {
Self {
handles_enabled: true,
}
}
}

impl<T: Config> Space<T> {
pub fn new(
id: SpaceId,
Expand All @@ -90,13 +67,11 @@ impl<T: Config> Space<T> {
updated: None,
owner: created_by,
parent_id,
handle: Default::default(),
content,
hidden: false,
posts_count: 0,
hidden_posts_count: 0,
followers_count: 0,
score: 0,
permissions,
}
}
Expand Down Expand Up @@ -139,8 +114,7 @@ impl<T: Config> Space<T> {
}

pub fn try_get_parent(&self) -> Result<SpaceId, DispatchError> {
self.parent_id
.ok_or_else(|| Error::<T>::SpaceIsAtRoot.into())
self.parent_id.ok_or_else(|| Error::<T>::SpaceIsAtRoot.into())
}

pub fn is_public(&self) -> bool {
Expand Down
Loading

0 comments on commit 04e70c8

Please sign in to comment.