From 0544c5b19e0127ac069cdc0cf0632d7c11352a97 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?=
 <hola@pablodorado.com>
Date: Mon, 22 Apr 2024 02:49:12 -0500
Subject: [PATCH] fix(pallet-communities): on tests, must unlock on casted
 votes

---
 pallets/communities/src/benchmarking.rs     |  2 +-
 pallets/communities/src/functions.rs        | 11 +++--------
 pallets/communities/src/lib.rs              | 10 +++-------
 pallets/communities/src/tests/governance.rs |  5 +----
 4 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/pallets/communities/src/benchmarking.rs b/pallets/communities/src/benchmarking.rs
index 3537bdaf..542b0f22 100644
--- a/pallets/communities/src/benchmarking.rs
+++ b/pallets/communities/src/benchmarking.rs
@@ -406,7 +406,7 @@ mod benchmarks {
 		T::BenchmarkHelper::finish_poll(index)?;
 
 		#[extrinsic_call]
-		_(RawOrigin::Signed(who.clone()), membership_id, 0u32);
+		_(RawOrigin::Signed(who.clone()), 0u32);
 
 		// verification code
 		assert_eq!(
diff --git a/pallets/communities/src/functions.rs b/pallets/communities/src/functions.rs
index 93f45690..08aa8355 100644
--- a/pallets/communities/src/functions.rs
+++ b/pallets/communities/src/functions.rs
@@ -122,7 +122,7 @@ impl<T: Config> Pallet<T> {
 			let (tally, class) = poll_status.ensure_ongoing().ok_or(Error::<T>::NotOngoing)?;
 			ensure!(community_id == class, Error::<T>::InvalidTrack);
 
-			let (vote, _) = CommunityVotes::<T>::get(poll_index, membership_id).ok_or(Error::<T>::NoVoteCasted)?;
+			let (vote, voter) = CommunityVotes::<T>::get(poll_index, membership_id).ok_or(Error::<T>::NoVoteCasted)?;
 			let vote_multiplier = match CommunityDecisionMethod::<T>::get(community_id) {
 				DecisionMethod::Rank => T::MemberMgmt::rank_of(&community_id, &membership_id)
 					.unwrap_or_default()
@@ -133,16 +133,11 @@ impl<T: Config> Pallet<T> {
 			let vote_weight = VoteWeight::from(&vote);
 			tally.remove_vote(vote.say(), vote_multiplier * vote_weight, vote_weight);
 
-			Self::do_unlock(membership_id, poll_index)
+			CommunityVotes::<T>::remove(poll_index, membership_id);
+			Self::update_locks(&voter, poll_index, &vote, LockUpdateType::Remove)
 		})
 	}
 
-	pub(crate) fn do_unlock(membership_id: MembershipIdOf<T>, poll_index: PollIndexOf<T>) -> DispatchResult {
-		let (vote, voter) = CommunityVotes::<T>::get(poll_index, membership_id).ok_or(Error::<T>::NoVoteCasted)?;
-		CommunityVotes::<T>::remove(poll_index, membership_id);
-		Self::update_locks(&voter, poll_index, &vote, LockUpdateType::Remove)
-	}
-
 	pub(crate) fn update_locks(
 		who: &AccountIdOf<T>,
 		poll_index: PollIndexOf<T>,
diff --git a/pallets/communities/src/lib.rs b/pallets/communities/src/lib.rs
index d15fb0bb..9524f2ee 100644
--- a/pallets/communities/src/lib.rs
+++ b/pallets/communities/src/lib.rs
@@ -517,15 +517,11 @@ pub mod pallet {
 		/// Make previously held or locked funds from a vote available
 		// if the refereundum  has finished
 		#[pallet::call_index(10)]
-		pub fn unlock(
-			origin: OriginFor<T>,
-			membership_id: MembershipIdOf<T>,
-			#[pallet::compact] poll_index: PollIndexOf<T>,
-		) -> DispatchResult {
+		pub fn unlock(origin: OriginFor<T>, #[pallet::compact] poll_index: PollIndexOf<T>) -> DispatchResult {
 			let who = ensure_signed(origin)?;
-			T::MemberMgmt::check_membership(&who, &membership_id).ok_or(Error::<T>::NotAMember)?;
 			ensure!(T::Polls::as_ongoing(poll_index).is_none(), Error::<T>::AlreadyOngoing);
-			Self::do_unlock(membership_id, poll_index)
+			let vote = CommunityVoteLocks::<T>::get(&who, poll_index).ok_or(Error::<T>::NoLocksInPlace)?;
+			Self::update_locks(&who, poll_index, &vote, LockUpdateType::Remove)
 		}
 
 		/// Dispatch a callable as the community account
diff --git a/pallets/communities/src/tests/governance.rs b/pallets/communities/src/tests/governance.rs
index 58eda71a..50410680 100644
--- a/pallets/communities/src/tests/governance.rs
+++ b/pallets/communities/src/tests/governance.rs
@@ -967,7 +967,6 @@ mod vote {
 
 				tick_blocks(2);
 
-				dbg!(System::events());
 				System::assert_has_event(pallet_referenda::Event::<Test>::ConfirmStarted { index: 3 }.into());
 			});
 		}
@@ -1144,7 +1143,7 @@ mod unlock {
 		new_test_ext().execute_with(|| {
 			// Since BOB never casted a vote, a lock wasn't put in place
 			assert_noop!(
-				Communities::unlock(RuntimeOrigin::signed(BOB), membership(COMMUNITY_C, 2), 1),
+				Communities::unlock(RuntimeOrigin::signed(BOB), 1),
 				Error::AlreadyOngoing
 			);
 		});
@@ -1171,13 +1170,11 @@ mod unlock {
 
 			assert_ok!(Communities::unlock(
 				RuntimeOrigin::signed(BOB),
-				membership(COMMUNITY_C, 2),
 				1
 			));
 
 			assert_ok!(Communities::unlock(
 				RuntimeOrigin::signed(CHARLIE),
-				membership(COMMUNITY_D, 3),
 				2
 			));
 		});