Removing redundant fields and checks

src/membership/members.rs

@@ -394,6 +394,8 @@ decl_module! {
     }
 }
 
+
+
 impl<T: Trait> Module<T> {
     /// Provided that the memberid exists return its profile. Returns error otherwise.
     pub fn ensure_profile(id: T::MemberId) -> Result<Profile<T>, &'static str> {
@@ -562,16 +564,11 @@ impl<T: Trait> Module<T> {
     // ** Note ** Role specific policies should be enforced by the client modules
     // this method should handle higher level policies
     pub fn can_register_role_on_member(
-        signing_account: &T::AccountId,
         member_id: T::MemberId,
         actor_in_role: ActorInRole<T::ActorId>,
     ) -> Result<(), &'static str> {
-        let profile = Self::ensure_profile(member_id)?;
 
-        ensure!(
-            profile.controller_account == *signing_account,
-            "OnlyMemberControllerCanRegisterRole"
-        );
+        let profile = Self::ensure_profile(member_id)?;
 
         // ensure is active member
         ensure!(!profile.suspended, "SuspendedMemberCannotEnterRole");
@@ -596,12 +593,12 @@ impl<T: Trait> Module<T> {
     }
 
     pub fn register_role_on_member(
-        signing_account: &T::AccountId,
         member_id: T::MemberId,
         actor_in_role: ActorInRole<T::ActorId>,
     ) -> Result<(), &'static str> {
+
         // policy check
-        Self::can_register_role_on_member(signing_account, member_id, actor_in_role)?;
+        Self::can_register_role_on_member(member_id, actor_in_role)?;
 
         let mut profile = Self::ensure_profile(member_id)?; // .expect().. ?
         assert!(profile.roles.register_role(&actor_in_role));
@@ -612,35 +609,25 @@ impl<T: Trait> Module<T> {
         Ok(())
     }
 
-    pub fn can_unregister_role_on_member(
-        signing_account: &T::AccountId,
-        member_id: T::MemberId,
+    pub fn can_unregister_role(
         actor_in_role: ActorInRole<T::ActorId>,
     ) -> Result<(), &'static str> {
-        let profile = Self::ensure_profile(member_id)?;
-
-        ensure!(
-            profile.controller_account == *signing_account,
-            "OnlyMemberControllerCanUnregisterRole"
-        );
 
         ensure!(
             <MembershipIdByActorInRole<T>>::exists(&actor_in_role),
             "ActorInRoleNotFound"
         );
-        ensure!(
-            <MembershipIdByActorInRole<T>>::get(&actor_in_role) == member_id,
-            "ActorInRoleNotRegisteredForMember"
-        );
+
         Ok(())
     }
 
-    pub fn unregister_role_on_member(
-        signing_account: &T::AccountId,
-        member_id: T::MemberId,
+    pub fn unregister_role(
         actor_in_role: ActorInRole<T::ActorId>,
     ) -> Result<(), &'static str> {
-        Self::can_unregister_role_on_member(signing_account, member_id, actor_in_role)?;
+
+        Self::can_unregister_role(actor_in_role)?;
+
+        let member_id = <MembershipIdByActorInRole<T>>::get(actor_in_role);
 
         let mut profile = Self::ensure_profile(member_id)?; // .expect().. ?
 

src/membership/tests.rs

@@ -326,7 +326,6 @@ fn registering_and_unregistering_roles_on_member() {
 
             // successful registration
             assert_ok!(Members::register_role_on_member(
-                &1,
                 member_id_1,
                 members::ActorInRole::new(members::Role::Publisher, DUMMY_ACTOR_ID)
             ));
@@ -338,7 +337,6 @@ fn registering_and_unregistering_roles_on_member() {
             // enter role a second time should fail
             assert_dispatch_error_message(
                 Members::register_role_on_member(
-                    &1,
                     member_id_1,
                     members::ActorInRole::new(members::Role::Publisher, DUMMY_ACTOR_ID + 1),
                 ),
@@ -348,7 +346,6 @@ fn registering_and_unregistering_roles_on_member() {
             // registering another member in same role and actorid combination should fail
             assert_dispatch_error_message(
                 Members::register_role_on_member(
-                    &2,
                     member_id_2,
                     members::ActorInRole::new(members::Role::Publisher, DUMMY_ACTOR_ID),
                 ),
@@ -359,28 +356,22 @@ fn registering_and_unregistering_roles_on_member() {
 
             // trying to unregister non existant actor role should fail
             assert_dispatch_error_message(
-                Members::unregister_role_on_member(
-                    &1,
-                    member_id_1,
+                Members::unregister_role(
                     members::ActorInRole::new(members::Role::Curator, 222),
                 ),
                 "ActorInRoleNotFound",
             );
-
+/*
             // trying to unregister actor role on wrong member should fail
             assert_dispatch_error_message(
-                Members::unregister_role_on_member(
-                    &2,
-                    member_id_2,
+                Members::unregister_role(
                     members::ActorInRole::new(members::Role::Publisher, DUMMY_ACTOR_ID),
                 ),
                 "ActorInRoleNotRegisteredForMember",
             );
-
+*/
             // successfully unregister role
-            assert_ok!(Members::unregister_role_on_member(
-                &1,
-                member_id_1,
+            assert_ok!(Members::unregister_role(
                 members::ActorInRole::new(members::Role::Publisher, DUMMY_ACTOR_ID)
             ));
             assert!(!Members::member_is_in_role(