Skip to content

Commit

Permalink
refactor(routes): Remove parentId parameter from page routes
Browse files Browse the repository at this point in the history 
The `parentId` has been been superfluous information all the time for
all routes except "create new page". If we need the parent, it's better
to retrieve it from the `pageId` instead of trusting the client that
they passed the right `parentId`.

Signed-off-by: Jonas <[email protected]>
  • Loading branch information
@mejo-
mejo- committed Oct 23, 2023
1 parent 3acd18e commit 4ad8293
Show file tree
Hide file tree
Showing 15 changed files with 185 additions and 222 deletions.
68 changes: 34 additions & 34 deletions appinfo/routes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,24 @@
// pages API
['name' => 'page#index', 'url' => '/_api/{collectiveId}/_pages',
'verb' => 'GET', 'requirements' => ['collectiveId' => '\d+']],
['name' => 'page#get', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}',
'verb' => 'GET', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#create', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}',
['name' => 'page#get', 'url' => '/_api/{collectiveId}/_pages/{id}',
'verb' => 'GET', 'requirements' => ['collectiveId' => '\d+', 'id' => '\d+']],
['name' => 'page#create', 'url' => '/_api/{collectiveId}/_pages/{parentId}',
'verb' => 'POST', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+']],
['name' => 'page#touch', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}/touch',
'verb' => 'GET', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#rename', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}',
'verb' => 'PUT', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#setEmoji', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}/emoji',
'verb' => 'PUT', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#setSubpageOrder', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}/subpageOrder',
'verb' => 'PUT', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#trash', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}',
'verb' => 'DELETE', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#getBacklinks', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}/backlinks',
'verb' => 'GET', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#getAttachments', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}/attachments',
'verb' => 'GET', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#touch', 'url' => '/_api/{collectiveId}/_pages/{id}/touch',
'verb' => 'GET', 'requirements' => ['collectiveId' => '\d+', 'id' => '\d+']],
['name' => 'page#rename', 'url' => '/_api/{collectiveId}/_pages/{id}',
'verb' => 'PUT', 'requirements' => ['collectiveId' => '\d+', 'id' => '\d+']],
['name' => 'page#setEmoji', 'url' => '/_api/{collectiveId}/_pages/{id}/emoji',
'verb' => 'PUT', 'requirements' => ['collectiveId' => '\d+', 'id' => '\d+']],
['name' => 'page#setSubpageOrder', 'url' => '/_api/{collectiveId}/_pages/{id}/subpageOrder',
'verb' => 'PUT', 'requirements' => ['collectiveId' => '\d+', 'id' => '\d+']],
['name' => 'page#trash', 'url' => '/_api/{collectiveId}/_pages/{id}',
'verb' => 'DELETE', 'requirements' => ['collectiveId' => '\d+', 'id' => '\d+']],
['name' => 'page#getBacklinks', 'url' => '/_api/{collectiveId}/_pages/{id}/backlinks',
'verb' => 'GET', 'requirements' => ['collectiveId' => '\d+', 'id' => '\d+']],
['name' => 'page#getAttachments', 'url' => '/_api/{collectiveId}/_pages/{id}/attachments',
'verb' => 'GET', 'requirements' => ['collectiveId' => '\d+', 'id' => '\d+']],

// pages trash API
['name' => 'pageTrash#index', 'url' => '/_api/{collectiveId}/_pages/trash', 'verb' => 'GET'],
Expand All @@ -75,24 +75,24 @@

