Merge pull request #3118 from shamil-gadelshin/olympia-moving-threads

Olympia. Runtime. Forum: fix `move_thread_to_category` extrinsic bug.

runtime-modules/forum/src/benchmarking.rs

@@ -1152,7 +1152,10 @@ benchmarks! {
         assert_eq!(Module::<T>::category_by_id(new_category_id), new_category);
 
         assert!(!<ThreadById<T>>::contains_key(category_id, thread_id));
-        assert_eq!(Module::<T>::thread_by_id(new_category_id, thread_id), thread);
+        assert_eq!(
+            Module::<T>::thread_by_id(new_category_id, thread_id),
+            Thread { category_id: new_category_id, ..thread}
+        );
 
         assert_last_event::<T>(
             RawEvent::ThreadMoved(
@@ -1226,7 +1229,10 @@ benchmarks! {
         assert_eq!(Module::<T>::category_by_id(new_category_id), new_category);
 
         assert!(!<ThreadById<T>>::contains_key(category_id, thread_id));
-        assert_eq!(Module::<T>::thread_by_id(new_category_id, thread_id), thread);
+        assert_eq!(
+            Module::<T>::thread_by_id(new_category_id, thread_id),
+            Thread { category_id: new_category_id, ..thread}
+        );
 
         assert_last_event::<T>(
             RawEvent::ThreadMoved(

runtime-modules/forum/src/lib.rs

@@ -1102,7 +1102,13 @@ decl_module! {
         ).max(WeightInfoForum::<T>::move_thread_to_category_moderator(
             T::MaxCategoryDepth::get() as u32,
         ))]
-        fn move_thread_to_category(origin, actor: PrivilegedActor<T>, category_id: T::CategoryId, thread_id: T::ThreadId, new_category_id: T::CategoryId) -> DispatchResult {
+        fn move_thread_to_category(
+            origin,
+            actor: PrivilegedActor<T>,
+            category_id: T::CategoryId,
+            thread_id: T::ThreadId,
+            new_category_id: T::CategoryId
+        ) -> DispatchResult {
             // Ensure data migration is done
             Self::ensure_data_migration_done()?;
 
@@ -1115,8 +1121,13 @@ decl_module! {
             // == MUTATION SAFE ==
             //
 
+            let updated_thread = Thread {
+                category_id: new_category_id,
+                ..thread
+            };
+
             <ThreadById<T>>::remove(thread.category_id, thread_id);
-            <ThreadById<T>>::insert(new_category_id, thread_id, thread.clone());
+            <ThreadById<T>>::insert(new_category_id, thread_id, updated_thread);
             <CategoryById<T>>::mutate(thread.category_id, |category| category.num_direct_threads -= 1);
             <CategoryById<T>>::mutate(new_category_id, |category| category.num_direct_threads += 1);
 

runtime-modules/forum/src/tests.rs

@@ -1350,6 +1350,126 @@ fn move_thread_moderator_permissions() {
     });
 }
 
+#[test]
+fn category_updated_successfully_on_thread_moving() {
+    let moderator = FORUM_MODERATOR_ORIGIN_ID;
+    let moderator_origin = FORUM_MODERATOR_ORIGIN;
+
+    let forum_lead = FORUM_LEAD_ORIGIN_ID;
+    let origin = OriginType::Signed(forum_lead);
+    let initial_balance = 10_000_000;
+    with_test_externalities(|| {
+        balances::Module::<Runtime>::make_free_balance_be(&forum_lead, initial_balance);
+
+        let category_id_1 = create_category_mock(
+            origin.clone(),
+            None,
+            good_category_title(),
+            good_category_description(),
+            Ok(()),
+        );
+        let category_id_2 = create_category_mock(
+            origin.clone(),
+            None,
+            good_category_title(),
+            good_category_description(),
+            Ok(()),
+        );
+
+        // sanity check
+        assert_ne!(category_id_1, category_id_2);
+
+        let thread_id = create_thread_mock(
+            origin.clone(),
+            forum_lead,
+            forum_lead,
+            category_id_1,
+            good_thread_metadata(),
+            good_thread_text(),
+            None,
+            Ok(()),
+        );
+
+        // set incomplete permissions for first user (only category 1)
+        update_category_membership_of_moderator_mock(
+            moderator_origin.clone(),
+            moderator,
+            category_id_1,
+            true,
+            Ok(()),
+        );
+
+        // give the rest of necessary permissions to the first moderator
+        update_category_membership_of_moderator_mock(
+            moderator_origin.clone(),
+            moderator,
+            category_id_2,
+            true,
+            Ok(()),
+        );
+
+        // check counters of threads in category
+        assert_eq!(
+            <CategoryById<Runtime>>::get(category_id_1).num_direct_threads,
+            1,
+        );
+        assert_eq!(
+            <CategoryById<Runtime>>::get(category_id_2).num_direct_threads,
+            0,
+        );
+
+        // move in one direction
+        move_thread_mock(
+            moderator_origin.clone(),
+            moderator,
+            category_id_1,
+            thread_id,
+            category_id_2,
+            Ok(()),
+        );
+
+        assert_eq!(
+            TestForumModule::thread_by_id(category_id_2, thread_id).category_id,
+            category_id_2
+        );
+
+        // check counters of threads in category
+        assert_eq!(
+            <CategoryById<Runtime>>::get(category_id_1).num_direct_threads,
+            0,
+        );
+        assert_eq!(
+            <CategoryById<Runtime>>::get(category_id_2).num_direct_threads,
+            1,
+        );
+
+        // move in opposite direction
+        move_thread_mock(
+            moderator_origin.clone(),
+            moderator,
+            category_id_2,
+            thread_id,
+            category_id_1,
+            Ok(()),
+        );
+
+        // check counters of threads in category
+        assert_eq!(
+            <CategoryById<Runtime>>::get(category_id_1).num_direct_threads,
+            1,
+        );
+        assert_eq!(
+            <CategoryById<Runtime>>::get(category_id_2).num_direct_threads,
+            0,
+        );
+
+        assert_eq!(
+            TestForumModule::thread_by_id(category_id_1, thread_id).category_id,
+            category_id_1
+        );
+    });
+}
+
 #[test]
 // test if error is thrown when origin and destination category is the same
 fn move_thread_invalid_move() {