Merge pull request #1774 from iorveth/issue_1363

Bug in update_category_membership_of_moderator of forum & dry fixes

runtime-modules/forum/src/lib.rs

@@ -441,7 +441,7 @@ decl_module! {
 
             let account_id = ensure_signed(origin)?;
 
-            Self::ensure_can_update_category_membership_of_moderator(account_id, &category_id)?;
+            Self::ensure_can_update_category_membership_of_moderator(account_id, &category_id, new_value)?;
 
             //
             // == MUTATION SAFE ==
@@ -451,14 +451,12 @@ decl_module! {
                 <CategoryByModerator<T>>::insert(category_id, moderator_id, ());
 
                 <CategoryById<T>>::mutate(category_id, |category| category.num_direct_moderators += 1);
+            } else {
+                <CategoryByModerator<T>>::remove(category_id, moderator_id);
 
-                return Ok(());
+                <CategoryById<T>>::mutate(category_id, |category| category.num_direct_moderators -= 1);
             }
 
-            <CategoryByModerator<T>>::remove(category_id, moderator_id);
-
-            <CategoryById<T>>::mutate(category_id, |category| category.num_direct_moderators -= 1);
-
             Ok(())
         }
 
@@ -1330,19 +1328,10 @@ impl<T: Trait> Module<T> {
         category_id: &T::CategoryId,
     ) -> Result<Category<T::CategoryId, T::ThreadId, T::Hash>, Error<T>> {
         // Check actor's role
-        match actor {
-            PrivilegedActor::Lead => Self::ensure_is_forum_lead_account(&account_id)?,
-            PrivilegedActor::Moderator(moderator_id) => {
-                Self::ensure_is_moderator_account(&account_id, &moderator_id)?
-            }
-        };
+        Self::ensure_actor_role(account_id, actor)?;
 
         // Ensure category exists
-        if !<CategoryById<T>>::contains_key(category_id) {
-            return Err(Error::<T>::CategoryDoesNotExist);
-        }
-
-        let category = <CategoryById<T>>::get(category_id);
+        let category = Self::ensure_category_exists(category_id)?;
 
         // Ensure category is empty
         ensure!(
@@ -1419,23 +1408,32 @@ impl<T: Trait> Module<T> {
     fn ensure_can_update_category_membership_of_moderator(
         account_id: T::AccountId,
         category_id: &T::CategoryId,
-    ) -> Result<Category<T::CategoryId, T::ThreadId, T::Hash>, Error<T>> {
+        new_value: bool,
+    ) -> Result<(), Error<T>> {
         // Not signed by forum LEAD
         Self::ensure_is_forum_lead_account(&account_id)?;
 
         // ensure category exists.
+        let category = Self::ensure_category_exists(category_id)?;
+
+        if new_value {
+            Self::ensure_map_limits::<<<T>::MapLimits as StorageLimits>::MaxModeratorsForCategory>(
+                category.num_direct_moderators as u64,
+            )?;
+        }
+
+        Ok(())
+    }
+
+    fn ensure_category_exists(
+        category_id: &T::CategoryId,
+    ) -> Result<Category<T::CategoryId, T::ThreadId, T::Hash>, Error<T>> {
         ensure!(
             <CategoryById<T>>::contains_key(&category_id),
             Error::<T>::CategoryDoesNotExist
         );
 
-        let category = <CategoryById<T>>::get(&category_id);
-
-        Self::ensure_map_limits::<<<T>::MapLimits as StorageLimits>::MaxModeratorsForCategory>(
-            category.num_direct_moderators as u64,
-        )?;
-
-        Ok(category)
+        Ok(<CategoryById<T>>::get(category_id))
     }
 
     fn ensure_can_create_category(