Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: transfer ownership via webgui #741

Merged
merged 14 commits into from
Jan 11, 2024
1 change: 1 addition & 0 deletions appinfo/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
['name' => 'ApiTables#create', 'url' => '/api/2/tables', 'verb' => 'POST'],
['name' => 'ApiTables#update', 'url' => '/api/2/tables/{id}', 'verb' => 'PUT'],
['name' => 'ApiTables#destroy', 'url' => '/api/2/tables/{id}', 'verb' => 'DELETE'],
['name' => 'ApiTables#transfer', 'url' => '/api/2/tables/{id}/transfer', 'verb' => 'PUT'],

['name' => 'ApiColumns#index', 'url' => '/api/2/columns/{nodeType}/{nodeId}', 'verb' => 'GET'],
['name' => 'ApiColumns#show', 'url' => '/api/2/columns/{id}', 'verb' => 'GET'],
Expand Down
2 changes: 1 addition & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions cypress/e2e/tables-table.cy.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
let localUser
let targetUserTransfer

describe('Manage a table', () => {

before(function() {
cy.createRandomUser().then(user => {
localUser = user
})
cy.createRandomUser().then(user => {
targetUserTransfer = user
})
})

beforeEach(function() {
Expand Down Expand Up @@ -51,4 +55,30 @@ describe('Manage a table', () => {
cy.wait(10).get('.toastify.toast-success').should('be.visible')
cy.get('.app-navigation__list').contains('to do list').should('not.exist')
})

it('Transfer', () => {
cy.contains('.app-menu-entry--label', 'Tables').click()
cy.contains('button', 'Create new table').click()
cy.get('.tile').contains('ToDo').click({ force: true })
cy.get('.modal__content').should('be.visible')
cy.get('.modal__content input[type="text"]').clear().type('test table')
cy.contains('button', 'Create table').click()

cy.get('.app-navigation__list').contains('test table').click({ force: true })
cy.get('[data-cy="customTableAction"] button').click()
cy.get('.action-button__text').contains('Edit table').click()

cy.get('[data-cy="editTableModal"]').should('be.visible')
cy.get('[data-cy="editTableModal"] button').contains('Change owner').click()
cy.get('[data-cy="editTableModal"]').should('not.exist')
cy.get('[data-cy="transferTableModal"]').should('be.visible')
cy.get('[data-cy="transferTableModal"] input[type="search"]').clear().type(targetUserTransfer.userId)
cy.get(`.vs__dropdown-menu [user="${targetUserTransfer.userId}"]`).click()
cy.get('[data-cy="transferTableButton"]').should('be.enabled').click()
cy.get('.toastify.toast-success').should('be.visible')
cy.get('.app-navigation__list').contains('test table').should('not.exist')
enjeck marked this conversation as resolved.
Show resolved Hide resolved
cy.login(targetUserTransfer)
cy.visit('apps/tables')
cy.get('.app-navigation__list').contains('test table')
})
})
14 changes: 12 additions & 2 deletions lib/Command/ChangeOwnershipTable.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
namespace OCA\Tables\Command;

use OCA\Tables\Errors\InternalError;
use OCA\Tables\Errors\NotFoundError;
use OCA\Tables\Errors\PermissionError;
use OCA\Tables\Service\TableService;
use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Command\Command;
Expand Down Expand Up @@ -68,12 +70,20 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$newOwnerUserId = $input->getArgument('user-id');

try {
$arr = $this->tableService->setOwner($id, $newOwnerUserId, '');
$output->writeln(json_encode($arr, JSON_PRETTY_PRINT));
$table = $this->tableService->setOwner($id, $newOwnerUserId, '');
$output->writeln(json_encode($table->jsonSerialize(), JSON_PRETTY_PRINT));
} catch (InternalError $e) {
$output->writeln('Error occurred: '.$e->getMessage());
$this->logger->warning('Following error occurred during executing occ command "'.self::class.'"', ['exception' => $e]);
return 1;
} catch (NotFoundError $e) {
$output->writeln('Not found error occurred: '.$e->getMessage());
$this->logger->warning('Following error occurred during executing occ command "'.self::class.'"', ['exception' => $e]);
return 1;
} catch (PermissionError $e) {
$output->writeln('Permission error occurred: '.$e->getMessage());
$this->logger->warning('Following error occurred during executing occ command "'.self::class.'"', ['exception' => $e]);
return 1;
}
return 0;
}
Expand Down
28 changes: 28 additions & 0 deletions lib/Controller/ApiTablesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,32 @@ public function destroy(int $id): DataResponse {
return $this->handleNotFoundError($e);
}
}

