Skip to content

Commit 9a45b4a

Browse files
dartcafedartcafe
authored
Merge pull request #3308 from nextcloud/fix/aftermath
Fixing some aftermaths
2 parents e5dbd93 + f2cdbde commit 9a45b4a

19 files changed

+180
-130
lines changed

‎lib/Controller/BasePublicController.php

+1-3
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ protected function responseCreate(Closure $callback, string $token): JSONRespons
9292
}
9393

9494
private function updateSessionToken(string $token): void {
95-
if ($token !== $this->session->get(AppConstants::SESSION_KEY_SHARE_TOKEN)) {
96-
$this->session->set(AppConstants::SESSION_KEY_SHARE_TOKEN, $token);
97-
}
95+
$this->session->set(AppConstants::SESSION_KEY_SHARE_TOKEN, $token);
9896
}
9997
}

‎lib/Controller/ShareController.php

+8-8
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,6 @@ public function add(int $pollId, string $type, string $userId = '', string $disp
5959
return $this->responseCreate(fn () => ['share' => $this->shareService->add($pollId, $type, $userId, $displayName, $emailAddress)]);
6060
}
6161

62-
/**
63-
* Convert user to poll admin
64-
*/
65-
#[NoAdminRequired]
66-
public function userToAdmin(string $token): JSONResponse {
67-
return $this->responseCreate(fn () => ['share' => $this->shareService->setType($token, Share::TYPE_ADMIN)]);
68-
}
69-
7062
/**
7163
* Change the contraints for email addresses in public polls
7264
*/
@@ -93,6 +85,14 @@ public function adminToUser(string $token): JSONResponse {
9385
return $this->responseCreate(fn () => ['share' => $this->shareService->setType($token, Share::TYPE_USER)]);
9486
}
9587

88+
/**
89+
* Convert user to poll admin
90+
*/
91+
#[NoAdminRequired]
92+
public function userToAdmin(string $token): JSONResponse {
93+
return $this->responseCreate(fn () => ['share' => $this->shareService->setType($token, Share::TYPE_ADMIN)]);
94+
}
95+
9696
/**
9797
* Set email address
9898
*/

‎lib/Db/PollMapper.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,9 @@ protected function buildQuery(): IQueryBuilder {
170170
$qb = $this->db->getQueryBuilder();
171171

172172
$qb->select(self::TABLE . '.*')
173+
// TODO: check if this is necessary, in case of empty table to avoid possibly nulled columns
174+
// ->groupBy(self::TABLE . '.id')
173175
->from($this->getTableName(), self::TABLE);
174-
175176
return $qb;
176177
}
177178

‎lib/Db/ShareMapper.php

+45-28
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
namespace OCA\Polls\Db;
2828

