From e22f49bc6abdfb3396654b2f54c06d9058e711e4 Mon Sep 17 00:00:00 2001 From: Bruno Fontes Date: Sun, 21 Oct 2018 12:36:02 -0300 Subject: [PATCH] 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 | 17 ++++++--- app/Http/Controllers/TakeController.php | 23 +++++++++--- app/Item.php | 48 +++++++++++++++++++++++- resources/lang/pt-br.json | 5 ++- 4 files changed, 78 insertions(+), 15 deletions(-) diff --git a/app/Http/Controllers/AlertController.php b/app/Http/Controllers/AlertController.php index dd24300..0baa303 100644 --- a/app/Http/Controllers/AlertController.php +++ b/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; - $item->save(); + + if ($item->used_by == Auth::id()) { + 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'); } } diff --git a/app/Http/Controllers/TakeController.php b/app/Http/Controllers/TakeController.php index 3206a2a..247177a 100644 --- a/app/Http/Controllers/TakeController.php +++ b/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')); + + 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)); - $item->returnItem(); return redirect('home'); } } diff --git a/app/Item.php b/app/Item.php index 64f2bba..19f8ada 100644 --- a/app/Item.php +++ b/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(); } } diff --git a/resources/lang/pt-br.json b/resources/lang/pt-br.json index b509c13..8f085e6 100644 --- a/resources/lang/pt-br.json +++ b/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 :itemname (:productname) is now available on **Share It**.": "O item :itemname (:productname) já está disponível no **Share 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!" } \ No newline at end of file