From 8f0e16b6c22163ecc70bcc532d5213d778fac7c8 Mon Sep 17 00:00:00 2001
From: cgeek <cem.moreau@gmail.com>
Date: Fri, 15 Dec 2023 13:30:43 +0100
Subject: [PATCH] feat(smith-members): refact: check_* and do_* functions

---
 pallets/smith-members/src/lib.rs | 181 ++++++++++++++++++-------------
 1 file changed, 105 insertions(+), 76 deletions(-)

diff --git a/pallets/smith-members/src/lib.rs b/pallets/smith-members/src/lib.rs
index 1e09fe8c9..c5596521f 100644
--- a/pallets/smith-members/src/lib.rs
+++ b/pallets/smith-members/src/lib.rs
@@ -27,10 +27,12 @@ mod traits;
 mod types;
 
 use codec::{Codec, Decode, Encode};
-use frame_support::dispatch::TypeInfo;
+use frame_support::dispatch::{DispatchResultWithPostInfo, TypeInfo};
 use frame_support::pallet_prelude::Get;
-use frame_support::RuntimeDebug;
-use sp_runtime::traits::AtLeast32BitUnsigned;
+use frame_support::{ensure, RuntimeDebug};
+use frame_system::ensure_signed;
+use frame_system::pallet_prelude::OriginFor;
+use sp_runtime::traits::{AtLeast32BitUnsigned, Convert};
 use sp_std::fmt::Debug;
 use sp_std::prelude::*;
 
@@ -238,8 +240,6 @@ pub mod pallet {
         NotAMember,
     }
 
-    // TODO: refactor with check_* and do_* functions
-
     #[pallet::call]
     impl<T: Config> Pallet<T> {
         #[pallet::call_index(0)]
@@ -250,29 +250,8 @@ pub mod pallet {
         ) -> DispatchResultWithPostInfo {
             let who = ensure_signed(origin.clone())?;
             let issuer = T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T>::UnknownIssuer)?;
-            let issuer_status = Smiths::<T>::get(issuer)
-                .ok_or(Error::<T>::IssuerHasNoSmithStatus)?
-                .status;
-            ensure!(
-                issuer_status == SmithStatus::Smith,
-                Error::<T>::CannotInvite
-            );
-            ensure!(
-                Smiths::<T>::get(receiver).is_none(),
-                Error::<T>::ReceveirAlreadyHasSmithStatus
-            );
-
-            let new_expires_on = CurrentSession::<T>::get() + T::InactivityMaxDuration::get();
-            Smiths::<T>::insert(
-                receiver,
-                SmithMeta {
-                    status: SmithStatus::Invited,
-                    expires_on: Some(new_expires_on),
-                    received_certs: vec![],
-                },
-            );
-            ExpiresOn::<T>::append(new_expires_on, receiver);
-
+            Self::check_invite_smith(issuer, receiver)?;
+            Self::do_invite_smith(receiver);
             Ok(().into())
         }
 
@@ -280,20 +259,9 @@ pub mod pallet {
         #[pallet::weight(1_000_000_000)]
         pub fn accept_invitation(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
             let who = ensure_signed(origin.clone())?;
-            let pretender = T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T>::UnknownIssuer)?;
-            let pretender_status = Smiths::<T>::get(pretender)
-                .ok_or(Error::<T>::NotInvited)?
-                .status;
-            ensure!(
-                pretender_status == SmithStatus::Invited,
-                Error::<T>::AlreadyAcceptedInvitation
-            );
-
-            Smiths::<T>::mutate(pretender, |maybe_smith_meta| {
-                let maybe_smith_meta = maybe_smith_meta.as_mut().expect("status checked earlier");
-                maybe_smith_meta.status = SmithStatus::Pending;
-            });
-
+            let receiver = T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T>::UnknownIssuer)?;
+            Self::check_accept_invitation(receiver)?;
+            Self::do_accept_invitation(receiver)?;
             Ok(().into())
         }
 
@@ -305,46 +273,107 @@ pub mod pallet {
         ) -> DispatchResultWithPostInfo {
             let who = ensure_signed(origin)?;
             let issuer = T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T>::UnknownIssuer)?;
