handle self transfer better (#307)

nft/src/lib.rs

@@ -141,12 +141,14 @@ impl<T: Trait> Module<T> {
 
 	/// Transfer NFT(non fungible token) from `from` account to `to` account
 	pub fn transfer(from: &T::AccountId, to: &T::AccountId, token: (T::ClassId, T::TokenId)) -> DispatchResult {
-		if from == to {
-			return Ok(());
-		}
-
 		TokensByOwner::<T>::try_mutate_exists(from, token, |token_by_owner| -> DispatchResult {
-			ensure!(token_by_owner.take().is_some(), Error::<T>::NoPermission);
+			ensure!(token_by_owner.is_some(), Error::<T>::NoPermission);
+			if from == to {
+				// no change needed
+				return Ok(());
+			}
+
+			*token_by_owner = None;
 			TokensByOwner::<T>::insert(to, token, ());
 
 			Tokens::<T>::try_mutate_exists(token.0, token.1, |token_info| -> DispatchResult {
@@ -218,4 +220,8 @@ impl<T: Trait> Module<T> {
 			Ok(())
 		})
 	}
+
+	pub fn is_owner(account: &T::AccountId, token: (T::ClassId, T::TokenId)) -> bool {
+		TokensByOwner::<T>::contains_key(account, token)
+	}
 }

nft/src/tests.rs

@@ -62,6 +62,7 @@ fn transfer_should_work() {
 		assert_ok!(NonFungibleTokenModule::transfer(&BOB, &BOB, (CLASS_ID, TOKEN_ID)));
 		assert_ok!(NonFungibleTokenModule::transfer(&BOB, &ALICE, (CLASS_ID, TOKEN_ID)));
 		assert_ok!(NonFungibleTokenModule::transfer(&ALICE, &BOB, (CLASS_ID, TOKEN_ID)));
+		assert!(NonFungibleTokenModule::is_owner(&BOB, (CLASS_ID, TOKEN_ID)));
 	});
 }
 
@@ -82,6 +83,10 @@ fn transfer_should_fail() {
 			NonFungibleTokenModule::mint(&BOB, CLASS_ID_NOT_EXIST, vec![1], ()),
 			Error::<Runtime>::ClassNotFound
 		);
+		assert_noop!(
+			NonFungibleTokenModule::transfer(&ALICE, &ALICE, (CLASS_ID, TOKEN_ID)),
+			Error::<Runtime>::NoPermission
+		);
 	});
 }