29+
use Exception;
2930
use OCA\Polls\Exceptions\ShareNotFoundException;
3031
use OCA\Polls\Model\UserBase;
3132
use OCP\AppFramework\Db\DoesNotExistException;
@@ -56,28 +57,19 @@ public function __construct(
5657
* @psalm-return array<array-key, Share>
5758
*/
5859
public function findByPoll(int $pollId, bool $getDeleted = false): array {
59-
6060
$qb = $this->db->getQueryBuilder();
6161

62-
$qb->select('shares.*')
63-
->from($this->getTableName(), 'shares')
64-
->groupBy('shares.id')
65-
->where($qb->expr()->eq('shares.poll_id', $qb->createNamedParameter($pollId, IQueryBuilder::PARAM_INT)))
66-
->leftJoin(
67-
'shares',
68-
Vote::TABLE,
69-
'votes',
70-
$qb->expr()->andX(
71-
$qb->expr()->eq('shares.poll_id', 'votes.poll_id'),
72-
$qb->expr()->eq('shares.user_id', 'votes.user_id'),
73-
)
74-
)
75-
->addSelect($qb->func()->count('votes.id', 'voted'));
62+
$qb->select(self::TABLE . '.*')
63+
->from($this->getTableName(), self::TABLE)
64+
->groupBy(self::TABLE . '.id')
65+
->where($qb->expr()->eq(self::TABLE . '.poll_id', $qb->createNamedParameter($pollId, IQueryBuilder::PARAM_INT)));
7666

7767
if (!$getDeleted) {
78-
$qb->andWhere($qb->expr()->eq('shares' . '.deleted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)));
68+
$qb->andWhere($qb->expr()->eq(self::TABLE . '.deleted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)));
7969
}
8070

71+
$this->joinUserVoteCount($qb, self::TABLE);
72+
8173
return $this->findEntities($qb);
8274
}
8375

@@ -127,18 +119,20 @@ public function findByPollUnreminded(int $pollId, bool $getDeleted = false): arr
127119
public function findByPollAndUser(int $pollId, string $userId, bool $findDeleted = false): Share {
128120
$qb = $this->db->getQueryBuilder();
129121

130-
$qb->select('*')
131-
->from($this->getTableName())
132-
->where($qb->expr()->eq('poll_id', $qb->createNamedParameter($pollId, IQueryBuilder::PARAM_INT)))
133-
->andWhere($qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)));
122+
$qb->select(self::TABLE . '.*')
123+
->from($this->getTableName(), self::TABLE)
124+
->where($qb->expr()->eq(self::TABLE . '.poll_id', $qb->createNamedParameter($pollId, IQueryBuilder::PARAM_INT)))
125+
->andWhere($qb->expr()->eq(self::TABLE . '.user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)))
126+
->andWhere($qb->expr()->isNotNull(self::TABLE . '.id'));
134127

135128
if (!$findDeleted) {
136-
$qb->andWhere($qb->expr()->eq('deleted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)));
129+
$qb->andWhere($qb->expr()->eq(self::TABLE . '.deleted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)));
137130
}
131+
$this->joinUserVoteCount($qb, self::TABLE);
138132

139133
try {
140134
return $this->findEntity($qb);
141-
} catch (DoesNotExistException $e) {
135+
} catch (Exception $e) {
142136
throw new ShareNotFoundException("Share not found by userId and pollId");
143137
}
144138
}
@@ -165,13 +159,15 @@ public function getReplacement(int $pollId, string $userId): Share {
165159
public function findByToken(string $token, bool $getDeleted = false): Share {
166160
$qb = $this->db->getQueryBuilder();
167161

168-
$qb->select('*')
169-
->from($this->getTableName())
170-
->where($qb->expr()->eq('token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR)));
162+
$qb->select(self::TABLE . '.*')
163+
->from($this->getTableName(), self::TABLE)
164+
->where($qb->expr()->eq(self::TABLE . '.token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR)));
171165

172-
// if (!$getDeleted) {
173-
// $qb->andWhere($qb->expr()->eq('deleted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)));
174-
// }
166+
if (!$getDeleted) {
167+
$qb->andWhere($qb->expr()->eq(self::TABLE . '.deleted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)));
168+
}
169+
170+
$this->joinUserVoteCount($qb, self::TABLE);
175171

176172
try {
177173
return $this->findEntity($qb);
@@ -205,4 +201,25 @@ public function purgeDeletedShares(int $offset): void {
205201
$query->executeStatement();
206202
}
207203

204+
/**
205+
* Joins votes count of the share user in the given poll
206+
*/
207+
protected function joinUserVoteCount(IQueryBuilder &$qb, string $fromAlias): void {
208+
$joinAlias = 'votes';
209+
210+
$qb->addSelect($qb->func()->count($joinAlias . '.id', 'voted'));
211+
212+
$qb->leftJoin(
213+
$fromAlias,
214+
Vote::TABLE,
215+
$joinAlias,
216+
$qb->expr()->andX(
217+
$qb->expr()->eq($fromAlias . '.poll_id', $joinAlias . '.poll_id'),
218+
$qb->expr()->eq($fromAlias . '.user_id', $joinAlias . '.user_id'),
219+
)
220+
);
221+
// avoid result with nulled columns
222+
$qb->groupBy($fromAlias . '.id');
223+
}
224+
208225
}

‎lib/Db/UserMapper.php

+11-2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
namespace OCA\Polls\Db;
2727

28+
use Exception;
2829
use OCA\Polls\AppConstants;
2930
use OCA\Polls\Exceptions\InvalidShareTypeException;
3031
use OCA\Polls\Exceptions\ShareNotFoundException;
@@ -121,7 +122,7 @@ public function getParticipant(string $userId, ?int $pollId = null): UserBase {
121122
}
122123

123124
try {
124-
return $this->getUserFromUserBase($userId);
125+
return $this->getUserFromUserBase($userId, $pollId);
125126
} catch (UserNotFoundException $e) {
126127
// just catch and continue if not found and try to find user by share;
127128
}
@@ -169,9 +170,17 @@ public function getUserFromShare(Share $share): GenericUser|Email|User|ContactGr
169170
);
170171
}
171172

172-
public function getUserFromUserBase(string $userId): User {
173+
public function getUserFromUserBase(string $userId, ?int $pollId = null): User {
173174
$user = $this->userManager->get($userId);
174175
if ($user instanceof IUser) {
176+
try {
177+
// check if we find a share, where the user got admin rights for the particular poll
178+
if ($this->getShareByPollAndUser($userId, $pollId)->getType() === Share::TYPE_ADMIN) {
179+
return new Admin($userId);
180+
}
181+
} catch (Exception $e) {
182+
// silent catch
183+
}
175184
return new User($userId);
176185
}
177186
throw new UserNotFoundException();

‎lib/Model/Acl.php

+9-14
Original file line numberDiff line numberDiff line change
@@ -173,14 +173,6 @@ public function getShare(): ?Share {
173173

174174
return $this->share;
175175
}
176-
177-
private function verifyConstraints(): void {
178-
if ($this->getToken()) {
179-
$this->loadShare();
180-
}
181-
$this->getCurrentUser();
182-
$this->loadPoll();
183-
}
184176

185177
/**
186178
* load poll
@@ -201,34 +193,37 @@ private function loadPoll(): void {
201193
}
202194

203195
/**
204-
* load share from db by session token or rely on cahced share
196+
* load share from db by session token or rely on cached share
205197
* If the share token has changed, the share gets loaded from the db,
206198
* the poll will get invalidated (set to null)
207199
* and the pollId will get set to the share's pollId
208200
*/
209201
private function loadShare(): void {
210-
// if share is already cached, verify against session token
202+
// no token in session, try to find a user, who matches
211203
if (!$this->getToken()) {
212204
if ($this->getCurrentUser()->getIsLoggedIn()) {
213205
// search for logged in user's share, load it and return
214206
$this->share = $this->shareMapper->findByPollAndUser($this->getPollId(), $this->getUserId());
215207
// store share in session for further validations
216-
$this->session->set(AppConstants::SESSION_KEY_SHARE_TOKEN, $this->share->getToken());
208+
// $this->session->set(AppConstants::SESSION_KEY_SHARE_TOKEN, $this->share->getToken());
217209
return;
218210
} else {
211+
$this->share = new Share();
219212
// must fail, if no token is present and not logged in
220213
throw new ShareNotFoundException('No token was set for ACL');
221214
}
222215
}
223-
216+
217+
// if share is already cached, verify against session token
224218
if ($this->share?->getToken() === $this->getToken()) {
225219
return;
226220
}
227221

228222
$this->share = $this->shareMapper->findByToken((string) $this->getToken());
229223

230-
// ensure, the poll is reset
224+
// ensure, poll and currentUser get reset
231225
$this->poll = null;
226+
$this->currentUser = null;
232227

233228
// set the poll id based on the share
234229
$this->pollId = $this->share->getPollId();
@@ -278,7 +273,7 @@ public function getIsOwner(): bool {
278273
}
279274

280275
public function getIsAllowed(string $permission, ?string $userId = null, ?int $pollId = null): bool|null {
281-
$this->verifyConstraints();
276+
// $this->verifyConstraints();
282277
return match ($permission) {
283278
self::PERMISSION_OVERRIDE => true,
284279
self::PERMISSION_POLL_CREATE => $this->appSettings->getPollCreationAllowed(),

‎lib/Service/SubscriptionService.php

+15-7
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
use OCA\Polls\Db\Subscription;
2929
use OCA\Polls\Db\SubscriptionMapper;
30+
use OCA\Polls\Exceptions\ForbiddenException;
3031
use OCA\Polls\Model\Acl;
3132
use OCP\AppFramework\Db\DoesNotExistException;
3233
use OCP\DB\Exception;
@@ -42,37 +43,44 @@ public function __construct(
4243
}
4344

4445
public function get(?int $pollId = null): bool {
45-
$this->acl->setPollId($pollId, Acl::PERMISSION_POLL_SUBSCRIBE);
4646
try {
47+
$this->acl->setPollId($pollId, Acl::PERMISSION_POLL_SUBSCRIBE);
4748
$this->subscriptionMapper->findByPollAndUser($this->acl->getPollId(), $this->acl->getUserId());
4849
// Subscription exists
4950
return true;
5051
} catch (DoesNotExistException $e) {
5152
return false;
53+
} catch (ForbiddenException $e) {
54+
return false;
5255
}
5356
}
5457

55-
public function set(bool $subscribed, ?int $pollId = null): bool {
56-
$this->acl->setPollId($pollId, Acl::PERMISSION_POLL_SUBSCRIBE);
57-
if (!$subscribed) {
58+
public function set(bool $setToSubscribed, ?int $pollId = null): bool {
59+
if (!$setToSubscribed) {
60+
// user wants to unsubscribe
5861
try {
5962
$subscription = $this->subscriptionMapper->findByPollAndUser($this->acl->getPollId(), $this->acl->getUserId());
6063
$this->subscriptionMapper->delete($subscription);
6164
} catch (DoesNotExistException $e) {
62-
// catch silently (assume already unsubscribed)
65+
// Not found, assume already unsubscribed
66+
return false;
6367
}
6468
} else {
6569
try {
70+
$this->acl->setPollId($pollId, Acl::PERMISSION_POLL_SUBSCRIBE);
6671
$this->add($this->acl->getPollId(), $this->acl->getUserId());
72+
} catch (ForbiddenException $e) {
73+
return false;
6774
} catch (Exception $e) {
6875
if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
69-
// catch silently (assume already subscribed)
76+
// Already subscribed
77+
return true;
7078
} else {
7179
throw $e;
7280
}
7381
}
7482
}
75-
return $subscribed;
83+
return $setToSubscribed;
7684
}
7785

7886
private function add(int $pollId, string $userId): void {

‎psalm-baseline.xml

-3
Original file line numberDiff line numberDiff line change
@@ -189,9 +189,6 @@
189189
<PossiblyUnusedMethod>
190190
<code>getReplacement</code>
191191
</PossiblyUnusedMethod>
192-
<PossiblyUnusedParam>
193-
<code>$getDeleted</code>
194-
</PossiblyUnusedParam>
195192
<UnusedProperty>
196193
<code>$config</code>
197194
</UnusedProperty>

‎src/js/components/Configuration/ConfigAutoReminder.vue

+1-2
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,10 @@
2828
<NcPopover :focus-trap="false">
2929
<template #trigger>
3030
<NcActions>
31-
<NcActionButton>
31+
<NcActionButton :name="t('polls', 'Autoreminder informations')">
3232
<template #icon>
3333
<InformationIcon />
3434
</template>
35-
{{ t('polls', 'Autoreminder informations') }}
3635
</NcActionButton>
3736
</NcActions>
3837
</template>

0 commit comments

Comments
 (0)