-            ensure!(issuer != receiver, Error::<T>::CannotCertifySelf);
-            let issuer_status = Smiths::<T>::get(issuer)
-                .ok_or(Error::<T>::IssuerHasNoSmithStatus)?
-                .status;
-            ensure!(
-                issuer_status == SmithStatus::Smith,
-                Error::<T>::IssuerIsNotASmith
-            );
-            let receiver_status = Smiths::<T>::get(receiver)
-                .ok_or(Error::<T>::ReceiverHasNotBeenInvited)?
-                .status;
-            ensure!(
-                receiver_status != SmithStatus::Invited,
-                Error::<T>::ReceveirMustAcceptInvitation
-            );
-
-            Smiths::<T>::mutate(receiver, |maybe_smith_meta| {
-                let maybe_smith_meta = maybe_smith_meta.as_mut().expect("status checked earlier");
-                maybe_smith_meta.received_certs.push(issuer);
-                maybe_smith_meta.received_certs.sort();
-                // TODO: "as u32" allowed?
-                maybe_smith_meta.status = if maybe_smith_meta.received_certs.len() as u32
-                    >= T::MinCertForMembership::get()
-                {
-                    SmithStatus::Smith
-                } else {
-                    SmithStatus::Pending
-                };
-                // expiry postponed
-                let new_expires_on = CurrentSession::<T>::get() + T::InactivityMaxDuration::get();
-                maybe_smith_meta.expires_on = Some(new_expires_on);
-                // TODO: unschedule old expiry
-            });
-
+            Self::check_certify_smith(issuer, receiver)?;
+            Self::do_certify_smith(receiver, issuer);
             Ok(().into())
         }
     }
 }
 
 impl<T: Config> Pallet<T> {
+    fn check_invite_smith(
+        issuer: T::IdtyIndex,
+        receiver: T::IdtyIndex,
+    ) -> DispatchResultWithPostInfo {
+        let issuer_status = Smiths::<T>::get(issuer)
+            .ok_or(Error::<T>::IssuerHasNoSmithStatus)?
+            .status;
+        ensure!(
+            issuer_status == SmithStatus::Smith,
+            Error::<T>::CannotInvite
+        );
+        ensure!(
+            Smiths::<T>::get(receiver).is_none(),
+            Error::<T>::ReceveirAlreadyHasSmithStatus
+        );
+
+        Ok(().into())
+    }
+
+    fn do_invite_smith(receiver: T::IdtyIndex) {
+        let new_expires_on = CurrentSession::<T>::get() + T::InactivityMaxDuration::get();
+        Smiths::<T>::insert(
+            receiver,
+            SmithMeta {
+                status: SmithStatus::Invited,
+                expires_on: Some(new_expires_on),
+                received_certs: vec![],
+            },
+        );
+        ExpiresOn::<T>::append(new_expires_on, receiver);
+    }
+
+    fn check_accept_invitation(receiver: T::IdtyIndex) -> DispatchResultWithPostInfo {
+        let pretender_status = Smiths::<T>::get(receiver)
+            .ok_or(Error::<T>::NotInvited)?
+            .status;
+        ensure!(
+            pretender_status == SmithStatus::Invited,
+            Error::<T>::AlreadyAcceptedInvitation
+        );
+        Ok(().into())
+    }
+
+    fn do_accept_invitation(receiver: T::IdtyIndex) -> DispatchResultWithPostInfo {
+        Smiths::<T>::mutate(receiver, |maybe_smith_meta| {
+            let maybe_smith_meta = maybe_smith_meta.as_mut().expect("status checked earlier");
+            maybe_smith_meta.status = SmithStatus::Pending;
+        });
+        Ok(().into())
+    }
+
+    fn check_certify_smith(
+        issuer: T::IdtyIndex,
+        receiver: T::IdtyIndex,
+    ) -> DispatchResultWithPostInfo {
+        ensure!(issuer != receiver, Error::<T>::CannotCertifySelf);
+        let issuer_status = Smiths::<T>::get(issuer)
+            .ok_or(Error::<T>::IssuerHasNoSmithStatus)?
+            .status;
+        ensure!(
+            issuer_status == SmithStatus::Smith,
+            Error::<T>::IssuerIsNotASmith
+        );
+        let receiver_status = Smiths::<T>::get(receiver)
+            .ok_or(Error::<T>::ReceiverHasNotBeenInvited)?
+            .status;
+        ensure!(
+            receiver_status != SmithStatus::Invited,
+            Error::<T>::ReceveirMustAcceptInvitation
+        );
+
+        Ok(().into())
+    }
+
+    fn do_certify_smith(receiver: T::IdtyIndex, issuer: T::IdtyIndex) {
+        Smiths::<T>::mutate(receiver, |maybe_smith_meta| {
+            let maybe_smith_meta = maybe_smith_meta.as_mut().expect("status checked earlier");
+            maybe_smith_meta.received_certs.push(issuer);
+            maybe_smith_meta.received_certs.sort();
+            // TODO: "as u32" allowed?
+            maybe_smith_meta.status =
+                if maybe_smith_meta.received_certs.len() as u32 >= T::MinCertForMembership::get() {
+                    SmithStatus::Smith
+                } else {
+                    SmithStatus::Pending
+                };
+            // expiry postponed
+            let new_expires_on = CurrentSession::<T>::get() + T::InactivityMaxDuration::get();
+            maybe_smith_meta.expires_on = Some(new_expires_on);
+            // TODO: unschedule old expiry
+        });
+    }
+
     // TODO: return what?
     fn remove_expired_smiths(at: SessionIndex) {
         if let Some(smiths_to_remove) = ExpiresOn::<T>::get(at) {
-- 
GitLab