From 458cea36449aeb037bc34b462ad135d426eed204 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jean-Ren=C3=A9=20ROUET?= <rouet@in2p3.fr>
Date: Mon, 12 Jun 2023 15:12:46 +0200
Subject: [PATCH 1/4] [API] add priority in book read [API] add priority in
chapter create and update [API] add priority in page create and update
---
app/Api/ApiEntityListFormatter.php | 1 +
app/Entities/Controllers/ChapterApiController.php | 2 ++
app/Entities/Controllers/PageApiController.php | 2 ++
3 files changed, 5 insertions(+)
diff --git a/app/Api/ApiEntityListFormatter.php b/app/Api/ApiEntityListFormatter.php
index c170ecf0c..7d00834e5 100644
--- a/app/Api/ApiEntityListFormatter.php
+++ b/app/Api/ApiEntityListFormatter.php
@@ -22,6 +22,7 @@ class ApiEntityListFormatter
protected $fields = [
'id', 'name', 'slug', 'book_id', 'chapter_id',
'draft', 'template', 'created_at', 'updated_at',
+ 'priority'
];
public function __construct(array $list)
diff --git a/app/Entities/Controllers/ChapterApiController.php b/app/Entities/Controllers/ChapterApiController.php
index 403c58de3..7f01e445a 100644
--- a/app/Entities/Controllers/ChapterApiController.php
+++ b/app/Entities/Controllers/ChapterApiController.php
@@ -19,12 +19,14 @@ class ChapterApiController extends ApiController
'name' => ['required', 'string', 'max:255'],
'description' => ['string', 'max:1000'],
'tags' => ['array'],
+ 'priority' => ['integer'],
],
'update' => [
'book_id' => ['integer'],
'name' => ['string', 'min:1', 'max:255'],
'description' => ['string', 'max:1000'],
'tags' => ['array'],
+ 'priority' => ['integer'],
],
];
diff --git a/app/Entities/Controllers/PageApiController.php b/app/Entities/Controllers/PageApiController.php
index 28dd36f97..c83126df9 100644
--- a/app/Entities/Controllers/PageApiController.php
+++ b/app/Entities/Controllers/PageApiController.php
@@ -23,6 +23,7 @@ class PageApiController extends ApiController
'html' => ['required_without:markdown', 'string'],
'markdown' => ['required_without:html', 'string'],
'tags' => ['array'],
+ 'priority' => ['integer'],
],
'update' => [
'book_id' => ['integer'],
@@ -31,6 +32,7 @@ class PageApiController extends ApiController
'html' => ['string'],
'markdown' => ['string'],
'tags' => ['array'],
+ 'priority' => ['integer'],
],
];
From 4d399f6ba77610635d4d1399b9ac3d5302c63b9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jean-Ren=C3=A9=20ROUET?= <rouet@in2p3.fr>
Date: Tue, 11 Jul 2023 13:28:20 +0200
Subject: [PATCH 2/4] add priority on page and chapter create
---
app/Entities/Repos/ChapterRepo.php | 2 +-
app/Entities/Repos/PageRepo.php | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php
index 588854c7e..dadeec7f8 100644
--- a/app/Entities/Repos/ChapterRepo.php
+++ b/app/Entities/Repos/ChapterRepo.php
@@ -49,7 +49,7 @@ class ChapterRepo
{
$chapter = new Chapter();
$chapter->book_id = $parentBook->id;
- $chapter->priority = (new BookContents($parentBook))->getLastPriority() + 1;
+ $chapter->priority = $chapter->priority ?: (new BookContents($parentBook))->getLastPriority() + 1;
$this->baseRepo->create($chapter, $input);
Activity::add(ActivityType::CHAPTER_CREATE, $chapter);
diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php
index 521519dc0..637f4133a 100644
--- a/app/Entities/Repos/PageRepo.php
+++ b/app/Entities/Repos/PageRepo.php
@@ -164,7 +164,7 @@ class PageRepo
$draft->draft = false;
$draft->revision_count = 1;
- $draft->priority = $this->getNewPriority($draft);
+ $draft->priority = $draft->priority ?: $this->getNewPriority($draft);
$draft->save();
$this->revisionRepo->storeNewForPage($draft, trans('entities.pages_initial_revision'));
From 3a36d3c847d235da3926f80303715e5349ee449f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jean-Ren=C3=A9=20ROUET?= <rouet@in2p3.fr>
Date: Tue, 11 Jul 2023 14:11:13 +0200
Subject: [PATCH 3/4] add tests for priority
---
dev/api/requests/chapters-create.json | 6 +++--
dev/api/requests/chapters-update.json | 5 ++--
dev/api/requests/pages-create.json | 5 ++--
dev/api/requests/pages-update.json | 5 ++--
tests/Api/ChaptersApiTest.php | 32 +++++++++++++++++++++++++
tests/Api/PagesApiTest.php | 34 +++++++++++++++++++++++++++
6 files changed, 79 insertions(+), 8 deletions(-)
diff --git a/dev/api/requests/chapters-create.json b/dev/api/requests/chapters-create.json
index ca06fc298..afd3c71f1 100644
--- a/dev/api/requests/chapters-create.json
+++ b/dev/api/requests/chapters-create.json
@@ -5,5 +5,7 @@
"tags": [
{"name": "Category", "value": "Top Content"},
{"name": "Rating", "value": "Highest"}
- ]
-}
\ No newline at end of file
+ ],
+ "priority": 15
+}
+}
diff --git a/dev/api/requests/chapters-update.json b/dev/api/requests/chapters-update.json
index 6bd3a3e5c..f62d63d1e 100644
--- a/dev/api/requests/chapters-update.json
+++ b/dev/api/requests/chapters-update.json
@@ -5,5 +5,6 @@
"tags": [
{"name": "Category", "value": "Kinda Good Content"},
{"name": "Rating", "value": "Medium"}
- ]
-}
\ No newline at end of file
+ ],
+ "priority": 15
+}
diff --git a/dev/api/requests/pages-create.json b/dev/api/requests/pages-create.json
index 1f53b42d4..5a21a80c7 100644
--- a/dev/api/requests/pages-create.json
+++ b/dev/api/requests/pages-create.json
@@ -5,5 +5,6 @@
"tags": [
{"name": "Category", "value": "Not Bad Content"},
{"name": "Rating", "value": "Average"}
- ]
-}
\ No newline at end of file
+ ],
+ "priority": 15
+}
diff --git a/dev/api/requests/pages-update.json b/dev/api/requests/pages-update.json
index b9bfeb630..fc8c73c0c 100644
--- a/dev/api/requests/pages-update.json
+++ b/dev/api/requests/pages-update.json
@@ -5,5 +5,6 @@
"tags": [
{"name": "Category", "value": "API Examples"},
{"name": "Rating", "value": "Alright"}
- ]
-}
\ No newline at end of file
+ ],
+ "priority": 15
+}
diff --git a/tests/Api/ChaptersApiTest.php b/tests/Api/ChaptersApiTest.php
index 713d8bba4..a99a85af8 100644
--- a/tests/Api/ChaptersApiTest.php
+++ b/tests/Api/ChaptersApiTest.php
@@ -61,6 +61,37 @@ class ChaptersApiTest extends TestCase
$this->assertActivityExists('chapter_create', $newItem);
}
+ public function test_create_applies_correct_priority()
+ {
+ $this->actingAsApiEditor();
+ $book = $this->entities->book();
+ $details = [
+ 'name' => 'My API chapter',
+ 'description' => 'A chapter created via the API',
+ 'book_id' => $book->id,
+ 'tags' => [
+ [
+ 'name' => 'tagname',
+ 'value' => 'tagvalue',
+ ],
+ ],
+ 'priority' => 15,
+ ];
+
+ $resp = $this->postJson($this->baseEndpoint, $details);
+ $resp->assertStatus(200);
+ $newItem = Chapter::query()->orderByDesc('id')->where('name', '=', $details['name'])->first();
+ $resp->assertJson(array_merge($details, ['id' => $newItem->id, 'slug' => $newItem->slug]));
+ $this->assertDatabaseHas('tags', [
+ 'entity_id' => $newItem->id,
+ 'entity_type' => $newItem->getMorphClass(),
+ 'name' => 'tagname',
+ 'value' => 'tagvalue',
+ ]);
+ $resp->assertJsonMissing(['pages' => []]);
+ $this->assertActivityExists('chapter_create', $newItem);
+ }
+
public function test_chapter_name_needed_to_create()
{
$this->actingAsApiEditor();
@@ -137,6 +168,7 @@ class ChaptersApiTest extends TestCase
'value' => 'freshtagval',
],
],
+ 'priority' => 15,
];
$resp = $this->putJson($this->baseEndpoint . "/{$chapter->id}", $details);
diff --git a/tests/Api/PagesApiTest.php b/tests/Api/PagesApiTest.php
index 4a81f738b..a1f65692f 100644
--- a/tests/Api/PagesApiTest.php
+++ b/tests/Api/PagesApiTest.php
@@ -63,6 +63,39 @@ class PagesApiTest extends TestCase
$this->assertActivityExists('page_create', $newItem);
}
+ public function test_create_applies_correct_priority()
+ {
+ $this->actingAsApiEditor();
+ $book = $this->entities->book();
+ $details = [
+ 'name' => 'My API page',
+ 'book_id' => $book->id,
+ 'html' => '<p>My new page content</p>',
+ 'tags' => [
+ [
+ 'name' => 'tagname',
+ 'value' => 'tagvalue',
+ ],
+ ],
+ 'priority' => 15,
+ ];
+
+ $resp = $this->postJson($this->baseEndpoint, $details);
+ unset($details['html']);
+ $resp->assertStatus(200);
+ $newItem = Page::query()->orderByDesc('id')->where('name', '=', $details['name'])->first();
+ $resp->assertJson(array_merge($details, ['id' => $newItem->id, 'slug' => $newItem->slug]));
+ $this->assertDatabaseHas('tags', [
+ 'entity_id' => $newItem->id,
+ 'entity_type' => $newItem->getMorphClass(),
+ 'name' => 'tagname',
+ 'value' => 'tagvalue',
+ ]);
+ $resp->assertSeeText('My new page content');
+ $resp->assertJsonMissing(['book' => []]);
+ $this->assertActivityExists('page_create', $newItem);
+ }
+
public function test_page_name_needed_to_create()
{
$this->actingAsApiEditor();
@@ -207,6 +240,7 @@ class PagesApiTest extends TestCase
'value' => 'freshtagval',
],
],
+ 'priority' => 15,
];
$resp = $this->putJson($this->baseEndpoint . "/{$page->id}", $details);
From 9ca1139ab0d0f5dd8d8eda4e9cd96f6c33530344 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Mon, 21 Aug 2023 15:40:53 +0100
Subject: [PATCH 4/4] API: Reviewed changes for API priority control
Review of #4313
- Made constructor changes while reviewing some classes.
- Updated API examples for consistency.
- Tweaked formatting for some array changes.
- Simplified added tests.
- Tweaked chapter/page repo priority handling to be simpler.
Performed manual API endpoint testing of page/chapter create/update.
---
app/Api/ApiEntityListFormatter.php | 9 ++++----
app/Entities/Repos/ChapterRepo.php | 13 ++++-------
app/Entities/Repos/PageRepo.php | 28 ++++++----------------
dev/api/requests/chapters-create.json | 5 ++--
dev/api/requests/chapters-update.json | 4 ++--
dev/api/requests/pages-create.json | 4 ++--
dev/api/requests/pages-update.json | 4 ++--
dev/api/responses/chapters-create.json | 2 +-
dev/api/responses/chapters-update.json | 2 +-
dev/api/responses/pages-create.json | 2 +-
tests/Api/ChaptersApiTest.php | 32 +-------------------------
tests/Api/PagesApiTest.php | 32 --------------------------
12 files changed, 27 insertions(+), 110 deletions(-)
diff --git a/app/Api/ApiEntityListFormatter.php b/app/Api/ApiEntityListFormatter.php
index 7d00834e5..436d66d59 100644
--- a/app/Api/ApiEntityListFormatter.php
+++ b/app/Api/ApiEntityListFormatter.php
@@ -10,7 +10,7 @@ class ApiEntityListFormatter
* The list to be formatted.
* @var Entity[]
*/
- protected $list = [];
+ protected array $list = [];
/**
* The fields to show in the formatted data.
@@ -19,10 +19,9 @@ class ApiEntityListFormatter
* will be used for the resultant value. A null return value will omit the property.
* @var array<string|int, string|callable>
*/
- protected $fields = [
- 'id', 'name', 'slug', 'book_id', 'chapter_id',
- 'draft', 'template', 'created_at', 'updated_at',
- 'priority'
+ protected array $fields = [
+ 'id', 'name', 'slug', 'book_id', 'chapter_id', 'draft',
+ 'template', 'priority', 'created_at', 'updated_at',
];
public function __construct(array $list)
diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php
index dadeec7f8..977193d85 100644
--- a/app/Entities/Repos/ChapterRepo.php
+++ b/app/Entities/Repos/ChapterRepo.php
@@ -16,14 +16,9 @@ use Exception;
class ChapterRepo
{
- protected $baseRepo;
-
- /**
- * ChapterRepo constructor.
- */
- public function __construct(BaseRepo $baseRepo)
- {
- $this->baseRepo = $baseRepo;
+ public function __construct(
+ protected BaseRepo $baseRepo
+ ) {
}
/**
@@ -49,7 +44,7 @@ class ChapterRepo
{
$chapter = new Chapter();
$chapter->book_id = $parentBook->id;
- $chapter->priority = $chapter->priority ?: (new BookContents($parentBook))->getLastPriority() + 1;
+ $chapter->priority = (new BookContents($parentBook))->getLastPriority() + 1;
$this->baseRepo->create($chapter, $input);
Activity::add(ActivityType::CHAPTER_CREATE, $chapter);
diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php
index 637f4133a..61a1db63e 100644
--- a/app/Entities/Repos/PageRepo.php
+++ b/app/Entities/Repos/PageRepo.php
@@ -23,24 +23,12 @@ use Illuminate\Pagination\LengthAwarePaginator;
class PageRepo
{
- protected BaseRepo $baseRepo;
- protected RevisionRepo $revisionRepo;
- protected ReferenceStore $referenceStore;
- protected ReferenceUpdater $referenceUpdater;
-
- /**
- * PageRepo constructor.
- */
public function __construct(
- BaseRepo $baseRepo,
- RevisionRepo $revisionRepo,
- ReferenceStore $referenceStore,
- ReferenceUpdater $referenceUpdater
+ protected BaseRepo $baseRepo,
+ protected RevisionRepo $revisionRepo,
+ protected ReferenceStore $referenceStore,
+ protected ReferenceUpdater $referenceUpdater
) {
- $this->baseRepo = $baseRepo;
- $this->revisionRepo = $revisionRepo;
- $this->referenceStore = $referenceStore;
- $this->referenceUpdater = $referenceUpdater;
}
/**
@@ -159,13 +147,11 @@ class PageRepo
*/
public function publishDraft(Page $draft, array $input): Page
{
- $this->updateTemplateStatusAndContentFromInput($draft, $input);
- $this->baseRepo->update($draft, $input);
-
$draft->draft = false;
$draft->revision_count = 1;
- $draft->priority = $draft->priority ?: $this->getNewPriority($draft);
- $draft->save();
+ $draft->priority = $this->getNewPriority($draft);
+ $this->updateTemplateStatusAndContentFromInput($draft, $input);
+ $this->baseRepo->update($draft, $input);
$this->revisionRepo->storeNewForPage($draft, trans('entities.pages_initial_revision'));
$this->referenceStore->updateForPage($draft);
diff --git a/dev/api/requests/chapters-create.json b/dev/api/requests/chapters-create.json
index afd3c71f1..a7a0e072c 100644
--- a/dev/api/requests/chapters-create.json
+++ b/dev/api/requests/chapters-create.json
@@ -2,10 +2,9 @@
"book_id": 1,
"name": "My fantastic new chapter",
"description": "This is a great new chapter that I've created via the API",
+ "priority": 15,
"tags": [
{"name": "Category", "value": "Top Content"},
{"name": "Rating", "value": "Highest"}
- ],
- "priority": 15
-}
+ ]
}
diff --git a/dev/api/requests/chapters-update.json b/dev/api/requests/chapters-update.json
index f62d63d1e..18c40301b 100644
--- a/dev/api/requests/chapters-update.json
+++ b/dev/api/requests/chapters-update.json
@@ -2,9 +2,9 @@
"book_id": 1,
"name": "My fantastic updated chapter",
"description": "This is an updated chapter that I've altered via the API",
+ "priority": 16,
"tags": [
{"name": "Category", "value": "Kinda Good Content"},
{"name": "Rating", "value": "Medium"}
- ],
- "priority": 15
+ ]
}
diff --git a/dev/api/requests/pages-create.json b/dev/api/requests/pages-create.json
index 5a21a80c7..bb32943a2 100644
--- a/dev/api/requests/pages-create.json
+++ b/dev/api/requests/pages-create.json
@@ -2,9 +2,9 @@
"book_id": 1,
"name": "My API Page",
"html": "<p>my new API page</p>",
+ "priority": 15,
"tags": [
{"name": "Category", "value": "Not Bad Content"},
{"name": "Rating", "value": "Average"}
- ],
- "priority": 15
+ ]
}
diff --git a/dev/api/requests/pages-update.json b/dev/api/requests/pages-update.json
index fc8c73c0c..e3ca9004e 100644
--- a/dev/api/requests/pages-update.json
+++ b/dev/api/requests/pages-update.json
@@ -2,9 +2,9 @@
"chapter_id": 1,
"name": "My updated API Page",
"html": "<p>my new API page - Updated</p>",
+ "priority": 16,
"tags": [
{"name": "Category", "value": "API Examples"},
{"name": "Rating", "value": "Alright"}
- ],
- "priority": 15
+ ]
}
diff --git a/dev/api/responses/chapters-create.json b/dev/api/responses/chapters-create.json
index 4dbc764b1..cf47b123d 100644
--- a/dev/api/responses/chapters-create.json
+++ b/dev/api/responses/chapters-create.json
@@ -4,7 +4,7 @@
"slug": "my-fantastic-new-chapter",
"name": "My fantastic new chapter",
"description": "This is a great new chapter that I've created via the API",
- "priority": 6,
+ "priority": 15,
"created_by": 1,
"updated_by": 1,
"owned_by": 1,
diff --git a/dev/api/responses/chapters-update.json b/dev/api/responses/chapters-update.json
index cc454d740..a4940af2d 100644
--- a/dev/api/responses/chapters-update.json
+++ b/dev/api/responses/chapters-update.json
@@ -4,7 +4,7 @@
"slug": "my-fantastic-updated-chapter",
"name": "My fantastic updated chapter",
"description": "This is an updated chapter that I've altered via the API",
- "priority": 7,
+ "priority": 16,
"created_at": "2020-05-22T23:03:35.000000Z",
"updated_at": "2020-05-22T23:07:20.000000Z",
"created_by": 1,
diff --git a/dev/api/responses/pages-create.json b/dev/api/responses/pages-create.json
index 385d5384e..11f5ab8c8 100644
--- a/dev/api/responses/pages-create.json
+++ b/dev/api/responses/pages-create.json
@@ -6,7 +6,7 @@
"slug": "my-api-page",
"html": "<p id=\"bkmrk-my-new-api-page\">my new API page</p>",
"raw_html": "<p id=\"bkmrk-my-new-api-page\">my new API page</p>",
- "priority": 14,
+ "priority": 15,
"created_at": "2020-11-28T15:01:39.000000Z",
"updated_at": "2020-11-28T15:01:39.000000Z",
"created_by": {
diff --git a/tests/Api/ChaptersApiTest.php b/tests/Api/ChaptersApiTest.php
index a99a85af8..0629f3aed 100644
--- a/tests/Api/ChaptersApiTest.php
+++ b/tests/Api/ChaptersApiTest.php
@@ -45,37 +45,7 @@ class ChaptersApiTest extends TestCase
'value' => 'tagvalue',
],
],
- ];
-
- $resp = $this->postJson($this->baseEndpoint, $details);
- $resp->assertStatus(200);
- $newItem = Chapter::query()->orderByDesc('id')->where('name', '=', $details['name'])->first();
- $resp->assertJson(array_merge($details, ['id' => $newItem->id, 'slug' => $newItem->slug]));
- $this->assertDatabaseHas('tags', [
- 'entity_id' => $newItem->id,
- 'entity_type' => $newItem->getMorphClass(),
- 'name' => 'tagname',
- 'value' => 'tagvalue',
- ]);
- $resp->assertJsonMissing(['pages' => []]);
- $this->assertActivityExists('chapter_create', $newItem);
- }
-
- public function test_create_applies_correct_priority()
- {
- $this->actingAsApiEditor();
- $book = $this->entities->book();
- $details = [
- 'name' => 'My API chapter',
- 'description' => 'A chapter created via the API',
- 'book_id' => $book->id,
- 'tags' => [
- [
- 'name' => 'tagname',
- 'value' => 'tagvalue',
- ],
- ],
- 'priority' => 15,
+ 'priority' => 15,
];
$resp = $this->postJson($this->baseEndpoint, $details);
diff --git a/tests/Api/PagesApiTest.php b/tests/Api/PagesApiTest.php
index a1f65692f..0d084472d 100644
--- a/tests/Api/PagesApiTest.php
+++ b/tests/Api/PagesApiTest.php
@@ -32,38 +32,6 @@ class PagesApiTest extends TestCase
}
public function test_create_endpoint()
- {
- $this->actingAsApiEditor();
- $book = $this->entities->book();
- $details = [
- 'name' => 'My API page',
- 'book_id' => $book->id,
- 'html' => '<p>My new page content</p>',
- 'tags' => [
- [
- 'name' => 'tagname',
- 'value' => 'tagvalue',
- ],
- ],
- ];
-
- $resp = $this->postJson($this->baseEndpoint, $details);
- unset($details['html']);
- $resp->assertStatus(200);
- $newItem = Page::query()->orderByDesc('id')->where('name', '=', $details['name'])->first();
- $resp->assertJson(array_merge($details, ['id' => $newItem->id, 'slug' => $newItem->slug]));
- $this->assertDatabaseHas('tags', [
- 'entity_id' => $newItem->id,
- 'entity_type' => $newItem->getMorphClass(),
- 'name' => 'tagname',
- 'value' => 'tagvalue',
- ]);
- $resp->assertSeeText('My new page content');
- $resp->assertJsonMissing(['book' => []]);
- $this->assertActivityExists('page_create', $newItem);
- }
-
- public function test_create_applies_correct_priority()
{
$this->actingAsApiEditor();
$book = $this->entities->book();