/**
* [api v2] Transfer table
*
* Transfer table from one user to another
*
* @NoAdminRequired
*
* @param int $id Table ID
* @param string $newOwnerUserId New user ID
*
* @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_NOT_FOUND, array{message: string}, array{}>
*
* 200: Ownership changed
* 403: No permissions
* 404: Not found
*/
public function transfer(int $id, string $newOwnerUserId): DataResponse {
try {
return new DataResponse($this->service->setOwner($id, $newOwnerUserId)->jsonSerialize());
} catch (PermissionError $e) {
return $this->handlePermissionError($e);
} catch (InternalError $e) {
return $this->handleError($e);
} catch (NotFoundError $e) {
return $this->handleNotFoundError($e);
}
}
}
9 changes: 7 additions & 2 deletions lib/Service/PermissionsService.php
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,12 @@ private function userIsElementOwner($element, string $userId = null): bool {
return $element->getOwnership() === $userId;
}

public function canChangeElementOwner(string $userId): bool {
return $userId === '';
/**
* @param View|Table $element
* @param string $userId
* @return bool
*/
public function canChangeElementOwner($element, string $userId): bool {
return $userId === '' || $this->userIsElementOwner($element, $userId);
}
}
64 changes: 36 additions & 28 deletions lib/Service/TableService.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace OCA\Tables\Service;

use DateTime;
use Exception;

use OCA\Tables\Db\Table;
use OCA\Tables\Db\TableMapper;
Expand Down Expand Up @@ -287,45 +286,54 @@ public function create(string $title, string $template, ?string $emoji, ?string
}

/**
* Set a new owner for a table and adjust all related ressources
*
* @param int $id
* @param string $newOwnerUserId
* @param string|null $userId
*
* @return Table
*
* @throws InternalError
* @throws NotFoundError
* @throws PermissionError
*/
public function setOwner(int $id, string $newOwnerUserId, ?string $userId = null): array {
public function setOwner(int $id, string $newOwnerUserId, ?string $userId = null): Table {
$userId = $this->permissionsService->preCheckUserId($userId);

try {
$table = $this->mapper->find($id);
} catch (DoesNotExistException $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new NotFoundError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage());
} catch (MultipleObjectsReturnedException|OcpDbException $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage());
}

// security
if (!$this->permissionsService->canChangeElementOwner($userId)) {
throw new PermissionError('PermissionError: can not change table owner with table id '.$id);
}
// security
if (!$this->permissionsService->canChangeElementOwner($table, $userId)) {
throw new PermissionError('PermissionError: can not change table owner with table id '.$id);
}

$table->setOwnership($newOwnerUserId);
$table->setOwnership($newOwnerUserId);
try {
$table = $this->mapper->update($table);
} catch (OcpDbException $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage());
}

// also change owners of related shares
$updatedShares = $this->shareService->changeSenderForNode('table', $id, $newOwnerUserId, $userId);
$updatesSharesOutput = [];
foreach ($updatedShares as $share) {
$updatesSharesOutput[] = [
'shareId' => $share->getId(),
'nodeType' => $share->getNodeType(),
'sender' => $share->getSender(),
'receiverType' => $share->getReceiverType(),
'receiver' => $share->getReceiver()
];
}

return [
'tableId' => $table->getId(),
'title' => $table->getTitle(),
'ownership' => $table->getOwnership(),
'updatedShares' => $updatesSharesOutput
];
} catch (Exception $e) {
// change owners of related shares
try {
$this->shareService->changeSenderForNode('table', $id, $newOwnerUserId, $userId);
} catch (InternalError $e) {
$this->logger->error('Could not update related shares for a table transfer!');
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new InternalError($e->getMessage());
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage());
}

return $table;
}

/**
Expand Down
Loading
Loading