Apply fix suggestions to spaces pallet

pallets/spaces/Cargo.toml

@@ -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']

pallets/spaces/src/lib.rs

@@ -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,
@@ -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,
@@ -68,9 +68,6 @@ pub mod pallet {
 
         type IsContentBlocked: IsContentBlocked;
 
-        #[pallet::constant]
-        type MaxHandleLen: Get<u32>;
-
         #[pallet::constant]
         type MaxSpacesPerAccount: Get<u32>;
     }
@@ -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.
@@ -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> {
@@ -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());
         }
     }
 
@@ -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)?;
@@ -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));
@@ -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!(
@@ -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(())
         }
 
@@ -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;
@@ -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())
@@ -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 })
         }
     }
 

pallets/spaces/src/types.rs

@@ -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.
@@ -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
@@ -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>,
@@ -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,
@@ -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,
         }
     }
@@ -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 {