Browse Source

Merge pull request #151 from siman/alex-optional-channel-fields-rebased

Optional channel fields: title, description, avatar, banner
Bedeho Mender 5 years ago
parent
commit
c0a4e241cc
2 changed files with 96 additions and 117 deletions
  1. 61 84
      src/content_working_group/lib.rs
  2. 35 33
      src/content_working_group/tests.rs

+ 61 - 84
src/content_working_group/lib.rs

@@ -576,6 +576,8 @@ impl Default for ChannelCurationStatus {
     }
 }
 
+pub type OptionalText = Option<Vec<u8>>;
+
 /// A channel for publishing content.
 #[derive(Encode, Decode, Default, Debug, Clone, PartialEq)]
 pub struct Channel<MemberId, AccountId, BlockNumber, PrincipalId> {
@@ -586,16 +588,16 @@ pub struct Channel<MemberId, AccountId, BlockNumber, PrincipalId> {
     pub handle: Vec<u8>,
 
     /// Human readable title of channel. Not required to be unique.
-    pub title: Vec<u8>,
+    pub title: OptionalText,
 
     /// Human readable description of channel purpose and scope.
-    pub description: Vec<u8>,
+    pub description: OptionalText,
 
     /// URL of a small avatar (logo) image of this channel.
-    pub avatar: Vec<u8>,
+    pub avatar: OptionalText,
 
     /// URL of a big background image of this channel.
-    pub banner: Vec<u8>,
+    pub banner: OptionalText,
 
     /// The type of channel.
     pub content: ChannelContentType,
@@ -620,42 +622,6 @@ pub struct Channel<MemberId, AccountId, BlockNumber, PrincipalId> {
     pub principal_id: PrincipalId,
 }
 
-impl<MemberId, AccountId, BlockNumber, PrincipalId>
-    Channel<MemberId, AccountId, BlockNumber, PrincipalId>
-{
-    pub fn new(
-        title: Vec<u8>,
-        verified: bool,
-        description: Vec<u8>,
-        content: ChannelContentType,
-        owner: MemberId,
-        role_account: AccountId,
-        publishing_status: ChannelPublishingStatus,
-        curation_status: ChannelCurationStatus,
-        created: BlockNumber,
-        principal_id: PrincipalId,
-        avatar: Vec<u8>,
-        banner: Vec<u8>,
-        handle: Vec<u8>,
-    ) -> Self {
-        Self {
-            title,
-            verified,
-            description,
-            content,
-            owner,
-            role_account,
-            publishing_status,
-            curation_status,
-            created,
-            principal_id,
-            avatar,
-            banner,
-            handle,
-        }
-    }
-}
-
 /*
  * END: =========================================================
  * Channel stuff
@@ -1120,12 +1086,12 @@ decl_module! {
             origin,
             owner: T::MemberId,
             role_account: T::AccountId,
-            handle: Vec<u8>,
-            title: Vec<u8>,
-            description: Vec<u8>,
-            avatar: Vec<u8>,
-            banner: Vec<u8>,
             content: ChannelContentType,
+            handle: Vec<u8>,
+            title: OptionalText,
+            description: OptionalText,
+            avatar: OptionalText,
+            banner: OptionalText,
             publishing_status: ChannelPublishingStatus
         ) {
 
@@ -1255,10 +1221,10 @@ decl_module! {
             origin,
             channel_id: ChannelId<T>,
             new_handle: Option<Vec<u8>>,
-            new_title: Option<Vec<u8>>,
-            new_description: Option<Vec<u8>>,
-            new_avatar: Option<Vec<u8>>,
-            new_banner: Option<Vec<u8>>,
+            new_title: Option<OptionalText>,
+            new_description: Option<OptionalText>,
+            new_avatar: Option<OptionalText>,
+            new_banner: Option<OptionalText>,
             new_publishing_status: Option<ChannelPublishingStatus>
         ) {
 
@@ -1313,18 +1279,12 @@ decl_module! {
             curation_actor: CurationActor<CuratorId<T>>,
             channel_id: ChannelId<T>,
             new_verified: Option<bool>,
-            new_description: Option<Vec<u8>>,
             new_curation_status: Option<ChannelCurationStatus>
         ) {
 
             // Ensure curation actor signed
             Self::ensure_curation_actor_signed(origin, &curation_actor)?;
 
-            // If set, ensure description is acceptable length
-            if let Some(ref description) = new_description {
-                Self::ensure_channel_description_is_valid(description)?;
-            }
-
             //
             // == MUTATION SAFE ==
             //
@@ -1334,7 +1294,7 @@ decl_module! {
                 &new_verified,
                 &None, // handle
                 &None, // title
-                &new_description,
+                &None, // description,
                 &None, // avatar
                 &None, // banner
                 &None, // publishing_status
@@ -2122,36 +2082,53 @@ impl<T: Trait> Module<T> {
         Ok(())
     }
 
-    fn ensure_channel_title_is_valid(title: &Vec<u8>) -> dispatch::Result {
-        ChannelTitleConstraint::get().ensure_valid(
-            title.len(),
-            MSG_CHANNEL_TITLE_TOO_SHORT,
-            MSG_CHANNEL_TITLE_TOO_LONG,
-        )
+    
+    fn ensure_channel_title_is_valid(text_opt: &OptionalText) -> dispatch::Result {
+        if let Some(text) = text_opt {
+            ChannelTitleConstraint::get().ensure_valid(
+                text.len(),
+                MSG_CHANNEL_TITLE_TOO_SHORT,
+                MSG_CHANNEL_TITLE_TOO_LONG,
+            )
+        } else {
+            Ok(())
+        }
     }
 
-    fn ensure_channel_description_is_valid(description: &Vec<u8>) -> dispatch::Result {
-        ChannelDescriptionConstraint::get().ensure_valid(
-            description.len(),
-            MSG_CHANNEL_DESCRIPTION_TOO_SHORT,
-            MSG_CHANNEL_DESCRIPTION_TOO_LONG,
-        )
+    fn ensure_channel_description_is_valid(text_opt: &OptionalText) -> dispatch::Result {
+        if let Some(text) = text_opt {
+            ChannelDescriptionConstraint::get().ensure_valid(
+                text.len(),
+                MSG_CHANNEL_DESCRIPTION_TOO_SHORT,
+                MSG_CHANNEL_DESCRIPTION_TOO_LONG,
+            )
+        } else {
+            Ok(())
+        }
     }
 
-    fn ensure_channel_avatar_is_valid(avatar: &Vec<u8>) -> dispatch::Result {
-        ChannelAvatarConstraint::get().ensure_valid(
-            avatar.len(),
-            MSG_CHANNEL_AVATAR_TOO_SHORT,
-            MSG_CHANNEL_AVATAR_TOO_LONG,
-        )
+    fn ensure_channel_avatar_is_valid(text_opt: &OptionalText) -> dispatch::Result {
+        if let Some(text) = text_opt {
+            ChannelAvatarConstraint::get().ensure_valid(
+                text.len(),
+                MSG_CHANNEL_AVATAR_TOO_SHORT,
+                MSG_CHANNEL_AVATAR_TOO_LONG,
+            )
+        } else {
+            Ok(())
+        }
     }
 
-    fn ensure_channel_banner_is_valid(banner: &Vec<u8>) -> dispatch::Result {
-        ChannelBannerConstraint::get().ensure_valid(
-            banner.len(),
-            MSG_CHANNEL_BANNER_TOO_SHORT,
-            MSG_CHANNEL_BANNER_TOO_LONG,
-        )
+    fn ensure_channel_banner_is_valid(text_opt: &OptionalText) -> dispatch::Result {
+        if let Some(text) = text_opt {
+            ChannelBannerConstraint::get().ensure_valid(
+                text.len(),
+                MSG_CHANNEL_BANNER_TOO_SHORT,
+                MSG_CHANNEL_BANNER_TOO_LONG,
+            )
+        } else {
+            Ok(())
+        }
     }
 
     fn ensure_curator_application_text_is_valid(text: &Vec<u8>) -> dispatch::Result {
@@ -2631,10 +2608,10 @@ impl<T: Trait> Module<T> {
         channel_id: &ChannelId<T>,
         new_verified: &Option<bool>,
         new_handle: &Option<Vec<u8>>,
-        new_title: &Option<Vec<u8>>,
-        new_description: &Option<Vec<u8>>,
-        new_avatar: &Option<Vec<u8>>,
-        new_banner: &Option<Vec<u8>>,
+        new_title: &Option<OptionalText>,
+        new_description: &Option<OptionalText>,
+        new_avatar: &Option<OptionalText>,
+        new_banner: &Option<OptionalText>,
         new_publishing_status: &Option<ChannelPublishingStatus>,
         new_curation_status: &Option<ChannelCurationStatus>,
     ) {

+ 35 - 33
src/content_working_group/tests.rs

@@ -142,8 +142,9 @@ fn create_channel_description_too_long() {
                 None,
             );
 
-            fixture.description =
-                generate_too_long_length_buffer(&ChannelDescriptionConstraint::get());
+            fixture.description = Some(
+                generate_too_long_length_buffer(&ChannelDescriptionConstraint::get())
+            );
 
             fixture.call_and_assert_error(MSG_CHANNEL_DESCRIPTION_TOO_LONG);
         });
@@ -161,8 +162,9 @@ fn create_channel_description_too_short() {
                 None,
             );
 
-            fixture.description =
-                generate_too_short_length_buffer(&ChannelDescriptionConstraint::get());
+            fixture.description = Some(
+                generate_too_short_length_buffer(&ChannelDescriptionConstraint::get())
+            );
 
             fixture.call_and_assert_error(MSG_CHANNEL_DESCRIPTION_TOO_SHORT);
         });
@@ -178,7 +180,7 @@ struct UpdateChannelAsCurationActorFixture {
     pub origin: Origin,
     pub curation_actor: CurationActor<CuratorId<Test>>,
     pub new_verified: Option<bool>,
-    pub new_description: Option<Vec<u8>>,
+    pub new_description: Option<OptionalText>,
     pub new_curation_status: Option<ChannelCurationStatus>,
 }
 
@@ -192,7 +194,6 @@ impl UpdateChannelAsCurationActorFixture {
             self.curation_actor.clone(),
             channel_id,
             self.new_verified,
-            self.new_description.clone(),
             self.new_curation_status,
         )
     }
@@ -200,24 +201,25 @@ impl UpdateChannelAsCurationActorFixture {
     pub fn call_and_assert_success(&self, channel_id: ChannelId<Test>) {
         let old_channel = ChannelById::<Test>::get(channel_id);
 
-        let expected_updated_channel = lib::Channel::new(
-            old_channel.title,
-            self.new_verified.unwrap_or(old_channel.verified),
-            self.new_description
-                .clone()
-                .unwrap_or(old_channel.description),
-            old_channel.content,
-            old_channel.owner,
-            old_channel.role_account,
-            old_channel.publishing_status,
-            self.new_curation_status
-                .unwrap_or(old_channel.curation_status),
-            old_channel.created,
-            old_channel.principal_id,
-            old_channel.avatar,
-            old_channel.banner,
-            old_channel.handle,
-        );
+        let upd_verified = self.new_verified.unwrap_or(old_channel.verified);
+        let upd_description = self.new_description.clone().unwrap_or(old_channel.description);
+        let upd_curation_status = self.new_curation_status.unwrap_or(old_channel.curation_status);
+
+        let expected_updated_channel = Channel {
+            verified: upd_verified,
+            handle: old_channel.handle,
+            title: old_channel.title,
+            description: upd_description,
+            avatar: old_channel.avatar,
+            banner: old_channel.banner,
+            content: old_channel.content,
+            owner: old_channel.owner,
+            role_account: old_channel.role_account,
+            publishing_status: old_channel.publishing_status,
+            curation_status: upd_curation_status,
+            created: old_channel.created,
+            principal_id: old_channel.principal_id,
+        };
 
         // Call and check result
 
@@ -1818,10 +1820,10 @@ struct CreateChannelFixture {
     pub controller_account: <Test as system::Trait>::AccountId,
     pub channel_creator_role_account: <Test as system::Trait>::AccountId,
     pub channel_handle: Vec<u8>,
-    pub channel_title: Vec<u8>,
-    pub description: Vec<u8>,
-    pub avatar: Vec<u8>,
-    pub banner: Vec<u8>,
+    pub channel_title: OptionalText,
+    pub description: OptionalText,
+    pub avatar: OptionalText,
+    pub banner: OptionalText,
     pub content: ChannelContentType,
     pub publishing_status: ChannelPublishingStatus,
 }
@@ -1844,10 +1846,10 @@ impl CreateChannelFixture {
             controller_account,
             channel_creator_role_account: 527489,
             channel_handle: generate_valid_length_buffer(&ChannelHandleConstraint::get()),
-            channel_title: generate_valid_length_buffer(&ChannelTitleConstraint::get()),
-            avatar: generate_valid_length_buffer(&ChannelAvatarConstraint::get()),
-            banner: generate_valid_length_buffer(&ChannelBannerConstraint::get()),
-            description: generate_valid_length_buffer(&ChannelDescriptionConstraint::get()),
+            channel_title: Some(generate_valid_length_buffer(&ChannelTitleConstraint::get())),
+            avatar: Some(generate_valid_length_buffer(&ChannelAvatarConstraint::get())),
+            banner: Some(generate_valid_length_buffer(&ChannelBannerConstraint::get())),
+            description: Some(generate_valid_length_buffer(&ChannelDescriptionConstraint::get())),
             content: ChannelContentType::Video,
             publishing_status: ChannelPublishingStatus::NotPublished,
         }
@@ -1858,12 +1860,12 @@ impl CreateChannelFixture {
             Origin::signed(self.controller_account),
             self.channel_creator_member_id,
             self.channel_creator_role_account,
+            self.content.clone(),
             self.channel_handle.clone(),
             self.channel_title.clone(),
             self.description.clone(),
             self.avatar.clone(),
             self.banner.clone(),
-            self.content.clone(),
             self.publishing_status.clone(),
         )
     }