Merge pull request #3194 from nextcloud/fix/user-presentation

fixing some weird user apperance

appinfo/info.xml

@@ -4,7 +4,7 @@
   <name>Polls</name>
   <summary>A polls app, similar to Doodle/Dudle with the possibility to restrict access.</summary>
   <description>A polls app, similar to Doodle/Dudle with the possibility to restrict access (members, certain groups/users, hidden and public).</description>
-  <version>6.0.0</version>
+  <version>6.0.1</version>
   <licence>agpl</licence>
   <author>Vinzenz Rosenkranz</author>
   <author>René Gieling</author>

lib/Db/EntityWithUser.php

@@ -33,14 +33,14 @@
 
 /**
  * @method int getPollId()
- * @method string getUserId()
- * @method string getDisplayName()
- * @method string getEmailAdress()
- * @method string getUserType()
+ * @method ?string getUserId()
+ * @method ?string getDisplayName()
+ * @method ?string getEmailAdress()
+ * @method ?string getUserType()
  */
 
 abstract class EntityWithUser extends Entity {
-	protected string $publicUserId = '';
+	protected ?string $publicUserId = '';
 	protected ?string $displayName = '';
 	protected ?string $emailAddress = '';
 	protected ?string $userType = '';
@@ -57,6 +57,10 @@ public function getIsNoUser(): bool {
 	 * - otherwise assume a deleted user
 	 **/
 	public function getDisplayName(): ?string {
+		if (!$this->getUserId()) {
+			return null;
+		}
+
 		return Container::queryClass(IUserManager::class)->get($this->getUserId())?->getDisplayName()
 			?? $this->displayName
 			?? 'Deleted User';
@@ -68,12 +72,21 @@ public function getDisplayName(): ?string {
 	 * - first tries to get type from joined share
 	 * - then try to verify an internal user and set type user
 	 * - otherwise assume a deleted user
-	 **/
-	public function getUserType(): ?string {
-		return $this->userType
-			?? Container::queryClass(IUserManager::class)->get($this->getUserId())
-				? User::TYPE_USER
-				: User::TYPE_GHOST;
+	 *
+	 * @return null|string
+	 */
+	public function getUserType(): string|null {
+		if (!$this->getUserId()) {
+			return null;
+		}
+
+		if ($this->userType) {
+			return $this->userType;
+		}
+
+		return Container::queryClass(IUserManager::class)->get($this->getUserId())
+			? User::TYPE_USER
+			: User::TYPE_GHOST;
 	}
 
 	/**
@@ -83,6 +96,9 @@ public function getUserType(): ?string {
 	 * - then get it from joined share
 	 **/
 	public function getEmailAddress(): ?string {
+		if (!$this->getUserId()) {
+			return null;
+		}
 		return Container::queryClass(IUserManager::class)->get($this->getUserId())?->getEmailAddress()
 			?? $this->emailAddress;
 	}
@@ -92,9 +108,9 @@ public function getEmailAddress(): ?string {
 	 *
 	 * Avoids leaking internal userIds by replacing the actual userId by another string in public access
 	 **/
-	private function getPublicUserId(): string {
+	private function getPublicUserId(): ?string {
 		if (!$this->getUserId()) {
-			return '';
+			return null;
 		}
 
 		if ($this->publicUserId) {
@@ -105,6 +121,10 @@ private function getPublicUserId(): string {
 	}
 
 	public function generateHashedUserId(): void {
+		if (!$this->getUserId()) {
+			$this->publicUserId = null;
+		}
+
 		$this->publicUserId = hash('md5', $this->getUserId());
 	}
 

lib/Db/VoteMapper.php

@@ -40,14 +40,16 @@ public function __construct(IDBConnection $db) {
 		parent::__construct($db, self::TABLE, Vote::class);
 	}
 
-	public function update(Entity $entity): Entity {
+	public function update(Entity $entity): Vote {
 		$entity->setVoteOptionHash(hash('md5', $entity->getPollId() . $entity->getUserId() . $entity->getVoteOptionText()));
-		return parent::update($entity);
+		$entity = parent::update($entity);
+		return $this->find($entity->getId());
 	}
-
-	public function insert(Entity $entity): Entity {
+	
+	public function insert(Entity $entity): Vote {
 		$entity->setVoteOptionHash(hash('md5', $entity->getPollId() . $entity->getUserId() . $entity->getVoteOptionText()));
-		return parent::insert($entity);
+		$entity = parent::insert($entity);
+		return $this->find($entity->getId());
 	}
 
 	/**
@@ -187,6 +189,12 @@ public function fixVoteOptionText(int $pollId, int $optionId, string $searchOpti
 	/**
 	 * Build the enhanced query with joined tables
 	 */
+	protected function find(int $id): Vote {
+		$qb = $this->buildQuery();
+		$qb->where($qb->expr()->eq(self::TABLE . '.id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)));
+		return $this->findEntity($qb);
+	}
+
 	protected function buildQuery(): IQueryBuilder {
 		$qb = $this->db->getQueryBuilder();
 

lib/Event/BaseEvent.php

@@ -70,7 +70,7 @@ public function getPollOwner(): string {
 		return $this->poll->getOwner();
 	}
 
-	public function getActor(): string {
+	public function getActor(): ?string {
 		return $this->userSession->getUser()?->getUID() ?? $this->eventObject->getUserId();
 	}
 

lib/Model/Acl.php

@@ -181,10 +181,17 @@ public function setPoll(Poll $poll, string $permission = self::PERMISSION_POLL_V
 	/**
 	 * Property getters
 	 */
-	public function getPoll(): Poll|null {
+	public function getPoll(): ?Poll {
 		return $this->poll;
 	}
 
+	/**
+	 * Property getters
+	 */
+	public function getShare(): ?Share {
+		return $this->share;
+	}
+
 	public function getPollId(): int {
 		return $this->poll->getId();
 	}

lib/Service/PollService.php

@@ -403,9 +403,6 @@ public function clone(int $pollId): Poll {
 	/**
 	 * Collect email addresses from particitipants
 	 *
-	 * @return string[][]
-	 *
-	 * @psalm-return list<array{displayName: string, emailAddress: string, combined: string}>
 	 */
 	public function getParticipantsEmailAddresses(int $pollId): array {
 		$this->acl->setPollId($pollId, Acl::PERMISSION_POLL_EDIT);

lib/Service/VoteService.php

@@ -104,12 +104,12 @@ public function set(int $optionId, string $setTo, ?Acl $acl = null): ?Vote {
 		$option = $this->optionMapper->find($optionId);
 
 		if ($acl) {
-			$this->acl = $acl; // ->setToken($token, Acl::PERMISSION_VOTE_EDIT, $option->getPollId());
-		// $this->acl->setToken($token, Acl::PERMISSION_VOTE_EDIT, $option->getPollId());
+			$this->acl = $acl;
+			$this->acl->request(Acl::PERMISSION_VOTE_EDIT);
 		} else {
 			$this->acl->setPollId($option->getPollId(), Acl::PERMISSION_VOTE_EDIT);
 		}
-
+		
 		if ($setTo === Vote::VOTE_YES) {
 			$this->checkLimits($option, $this->acl->getUserId());
 		}
@@ -122,7 +122,7 @@ public function set(int $optionId, string $setTo, ?Acl $acl = null): ?Vote {
 				$this->voteMapper->delete($this->vote);
 			} else {
 				$this->vote->setVoteAnswer($setTo);
-				$this->voteMapper->update($this->vote);
+				$this->vote = $this->voteMapper->update($this->vote);
 			}
 		} catch (DoesNotExistException $e) {
 			// Vote does not exist, insert as new Vote
@@ -133,7 +133,7 @@ public function set(int $optionId, string $setTo, ?Acl $acl = null): ?Vote {
 			$this->vote->setVoteOptionText($option->getPollOptionText());
 			$this->vote->setVoteOptionId($option->getId());
 			$this->vote->setVoteAnswer($setTo);
-			$this->voteMapper->insert($this->vote);
+			$this->vote = $this->voteMapper->insert($this->vote);
 		}
 
 		$this->eventDispatcher->dispatchTyped(new VoteSetEvent($this->vote));

package.json

@@ -1,7 +1,7 @@
 {
 	"name": "polls",
 	"description": "Polls app for nextcloud",
-	"version": "6.0.0",
+	"version": "6.0.1",
 	"authors": [
 		{
 			"name": "Vinzenz Rosenkranz",

src/js/components/Shares/ShareItem.vue

@@ -24,6 +24,7 @@
 	<UserItem v-bind="share"
 		show-email
 		resolve-info
+		show-type-icon
 		:icon="true">
 		<template #status>
 			<div v-if="share.voted">

src/js/components/User/UserItem.vue

@@ -22,30 +22,33 @@
 
 <template>
 	<div :class="['user-item', type, { disabled, condensed: condensed }]">
-		<NcAvatar :disable-menu="disableMenu"
-			:disable-tooltip="disableTooltip"
-			class="user-item__avatar"
-			:is-guest="isGuestComputed"
-			:menu-position="menuPosition"
-			:size="iconSize"
-			:show-user-status="showUserStatus"
-			:user="avatarUserId"
-			:display-name="name"
-			:is-no-user="isNoUser"
-			@click="showMenu()">
-			<template v-if="useIconSlot" #icon>
-				<LinkIcon v-if="type==='public'" :size="mdIconSize" />
-				<InternalLinkIcon v-if="type==='internalAccess'" :size="mdIconSize" />
-				<ContactIcon v-if="type==='contact'" :size="mdIconSize" />
-				<EmailIcon v-if="type==='email'" :size="mdIconSize" />
-				<ShareIcon v-if="type==='external'" :size="mdIconSize" />
-				<ContactGroupIcon v-if="type==='contactGroup'" :size="mdIconSize" />
-				<GroupIcon v-if="type==='group'" :size="mdIconSize" />
-				<CircleIcon v-if="type==='circle'" :size="mdIconSize" />
-			</template>
-		</NcAvatar>
+		<div class="avatar-wrapper">
+			<NcAvatar :disable-menu="disableMenu"
+				:disable-tooltip="disableTooltip"
+				class="user-item__avatar"
+				:is-guest="isGuestComputed"
+				:menu-position="menuPosition"
+				:size="iconSize"
+				:show-user-status="showUserStatus"
+				:user="avatarUserId"
+				:display-name="name"
+				:is-no-user="isNoUser"
+				@click="showMenu()">
+				<template v-if="useIconSlot" #icon>
+					<LinkIcon v-if="type==='public'" :size="mdIconSize" />
+					<InternalLinkIcon v-if="type==='internalAccess'" :size="mdIconSize" />
+					<ContactGroupIcon v-if="type==='contactGroup'" :size="mdIconSize" />
+					<GroupIcon v-if="type==='group'" :size="mdIconSize" />
+					<CircleIcon v-if="type==='circle'" :size="mdIconSize" />
+					<DeletedUserIcon v-if="type==='deleted'" :size="mdIconSize" />
+				</template>
+			</NcAvatar>
 
-		<AdminIcon v-if="icon && type === 'admin'" :size="16" class="type-icon" />
+			<AdminIcon v-if="type === 'admin' && showTypeIcon" :size="16" class="type-icon" />
+			<ContactIcon v-if="type==='contact' && showTypeIcon" :size="16" class="type-icon" />
+			<EmailIcon v-if="type==='email' && showTypeIcon" :size="16" class="type-icon" />
+			<ShareIcon v-if="type==='external' && showTypeIcon" :size="16" class="type-icon" />
+		</div>
 
 		<slot name="status" />
 
@@ -80,6 +83,7 @@ export default {
 		ContactGroupIcon: () => import('vue-material-design-icons/AccountGroupOutline.vue'),
 		GroupIcon: () => import('vue-material-design-icons/AccountMultiple.vue'),
 		CircleIcon: () => import('vue-material-design-icons/GoogleCirclesExtended.vue'),
+		DeletedUserIcon: () => import('vue-material-design-icons/AccountOff.vue'),
 	},
 
 	inheritAttrs: false,
@@ -144,6 +148,7 @@ export default {
 					'circle',
 					'external',
 					'email',
+					'deleted',
 				].includes(value)
 			},
 
@@ -152,11 +157,11 @@ export default {
 			type: Boolean,
 			default: false,
 		},
-		isGuest: {
+		showTypeIcon: {
 			type: Boolean,
 			default: false,
 		},
-		icon: {
+		isGuest: {
 			type: Boolean,
 			default: false,
 		},
@@ -180,16 +185,19 @@ export default {
 		},
 
 		useIconSlot() {
-			return !['user', 'admin'].includes(this.type)
+			return ['internalAccess', 'public', 'contactGroup', 'group', 'circle', 'deleted'].includes(this.type)
 		},
 		description() {
+			if (this.condensed) return ''
 			if (this.forcedDescription) return this.forcedDescription
+			if (this.type === 'deleted') return t('polls', 'The participant got removed from this poll')
 			if (this.type === 'admin') return t('polls', 'Is granted admin rights for this poll')
 			if (this.displayEmailAddress) return this.displayEmailAddress
 			return ''
 		},
 
 		name() {
+			if (this.type === 'deleted') return t('polls', 'Deleted User')
 			if (this.type === 'internalAccess') return t('polls', 'Internal access')
 			if (this.displayName) return this.displayName
 			if (this.type === 'public' && this.userId !== 'addPublic') return t('polls', 'Public link')
@@ -236,11 +244,14 @@ export default {
 </script>
 
 <style lang="scss">
-.type-icon {
-	position: absolute;
-	background-size: 16px;
-	top: 3px;
-	left: 0;
+.avatar-wrapper {
+	position: relative;
+	.type-icon {
+		position: absolute;
+		background-size: 16px;
+		top: -6px;
+		left: -6px;
+	}
 }
 
 .user-item {

src/js/components/VoteTable/VoteTable.vue

@@ -33,6 +33,7 @@
 				<UserItem v-bind="participant" condensed />
 
 				<ActionDelete v-if="acl.allowEdit"
+					class="user-actions"
 					:name="t('polls', 'Delete votes')"
 					@delete="removeUser(participant.userId)" />
 			</div>
@@ -124,14 +125,14 @@ export default {
 		align-self: stretch;
 		justify-content: center;
 
-		.material-design-icon {
+		.user-actions {
 			// display: none;
 			visibility: hidden;
 		}
 
 		&:hover {
 			background: var(--color-background-hover);
-			.material-design-icon {
+			.user-actions {
 				visibility: visible;
 			}
 		}