Merge pull request #80 from pacoorozco/issue-78

Increase performance by reducing queries
pacoorozco · Mar 25, 2020 · 055d09a · 055d09a
2 parents 3d04109 + c02d3d9
commit 055d09a
Show file tree

Hide file tree

Showing 38 changed files with 451 additions and 476 deletions.
diff --git a/.gitignore b/.gitignore
@@ -10,6 +10,7 @@ package-lock.json
 /public/hot
 /storage/*.key
 /storage/debugbar
+/test-coverage
 .env
 .env.backup
 .phpunit.result.cache

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,11 +4,21 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/) and this project adheres to [Semantic Versioning](https://semver.org/).
 
 ## Unreleased
+
+## 2.0.0 - 2020-03-25
+
 > **Note**: This application has been updated to use [Laravel 6.x](https://laravel.com/docs). It's still backwards compatibility, but database needs to be updated too. Some tests have been added but coverage is still very low.
 
+### Added
+- Ensure that users select at least one answer before proceeding. ([#79][i79]) 
+- Two composer commands: `test` and `test-coverage`.
+
+[i79]: https://github.com/pacoorozco/gamify-laravel/issues/79
+
 ### Changed
 - **Important**: This application has been upgraded to [Laravel 6](https://laravel.com/docs). A lot of refactors has been done in order to adopt Laravel 6.x best practices. 
 ([#66][i66])
+- Refactors to reduce the number of queries. 
 - Change the editor from TinyMCE to [Bootstrap-wysihtml5](https://github.com/bootstrap-wysiwyg/bootstrap3-wysiwyg). ([#36][i36]) 
 - Reputation is handled by Events. Added `experience` attribute to the User model. ([#72][i72], [#73][i73])
 - Repository name has been changed to `gamify-laravel`, current URL is https://github.com/pacoorozco/gamify-laravel.

diff --git a/app/Http/Controllers/Admin/AdminQuestionController.php b/app/Http/Controllers/Admin/AdminQuestionController.php
@@ -88,32 +88,45 @@ public function store(QuestionCreateRequest $request)
         }
 
         // Save Question Choices
-        if (is_array($request->input('choice_text'))) {
-            $choice_texts = $request->input('choice_text');
-            $choice_scores = $request->input('choice_score');
-
-            $numberOfChoices = count($choice_texts);
-            for ($i = 0; $i < $numberOfChoices; $i++) {
-                if (empty($choice_texts[$i])) {
-                    continue;
-                }
-
-                if (is_null($choice_scores[$i])) {
-                    $choice_scores[$i] = 0;
-                }
-
-                $question->choices()->create([
-                    'text'    => $choice_texts[$i],
-                    'score'   => $choice_scores[$i],
-                    'correct' => ($choice_scores[$i] > 0),
-                ]);
-            }
-        }
+        $question->choices()->createMany($this->prepareQuestionChoices($request));
 
         return redirect()->route('admin.questions.index')
             ->with('success', __('admin/question/messages.create.success'));
     }
 
+    /**
+     * Return an array of choices to be associated to a Question.
+     *
+     * @param \Illuminate\Http\Request $request
+     *
+     * @return array
+     */
+    private function prepareQuestionChoices(Request $request): array
+    {
+        if (! is_array($request->input('choice_text'))) {
+            return [];
+        }
+
+        $choice_texts = $request->input('choice_text');
+        $choice_scores = $request->input('choice_score');
+
+        $numberOfChoices = count($choice_texts);
+        $choices = [];
+        for ($i = 0; $i < $numberOfChoices; $i++) {
+            if (empty($choice_texts[$i])) {
+                continue;
+            }
+
+            array_push($choices, [
+                'text' => $choice_texts[$i],
+                'score' => is_numeric($choice_scores[$i]) ? $choice_scores[$i] : 0,
+                'correct' => ($choice_scores[$i] > 0),
+            ]);
+        }
+
+        return $choices;
+    }
+
     /**
      * Display the specified resource.
      *
@@ -170,28 +183,7 @@ public function update(QuestionUpdateRequest $request, Question $question)
         // 1st. Deletes the old ones
         $question->choices()->delete();
         // 2nd. Adds the new ones
-
-        if (is_array($request->input('choice_text'))) {
-            $choice_texts = $request->input('choice_text');
-            $choice_scores = $request->input('choice_score');
-
-            $numberOfChoices = count($choice_texts);
-            for ($i = 0; $i < $numberOfChoices; $i++) {
-                if (empty($choice_texts[$i])) {
-                    continue;
-                }
-
-                if (is_null($choice_scores[$i])) {
-                    $choice_scores[$i] = 0;
-                }
-
-                $question->choices()->create([
-                    'text'    => $choice_texts[$i],
-                    'score'   => $choice_scores[$i],
-                    'correct' => ($choice_scores[$i] > 0),
-                ]);
-            }
-        }
+        $question->choices()->createMany($this->prepareQuestionChoices($request));
 
         // Are you trying to publish a question?
         if ($request->input('status') == 'publish') {
@@ -230,9 +222,8 @@ public function delete(Question $question)
      *
      * @param Question $question
      *
-     * @throws \Exception
-     *
      * @return \Illuminate\Http\RedirectResponse
+     * @throws \Exception
      */
     public function destroy(Question $question)
     {
@@ -251,9 +242,8 @@ public function destroy(Question $question)
      * @param \Illuminate\Http\Request     $request
      * @param \Yajra\Datatables\Datatables $dataTable
      *
-     * @throws \Exception
-     *
      * @return \Illuminate\Contracts\Routing\ResponseFactory|\Symfony\Component\HttpFoundation\Response
+     * @throws \Exception
      */
     public function data(Request $request, Datatables $dataTable)
     {
@@ -270,9 +260,9 @@ public function data(Request $request, Datatables $dataTable)
         ])->orderBy('name', 'ASC');
 
         $statusLabel = [
-            'draft'     => '<span class="label label-default">'.__('admin/question/model.status_list.draft').'</span>',
-            'publish'   => '<span class="label label-success">'.__('admin/question/model.status_list.publish').'</span>',
-            'unpublish' => '<span class="label label-warning">'.__('admin/question/model.status_list.unpublish').'</span>',
+            'draft' => '<span class="label label-default">'.strval(__('admin/question/model.status_list.draft')).'</span>',
+            'publish' => '<span class="label label-success">'.strval(__('admin/question/model.status_list.publish')).'</span>',
+            'unpublish' => '<span class="label label-warning">'.strval(__('admin/question/model.status_list.unpublish')).'</span>',
         ];
 
         return $dataTable->of($question)

