From 60ccd78540ec8ea0706e8b296c0c927cf59f59e6 Mon Sep 17 00:00:00 2001
From: Hussein Ait Lahcen <hussein.aitlahcen@gmail.com>
Date: Tue, 12 Oct 2021 09:58:15 +0200
Subject: [PATCH] refactor promote/revoke behavior to be noop over
 privileged/nonprivileged user

---
 frame/privilege/src/lib.rs | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/frame/privilege/src/lib.rs b/frame/privilege/src/lib.rs
index 2225ab2761a..ba3eab73553 100644
--- a/frame/privilege/src/lib.rs
+++ b/frame/privilege/src/lib.rs
@@ -60,7 +60,6 @@ pub mod pallet {
 		GroupPrivilegeNotHeld,
 		NotGroupMember,
 		AlreadyGroupMember,
-		NotPrivileged,
 	}
 
 	#[pallet::config]
@@ -127,34 +126,33 @@ pub mod pallet {
 	impl<T: Config> MutatePrivilege for Pallet<T> {
 		fn promote(account_id: &Self::AccountId, privilege: Privilege) -> DispatchResult {
 			AccountPrivileges::<T>::try_mutate(account_id, |account_privileges| {
-				account_privileges.insert(privilege);
-				Self::deposit_event(Event::PrivilegeAdded { account_id: *account_id, privilege });
+				if !account_privileges.contains(privilege) {
+					account_privileges.insert(privilege);
+					Self::deposit_event(Event::PrivilegeAdded {
+						account_id: *account_id,
+						privilege,
+					});
+				}
 				Ok(())
 			})
 		}
 
 		fn revoke(account_id: &Self::AccountId, privilege: Privilege) -> DispatchResult {
 			AccountPrivileges::<T>::try_mutate(account_id, |account_privileges| {
-				if account_privileges.is_empty() {
-					Err(Error::<T>::NotPrivileged)
-				} else {
+				if !account_privileges.is_empty() {
 					account_privileges.remove(privilege);
 					Self::deposit_event(Event::PrivilegeRemoved {
 						account_id: *account_id,
 						privilege,
 					});
-					Ok(())
+					GroupPrivileges::<T>::iter()
+						.filter(|(_, group_privileges)| group_privileges.contains(privilege))
+						.for_each(|(group_id, _)| {
+							let _ = <Self as MutatePrivilegeGroup>::revoke(group_id, account_id);
+						});
 				}
-			})?;
-			/* NOTE(hussein-aitlahcen):
-				 Not ideal
-			*/
-			GroupPrivileges::<T>::iter()
-				.filter(|(_, group_privileges)| group_privileges.contains(privilege))
-				.for_each(|(group_id, _)| {
-					let _ = <Self as MutatePrivilegeGroup>::revoke(group_id, account_id);
-				});
-			Ok(())
+				Ok(())
+			})
 		}
 	}