Merge pull request '[MTM] Update existing draft articles instead of erroring on a repeat Micropub post' (#139) from develop into main

Reviewed-on: #139

app/Http/Controllers/MicropubController.php

@@ -70,7 +70,9 @@ class MicropubController extends Controller
                 'error' => 'invalid_request',
                 'error_description' => 'No known note with given ID',
             ], 404);
-        } catch (MicropubUnsupportedModelException) {
+        } catch (MicropubUnsupportedModelException $e) {
+            report($e);
+
             return response()->json([
                 'error' => 'invalid',
                 'error_description' => 'This implementation currently only supports the updating of notes',
@@ -80,12 +82,16 @@ class MicropubController extends Controller
                 'error' => 'invalid_request',
                 'error_description' => $e->getMessage(),
             ], 400);
-        } catch (MicropubHandlerException) {
+        } catch (MicropubHandlerException $e) {
+            report($e);
+
             return response()->json([
                 'error' => 'unsupported_operation',
                 'error_description' => 'The request could not be processed by this server',
             ], 500);
-        } catch (\Exception $e) {
+        } catch (\Throwable $e) {
+            report($e);
+
             return response()->json([
                 'error' => 'server_error',
                 'error_description' => 'An error occurred processing the request',

app/Models/Article.php

@@ -40,6 +40,7 @@ class Article extends Model
         return [
             'titleurl' => [
                 'source' => 'title',
+                'includeTrashed' => true,
             ],
         ];
     }
@@ -93,6 +94,13 @@ class Article extends Model
         );
     }
 
+    protected function uri(): Attribute
+    {
+        return Attribute::get(
+            get: fn () => config('app.url').$this->link,
+        );
+    }
+
     /**
      * Scope a query to only include articles from a particular year/month.
      */

app/Services/ArticleService.php

@@ -8,12 +8,29 @@ use App\Models\Article;
 
 class ArticleService
 {
+    /**
+     * @throws \InvalidArgumentException if a published article already has this title
+     */
     public function create(array $data): Article
     {
-        return Article::create([
+        $attributes = [
             'title' => $data['name'],
             'main' => $data['content'],
             'published' => ($data['post-status'] ?? null) !== 'draft',
-        ]);
+        ];
+
+        $existing = Article::where('title', $data['name'])->first();
+
+        if ($existing !== null) {
+            if ($existing->published) {
+                throw new \InvalidArgumentException("An article titled \"{$data['name']}\" has already been published");
+            }
+
+            $existing->update($attributes);
+
+            return $existing;
+        }
+
+        return Article::create($attributes);
     }
 }

app/Services/Micropub/Handlers/EntryHandler.php

@@ -37,7 +37,7 @@ class EntryHandler implements MicropubHandlerInterface
         $location = match (true) {
             isset($dataArray['like-of']) => resolve(LikeService::class)->create($dataArray)->url,
             isset($dataArray['bookmark-of']) => resolve(BookmarkService::class)->create($dataArray)->uri,
-            isset($dataArray['name']) => resolve(ArticleService::class)->create($dataArray)->link,
+            isset($dataArray['name']) => resolve(ArticleService::class)->create($dataArray)->uri,
             default => resolve(NoteService::class)->create($dataArray)->uri,
         };
 

tests/Feature/MicropubControllerTest.php

@@ -4,14 +4,17 @@ declare(strict_types=1);
 
 namespace Tests\Feature;
 
+use App\Exceptions\MicropubHandlerException;
 use App\Jobs\SendWebMentions;
 use App\Jobs\SyndicateNoteToBluesky;
 use App\Jobs\SyndicateNoteToMastodon;
+use App\Models\Article;
 use App\Models\Media;
 use App\Models\Note;
 use App\Models\Place;
 use App\Models\SyndicationTarget;
 use Faker\Factory;
+use Illuminate\Contracts\Debug\ExceptionHandler;
 use Illuminate\Foundation\Testing\RefreshDatabase;
 use Illuminate\Support\Carbon;
 use Illuminate\Support\Facades\Queue;
@@ -457,6 +460,11 @@ class MicropubControllerTest extends TestCase
     #[Test]
     public function micropub_client_api_request_for_unsupported_post_type_returns_error(): void
     {
+        $this->mock(ExceptionHandler::class)
+            ->shouldReceive('report')
+            ->once()
+            ->with(\Mockery::type(MicropubHandlerException::class));
+
         $response = $this->postJson(
             '/api/post',
             [
@@ -871,6 +879,8 @@ class MicropubControllerTest extends TestCase
             'main' => $content,
             'published' => true,
         ]);
+        $response->assertHeader('Location');
+        $this->assertStringStartsWith(config('app.url').'/blog/', $response->headers->get('Location'));
     }
 
     #[Test]
@@ -902,4 +912,64 @@ class MicropubControllerTest extends TestCase
             'published' => false,
         ]);
     }
+
+    #[Test]
+    public function micropub_client_api_request_updates_an_existing_draft_article_with_the_same_name(): void
+    {
+        $draft = Article::create([
+            'title' => 'WireGuard',
+            'main' => 'Early draft content',
+            'published' => false,
+        ]);
+
+        $response = $this->postJson(
+            '/api/post',
+            [
+                'type' => ['h-entry'],
+                'properties' => [
+                    'name' => ['WireGuard'],
+                    'content' => ['Finished content'],
+                ],
+            ],
+            ['HTTP_Authorization' => 'Bearer '.$this->getToken()]
+        );
+
+        $response
+            ->assertJson(['response' => 'created'])
+            ->assertStatus(201);
+        $this->assertSame(1, Article::where('title', 'WireGuard')->count());
+        $this->assertDatabaseHas('articles', [
+            'id' => $draft->id,
+            'title' => 'WireGuard',
+            'main' => 'Finished content',
+            'published' => true,
+        ]);
+    }
+
+    #[Test]
+    public function micropub_client_api_request_errors_when_an_article_with_the_same_name_is_already_published(): void
+    {
+        Article::create([
+            'title' => 'WireGuard',
+            'main' => 'Published content',
+            'published' => true,
+        ]);
+
+        $response = $this->postJson(
+            '/api/post',
+            [
+                'type' => ['h-entry'],
+                'properties' => [
+                    'name' => ['WireGuard'],
+                    'content' => ['Some other content'],
+                ],
+            ],
+            ['HTTP_Authorization' => 'Bearer '.$this->getToken()]
+        );
+
+        $response
+            ->assertJson(['error' => 'invalid_request'])
+            ->assertStatus(400);
+        $this->assertSame(1, Article::where('title', 'WireGuard')->count());
+    }
 }

tests/Unit/ArticlesTest.php

@@ -63,6 +63,28 @@ class ArticlesTest extends TestCase
         );
     }
 
+    #[Test]
+    public function uri_is_the_absolute_form_of_the_link(): void
+    {
+        $article = Article::create([
+            'title' => 'Test',
+            'main' => 'Test',
+        ]);
+
+        $this->assertEquals(config('app.url').$article->link, $article->uri);
+    }
+
+    #[Test]
+    public function slug_is_suffixed_when_a_trashed_article_already_used_it(): void
+    {
+        $original = Article::create(['title' => 'My Title', 'main' => 'Content']);
+        $original->delete();
+
+        $newArticle = Article::create(['title' => 'My Title', 'main' => 'Other content']);
+
+        $this->assertEquals('my-title-2', $newArticle->titleurl);
+    }
+
     #[Test]
     public function date_scope_returns_expected_articles(): void
     {