diff --git a/app/Http/Controllers/Admin/AdminUserController.php b/app/Http/Controllers/Admin/AdminUserController.php
@@ -163,7 +163,7 @@ public function delete(User $user)
     public function destroy(User $user)
     {
         // Can't remove myself
-        if ($user->id === Auth::user()->id) {
+        if ($user->id === Auth::id()) {
             return redirect()->route('admin.users.index')
                 ->with('error', __('admin/user/messages.delete.impossible'));
         }

diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php
@@ -21,6 +21,20 @@ class LoginController extends Controller
 
     use AuthenticatesUsers;
 
+    /**
+     * Maximum number of attempts to allow.
+     *
+     * @var int
+     */
+    public $maxAttempts = 5;
+
+    /**
+     * Number of minutes to throttle for.
+     *
+     * @var int
+     */
+    protected $decayMinutes = 1;
+
     /**
      * Where to redirect users after login.
      *
@@ -36,5 +50,7 @@ class LoginController extends Controller
     public function __construct()
     {
         $this->middleware('guest')->except('logout');
+        $this->maxAttempts = config('auth.login.max_attempts', 5);
+        $this->decayMinutes = config('auth.login.decay_minutes', 1);
     }
 }
diff --git a/app/Http/Controllers/HomeController.php b/app/Http/Controllers/HomeController.php
@@ -25,12 +25,9 @@
 
 namespace Gamify\Http\Controllers;
 
-use Gamify\Level;
 use Gamify\Libs\Game\Game;
 use Gamify\User;
-use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Auth;
-use Yajra\DataTables\DataTables;
 
 class HomeController extends Controller
 {
@@ -45,34 +42,6 @@ public function index()
         $questions = $user->pendingQuestions(3);
         $usersInRanking = Game::getRanking();
 
-        return view('dashboard.index', compact('user', 'questions', 'usersInRanking'));
-    }
-
-    /**
-     * Return the ranking..
-     *
-     * @param \Illuminate\Http\Request     $request
-     * @param \Yajra\Datatables\Datatables $dataTable
-     *
-     * @return \Illuminate\Contracts\Routing\ResponseFactory|\Symfony\Component\HttpFoundation\Response
-     * @throws \Exception
-     */
-    public function data(Request $request, Datatables $dataTable)
-    {
-        $users = User::select([
-            'username',
-            'name',
-        ])
-            //->with('points')
-            //->sortbyDesc(function (User $user) {
-            //  return $user->getExperiencePoints();
-//            })
-            ->take(10);
-
-        return $dataTable->eloquent($users)
-            ->addColumn('level', function (User $user) {
-                return 'level 0';
-            })
-            ->toJson();
+        return view('dashboard.index', compact('questions', 'usersInRanking'));
     }
 }
diff --git a/app/Http/Controllers/QuestionController.php b/app/Http/Controllers/QuestionController.php
@@ -3,37 +3,37 @@
 namespace Gamify\Http\Controllers;
 
 use Gamify\Events\QuestionAnswered;
+use Gamify\Http\Requests\QuestionAnswerRequest;
 use Gamify\Libs\Game\Game;
 use Gamify\Question;
-use Illuminate\Http\Request;
+use Gamify\User;
 use Illuminate\Support\Facades\Auth;
 
 class QuestionController extends Controller
 {
     /**
      * Display a listing of the resource.
      *
-     * @return \Illuminate\Http\Response
+     * @return \Illuminate\View\View
      */
     public function index()
     {
-        $questions = Auth::user()->getPendingQuestions();
+        $user = User::findOrFail(Auth::id());
+        $questions = $user->pendingQuestions();
 
         return view('question.index', compact('questions'));
     }
 
     /**
-     * @param \Illuminate\Http\Request $request
-     * @param Question                 $question
+     * @param QuestionAnswerRequest $request
+     * @param Question              $question
      *
-     * @return \Illuminate\Http\Response
+     * @return \Illuminate\Http\RedirectResponse
      */
-    public function answer(Request $request, Question $question)
+    public function answer(QuestionAnswerRequest $request, Question $question)
     {
         // TODO: If question has been answered can't answer again
 
-        // TODO: Validate
-
         // TODO: AI. Global Badges
 
         // Obtain how many points has its answer obtained
@@ -51,13 +51,14 @@ public function answer(Request $request, Question $question)
         }
 
         // Create relation between User and Question
-        Auth::user()->answeredQuestions()->attach($question, [
-            'points'  => $points,
+        $user = User::findOrFail(Auth::id());
+        $user->answeredQuestions()->attach($question, [
+            'points' => $points,
             'answers' => implode(',', $request->choices),
         ]);
 
         // Trigger an event that will update XP, badges...
-        event(new QuestionAnswered(Auth::user(), $question, $answerCorrectness, $points));
+        event(new QuestionAnswered($user, $question, $answerCorrectness, $points));
 
         // Deal with Question specific Badges
         if ($answerCorrectness) {
@@ -70,7 +71,7 @@ public function answer(Request $request, Question $question)
 
         // AI. Increment actions
         foreach ($badges as $badge) {
-            Game::incrementBadge(Auth::user(), $badge);
+            Game::incrementBadge($user, $badge);
         }
 
         // AI. Add notifications and return view
@@ -82,12 +83,13 @@ public function answer(Request $request, Question $question)
      *
      * @param Question $question
      *
-     * @return \Illuminate\Http\Response
+     * @return \Illuminate\View\View
      */
     public function show(Question $question)
     {
         // TODO: If question has been answered, not show form
-        if ($answer = Auth::user()->answeredQuestions()->find($question->id)) {
+        $user = User::findOrFail(Auth::id());
+        if ($answer = $user->answeredQuestions()->find($question->id)) {
             // User has answered this question
             return view('question.show-answered', compact('answer', 'question'));
         }

diff --git a/app/Http/Requests/QuestionAnswerRequest.php b/app/Http/Requests/QuestionAnswerRequest.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace Gamify\Http\Requests;
+
+class QuestionAnswerRequest extends Request
+{
+    /**
+     * Determine if the user is authorized to make this request.
+     *
+     * @return bool
+     */
+    public function authorize()
+    {
+        return true;
+    }
+
+    /**
+     * Get the validation rules that apply to the request.
+     *
+     * @return array
+     */
+    public function rules()
+    {
+        return [
+            'choices' => ['required'],
+        ];
+    }
+}