Merge pull request #1494 from fey/fix-flow-exercise-member

Fix flow exercise member

Makefile

@@ -47,7 +47,7 @@ test-coverage:
 	XDEBUG_MODE=coverage php artisan test --coverage-clover build/logs/clover.xml
 
 analyse:
-	composer exec phpstan analyse -v -- --memory-limit=512M
+	composer exec phpstan analyse -v -- --memory-limit=512M --xdebug
 
 check: test lint analyse
 

Procfile.dev

@@ -1,2 +1,2 @@
-web: make start-app
+web: PORT=8000 make start-app
 frontend: make start-frontend

app/Http/Controllers/Api/Exercise/CheckController.php

@@ -14,7 +14,7 @@ class CheckController extends Controller
     public function store(Exercise $exercise, Request $request, ExerciseService $exerciseService): Response
     {
         $data = $request->validate([
-            'user_id' => 'nullable',
+            'user_id' => 'integer|nullable',
             'solution_code' => 'required',
         ]);
 

app/Http/Controllers/ExerciseController.php

@@ -2,7 +2,6 @@
 
 namespace App\Http\Controllers;
 
-use App\Models\ExerciseMember;
 use App\Models\Exercise;
 use App\Models\User;
 use Illuminate\View\View;
@@ -35,19 +34,6 @@ public function show(Exercise $exercise): View
             ->get();
 
         $isShowSavedSolutionsButton = collect($userSolutions)->isEmpty();
-        /** @var ExerciseMember $completedExercise */
-        $completedExercise = $authUser
-            ->exerciseMembers()
-            ->whereBelongsTo($exercise)->firstOrNew([]);
-
-        if (
-            $authUser->isRegistered() &&
-            $completedExercise->isNotFinished()
-        ) {
-            $completedExercise->user()->associate($authUser);
-            $completedExercise->exercise()->associate($exercise);
-            $completedExercise->save();
-        }
 
         return view('exercise.show', compact(
             'exercise',

app/Models/Exercise.php

@@ -40,6 +40,11 @@ public function users(): BelongsToMany
         return $this->belongsToMany(User::class, 'exercise_members');
     }
 
+    public function members(): HasMany
+    {
+        return $this->hasMany(ExerciseMember::class);
+    }
+
     public function solutions(): HasMany
     {
         return $this->hasMany(Solution::class);

app/Models/ExerciseMember.php

@@ -41,6 +41,11 @@ public function exercise(): BelongsTo
         return $this->belongsTo(Exercise::class);
     }
 
+    public function mayFinish(): bool
+    {
+        return $this->canApply('finish');
+    }
+
     public function isFinished(): bool
     {
         return $this->state === 'finished';
@@ -56,6 +61,11 @@ public function finish(): void
         $this->apply('finish');
     }
 
+    public function isStarted(): bool
+    {
+        return $this->state === 'started';
+    }
+
     public function scopeFinished(Builder $builder): Builder
     {
         return $builder->where('state', '=', 'finished');

app/Services/CheckResult.php

@@ -16,7 +16,7 @@ class CheckResult
     private int $exitCode;
     private string $output;
 
-    public function __construct(int $exitCode, string $output)
+    public function __construct(int $exitCode, string $output = '')
     {
         $this->exitCode = $exitCode;
         $this->output = $output;

app/Services/ExerciseService.php

@@ -6,6 +6,7 @@
 
 use App\Models\User;
 use App\Models\Exercise;
+use App\Models\ExerciseMember;
 use App\Models\Solution;
 use App\Services\SolutionChecker;
 use Illuminate\Support\Facades\Log;
@@ -18,41 +19,40 @@ public function __construct(private SolutionChecker $checker, private ActivitySe
 
     public function check(User $user, Exercise $exercise, string $solutionCode): CheckResult
     {
-        if (!$exercise->hasTests()) {
-            $this->completeExercise($user, $exercise);
-            return new CheckResult(0, '');
-        }
-
-        $checkResult = $this->checker->check($user, $exercise, $solutionCode);
-
-        if ($checkResult->isSuccess()) {
-            $this->completeExercise($user, $exercise);
-        }
+        $checkResult = $this->checker->check($exercise, $solutionCode);
 
         if ($checkResult->isCheckExecutionError()) {
             Log::error(
                 "Failed to execute check. Output: {$checkResult->getOutput()}"
             );
-        }
 
-        return $checkResult;
-    }
+            return $checkResult;
+        }
 
-    public function completeExercise(User $user, Exercise $exercise): void
-    {
-        if ($user->hasCompletedExercise($exercise) || $user->isGuest()) {
-            return;
+        if ($user->isGuest()) {
+            return $checkResult;
         }
 
-        $user->exercises()->syncWithoutDetaching($exercise);
-        $this->activityService->logCompletedExercise($user, $exercise);
-    }
+        /** @var \App\Models\ExerciseMember $exerciseMember */
+        $exerciseMember = ExerciseMember
+            ::whereBelongsTo($user)
+            ->whereBelongsTo($exercise)
+            ->firstOrNew();
+
+        $exerciseMember->user()->associate($user);
+        $exerciseMember->exercise()->associate($exercise);
+        $exerciseMember->save();
+
+        if ($checkResult->isSuccess() && $user->isRegistered()) {
+            if ($exerciseMember->mayFinish()) {
+                $exerciseMember->finish();
+                $exerciseMember->save();
+                Log::info('finished');
+                $this->activityService->logCompletedExercise($user, $exercise);
+            }
+        }
 
-    // TODO: remove me
-    public function removeCompletedExercise(User $user, Exercise $exercise): void
-    {
-        $user->exercises()->detach($exercise);
-        $this->activityService->logRemovedExercise($user, $exercise);
+        return $checkResult;
     }
 
     public function createSolution(User $user, Exercise $exercise, string $solutionCode): Solution

app/Services/SolutionChecker.php

@@ -3,14 +3,17 @@
 namespace App\Services;
 
 use App\Models\Exercise;
-use App\Models\User;
 use Illuminate\Support\Facades\Storage;
 use mikehaertl\shellcommand\Command;
 
 class SolutionChecker
 {
-    public function check(User $user, Exercise $exercise, string $solutionCode): CheckResult
+    public function check(Exercise $exercise, string $solutionCode): CheckResult
     {
+        if (!$exercise->hasTests()) {
+            return new CheckResult(CheckResult::SUCCESS_EXIT_CODE);
+        }
+
         $tests = $exercise->getExerciseTests();
         $contents = view(
             'exercise.solution_sandbox_wrapper',
@@ -20,8 +23,7 @@ public function check(User $user, Exercise $exercise, string $solutionCode): Che
             ]
         )->render();
 
-
-        $hashedUserExerciseSolutionId = hash('sha256', implode('', [$user->id, $exercise->id]));
+        $hashedUserExerciseSolutionId = hash('sha256', implode('', [time(), $exercise->id]));
         $userSolutionPath = sprintf('solutions/%s.rkt', $hashedUserExerciseSolutionId);
 
         Storage::put($userSolutionPath, $contents);

database/factories/ExerciseMemberFactory.php

@@ -38,15 +38,15 @@ public function exercise(Exercise $exercise): self
     public function configure(): self
     {
         return $this
-            ->afterMaking(function (ExerciseMember $completedExercise) {
-                $completedExercise->finish();
+            ->afterMaking(function (ExerciseMember $exerciseMember) {
+                $exerciseMember->finish();
             })
-            ->afterCreating(function (ExerciseMember $completedExercise) {
+            ->afterCreating(function (ExerciseMember $exerciseMember) {
                 /** @var ActivityService $service */
                 $service = app()->make(ActivityService::class);
                 $service->logCompletedExercise(
-                    $completedExercise->user,
-                    $completedExercise->exercise
+                    $exerciseMember->user,
+                    $exerciseMember->exercise
                 );
             });
     }

resources/views/partials/chapter_form_element.blade.php

@@ -1,70 +1,43 @@
 @php
 /**
- * @var \App\Models\User $user
- */
+* @var \App\Models\User $user
+* @var ExerciseMembers[]
+*/
 @endphp
 
 @if($chapter->children->count())
-    <{{ getChapterHeaderTag($chapter) }} class="mt-3">
-        {{ App\Helpers\ChapterHelper::fullChapterName($chapter->path) }}
-    </{{ getChapterHeaderTag($chapter) }}>
-    @foreach($chapter->children as $subChapter)
-    @include('partials.chapter_form_element', ['chapter' => $chapters->find($subChapter->id)])
-    @endforeach
+<{{ getChapterHeaderTag($chapter) }} class="mt-3">
+    {{ App\Helpers\ChapterHelper::fullChapterName($chapter->path) }}
+</{{ getChapterHeaderTag($chapter) }}>
+@foreach($chapter->children as $subChapter)
+@include('partials.chapter_form_element', ['chapter' => $chapters->find($subChapter->id)])
+@endforeach
 @else
 <div class="form-check m-2 border-bottom">
-    <input
-        type="checkbox"
-        name="chapters_id[]"
-        id="subChapter{{ $chapter->id }}"
-        value="{{ $chapter->id }}"
-        class="form-check-input"
-        @checked($user->haveRead($chapter))
-        >
+    <input type="checkbox" name="chapters_id[]" id="subChapter{{ $chapter->id }}" value="{{ $chapter->id }}"
+        class="form-check-input" @checked($user->haveRead($chapter))
+    >
     <label for="subChapter{{ $chapter->id }}" class="form-check-label">
         {{ App\Helpers\ChapterHelper::fullChapterName($chapter->path) }}
     </label>
     @if ($chapter->exercises->isNotEmpty())
-    <div>
-        <a data-bs-toggle="collapse"
-           href="#collapse{{ $chapter->id }}"
-           aria-expanded="false"
-           aria-controls="collapse{{ $chapter->id }}">
-            {{ __('exercise.show.exercises') }}
-        </a>
-    </div>
-    <div class="collapse" id="collapse{{ $chapter->id }}">
-        <ul class="list-unstyled">
+    <ul class="list-unstyled">
         @foreach($chapter->exercises as $exercise)
-            <li>
-                @if($exerciseMembers->has($exercise->id))
-                <a href="{{ route('users.exercises.destroy', [$user, $exercise]) }}"
-                   class="text-decoration-none"
-                   data-bs-toggle="tooltip"
-                   data-bs-placement="bottom"
-                   title="{{ __('exercise.remove_completed_exercise', ['exercise_path' => $exercise->path]) }}"
-                   data-confirm="{{ __('exercise.remove_completed_exercise', ['exercise_path' => $exercise->path]) }}?"
-                   data-method="delete">
-                    <i class="bi bi-check"></i>
-                </a>
-                @else
-                <a href="{{ route('users.exercises.update', [$user, $exercise]) }}"
-                   class="text-decoration-none"
-                   data-bs-toggle="tooltip"
-                   data-bs-placement="bottom"
-                   title="{{ __('exercise.mark_exercise', ['exercise_path' => $exercise->path]) }}"
-                   data-method="patch">
-                    <i class="bi bi-square"></i>
-                </a>
+        <li>
+            @if ($exerciseMembers->get($exercise->id))
+                @if ($exerciseMembers->get($exercise->id)->isStarted())
+                <i class="bi bi-hourglass-split"></i>
+                @endif
+                @if ($exerciseMembers->get($exercise->id)->isFinished())
+                <i class="bi bi-check-circle"></i>
                 @endif
-                <a href="{{ route('exercises.show', [$exercise]) }}"
-                   class="link-dark">
-                    {{ $exercise->getFullTitle() }}
-                </a>
-            </li>
+            @endif
+            <a href="{{ route('exercises.show', [$exercise]) }}" class="link-dark">
+                {{ $exercise->getFullTitle() }}
+            </a>
+        </li>
         @endforeach
-        </ul>
-    </div>
+    </ul>
     @endif
 </div>
 @endif

routes/web.php

@@ -23,7 +23,6 @@
     Route::resource('users', 'UserController')->only('show');
     Route::namespace('User')->group(function (): void {
         Route::resource('users.chapters', 'UserChapterController')->only('store', 'destroy');
-        Route::resource('users.exercises', 'UserExerciseController')->only('store', 'update', 'destroy');
         Route::resource('users.solutions', 'SolutionController')->only('store', 'show', 'destroy');
         Route::resource('users.comments', 'UserCommentController')->only('index');
     });

tests/Exercises/TeacherSolutionsTest.php

@@ -22,9 +22,6 @@ protected function setUp(): void
         parent::setUp();
         $this->seed(ChaptersTableSeeder::class);
         $this->seed(ExercisesTableSeeder::class);
-        /** @var User $user */
-        $user = User::factory()->create();
-        $this->user = $user;
 
         $this->solutionChecker = new SolutionChecker();
     }
@@ -57,7 +54,7 @@ public function testTeacherSolutions(string $exercisePath): void
             $this->markTestSkipped("Exercise {$exercisePath} doesnt have teacher solution");
         }
         $teacherSolution = $exercise->getExerciseTeacherSolution();
-        $checkResult = $this->solutionChecker->check($this->user, $exercise, $teacherSolution);
+        $checkResult = $this->solutionChecker->check($exercise, $teacherSolution);
 
         $exercisePath = $exercise->path;
         $output = $checkResult->getOutput();