Skip to content

Commit

Permalink
fixup! feat(polls): allow editing of draft polls
Browse files Browse the repository at this point in the history
  • Loading branch information
@miaulalala
miaulalala committed Dec 2, 2024
1 parent 5b57919 commit 9dcf018
Show file tree
Hide file tree
Showing 7 changed files with 365 additions and 56 deletions.
28 changes: 28 additions & 0 deletions docs/poll.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,34 @@ Base endpoint is: `/ocs/v2.php/apps/spreed/api/v1`

See [Poll data](#poll-data)

# Edit a draft poll in a conversation

* Federation capability: `federation-v1`
* Method: `POST`
* Endpoint: `/poll/{token}/draft/{pollId]}`
* Data:

| field | type | Description |
|--------------|--------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `question` | string | The question of the poll |
| `options` | string[] | Array of strings with the voting options |
| `resultMode` | int | The result and voting mode of the poll, `0` means participants can immediatelly see the result and who voted for which option. `1` means the result is hidden until the poll is closed and then only the summary is published. |
| `maxVotes` | int | Maximum amount of options a participant can vote for |

* Response:
- Status code:
+ `201 Created`
+ `400 Bad Request` When the room is a one-to-one conversation
+ `400 Bad Request` When the question or the options were too long or invalid (not strings)
+ `403 Forbidden` When the conversation is read-only
+ `403 Forbidden` When the actor does not have chat permissions
+ `404 Not Found` When the conversation could not be found for the participant
+ `412 Precondition Failed` When the lobby is active and the user is not a moderator

- Data:

See [Poll data](#poll-data)

## Get state or result of a poll

* Federation capability: `federation-v1`
Expand Down
37 changes: 24 additions & 13 deletions lib/Controller/PollController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,14 @@ public function createPoll(string $question, array $options, int $resultMode, in
*
* Required capability: `edit-draft-poll`
*
* @param int $pollId
* @param int $pollId The poll id
* @param string $question Question of the poll
* @param string[] $options Options of the poll
* @param int $resultMode
* @psalm-param list<string> $options
* @param 0|1 $resultMode Mode how the results will be shown
* @psalm-param Poll::MODE_* $resultMode Mode how the results will be shown
* @param int $maxVotes Number of maximum votes per voter
* @return DataResponse<Http::STATUS_OK, TalkPollDraft, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'draft'|'options'|'question'|'room'}, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{error: string}, array{}>
* @return DataResponse<Http::STATUS_OK, TalkPollDraft, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'draft'|'options'|'question'|'room'}, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{error: string}, array{}>

Check failure on line 148 in lib/Controller/PollController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

InvalidReturnType 

lib/Controller/PollController.php:148:12: InvalidReturnType: The declared return type 'OCP\AppFramework\Http\DataResponse<200|400|404, array{actorDisplayName?: string, actorId?: non-empty-string, actorType?: 'bots'|'bridged'|'circles'|'emails'|'federated_users'|'groups'|'guests'|'phones'|'users', error?: string, id?: int<1, max>, maxVotes?: int<0, max>, options?: list<string>, question?: non-empty-string, resultMode?: 0|1, status?: 0|1|2}, array<never, never>>' for OCA\Talk\Controller\PollController::updateDraftPoll is incorrect, got 'OCP\AppFramework\Http\DataResponse<200|201|400|404, array{actorDisplayName?: string, actorId?: non-empty-string, actorType?: 'bots'|'bridged'|'circles'|'emails'|'federated_users'|'groups'|'guests'|'phones'|'users', details?: list<array{actorDisplayName: string, actorId: string, actorType: string, optionId: int}>, error?: mixed|string, id?: int<1, max>, maxVotes?: int<0, max>, numVoters?: int<0, max>, options?: list<string>, question?: non-empty-string, resultMode?: 0|1, status?: 0|1|2, votedSelf?: list<int>, votes?: array<string, int>}, array<never, never>>' (see https://psalm.dev/011)
*
* 200: Draft modified successfully
* 400: Modifying poll is not possible
Expand All @@ -157,7 +159,6 @@ public function createPoll(string $question, array $options, int $resultMode, in
#[RequireReadWriteConversation]
public function updateDraftPoll(int $pollId, string $question, array $options, int $resultMode, int $maxVotes): DataResponse {
if ($this->room->isFederatedConversation()) {
// TODO: add editing a draft to federation
/** @var \OCA\Talk\Federation\Proxy\TalkV1\Controller\PollController $proxy */
$proxy = \OCP\Server::get(\OCA\Talk\Federation\Proxy\TalkV1\Controller\PollController::class);
return $proxy->updateDraftPoll($pollId, $this->room, $this->participant, $question, $options, $resultMode, $maxVotes, $draft);

Check failure on line 164 in lib/Controller/PollController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

InvalidReturnStatement 

lib/Controller/PollController.php:164:11: InvalidReturnStatement: The inferred type 'OCP\AppFramework\Http\DataResponse<200|201|400, array{actorDisplayName?: string, actorId?: non-empty-string, actorType?: 'bots'|'bridged'|'circles'|'emails'|'federated_users'|'groups'|'guests'|'phones'|'users', details?: list<array{actorDisplayName: string, actorId: string, actorType: string, optionId: int}>, error?: 'draft'|'options'|'question'|'room', id?: int<1, max>, maxVotes?: int<0, max>, numVoters?: int<0, max>, options?: list<string>, question?: non-empty-string, resultMode?: 0|1, status?: 0|1|2, votedSelf?: list<int>, votes?: array<string, int>}, array<never, never>>' does not match the declared return type 'OCP\AppFramework\Http\DataResponse<200|400|404, array{actorDisplayName?: string, actorId?: non-empty-string, actorType?: 'bots'|'bridged'|'circles'|'emails'|'federated_users'|'groups'|'guests'|'phones'|'users', error?: string, id?: int<1, max>, maxVotes?: int<0, max>, options?: list<string>, question?: non-empty-string, resultMode?: 0|1, status?: 0|1|2}, array<never, never>>' for OCA\Talk\Controller\PollController::updateDraftPoll (see https://psalm.dev/128)

Check failure on line 164 in lib/Controller/PollController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

TooManyArguments 

lib/Controller/PollController.php:164:19: TooManyArguments: Too many arguments for method OCA\Talk\Federation\Proxy\TalkV1\Controller\PollController::updatedraftpoll - saw 8 (see https://psalm.dev/026)

Check failure on line 164 in lib/Controller/PollController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

UndefinedVariable 

lib/Controller/PollController.php:164:122: UndefinedVariable: Cannot find referenced variable $draft (see https://psalm.dev/024)
Expand All @@ -168,10 +169,6 @@ public function updateDraftPoll(int $pollId, string $question, array $options, i
return new DataResponse(['error' => PollPropertyException::REASON_ROOM], Http::STATUS_BAD_REQUEST);
}

if (!$this->participant->hasModeratorPermissions()) {
return new DataResponse(['error' => PollPropertyException::REASON_DRAFT], Http::STATUS_BAD_REQUEST);
}

try {
$poll = $this->pollService->getPoll($this->room->getId(), $pollId);
} catch (DoesNotExistException $e) {
Expand All @@ -182,8 +179,21 @@ public function updateDraftPoll(int $pollId, string $question, array $options, i
return new DataResponse(['error' => PollPropertyException::REASON_POLL], Http::STATUS_BAD_REQUEST);
}

if (!$this->participant->hasModeratorPermissions()
&& ($poll->getActorType() !== $this->participant->getAttendee()->getActorType()
|| $poll->getActorId() !== $this->participant->getAttendee()->getActorId())) {
return new DataResponse(['error' => PollPropertyException::REASON_DRAFT], Http::STATUS_BAD_REQUEST);
}

try {
$question = $this->pollService->validatePollQuestion($question);
$encodedOptions = $this->pollService->validatePollOptions($options);
} catch (PollPropertyException $e) {
$this->logger->error('Error modifying poll', ['exception' => $e]);
return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
}

$poll->setQuestion($question);
$encodedOptions = json_encode($options, JSON_THROW_ON_ERROR, 1);
$poll->setOptions($encodedOptions);
$poll->setResultMode($resultMode);
$poll->setMaxVotes($maxVotes);
Expand Down Expand Up @@ -372,12 +382,13 @@ public function closePoll(int $pollId): DataResponse {
return new DataResponse(['error' => 'poll'], Http::STATUS_BAD_REQUEST);
}

$poll->setStatus(Poll::STATUS_CLOSED);

try {
$this->pollService->updatePoll($this->participant, $poll);
} catch (WrongPermissionsException) {
$this->pollService->closePoll($this->participant, $poll);
} catch (WrongPermissionsException $e) {
return new DataResponse(['error' => 'poll'], Http::STATUS_FORBIDDEN);
} catch (Exception $e) {

Check failure on line 389 in lib/Controller/PollController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

UndefinedClass 

lib/Controller/PollController.php:389:12: UndefinedClass: Class, interface or enum named OCA\Talk\Controller\Exception does not exist (see https://psalm.dev/019)
$this->logger->error($e->getMessage(), ['exception' => $e]);

Check failure on line 390 in lib/Controller/PollController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

UndefinedClass 

lib/Controller/PollController.php:390:25: UndefinedClass: Class, interface or enum named OCA\Talk\Controller\Exception does not exist (see https://psalm.dev/019)
return new DataResponse(['error' => 'poll'], Http::STATUS_INTERNAL_SERVER_ERROR);
}

$attendee = $this->participant->getAttendee();
Expand Down
114 changes: 75 additions & 39 deletions lib/Service/PollService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,43 +33,8 @@ public function __construct(
* @throws PollPropertyException
*/
public function createPoll(int $roomId, string $actorType, string $actorId, string $displayName, string $question, array $options, int $resultMode, int $maxVotes, bool $draft): Poll {
$question = trim($question);

if ($question === '' || strlen($question) > 32_000) {
throw new PollPropertyException(PollPropertyException::REASON_QUESTION);
}

try {
json_encode($options, JSON_THROW_ON_ERROR, 1);
} catch (\Exception) {
throw new PollPropertyException(PollPropertyException::REASON_OPTIONS);
}

$validOptions = [];
foreach ($options as $option) {
if (!is_string($option)) {
throw new PollPropertyException(PollPropertyException::REASON_OPTIONS);
}

$option = trim($option);
if ($option !== '') {
$validOptions[] = $option;
}
}

if (count($validOptions) < 2) {
throw new PollPropertyException(PollPropertyException::REASON_OPTIONS);
}

try {
$jsonOptions = json_encode($validOptions, JSON_THROW_ON_ERROR, 1);
} catch (\Exception) {
throw new PollPropertyException(PollPropertyException::REASON_OPTIONS);
}

if (strlen($jsonOptions) > 60_000) {
throw new PollPropertyException(PollPropertyException::REASON_OPTIONS);
}
$question = $this->validatePollQuestion($question);
$jsonOptions = $this->validatePollOptions($options);

$poll = new Poll();
$poll->setRoomId($roomId);
Expand Down Expand Up @@ -115,12 +80,71 @@ public function getPoll(int $roomId, int $pollId): Poll {
*/
public function updatePoll(Participant $participant, Poll $poll): void {
if (!$participant->hasModeratorPermissions()
&& ($poll->getActorType() !== $participant->getAttendee()->getActorType()
|| $poll->getActorId() !== $participant->getAttendee()->getActorId())) {
&& ($poll->getActorType() !== $participant->getAttendee()->getActorType()
|| $poll->getActorId() !== $participant->getAttendee()->getActorId())) {
// Only moderators and the author of the poll can update it
throw new WrongPermissionsException();
}
$this->pollMapper->update($poll);
}

/**
* @param array $options
* @return string
*
* @since 21.0.0
*/
public function validatePollOptions(array $options): string {
try {
json_encode($options, JSON_THROW_ON_ERROR, 1);
} catch (\Exception) {
throw new PollPropertyException(PollPropertyException::REASON_OPTIONS);
}

$validOptions = [];
foreach ($options as $option) {
if (!is_string($option)) {
throw new PollPropertyException(PollPropertyException::REASON_OPTIONS);
}

$option = trim($option);
if ($option !== '') {
$validOptions[] = $option;
}
}

if (count($validOptions) < 2) {
throw new PollPropertyException(PollPropertyException::REASON_OPTIONS);
}

try {
$jsonOptions = json_encode($validOptions, JSON_THROW_ON_ERROR, 1);
} catch (\Exception) {
throw new PollPropertyException(PollPropertyException::REASON_OPTIONS);
}

if (strlen($jsonOptions) > 60_000) {
throw new PollPropertyException(PollPropertyException::REASON_OPTIONS);
}

return $jsonOptions;
}

/**
* @param Participant $participant
* @param Poll $poll
* @return void
* @throws WrongPermissionsException
*/
public function closePoll(Participant $participant, Poll $poll): void {
if (!$participant->hasModeratorPermissions()
&& ($poll->getActorType() !== $participant->getAttendee()->getActorType()
|| $poll->getActorId() !== $participant->getAttendee()->getActorId())) {
// Only moderators and the author of the poll can update it
throw new WrongPermissionsException();
}

$poll->setStatus(Poll::STATUS_CLOSED);
$this->pollMapper->update($poll);
}

Expand Down Expand Up @@ -345,4 +369,16 @@ public function neutralizeDeletedUser(string $actorType, string $actorId): void
->andWhere($update->expr()->eq('actor_id', $update->createNamedParameter($actorId)));
$update->executeStatement();
}

/**
* @param string $question
* @return string
*/
public function validatePollQuestion(string $question): string {
$question = trim($question);
if ($question === '' || strlen($question) > 32_000) {
throw new PollPropertyException(PollPropertyException::REASON_QUESTION);
}
return $question;
}
}
15 changes: 13 additions & 2 deletions openapi-full.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9381,6 +9381,7 @@
"required": [
"question",
"options",
"resultMode",
"maxVotes"
],
"properties": {
Expand All @@ -9395,6 +9396,15 @@
"type": "string"
}
},
"resultMode": {
"type": "integer",
"format": "int64",
"enum": [
0,
1
],
"description": "Mode how the results will be shown"
},
"maxVotes": {
"type": "integer",
"format": "int64",
Expand Down Expand Up @@ -9430,10 +9440,11 @@
{
"name": "pollId",
"in": "path",
"description": "The poll id",
"required": true,
"schema": {
"type": "string",
"pattern": "^\\d+$"
"type": "integer",
"format": "int64"
}
},
{
Expand Down
15 changes: 13 additions & 2 deletions openapi.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9268,6 +9268,7 @@
"required": [
"question",
"options",
"resultMode",
"maxVotes"
],
"properties": {
Expand All @@ -9282,6 +9283,15 @@
"type": "string"
}
},
"resultMode": {
"type": "integer",
"format": "int64",
"enum": [
0,
1
],
"description": "Mode how the results will be shown"
},
"maxVotes": {
"type": "integer",
"format": "int64",
Expand Down Expand Up @@ -9317,10 +9327,11 @@
{
"name": "pollId",
"in": "path",
"description": "The poll id",
"required": true,
"schema": {
"type": "string",
"pattern": "^\\d+$"
"type": "integer",
"format": "int64"
}
},
{
Expand Down
Loading

0 comments on commit 9dcf018

Please sign in to comment.