// public pages API
['name' => 'publicPage#index', 'url' => '/_api/p/{token}/_pages', 'verb' => 'GET'],
['name' => 'publicPage#get', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}',
'verb' => 'GET', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#create', 'url' => '/_api/p/{token}/_pages/parent/{parentId}',
['name' => 'publicPage#get', 'url' => '/_api/p/{token}/_pages/{id}',
'verb' => 'GET', 'requirements' => ['id' => '\d+']],
['name' => 'publicPage#create', 'url' => '/_api/p/{token}/_pages/{parentId}',
'verb' => 'POST', 'requirements' => ['parentId' => '\d+']],
['name' => 'publicPage#touch', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}/touch',
'verb' => 'GET', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#rename', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}',
'verb' => 'PUT', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#setEmoji', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}/emoji',
'verb' => 'PUT', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#setSubpageOrder', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}/subpageOrder',
'verb' => 'PUT', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#trash', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}',
'verb' => 'DELETE', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#getAttachments', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}/attachments',
'verb' => 'GET', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#getBacklinks', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}/backlinks',
'verb' => 'GET', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#touch', 'url' => '/_api/p/{token}/_pages/{id}/touch',
'verb' => 'GET', 'requirements' => ['id' => '\d+']],
['name' => 'publicPage#rename', 'url' => '/_api/p/{token}/_pages/{id}',
'verb' => 'PUT', 'requirements' => ['id' => '\d+']],
['name' => 'publicPage#setEmoji', 'url' => '/_api/p/{token}/_pages/{id}/emoji',
'verb' => 'PUT', 'requirements' => ['id' => '\d+']],
['name' => 'publicPage#setSubpageOrder', 'url' => '/_api/p/{token}/_pages/{id}/subpageOrder',
'verb' => 'PUT', 'requirements' => ['id' => '\d+']],
['name' => 'publicPage#trash', 'url' => '/_api/p/{token}/_pages/{id}',
'verb' => 'DELETE', 'requirements' => ['id' => '\d+']],
['name' => 'publicPage#getAttachments', 'url' => '/_api/p/{token}/_pages/{id}/attachments',
'verb' => 'GET', 'requirements' => ['id' => '\d+']],
['name' => 'publicPage#getBacklinks', 'url' => '/_api/p/{token}/_pages/{id}/backlinks',
'verb' => 'GET', 'requirements' => ['id' => '\d+']],

