Merge pull request #2535 from iorveth/improve_deletionsignature

forum: improve deletion signature

runtime-modules/forum/src/benchmarking.rs

@@ -1694,20 +1694,21 @@ benchmarks! {
             text.clone(), text.clone(), poll
         );
         let hide = false;
-        let mut posts = Vec::new();
+        let mut posts = BTreeMap::new();
         for _ in 0 .. k {
-            posts.push((
+            posts.insert(
+                ExtendedPostIdObject {
                     category_id,
                     thread_id,
-                    add_thread_post::<T>(
+                    post_id: add_thread_post::<T>(
                         caller_id.clone(),
                         forum_user_id.saturated_into(),
                         category_id,
                         thread_id,
                         vec![0u8],
                     ),
-                    hide
-                )
+                },
+                hide
             );
         }
 
@@ -1734,8 +1735,8 @@ benchmarks! {
         thread.number_of_posts -= k as u64;
         assert_eq!(Module::<T>::thread_by_id(category_id, thread_id), thread);
 
-        for post in posts.clone() {
-            assert!(!<PostById<T>>::contains_key(post.1, post.2));
+        for (extended_post, _) in &posts {
+            assert!(!<PostById<T>>::contains_key(extended_post.thread_id, extended_post.post_id));
         }
 
         assert_last_event::<T>(

runtime-modules/forum/src/lib.rs

@@ -17,6 +17,7 @@ use sp_arithmetic::traits::{BaseArithmetic, One};
 pub use sp_io::storage::clear_prefix;
 use sp_runtime::traits::{AccountIdConversion, MaybeSerialize, Member};
 use sp_runtime::{ModuleId, SaturatedConversion};
+use sp_std::collections::btree_map::BTreeMap;
 use sp_std::collections::btree_set::BTreeSet;
 use sp_std::fmt::Debug;
 use sp_std::prelude::*;
@@ -51,6 +52,19 @@ pub type ThreadOf<T> = Thread<
     BalanceOf<T>,
 >;
 
+/// Type alias for `ExtendedPostIdObject`
+pub type ExtendedPostId<T> =
+    ExtendedPostIdObject<<T as Trait>::CategoryId, <T as Trait>::ThreadId, <T as Trait>::PostId>;
+
+/// Extended post id representation
+#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
+#[derive(Encode, Decode, Default, Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
+pub struct ExtendedPostIdObject<CategoryId, ThreadId, PostId> {
+    pub category_id: CategoryId,
+    pub thread_id: ThreadId,
+    pub post_id: PostId,
+}
+
 type Balances<T> = balances::Module<T>;
 
 /// pallet_forum WeightInfo.
@@ -494,6 +508,7 @@ decl_event!(
         ForumUserId = ForumUserId<T>,
         <T as Trait>::PostReactionId,
         PrivilegedActor = PrivilegedActor<T>,
+        ExtendedPostId = ExtendedPostId<T>,
         PollInput = PollInput<<T as pallet_timestamp::Trait>::Moment>,
     {
         /// A category was introduced
@@ -541,7 +556,7 @@ decl_event!(
         PostModerated(PostId, Vec<u8>, PrivilegedActor, CategoryId, ThreadId),
 
         /// Post with givne id was deleted.
-        PostDeleted(Vec<u8>, ForumUserId, Vec<(CategoryId, ThreadId, PostId, bool)>),
+        PostDeleted(Vec<u8>, ForumUserId, BTreeMap<ExtendedPostId, bool>),
 
         /// Post with given id had its text updated.
         /// The second argument reflects the number of total edits when the text update occurs.
@@ -1471,20 +1486,17 @@ decl_module! {
         fn delete_posts(
             origin,
             forum_user_id: ForumUserId<T>,
-            posts: Vec<(T::CategoryId, T::ThreadId, T::PostId, bool)>,
+            posts: BTreeMap<ExtendedPostId<T>, bool>,
             rationale: Vec<u8>,
         ) -> DispatchResult {
 
-            // Check only unique post instances.
-            let unique_posts: BTreeSet<_> = posts.into_iter().collect();
-
             // Ensure data migration is done
             Self::ensure_data_migration_done()?;
 
             let account_id = ensure_signed(origin)?;
 
-            let mut deleting_posts = BTreeSet::new();
-            for (category_id, thread_id, post_id, hide) in &unique_posts {
+            let mut deleting_posts = Vec::new();
+            for (ExtendedPostIdObject {category_id, thread_id, post_id}, hide) in &posts {
                 // Ensure actor is allowed to moderate post and post is editable
                 let post = Self::ensure_can_delete_post(
                     &account_id,
@@ -1495,7 +1507,7 @@ decl_module! {
                     *hide,
                 )?;
 
-                deleting_posts.insert((category_id, thread_id, post_id, post));
+                deleting_posts.push((category_id, thread_id, post_id, post));
             }
 
             //
@@ -1511,7 +1523,7 @@ decl_module! {
 
             // Generate event
             Self::deposit_event(
-                RawEvent::PostDeleted(rationale, forum_user_id, unique_posts.into_iter().collect())
+                RawEvent::PostDeleted(rationale, forum_user_id, posts)
             );
 
             Ok(())

runtime-modules/forum/src/mock.rs

@@ -834,7 +834,14 @@ pub fn delete_post_mock(
 ) {
     let initial_balance = balances::Module::<Runtime>::free_balance(&account_id);
     let number_of_posts = <ThreadById<Runtime>>::get(category_id, thread_id).number_of_posts;
-    let deleted_posts = vec![(category_id, thread_id, post_id, hide)];
+    let mut deleted_posts = BTreeMap::new();
+    let extended_post_id = ExtendedPostIdObject {
+        category_id,
+        thread_id,
+        post_id,
+    };
+
+    deleted_posts.insert(extended_post_id, hide);
 
     assert_eq!(
         TestForumModule::delete_posts(