diff --git a/integration-tests/src/tests/comments.rs b/integration-tests/src/tests/comments.rs index 2a60cddd..80821547 100644 --- a/integration-tests/src/tests/comments.rs +++ b/integration-tests/src/tests/comments.rs @@ -96,10 +96,10 @@ fn create_comment_should_fail_when_ipfs_cid_is_invalid() { #[test] fn create_comment_should_fail_when_trying_to_create_in_hidden_space_scope() { ExtBuilder::build_with_post().execute_with(|| { - assert_ok!(_update_space( - None, - None, - Some(space_update(None, Some(true))) + assert_ok!(Spaces::hide_space( + RuntimeOrigin::signed(ACCOUNT1), + SPACE1, + true, )); assert_noop!( @@ -115,7 +115,13 @@ fn create_comment_should_fail_when_trying_create_in_hidden_post_scope() { assert_ok!(_update_post( None, None, - Some(post_update(None, None, Some(true))) + Some(post_update(None, another_space_content_ipfs().into())) + )); + + assert_ok!(Posts::hide_post( + RuntimeOrigin::signed(ACCOUNT1), + POST1, + true, )); assert_noop!( @@ -175,11 +181,16 @@ fn update_comment_hidden_should_work_when_comment_has_parents() { Some(should_hide_id), Some(post_update( None, - None, - Some(true) // make comment hidden + another_space_content_ipfs().into(), )) )); + assert_ok!(Posts::hide_post( + RuntimeOrigin::signed(ACCOUNT1), + POST1, + true, + )); + // We should check that counters weren't increased for all ancestor comments // except the last before hidden. for comment_id in first_comment_id..should_hide_by_one_id { @@ -223,7 +234,7 @@ fn update_comment_should_fail_when_ipfs_cid_is_invalid() { _update_comment( None, None, - Some(post_update(None, Some(invalid_content_ipfs()), None)) + Some(post_update(None, Some(invalid_content_ipfs()))) ), ContentError::InvalidIpfsCid, ); diff --git a/integration-tests/src/tests/posts.rs b/integration-tests/src/tests/posts.rs index 6455c512..7750d515 100644 --- a/integration-tests/src/tests/posts.rs +++ b/integration-tests/src/tests/posts.rs @@ -44,7 +44,7 @@ fn update_post_should_fail_when_content_is_blocked() { _update_post( None, // From ACCOUNT1 (has default permission to UpdateOwnPosts) None, - Some(post_update(None, Some(valid_content_ipfs()), Some(true))) + Some(post_update(None, Some(valid_content_ipfs()))) ), ModerationError::ContentIsBlocked, ); @@ -59,7 +59,7 @@ fn update_post_should_fail_when_account_is_blocked() { _update_post( None, // From ACCOUNT1 (has default permission to UpdateOwnPosts) None, - Some(post_update(None, Some(valid_content_ipfs()), Some(true))) + Some(post_update(None, Some(valid_content_ipfs()))) ), ModerationError::AccountIsBlocked, ); @@ -208,7 +208,6 @@ fn update_post_should_work() { Some(post_update( None, Some(expected_content_ipfs.clone()), - Some(true) )) )); @@ -216,7 +215,6 @@ fn update_post_should_work() { let post = Posts::post_by_id(POST1).unwrap(); assert_eq!(post.space_id, Some(SPACE1)); assert_eq!(post.content, expected_content_ipfs); - assert!(post.hidden); }); } @@ -283,10 +281,10 @@ fn move_hidden_post_should_work() { let expected_new_space_id = SPACE2; // Hide the post before moving it - assert_ok!(_update_post( - None, - Some(moved_post_id), - Some(post_update(None, None, Some(true))) + assert_ok!(Posts::hide_post( + RuntimeOrigin::signed(ACCOUNT1), + moved_post_id, + true, )); assert_ok!(_move_post_1_to_space_2()); @@ -382,7 +380,7 @@ fn should_fail_when_trying_to_move_comment() { #[test] fn update_post_should_work_after_transfer_space_ownership() { ExtBuilder::build_with_post().execute_with(|| { - let post_update = post_update(None, Some(updated_post_content()), Some(true)); + let post_update = post_update(None, Some(updated_post_content())); assert_ok!(_transfer_default_space_ownership()); @@ -395,7 +393,7 @@ fn update_post_should_work_after_transfer_space_ownership() { fn update_any_post_should_work_when_account_has_default_permission() { ExtBuilder::build_with_a_few_roles_granted_to_account2(vec![SP::CreatePosts]).execute_with( || { - let post_update = post_update(None, Some(updated_post_content()), Some(true)); + let post_update = post_update(None, Some(updated_post_content())); assert_ok!(_create_post( Some(RuntimeOrigin::signed(ACCOUNT2)), None, // SpaceId 1 @@ -417,7 +415,7 @@ fn update_any_post_should_work_when_account_has_default_permission() { fn update_any_post_should_work_when_one_of_roles_is_permitted() { ExtBuilder::build_with_a_few_roles_granted_to_account2(vec![SP::UpdateAnyPost]).execute_with( || { - let post_update = post_update(None, Some(updated_post_content()), Some(true)); + let post_update = post_update(None, Some(updated_post_content())); assert_ok!(_create_default_post()); // PostId 1 // Post update with ID 1 should be fine @@ -458,8 +456,7 @@ fn update_post_should_fail_when_post_not_found() { Some(post_update( // FIXME: when Post's `space_id` update is fully implemented None, /*Some(SPACE2)*/ - None, - Some(true) /*None*/ + valid_content_ipfs().into(), )) ), PostsError::::PostNotFound @@ -484,8 +481,7 @@ fn update_post_should_fail_when_account_has_no_permission_to_update_any_post() { Some(post_update( // FIXME: when Post's `space_id` update is fully implemented None, /*Some(SPACE2)*/ - None, - Some(true) /*None*/ + valid_content_ipfs().into(), )) ), PostsError::::NoPermissionToUpdateAnyPost @@ -501,7 +497,7 @@ fn update_post_should_fail_when_ipfs_cid_is_invalid() { _update_post( None, None, - Some(post_update(None, Some(invalid_content_ipfs()), None)) + Some(post_update(None, Some(invalid_content_ipfs()))) ), ContentError::InvalidIpfsCid, ); @@ -512,7 +508,7 @@ fn update_post_should_fail_when_ipfs_cid_is_invalid() { fn update_post_should_fail_when_no_right_permission_in_account_roles() { ExtBuilder::build_with_a_few_roles_granted_to_account2(vec![SP::UpdateAnyPost]).execute_with( || { - let post_update = post_update(None, Some(updated_post_content()), Some(true)); + let post_update = post_update(None, Some(updated_post_content())); assert_ok!(_create_default_post()); // PostId 1 assert_ok!(_delete_default_role()); diff --git a/integration-tests/src/tests/reactions.rs b/integration-tests/src/tests/reactions.rs index 54393715..f50c8e5a 100644 --- a/integration-tests/src/tests/reactions.rs +++ b/integration-tests/src/tests/reactions.rs @@ -86,10 +86,10 @@ fn create_post_reaction_should_fail_when_post_not_found() { fn create_post_reaction_should_fail_when_trying_to_react_in_hidden_space() { ExtBuilder::build_with_post().execute_with(|| { // Hide the space - assert_ok!(_update_space( - None, - None, - Some(space_update(None, Some(true))) + assert_ok!(Spaces::hide_space( + RuntimeOrigin::signed(ACCOUNT1), + SPACE1, + true, )); assert_noop!( @@ -103,10 +103,10 @@ fn create_post_reaction_should_fail_when_trying_to_react_in_hidden_space() { fn create_post_reaction_should_fail_when_trying_to_react_on_hidden_post() { ExtBuilder::build_with_post().execute_with(|| { // Hide the post - assert_ok!(_update_post( - None, - None, - Some(post_update(None, None, Some(true))) + assert_ok!(Posts::hide_post( + RuntimeOrigin::signed(ACCOUNT1), + POST1, + true, )); assert_noop!( diff --git a/integration-tests/src/tests/space_follows.rs b/integration-tests/src/tests/space_follows.rs index 18dffe17..abb66964 100644 --- a/integration-tests/src/tests/space_follows.rs +++ b/integration-tests/src/tests/space_follows.rs @@ -50,10 +50,10 @@ fn follow_space_should_fail_when_account_is_already_space_follower() { #[test] fn follow_space_should_fail_when_trying_to_follow_hidden_space() { ExtBuilder::build_with_space().execute_with(|| { - assert_ok!(_update_space( - None, - None, - Some(space_update(None, Some(true))) + assert_ok!(Spaces::hide_space( + RuntimeOrigin::signed(ACCOUNT1), + SPACE1, + true, )); assert_noop!( diff --git a/integration-tests/src/tests/spaces.rs b/integration-tests/src/tests/spaces.rs index bc941fb7..3cbdddad 100644 --- a/integration-tests/src/tests/spaces.rs +++ b/integration-tests/src/tests/spaces.rs @@ -36,7 +36,7 @@ fn update_space_should_fail_when_content_is_blocked() { _update_space( None, None, - Some(space_update(Some(valid_content_ipfs()), None)) + Some(space_update(Some(valid_content_ipfs()))) ), ModerationError::ContentIsBlocked, ); @@ -225,7 +225,6 @@ fn update_space_should_work() { None, Some(space_update( Some(expected_content_ipfs.clone()), - Some(true), )) )); @@ -233,7 +232,6 @@ fn update_space_should_work() { let space = Spaces::space_by_id(SPACE1).unwrap(); // assert_eq!(space.handle, Some(new_handle.clone())); assert_eq!(space.content, expected_content_ipfs); - assert!(space.hidden); // assert_eq!(find_space_id_by_handle(space_handle()), None); // assert_eq!(find_space_id_by_handle(new_handle), Some(SPACE1)); @@ -250,7 +248,6 @@ fn update_space_should_work_when_one_of_roles_is_permitted() { || { let space_update = space_update( Some(updated_space_content()), - Some(true), ); assert_ok!(_update_space( @@ -414,7 +411,7 @@ fn update_space_should_fail_when_ipfs_cid_is_invalid() { _update_space( None, None, - Some(space_update(Some(invalid_content_ipfs()), None,)) + Some(space_update(Some(invalid_content_ipfs()),)) ), ContentError::InvalidIpfsCid, ); @@ -427,7 +424,6 @@ fn update_space_should_fail_when_no_right_permission_in_account_roles() { || { let space_update = space_update( Some(updated_space_content()), - Some(true), ); assert_ok!(_delete_default_role()); diff --git a/integration-tests/src/utils/posts_utils.rs b/integration-tests/src/utils/posts_utils.rs index 14f74839..acfce4fe 100644 --- a/integration-tests/src/utils/posts_utils.rs +++ b/integration-tests/src/utils/posts_utils.rs @@ -17,12 +17,10 @@ pub(crate) fn updated_post_content() -> Content { pub(crate) fn post_update( space_id: Option, content: Option, - hidden: Option, ) -> PostUpdate { PostUpdate { space_id, content, - hidden, } } @@ -72,7 +70,7 @@ pub(crate) fn _update_post( Posts::update_post( origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT1)), post_id.unwrap_or(POST1), - update.unwrap_or_else(|| post_update(None, None, None)), + update.unwrap_or_else(|| post_update(None, None)), ) } @@ -127,7 +125,7 @@ pub(crate) fn _update_comment( origin, Some(post_id.unwrap_or(POST2)), Some(update.unwrap_or_else(|| - post_update(None, Some(reply_content_ipfs()), None)) + post_update(None, Some(reply_content_ipfs()))) ), ) } diff --git a/integration-tests/src/utils/spaces_utils.rs b/integration-tests/src/utils/spaces_utils.rs index 2d48296e..38870a9d 100644 --- a/integration-tests/src/utils/spaces_utils.rs +++ b/integration-tests/src/utils/spaces_utils.rs @@ -22,16 +22,14 @@ pub(crate) fn updated_space_content() -> Content { pub(crate) fn update_for_space_content( new_content: Content, ) -> SpaceUpdate { - space_update(Some(new_content), None) + space_update(Some(new_content)) } pub(crate) fn space_update( content: Option, - hidden: Option, ) -> SpaceUpdate { SpaceUpdate { content, - hidden, permissions: None, } } @@ -64,6 +62,6 @@ pub(crate) fn _update_space( Spaces::update_space( origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT1)), space_id.unwrap_or(SPACE1), - update.unwrap_or_else(|| space_update(None, None)), + update.unwrap_or_else(|| space_update(None)), ) } diff --git a/pallets/posts/src/benchmarking.rs b/pallets/posts/src/benchmarking.rs index 1a29291a..1bd0319b 100644 --- a/pallets/posts/src/benchmarking.rs +++ b/pallets/posts/src/benchmarking.rs @@ -117,7 +117,6 @@ benchmarks! { let new_content = Content::IPFS(b"Qme7ss3ARVgxv6rXqVPiikMJ8u2NLgmgszg13pYrDKEoiu".to_vec()); let update = PostUpdate { - hidden: Some(true), content: Some(new_content.clone()), space_id: None, }; diff --git a/pallets/posts/src/functions.rs b/pallets/posts/src/functions.rs index 2e0efff7..ee1c41d9 100644 --- a/pallets/posts/src/functions.rs +++ b/pallets/posts/src/functions.rs @@ -2,6 +2,7 @@ use frame_support::dispatch::DispatchResult; use sp_runtime::traits::Saturating; use subsocial_support::{remove_from_vec, SpaceId}; +use subsocial_support::traits::HidePost; use super::*; @@ -334,4 +335,36 @@ impl Pallet { Ok(()) } + + pub fn do_hide_post( + caller: Option, + post_id: PostId, + hidden: bool, + ) -> DispatchResult { + let mut post = Self::require_post(post_id)?; + + if let (Some(caller), Some(space)) = (caller, &post.try_get_space()) { + ensure!( + T::IsAccountBlocked::is_allowed_account(caller.clone(), space.id), + ModerationError::AccountIsBlocked + ); + Self::ensure_account_can_update_post(&caller, &post, space)?; + } + + if hidden != post.hidden { + post.hidden = hidden; + + >::insert(post.id, post); + Self::deposit_event(Event::PostHiddenChanged { hidden, post_id }); + } + + Ok(()) + } } + +impl HidePost for Pallet { + fn hide_post(caller: Option, post_id: PostId, hidden: bool) -> DispatchResult { + Self::do_hide_post(caller, post_id, hidden) + } + +} \ No newline at end of file diff --git a/pallets/posts/src/lib.rs b/pallets/posts/src/lib.rs index e4d611e1..3cc547b0 100644 --- a/pallets/posts/src/lib.rs +++ b/pallets/posts/src/lib.rs @@ -128,6 +128,10 @@ pub mod pallet { from_space: Option, to_space: Option, }, + PostHiddenChanged { + post_id: PostId, + hidden: bool, + } } #[pallet::error] @@ -275,7 +279,7 @@ pub mod pallet { ) -> DispatchResult { let editor = ensure_signed(origin)?; - let has_updates = update.content.is_some() || update.hidden.is_some(); + let has_updates = update.content.is_some(); ensure!(has_updates, Error::::NoUpdatesForPost); @@ -309,13 +313,6 @@ pub mod pallet { } } - if let Some(hidden) = update.hidden { - if hidden != post.hidden { - post.hidden = hidden; - is_update_applied = true; - } - } - // Update this post only if at least one field should be updated: if is_update_applied { >::insert(post.id, post); @@ -481,5 +478,18 @@ pub mod pallet { NextPostId::::put(post_id); Ok(Pays::No.into()) } + + #[pallet::call_index(6)] + // TODO: fix weight + #[pallet::weight(Weight::from_ref_time(10_000) + T::DbWeight::get().writes(1))] + pub fn hide_post( + origin: OriginFor, + post_id: PostId, + hidden: bool, + ) -> DispatchResult { + let caller = ensure_signed(origin)?; + + Self::do_hide_post(Some(caller), post_id, hidden) + } } } diff --git a/pallets/posts/src/types.rs b/pallets/posts/src/types.rs index ff0ab218..e6233721 100644 --- a/pallets/posts/src/types.rs +++ b/pallets/posts/src/types.rs @@ -43,7 +43,6 @@ pub struct PostUpdate { pub space_id: Option, pub content: Option, - pub hidden: Option, } /// Post extension provides specific information necessary for different kinds diff --git a/pallets/posts/tests/src/comments_tests.rs b/pallets/posts/tests/src/comments_tests.rs index 265d8a1d..c8c87b50 100644 --- a/pallets/posts/tests/src/comments_tests.rs +++ b/pallets/posts/tests/src/comments_tests.rs @@ -89,7 +89,11 @@ fn create_comment_should_fail_when_ipfs_cid_is_invalid() { #[test] fn create_comment_should_fail_when_trying_to_create_in_hidden_space_scope() { ExtBuilder::build_with_post().execute_with(|| { - assert_ok!(_update_space(None, None, Some(space_update(None, Some(true))))); + assert_ok!(Spaces::hide_space( + RuntimeOrigin::signed(ACCOUNT1), + SPACE1, + true, + )); assert_noop!(_create_default_comment(), PostsError::::CannotCreateInHiddenScope); }); @@ -98,7 +102,11 @@ fn create_comment_should_fail_when_trying_to_create_in_hidden_space_scope() { #[test] fn create_comment_should_fail_when_trying_create_in_hidden_post_scope() { ExtBuilder::build_with_post().execute_with(|| { - assert_ok!(_update_post(None, None, Some(post_update(None, None, Some(true))))); + assert_ok!(Posts::hide_post( + RuntimeOrigin::signed(ACCOUNT1), + POST1, + true, + )); assert_noop!(_create_default_comment(), PostsError::::CannotCreateInHiddenScope); }); @@ -153,8 +161,7 @@ fn update_comment_hidden_should_work_when_comment_has_parents() { Some(should_hide_id), Some(post_update( None, - None, - Some(true) // make comment hidden + another_valid_content_ipfs().into(), )) )); @@ -201,7 +208,7 @@ fn update_comment_should_fail_when_ipfs_cid_is_invalid() { _update_comment( None, None, - Some(post_update(None, Some(invalid_content_ipfs()), None)) + Some(post_update(None, Some(invalid_content_ipfs()))) ), DispatchError::from(ContentError::InvalidIpfsCid) ); diff --git a/pallets/posts/tests/src/post_tests.rs b/pallets/posts/tests/src/post_tests.rs index 973c98ba..b1610bd7 100644 --- a/pallets/posts/tests/src/post_tests.rs +++ b/pallets/posts/tests/src/post_tests.rs @@ -38,7 +38,7 @@ fn update_post_should_fail_when_content_is_blocked() { _update_post( None, // From ACCOUNT1 (has default permission to UpdateOwnPosts) None, - Some(post_update(None, Some(valid_content_ipfs()), Some(true))) + Some(post_update(None, Some(valid_content_ipfs()))) ), DispatchError::Other(ModerationError::ContentIsBlocked.into()) ); @@ -53,7 +53,7 @@ fn update_post_should_fail_when_account_is_blocked() { _update_post( None, // From ACCOUNT1 (has default permission to UpdateOwnPosts) None, - Some(post_update(None, Some(valid_content_ipfs()), Some(true))) + Some(post_update(None, Some(valid_content_ipfs()))) ), DispatchError::Other(ModerationError::AccountIsBlocked.into()) ); @@ -196,14 +196,13 @@ fn update_post_should_work() { assert_ok!(_update_post( None, // From ACCOUNT1 (has default permission to UpdateOwnPosts) None, - Some(post_update(None, Some(expected_content_ipfs.clone()), Some(true))) + Some(post_update(None, Some(expected_content_ipfs.clone()))) )); // Check whether post updates correctly let post = Posts::post_by_id(POST1).unwrap(); assert_eq!(post.space_id, Some(SPACE1)); assert_eq!(post.content, expected_content_ipfs); - assert!(post.hidden); }); } @@ -261,10 +260,10 @@ fn move_hidden_post_should_work() { let expected_new_space_id = SPACE2; // Hide the post before moving it - assert_ok!(_update_post( - None, - Some(moved_post_id), - Some(post_update(None, None, Some(true))) + assert_ok!(Posts::hide_post( + RuntimeOrigin::signed(ACCOUNT1), + POST1, + true, )); assert_ok!(_move_post_1_to_space_2()); @@ -344,7 +343,7 @@ fn should_fail_when_trying_to_move_comment() { #[test] fn update_post_should_work_after_transfer_space_ownership() { ExtBuilder::build_with_post().execute_with(|| { - let post_update = post_update(None, Some(updated_post_content()), Some(true)); + let post_update = post_update(None, Some(updated_post_content())); assert_ok!(_transfer_default_space_ownership()); @@ -357,7 +356,7 @@ fn update_post_should_work_after_transfer_space_ownership() { fn update_any_post_should_work_when_account_has_default_permission() { ExtBuilder::build_with_a_few_roles_granted_to_account2(vec![SP::CreatePosts]).execute_with( || { - let post_update = post_update(None, Some(updated_post_content()), Some(true)); + let post_update = post_update(None, Some(updated_post_content())); assert_ok!(_create_post( Some(RuntimeOrigin::signed(ACCOUNT2)), None, // SpaceId 1 @@ -379,7 +378,7 @@ fn update_any_post_should_work_when_account_has_default_permission() { fn update_any_post_should_work_when_one_of_roles_is_permitted() { ExtBuilder::build_with_a_few_roles_granted_to_account2(vec![SP::UpdateAnyPost]).execute_with( || { - let post_update = post_update(None, Some(updated_post_content()), Some(true)); + let post_update = post_update(None, Some(updated_post_content())); assert_ok!(_create_default_post()); // PostId 1 // Post update with ID 1 should be fine @@ -413,8 +412,7 @@ fn update_post_should_fail_when_post_not_found() { Some(post_update( // FIXME: when Post's `space_id` update is fully implemented None, /* Some(SPACE2) */ - None, - Some(true) /* None */ + valid_content_ipfs().into(), )) ), PostsError::::PostNotFound @@ -435,8 +433,7 @@ fn update_post_should_fail_when_account_has_no_permission_to_update_any_post() { Some(post_update( // FIXME: when Post's `space_id` update is fully implemented None, /* Some(SPACE2) */ - None, - Some(true) /* None */ + valid_content_ipfs().into(), )) ), PostsError::::NoPermissionToUpdateAnyPost @@ -449,7 +446,7 @@ fn update_post_should_fail_when_ipfs_cid_is_invalid() { ExtBuilder::build_with_post().execute_with(|| { // Try to catch an error updating a post with invalid content assert_noop!( - _update_post(None, None, Some(post_update(None, Some(invalid_content_ipfs()), None))), + _update_post(None, None, Some(post_update(None, Some(invalid_content_ipfs())))), DispatchError::from(ContentError::InvalidIpfsCid) ); }); @@ -459,7 +456,7 @@ fn update_post_should_fail_when_ipfs_cid_is_invalid() { fn update_post_should_fail_when_no_right_permission_in_account_roles() { ExtBuilder::build_with_a_few_roles_granted_to_account2(vec![SP::UpdateAnyPost]).execute_with( || { - let post_update = post_update(None, Some(updated_post_content()), Some(true)); + let post_update = post_update(None, Some(updated_post_content())); assert_ok!(_create_default_post()); // PostId 1 assert_ok!(_delete_default_role()); diff --git a/pallets/posts/tests/src/tests_utils.rs b/pallets/posts/tests/src/tests_utils.rs index 98e9b4c2..29f03b5a 100644 --- a/pallets/posts/tests/src/tests_utils.rs +++ b/pallets/posts/tests/src/tests_utils.rs @@ -257,8 +257,8 @@ pub(crate) fn another_space_content_ipfs() -> Content { Content::IPFS(b"bafyrelt3cif35x4ribisxgq7unhpun525l54eib3mgbou4xln42qqcgj6q".to_vec()) } -pub(crate) fn space_update(content: Option, hidden: Option) -> SpaceUpdate { - SpaceUpdate { content, hidden, permissions: None } +pub(crate) fn space_update(content: Option) -> SpaceUpdate { + SpaceUpdate { content, permissions: None } } pub(crate) fn _create_default_space() -> DispatchResult { @@ -289,7 +289,7 @@ pub(crate) fn _update_space( Spaces::update_space( origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT1)), space_id.unwrap_or(SPACE1), - update.unwrap_or_else(|| space_update(None, None)), + update.unwrap_or_else(|| space_update(None)), ) } @@ -306,9 +306,8 @@ pub(crate) fn updated_post_content() -> Content { pub(crate) fn post_update( space_id: Option, content: Option, - hidden: Option, ) -> PostUpdate { - PostUpdate { space_id, content, hidden } + PostUpdate { space_id, content } } pub(crate) fn comment_content_ipfs() -> Content { @@ -357,7 +356,7 @@ pub(crate) fn _update_post( Posts::update_post( origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT1)), post_id.unwrap_or(POST1), - update.unwrap_or_else(|| post_update(None, None, None)), + update.unwrap_or_else(|| post_update(None, None)), ) } @@ -408,7 +407,7 @@ pub(crate) fn _update_comment( _update_post( origin, Some(post_id.unwrap_or(POST2)), - Some(update.unwrap_or_else(|| post_update(None, Some(reply_content_ipfs()), None))), + Some(update.unwrap_or_else(|| post_update(None, Some(reply_content_ipfs())))), ) } diff --git a/pallets/profiles/src/mock.rs b/pallets/profiles/src/mock.rs index 040cb88a..221ed46a 100644 --- a/pallets/profiles/src/mock.rs +++ b/pallets/profiles/src/mock.rs @@ -13,7 +13,7 @@ use sp_runtime::{ }; use sp_std::sync::{Mutex, MutexGuard}; use subsocial_support::{ - traits::{SpacePermissionsProvider, SpacesInterface}, + traits::{SpacePermissionsProvider, SpacesInterface, HideSpace}, Content, SpaceId, }; @@ -85,6 +85,10 @@ mock! { fn create_space(_owner: &AccountId, _content: Content) -> Result; } + + impl HideSpace for Spaces { + fn hide_space(caller: Option, space_id: SpaceId, hidden: bool) -> DispatchResult; + } } impl pallet_profiles::Config for Test { diff --git a/pallets/reactions/tests/src/tests.rs b/pallets/reactions/tests/src/tests.rs index 49fd9e6d..8bb935ef 100644 --- a/pallets/reactions/tests/src/tests.rs +++ b/pallets/reactions/tests/src/tests.rs @@ -2,6 +2,7 @@ use frame_support::{assert_noop, assert_ok}; use pallet_posts::Error as PostsError; use pallet_reactions::Error as ReactionsError; +use subsocial_support::mock_functions::valid_content_ipfs; use crate::{mock::*, tests_utils::*}; @@ -74,7 +75,11 @@ fn create_post_reaction_should_fail_when_post_not_found() { fn create_post_reaction_should_fail_when_trying_to_react_in_hidden_space() { ExtBuilder::build_with_post().execute_with(|| { // Hide the space - assert_ok!(_update_space(None, None, Some(space_update(None, Some(true))))); + assert_ok!(Spaces::hide_space( + RuntimeOrigin::signed(ACCOUNT1), + SPACE1, + true, + )); assert_noop!( _create_default_post_reaction(), @@ -87,7 +92,12 @@ fn create_post_reaction_should_fail_when_trying_to_react_in_hidden_space() { fn create_post_reaction_should_fail_when_trying_to_react_on_hidden_post() { ExtBuilder::build_with_post().execute_with(|| { // Hide the post - assert_ok!(_update_post(None, None, Some(post_update(None, None, Some(true))))); + assert_ok!(_update_post(None, None, Some(post_update(None, valid_content_ipfs().into())))); + assert_ok!(Posts::hide_post( + RuntimeOrigin::signed(ACCOUNT1), + POST1, + true, + )); assert_noop!( _create_default_post_reaction(), diff --git a/pallets/reactions/tests/src/tests_utils.rs b/pallets/reactions/tests/src/tests_utils.rs index 0f1688c1..8ca2c709 100644 --- a/pallets/reactions/tests/src/tests_utils.rs +++ b/pallets/reactions/tests/src/tests_utils.rs @@ -100,8 +100,8 @@ pub(crate) fn another_space_content_ipfs() -> Content { Content::IPFS(b"bafyrelt3cif35x4ribisxgq7unhpun525l54eib3mgbou4xln42qqcgj6q".to_vec()) } -pub(crate) fn space_update(content: Option, hidden: Option) -> SpaceUpdate { - SpaceUpdate { content, hidden, permissions: None } +pub(crate) fn space_update(content: Option) -> SpaceUpdate { + SpaceUpdate { content, permissions: None } } pub(crate) fn _create_default_space() -> DispatchResult { @@ -132,7 +132,7 @@ pub(crate) fn _update_space( Spaces::update_space( origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT1)), space_id.unwrap_or(SPACE1), - update.unwrap_or_else(|| space_update(None, None)), + update.unwrap_or_else(|| space_update(None)), ) } @@ -145,9 +145,8 @@ pub(crate) fn post_content_ipfs() -> Content { pub(crate) fn post_update( space_id: Option, content: Option, - hidden: Option, ) -> PostUpdate { - PostUpdate { space_id, content, hidden } + PostUpdate { space_id, content } } pub(crate) fn extension_regular_post() -> PostExtension { @@ -180,7 +179,7 @@ pub(crate) fn _update_post( Posts::update_post( origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT1)), post_id.unwrap_or(POST1), - update.unwrap_or_else(|| post_update(None, None, None)), + update.unwrap_or_else(|| post_update(None, None)), ) } diff --git a/pallets/space-follows/tests/src/tests.rs b/pallets/space-follows/tests/src/tests.rs index 8764de8e..27965277 100644 --- a/pallets/space-follows/tests/src/tests.rs +++ b/pallets/space-follows/tests/src/tests.rs @@ -33,7 +33,11 @@ fn follow_space_should_fail_when_account_is_already_space_follower() { #[test] fn follow_space_should_fail_when_trying_to_follow_hidden_space() { ExtBuilder::build_with_space().execute_with(|| { - assert_ok!(_update_space(None, None, Some(space_update(None, Some(true))))); + assert_ok!(Spaces::hide_space( + RuntimeOrigin::signed(ACCOUNT1), + SPACE1, + true, + )); assert_noop!(_default_follow_space(), SpaceFollowsError::::CannotFollowHiddenSpace); }); diff --git a/pallets/space-follows/tests/src/tests_utils.rs b/pallets/space-follows/tests/src/tests_utils.rs index eea6f25b..0e13cf6a 100644 --- a/pallets/space-follows/tests/src/tests_utils.rs +++ b/pallets/space-follows/tests/src/tests_utils.rs @@ -74,8 +74,8 @@ pub(crate) fn space_content_ipfs() -> Content { Content::IPFS(b"bafyreib3mgbou4xln42qqcgj6qlt3cif35x4ribisxgq7unhpun525l54e".to_vec()) } -pub(crate) fn space_update(content: Option, hidden: Option) -> SpaceUpdate { - SpaceUpdate { content, hidden, permissions: None } +pub(crate) fn space_update(content: Option) -> SpaceUpdate { + SpaceUpdate { content, permissions: None } } pub(crate) fn _create_default_space() -> DispatchResult { @@ -105,7 +105,7 @@ pub(crate) fn _update_space( Spaces::update_space( origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT1)), space_id.unwrap_or(SPACE1), - update.unwrap_or_else(|| space_update(None, None)), + update.unwrap_or_else(|| space_update(None)), ) } diff --git a/pallets/spaces/src/benchmarking.rs b/pallets/spaces/src/benchmarking.rs index f37f714b..0dcdc122 100644 --- a/pallets/spaces/src/benchmarking.rs +++ b/pallets/spaces/src/benchmarking.rs @@ -44,7 +44,6 @@ benchmarks! { let space_update = SpaceUpdate { content: dummy_space_content().into(), - hidden: true.into(), permissions: Some(Some(::DefaultSpacePermissions::get())), }; }: _(RawOrigin::Signed(caller), space.id, space_update) diff --git a/pallets/spaces/src/lib.rs b/pallets/spaces/src/lib.rs index b86c63ea..9a2c3184 100644 --- a/pallets/spaces/src/lib.rs +++ b/pallets/spaces/src/lib.rs @@ -43,7 +43,7 @@ pub mod pallet { }; use subsocial_support::{ ensure_content_is_valid, remove_from_bounded_vec, - traits::{IsAccountBlocked, IsContentBlocked, SpacePermissionsProvider, SpacesInterface}, + traits::{IsAccountBlocked, IsContentBlocked, SpacePermissionsProvider, SpacesInterface, HideSpace}, ModerationError, SpacePermissionsInfo, WhoAndWhen, WhoAndWhenOf, }; use types::*; @@ -84,6 +84,7 @@ pub mod pallet { pub enum Event { SpaceCreated { account: T::AccountId, space_id: SpaceId }, SpaceUpdated { account: T::AccountId, space_id: SpaceId }, + SpaceHiddenUpdated { space_id: SpaceId, hidden: bool }, } #[pallet::error] @@ -171,7 +172,7 @@ pub mod pallet { let owner = ensure_signed(origin)?; let has_updates = - update.content.is_some() || update.hidden.is_some() || update.permissions.is_some(); + update.content.is_some() || update.permissions.is_some(); ensure!(has_updates, Error::::NoUpdatesForSpace); @@ -206,13 +207,6 @@ pub mod pallet { } } - if let Some(hidden) = update.hidden { - if hidden != space.hidden { - space.hidden = hidden; - is_update_applied = true; - } - } - if let Some(overrides_opt) = update.permissions { if space.permissions != overrides_opt { if let Some(overrides) = overrides_opt.clone() { @@ -307,6 +301,19 @@ pub mod pallet { NextSpaceId::::put(space_id); Ok(Pays::No.into()) } + + #[pallet::call_index(4)] + // TODO fix weight + #[pallet::weight(Weight::from_ref_time(10_000) + T::DbWeight::get().writes(1))] + pub fn hide_space( + origin: OriginFor, + space_id: SpaceId, + hidden: bool, + ) -> DispatchResult { + let caller = ensure_signed(origin)?; + + Self::do_hide_space(Some(caller), space_id, hidden) + } } impl Pallet { @@ -351,6 +358,37 @@ pub mod pallet { Ok(space_id) } + fn do_hide_space( + caller: Option, + space_id: SpaceId, + hidden: bool, + ) -> DispatchResult { + let mut space = Self::require_space(space_id)?; + + if let Some(caller) = caller { + ensure!( + T::IsAccountBlocked::is_allowed_account(caller.clone(), space.id), + ModerationError::AccountIsBlocked, + ); + + Self::ensure_account_has_space_permission( + caller.clone(), + &space, + SpacePermission::UpdateSpace, + Error::::NoPermissionToUpdateSpace.into(), + )?; + } + + if hidden != space.hidden { + space.hidden = hidden; + + SpaceById::::insert(space_id, space); + Self::deposit_event(Event::SpaceHiddenUpdated { hidden, space_id }); + } + + Ok(()) + } + /// 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 { @@ -431,4 +469,11 @@ pub mod pallet { Self::do_create_space(owner, content, None) } } + + impl HideSpace for Pallet { + fn hide_space(caller: Option, space_id: SpaceId, hidden: bool) -> DispatchResult { + Self::do_hide_space(caller, space_id, hidden) + } + + } } diff --git a/pallets/spaces/src/types.rs b/pallets/spaces/src/types.rs index a801b633..bae2f074 100644 --- a/pallets/spaces/src/types.rs +++ b/pallets/spaces/src/types.rs @@ -38,7 +38,6 @@ pub struct Space { #[derive(Encode, Decode, Clone, Eq, PartialEq, Default, RuntimeDebug, TypeInfo)] pub struct SpaceUpdate { pub content: Option, - pub hidden: Option, pub permissions: Option>, } diff --git a/pallets/spaces/tests/src/tests.rs b/pallets/spaces/tests/src/tests.rs index ca4c76f6..13b76865 100644 --- a/pallets/spaces/tests/src/tests.rs +++ b/pallets/spaces/tests/src/tests.rs @@ -22,7 +22,7 @@ fn update_space_should_fail_when_content_is_blocked() { ExtBuilder::build_with_space().execute_with(|| { block_content_in_space_1(); assert_noop!( - _update_space(None, None, Some(space_update(Some(valid_content_ipfs()), None))), + _update_space(None, None, Some(space_update(Some(valid_content_ipfs())))), ModerationError::ContentIsBlocked, ); }); @@ -100,13 +100,12 @@ fn update_space_should_work() { assert_ok!(_update_space( None, // From ACCOUNT1 (has permission as he's an owner) None, - Some(space_update(Some(expected_content_ipfs.clone()), Some(true),)) + Some(space_update(Some(expected_content_ipfs.clone()))) )); // Check whether space updates correctly let space = Spaces::space_by_id(SPACE1).unwrap(); assert_eq!(space.content, expected_content_ipfs); - assert!(space.hidden); }); } @@ -114,7 +113,7 @@ fn update_space_should_work() { fn update_space_should_work_when_one_of_roles_is_permitted() { ExtBuilder::build_with_a_few_roles_granted_to_account2(vec![SP::UpdateSpace]).execute_with( || { - let space_update = space_update(Some(updated_space_content()), Some(true)); + let space_update = space_update(Some(updated_space_content()),); assert_ok!(_update_space( Some(RuntimeOrigin::signed(ACCOUNT2)), @@ -168,7 +167,7 @@ fn update_space_should_fail_when_ipfs_cid_is_invalid() { ExtBuilder::build_with_space().execute_with(|| { // Try to catch an error updating a space with invalid content assert_noop!( - _update_space(None, None, Some(space_update(Some(invalid_content_ipfs()), None,))), + _update_space(None, None, Some(space_update(Some(invalid_content_ipfs()),))), ContentError::InvalidIpfsCid, ); }); @@ -178,7 +177,7 @@ fn update_space_should_fail_when_ipfs_cid_is_invalid() { fn update_space_should_fail_when_no_right_permission_in_account_roles() { ExtBuilder::build_with_a_few_roles_granted_to_account2(vec![SP::UpdateSpace]).execute_with( || { - let space_update = space_update(Some(updated_space_content()), Some(true)); + let space_update = space_update(Some(updated_space_content()),); assert_ok!(_delete_default_role()); diff --git a/pallets/spaces/tests/src/tests_utils.rs b/pallets/spaces/tests/src/tests_utils.rs index 4b483b50..776699dd 100644 --- a/pallets/spaces/tests/src/tests_utils.rs +++ b/pallets/spaces/tests/src/tests_utils.rs @@ -235,11 +235,11 @@ pub(crate) fn updated_space_content() -> Content { } pub(crate) fn update_for_space_content(new_content: Content) -> SpaceUpdate { - space_update(Some(new_content), None) + space_update(Some(new_content)) } -pub(crate) fn space_update(content: Option, hidden: Option) -> SpaceUpdate { - SpaceUpdate { content, hidden, permissions: None } +pub(crate) fn space_update(content: Option) -> SpaceUpdate { + SpaceUpdate { content, permissions: None } } pub(crate) fn _create_default_space() -> DispatchResult { @@ -270,7 +270,7 @@ pub(crate) fn _update_space( Spaces::update_space( origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT1)), space_id.unwrap_or(SPACE1), - update.unwrap_or_else(|| space_update(None, None)), + update.unwrap_or_else(|| space_update(None)), ) } diff --git a/pallets/support/src/traits.rs b/pallets/support/src/traits.rs index c6ae4b7c..98e3b207 100644 --- a/pallets/support/src/traits.rs +++ b/pallets/support/src/traits.rs @@ -1,5 +1,5 @@ pub use common::{ - ProfileManager, SpaceFollowsProvider, SpacePermissionsProvider, SpacesInterface, PostFollowsProvider, + ProfileManager, SpaceFollowsProvider, SpacePermissionsProvider, SpacesInterface, HideSpace, HidePost, PostFollowsProvider, }; pub use moderation::{IsAccountBlocked, IsContentBlocked, IsPostBlocked, IsSpaceBlocked}; diff --git a/pallets/support/src/traits/common.rs b/pallets/support/src/traits/common.rs index 1b37cf48..825f3262 100644 --- a/pallets/support/src/traits/common.rs +++ b/pallets/support/src/traits/common.rs @@ -26,8 +26,16 @@ pub trait ProfileManager { fn unlink_space_from_profile(account: &AccountId, space_id: SpaceId); } -pub trait SpacesInterface { +pub trait SpacesInterface: HideSpace { fn get_space_owner(space_id: SpaceId) -> Result; fn create_space(owner: &AccountId, content: Content) -> Result; } + +pub trait HideSpace { + fn hide_space(caller: Option, space_id: SpaceId, hidden: bool) -> DispatchResult; +} + +pub trait HidePost { + fn hide_post(caller: Option, post_id: PostId, hidden: bool) -> DispatchResult; +} \ No newline at end of file