// public pages trash API
['name' => 'publicPageTrash#index', 'url' => '/_api/p/{token}/_pages/trash', 'verb' => 'GET'],
Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/collective-share.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('Collective Share', function() {
cy.logout()
cy.visit(shareUrl)
// Do some handstands to ensure that new page with editor is loaded before we edit the title
cy.intercept('POST', '**/_api/p/*/_pages/parent/*').as('createPage')
cy.intercept('POST', '**/_api/p/*/_pages/*').as('createPage')
cy.intercept('PUT', '**/apps/text/public/session/create').as('textCreateSession')
cy.contains('.app-content-list-item', 'Share me')
.find('button.action-button-add')
Expand Down
6 changes: 3 additions & 3 deletions cypress/e2e/pages.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('Page', function() {
describe('Creating a page from template', function() {
it('New page has template content', function() {
// Do some handstands to ensure that new page with editor is loaded before we edit the title
cy.intercept('POST', '**/_api/*/_pages/parent/*').as('createPage')
cy.intercept('POST', '**/_api/*/_pages/*').as('createPage')
cy.intercept('PUT', '**/apps/text/session/create').as('textCreateSession')
cy.contains('.app-content-list-item', 'Our Garden')
.find('button.action-button-add')
Expand All @@ -117,7 +117,7 @@ describe('Page', function() {
.should('not.have.attr', 'disabled')
cy.get('#titleform input.title')
.clear()
cy.intercept('PUT', '**/_api/*/_pages/parent/*/page/*').as('renamePage')
cy.intercept('PUT', '**/_api/*/_pages/*').as('renamePage')
cy.get('#titleform input.title')
.type('New page from Template')
cy.get('#titleform input.title')
Expand All @@ -134,7 +134,7 @@ describe('Page', function() {
describe('Creating a new subpage', function() {
it('Shows the title in the enabled titleform and full path in browser title', function() {
// Do some handstands to ensure that new page with editor is loaded before we edit the title
cy.intercept('POST', '**/_api/*/_pages/parent/*').as('createPage')
cy.intercept('POST', '**/_api/*/_pages/*').as('createPage')
cy.intercept('PUT', '**/apps/text/session/create').as('textCreateSession')
cy.contains('.app-content-list-item', '#% special chars')
.find('button.action-button-add')
Expand Down
53 changes: 23 additions & 30 deletions lib/Controller/PageController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,14 @@ public function index(int $collectiveId): DataResponse {
* @NoAdminRequired
*
* @param int $collectiveId
* @param int $parentId
* @param int $id
*
* @return DataResponse
*/
public function get(int $collectiveId, int $parentId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $parentId, $id): array {
public function get(int $collectiveId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $id): array {
$userId = $this->getUserId();
$pageInfo = $this->service->find($collectiveId, $parentId, $id, $userId);
$pageInfo = $this->service->find($collectiveId, $id, $userId);
return [
"data" => $pageInfo
];
Expand Down Expand Up @@ -99,15 +98,14 @@ public function create(int $collectiveId, int $parentId, string $title): DataRes
* @NoAdminRequired
*
* @param int $collectiveId
* @param int $parentId
* @param int $id
*
* @return DataResponse
*/
public function touch(int $collectiveId, int $parentId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $parentId, $id): array {
public function touch(int $collectiveId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $id): array {
$userId = $this->getUserId();
$pageInfo = $this->service->touch($collectiveId, $parentId, $id, $userId);
$pageInfo = $this->service->touch($collectiveId, $id, $userId);
return [
"data" => $pageInfo
];
Expand All @@ -118,17 +116,17 @@ public function touch(int $collectiveId, int $parentId, int $id): DataResponse {
* @NoAdminRequired
*
* @param int $collectiveId
* @param int $parentId
* @param int $id
* @param int|null $parentId
* @param string|null $title
* @param int|null $index
*
* @return DataResponse
*/
public function rename(int $collectiveId, int $parentId, int $id, ?string $title = null, ?int $index = 0): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $parentId, $id, $title, $index): array {
public function rename(int $collectiveId, int $id, ?int $parentId = null, ?string $title = null, ?int $index = 0): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $id, $parentId, $title, $index): array {
$userId = $this->getUserId();
$pageInfo = $this->service->rename($collectiveId, $parentId, $id, $title, $index, $userId);
$pageInfo = $this->service->rename($collectiveId, $id, $parentId, $title, $index, $userId);
return [
"data" => $pageInfo
];
Expand All @@ -139,16 +137,15 @@ public function rename(int $collectiveId, int $parentId, int $id, ?string $title
* @NoAdminRequired
*
* @param int $collectiveId
* @param int $parentId
* @param int $id
* @param string|null $emoji
*
* @return DataResponse
*/
public function setEmoji(int $collectiveId, int $parentId, int $id, ?string $emoji = null): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $parentId, $id, $emoji): array {
public function setEmoji(int $collectiveId, int $id, ?string $emoji = null): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $id, $emoji): array {
$userId = $this->getUserId();
$pageInfo = $this->service->setEmoji($collectiveId, $parentId, $id, $emoji, $userId);
$pageInfo = $this->service->setEmoji($collectiveId, $id, $emoji, $userId);
return [
"data" => $pageInfo
];
Expand All @@ -159,16 +156,15 @@ public function setEmoji(int $collectiveId, int $parentId, int $id, ?string $emo
* @NoAdminRequired
*
* @param int $collectiveId
* @param int $parentId
* @param int $id
* @param string|null $subpageOrder
*
* @return DataResponse
*/
public function setSubpageOrder(int $collectiveId, int $parentId, int $id, ?string $subpageOrder = null): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $parentId, $id, $subpageOrder): array {
public function setSubpageOrder(int $collectiveId, int $id, ?string $subpageOrder = null): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $id, $subpageOrder): array {
$userId = $this->getUserId();
$pageInfo = $this->service->setSubpageOrder($collectiveId, $parentId, $id, $subpageOrder, $userId);
$pageInfo = $this->service->setSubpageOrder($collectiveId, $id, $subpageOrder, $userId);
return [
"data" => $pageInfo
];
Expand All @@ -179,15 +175,14 @@ public function setSubpageOrder(int $collectiveId, int $parentId, int $id, ?stri
* @NoAdminRequired
*
* @param int $collectiveId
* @param int $parentId
* @param int $id
*
* @return DataResponse
*/
public function trash(int $collectiveId, int $parentId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $parentId, $id): array {
public function trash(int $collectiveId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $id): array {
$userId = $this->getUserId();
$pageInfo = $this->service->trash($collectiveId, $parentId, $id, $userId);
$pageInfo = $this->service->trash($collectiveId, $id, $userId);
return [
"data" => $pageInfo
];
Expand All @@ -198,12 +193,11 @@ public function trash(int $collectiveId, int $parentId, int $id): DataResponse {
* @NoAdminRequired
*
* @param int $collectiveId
* @param int $parentId
* @param int $id
*
* @return DataResponse
*/
public function getAttachments(int $collectiveId, int $parentId, int $id): DataResponse {
public function getAttachments(int $collectiveId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $id): array {
$userId = $this->getUserId();
$attachments = $this->attachmentService->getAttachments($collectiveId, $id, $userId);
Expand All @@ -217,15 +211,14 @@ public function getAttachments(int $collectiveId, int $parentId, int $id): DataR
* @NoAdminRequired
*
* @param int $collectiveId
* @param int $parentId
* @param int $id
*
* @return DataResponse
*/
public function getBacklinks(int $collectiveId, int $parentId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $parentId, $id): array {
public function getBacklinks(int $collectiveId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $id): array {
$userId = $this->getUserId();
$backlinks = $this->service->getBacklinks($collectiveId, $parentId, $id, $userId);
$backlinks = $this->service->getBacklinks($collectiveId, $id, $userId);
return [
"data" => $backlinks
];
Expand Down
Loading

0 comments on commit 4ad8293

Please sign in to comment.