Avoiding issues and refactoring code

I made the code more passive, avoiding issued at taking, returning,
storing alerts or removing alerts from an item.

Now they all check if it is with you before returning/deleting
alert etc. I am not sure if all cases are covered, but they are
better than before. I had one only issued on this on that time,
but I prefer to prioritize safety/security.

I took the opportunitie to move some code from Controllers to
the model itself, as they were changing with the DB.

app/Http/Controllers/AlertController.php

@@ -29,9 +29,12 @@ class AlertController extends Controller
             );
             return redirect('home');
         }
-        $item->waiting_user_id = Auth::id();
+
-        $item->timestamps = false;
+        if ($item->used_by == Auth::id()) {
-        $item->save();
+            return redirect('home');
+        }
+
+        $item->storeAlert();
 
         $loggedUser = Auth::user()->name;
         $userWithItem = User::find($item->used_by);
@@ -45,10 +48,12 @@ class AlertController extends Controller
     public function delete(Request $request)
     {
         $item = User::loggedIn()->items()->find(request('item'));
-        $item->waiting_user_id = null;
-        $item->timestamps = false;
-        $item->save();
 
+        if ($item->waiting_user_id != Auth::id()) {
+            return redirect('home');
+        }
+
+        $item->removeAlert();
         return redirect('home');
     }
 }

app/Http/Controllers/TakeController.php

@@ -8,6 +8,7 @@ use App\Item;
 use App\User;
 use App\Events\ReturnItem;
 use Illuminate\Http\Request;
+use PhpParser\Node\Stmt\TryCatch;
 
 /**
  * Responsible to Take and Return an Item.
@@ -24,14 +25,15 @@ class TakeController extends Controller
     public function store(Request $request)
     {
         $item = User::loggedIn()->items()->find(request('item'));
-        if ($item->used_by) {
+
+        try {
+            $item->takeItem();
+        } catch (\Exception $e) {
             return back()->withErrors(
-                Lang::getFromJson("This item is already taken")
+                Lang::getFromJson('This item is already taken')
             );
         }
-        $item->used_by = Auth::id();
+
-        $item->waiting_user_id = null;
-        $item->save();
         return redirect('home');
     }
 
@@ -46,8 +48,17 @@ class TakeController extends Controller
     public function delete(Request $request)
     {
         $item = User::loggedIn()->items()->find(request('item'));
-        event(new ReturnItem($item));
+
+        try {
             $item->returnItem();
+
+        } catch (\Exception $e) {
+            return back()->withErrors(
+                Lang::getFromJson("You cannot return an item that is not with you")
+            );
+        }
+
+        event(new ReturnItem($item));
         return redirect('home');
     }
 }

app/Item.php

@@ -2,7 +2,10 @@
 
 namespace App;
 
+use Auth;
+use Lang;
 use Illuminate\Database\Eloquent\Model;
+use Exception;
 
 class Item extends Model
 {
@@ -36,7 +39,23 @@ class Item extends Model
      */
     public static function fromAuthUser()
     {
-        return (new static)->where('user_id', \Auth::id());
+        return (new static)->where('user_id', Auth::id());
+    }
+
+    /**
+     * Take a specified item
+     * 
+     * @return void
+     */
+    public function takeItem()
+    {
+        if (isset($this->used_by)) {
+            throw new Exception("Trying to take an Item that is in use", 1);
+        }
+
+        $this->used_by = Auth::id();
+        $this->waiting_user_id = null;
+        $this->save();
     }
 
     /**
@@ -46,8 +65,35 @@ class Item extends Model
      */
     public function returnItem()
     {
+        if ($this->used_by != Auth::id()) {
+            throw new Exception("Trying to return an empty Item or from other user", 1);
+        }
+
         $this->used_by = null;
+        $this->save();
+    }
+
+    /**
+     * Store a waiting user to the item
+     * 
+     * @return void
+     */
+    public function storeAlert()
+    {
+        $this->waiting_user_id = Auth::id();
+        $this->timestamps = false;
+        $this->save();
+    }
+
+    /**
+     * Remove a waiting user to the item
+     * 
+     * @return void
+     */
+    public function removeAlert()
+    {
         $this->waiting_user_id = null;
+        $this->timestamps = false;
         $this->save();
     }
 }

resources/lang/pt-br.json

@@ -32,7 +32,7 @@
 
     ":itemname is available!": ":itemname está disponível!",
     "Hi, :username,": "Olá, :username,",
-    "Good news: :itemname is available!": "Uma boa notícia: :itemname está disponível!",
+    "Good news: :itemname is available!": "Boa notícia: :itemname está disponível!",
     "The item <em>:itemname (:productname)</em> is now available on **Share&nbsp;It**.": "O item <em>:itemname (:productname)</em> já está disponível no **Share&nbsp;It**.",
     "**Take It** before anyone else at the website:": "Entre no nosso site para usar o item antes de todo mundo.",
 
@@ -46,6 +46,7 @@
 
     "The item doesn't exist.": "O item não existe.",
     "The product doesn't exist or doesn't belongs to you.": "O produto não existe ou não é seu.",
-    "This item is already taken": "Esse item já está sendo usado",
+    "This item is already taken": "Esse item já está sendo usado.",
+    "You cannot return an item that is not with you": "Você não pode devolver um item que não está com você.",
     "Oh! This item has just being returned. Take it before anyone else!": "Opa! Esse item acabou de ser devolvido. Aproveite!"
 }