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.

Sign up for GitHub

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

Jump to bottom

Snooze mvp follow up #8754

Merged
merged 1 commit into from
Aug 17, 2023
Merged

Snooze mvp follow up #8754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions lib/Controller/MessagesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,18 +390,25 @@ public function move(int $id, int $destFolderId): JSONResponse {
*
* @param int $id
* @param int $unixTimestamp
* @param int $destMailboxId
*
* @return JSONResponse
* @throws ClientException
* @throws ServiceException
*/
#[TrapError]
public function snooze(int $id, int $unixTimestamp): JSONResponse {
public function snooze(int $id, int $unixTimestamp, int $destMailboxId): JSONResponse {
try {
$message = $this->mailManager->getMessage($this->currentUserId, $id);
$srcMailbox = $this->mailManager->getMailbox($this->currentUserId, $message->getMailboxId());
$dstMailbox = $this->mailManager->getMailbox($this->currentUserId, $destMailboxId);
$srcAccount = $this->accountService->find($this->currentUserId, $srcMailbox->getAccountId());
$dstAccount = $this->accountService->find($this->currentUserId, $dstMailbox->getAccountId());
} catch (DoesNotExistException $e) {
return new JSONResponse([], Http::STATUS_FORBIDDEN);
}

$this->snoozeService->snoozeMessage($message, $unixTimestamp);
$this->snoozeService->snoozeMessage($message, $unixTimestamp, $srcAccount, $srcMailbox, $dstAccount, $dstMailbox);

return new JSONResponse();
}
Expand Down
11 changes: 9 additions & 2 deletions lib/Controller/ThreadController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,18 +104,25 @@ public function move(int $id, int $destMailboxId): JSONResponse {
*
* @param int $id
* @param int $unixTimestamp
* @param int $destMailboxId
*
* @return JSONResponse
* @throws ClientException
* @throws ServiceException
*/
#[TrapError]
public function snooze(int $id, int $unixTimestamp): JSONResponse {
public function snooze(int $id, int $unixTimestamp, int $destMailboxId): JSONResponse {
try {
$selectedMessage = $this->mailManager->getMessage($this->currentUserId, $id);
$srcMailbox = $this->mailManager->getMailbox($this->currentUserId, $selectedMessage->getMailboxId());
$srcAccount = $this->accountService->find($this->currentUserId, $srcMailbox->getAccountId());
$dstMailbox = $this->mailManager->getMailbox($this->currentUserId, $destMailboxId);
$dstAccount = $this->accountService->find($this->currentUserId, $dstMailbox->getAccountId());
} catch (DoesNotExistException $e) {
return new JSONResponse([], Http::STATUS_FORBIDDEN);
}

$this->snoozeService->snoozeThread($selectedMessage, $unixTimestamp);
$this->snoozeService->snoozeThread($selectedMessage, $unixTimestamp, $srcAccount, $srcMailbox, $dstAccount, $dstMailbox);

return new JSONResponse();
}
Expand Down
10 changes: 3 additions & 7 deletions lib/Db/MessageSnoozeMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,15 @@ public function __construct(IDBConnection $db) {
/**
* Deletes DB Entry for a waked message
*
* @param string $messageId
* @param string[] $messageIds
*
* @return void
*/
public function deleteByMessageId(string $messageId): void {
public function deleteByMessageIds(array $messageIds): void {
$qb = $this->db->getQueryBuilder();

$delete = $qb->delete($this->getTableName())
->where($qb->expr()->eq(
'message_id',
$qb->createNamedParameter($messageId, IQueryBuilder::PARAM_STR),
IQueryBuilder::PARAM_STR,
));
->where($qb->expr()->in('message_id', $qb->createNamedParameter($messageIds, IQueryBuilder::PARAM_STR_ARRAY)));

$delete->executeStatement();
}
Expand Down
159 changes: 154 additions & 5 deletions lib/Service/SnoozeService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use OCA\Mail\Account;
use OCA\Mail\Contracts\IMailManager;
use OCA\Mail\Db\MailAccountMapper;
use OCA\Mail\Db\Mailbox;
use OCA\Mail\Db\MailboxMapper;
use OCA\Mail\Db\Message;
use OCA\Mail\Db\MessageMapper;
Expand All @@ -40,7 +41,9 @@
use OCA\Mail\IMAP\IMAPClientFactory;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\DB\Exception;
use Psr\Log\LoggerInterface;
use Throwable;

class SnoozeService {

Expand Down Expand Up @@ -81,29 +84,134 @@ public function wakeMessages(): void {
}
}

/**
* @param Message $message
* @param int $unixTimestamp
* @param Account $srcAccount
* @param Mailbox $srcMailbox
* @param Account $dstAccount
* @param Mailbox $dstMailbox
*
* @return void
*
* @throws Throwable
*/
public function snoozeMessage(
Message $message,
int $unixTimestamp,
Account $srcAccount,
Mailbox $srcMailbox,
Account $dstAccount,
Mailbox $dstMailbox
): void {
$this->snoozeMessageDB($message, $unixTimestamp);

try {
$this->mailManager->moveMessage(
$srcAccount,
$srcMailbox->getName(),
$message->getUid(),
$dstAccount,
$dstMailbox->getName()
);
} catch (Throwable $e) {
$this->unSnoozeMessageDB($message);
throw $e;
}
}

/**
* @param Message $selectedMessage
* @param int $unixTimestamp
* @param Account $srcAccount
* @param Mailbox $srcMailbox
* @param Account $dstAccount
* @param Mailbox $dstMailbox
*
* @return void
*
* @throws Throwable
*/
public function snoozeThread(
Message $selectedMessage,
int $unixTimestamp,
Account $srcAccount,
Mailbox $srcMailbox,
Account $dstAccount,
Mailbox $dstMailbox
):void {
$this->snoozeThreadDB($selectedMessage, $unixTimestamp);

try {
$this->mailManager->moveThread(
$srcAccount,
$srcMailbox,
$dstAccount,
$dstMailbox,
$selectedMessage->getThreadRootId()
);
} catch (Throwable $e) {
$this->unSnoozeThreadDB($selectedMessage);
throw $e;
}

}


/**
* Adds a DB entry for the message with a wake timestamp
*
* @param Message $message
* @param int $unixTimestamp
*
* @return void
*
* @throws Exception
*/
public function snoozeMessage(Message $message, int $unixTimestamp): void {
public function snoozeMessageDB(Message $message, int $unixTimestamp): void {
$snooze = new MessageSnooze();
$snooze->setMessageId($message->getMessageId());
$snooze->setSnoozedUntil($unixTimestamp);
$this->messageSnoozeMapper->insert($snooze);
try {
$this->messageSnoozeMapper->insert($snooze);
} catch(Exception $e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you check the reason of the error and only handle duplicates and rethrow the rest?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a check $e->getPrevious() instanceof \Doctrine\DBAL\Exception\UniqueConstraintViolationException. Do you think this is okay?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently, tests fail because the UniqueConstraintViolationException is part of 3rdparty. Is there a way to use it without errors or is this not an option?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\OCP\DB\Exception::getReason and comparison with \OCP\DB\Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION

if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
$messageId = $message->getMessageId();
if($messageId === null) {
throw $e;
}
$this->messageSnoozeMapper->deleteByMessageIds(array($messageId));
$this->messageSnoozeMapper->insert($snooze);
} else {
throw $e;
}
}
}

/**
* Removes the DB entry for the message
*
* @param Message $message
* @return void
*/
public function unSnoozeMessageDB(Message $message): void {
$messageId = $message->getMessageId();
if($messageId !== null) {
$this->messageSnoozeMapper->deleteByMessageIds(array($messageId));
}
}

/**
* Adds a DB entry for the messages with a wake timestamp
*
* @param Message $selectedMessage
* @param int $unixTimestamp
*
* @return void
*
* @throws Exception
*/
public function snoozeThread(Message $selectedMessage, int $unixTimestamp): void {
public function snoozeThreadDB(Message $selectedMessage, int $unixTimestamp): void {
$messages = $this->threadMapper->findMessageIdsByThreadRoot(
$selectedMessage->getThreadRootId()
);
Expand All @@ -112,7 +220,40 @@ public function snoozeThread(Message $selectedMessage, int $unixTimestamp): void
$snooze = new MessageSnooze();
$snooze->setMessageId($message['messageId']);
$snooze->setSnoozedUntil($unixTimestamp);
$this->messageSnoozeMapper->insert($snooze);
try {
$this->messageSnoozeMapper->insert($snooze);
} catch(Exception $e) {
if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
$messageId = $message['messageId'];
if($messageId === null) {
throw $e;
}
$this->messageSnoozeMapper->deleteByMessageIds(array($messageId));
$this->messageSnoozeMapper->insert($snooze);
} else {
throw $e;
}
}
}
}

/**
* Removes DB entry for the messages
*
* @param Message $selectedMessage
* @return void
*/
public function unSnoozeThreadDB(Message $selectedMessage): void {
$messages = $this->threadMapper->findMessageIdsByThreadRoot(
$selectedMessage->getThreadRootId()
);

$messageIdsToDelete = [];
foreach ($messages as $message) {
$messageIdsToDelete[] = $message['messageId'];
}
if(count($messageIdsToDelete) > 0) {
$this->messageSnoozeMapper->deleteByMessageIds($messageIdsToDelete);
}
}

Expand Down Expand Up @@ -144,6 +285,7 @@ private function wakeMessagesByAccount(Account $account): void {

$client = $this->clientFactory->getClient($account);
try {
$messageIdsToDelete = [];
foreach ($messages as $message) {
$this->mailManager->moveMessage(
$account,
Expand All @@ -155,7 +297,14 @@ private function wakeMessagesByAccount(Account $account): void {

//TODO mark message as unread?

$this->messageSnoozeMapper->deleteByMessageId($message->getMessageId());
$messageId = $message->getMessageId();
if($messageId !== null) {
$messageIdsToDelete[] = $messageId;
}

}
if(count($messageIdsToDelete) > 0) {
$this->messageSnoozeMapper->deleteByMessageIds($messageIdsToDelete);
}
} finally {
$client->logout();
Expand Down
23 changes: 13 additions & 10 deletions src/components/Envelope.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
:details="formatted()"
@click="onClick"
@click.ctrl="toggleSelected"
@update:menuOpen="moreActionsOpen=false">
@update:menuOpen="closeMoreAndSnoozeOptions">
<template #icon>
<Star
v-if="data.flags.flagged"
Expand All @@ -31,7 +31,7 @@
class="app-content-list-item-star svg icon-important"
:data-starred="isImportant ? 'true' : 'false'"
@click.prevent="hasWriteAcl ? onToggleImportant() : false"
v-html="importantSvg" />

Check warning on line 34 in src/components/Envelope.vue

View workflow job for this annotation

GitHub Actions / eslint 

'v-html' directive can lead to XSS attack
<JunkIcon
v-if="data.flags.$junk"
:size="18"
Expand Down Expand Up @@ -625,21 +625,23 @@
reminderOptions() {
const currentDateTime = moment()

// Same day 18:00 PM (or hidden)
const laterTodayTime = (currentDateTime.hour() < 18)
// Same day 18:00 PM (hidden if after 17:00 PM now)
const laterTodayTime = (currentDateTime.hour() < 17)
? moment().hour(18)
: null

// Tomorrow 08:00 AM
const tomorrowTime = moment().add(1, 'days').hour(8)

// Saturday 08:00 AM (or hidden)
const thisWeekendTime = (currentDateTime.day() !== 6 && currentDateTime.day() !== 0)
// Saturday 08:00 AM (hidden if Friday, Saturday or Sunday now)
const thisWeekendTime = (currentDateTime.day() > 0 && currentDateTime.day() < 5)
? moment().day(6).hour(8)
: null

// Next Monday 08:00 AM
const nextWeekTime = moment().add(1, 'weeks').day(1).hour(8)
// Next Monday 08:00 AM (hidden if Sunday now)
const nextWeekTime = (currentDateTime.day() !== 0)
? moment().add(1, 'weeks').day(1).hour(8)
: null

return [
{
Expand Down Expand Up @@ -764,6 +766,10 @@
this.snoozeOptions = true
this.moreActionsOpen = false
},
closeMoreAndSnoozeOptions() {
this.snoozeOptions = false
this.moreActionsOpen = false
},
async onArchive() {
// Remove from selection first
this.setSelected(false)
Expand All @@ -788,9 +794,6 @@
await this.$store.dispatch('snoozeThread', {
envelope: this.data,
unixTimestamp: timestamp / 1000,
})
await this.$store.dispatch('moveThread', {
envelope: this.data,
destMailboxId: this.account.snoozeMailboxId,
})
showSuccess(t('mail', 'Thread was snoozed'))
Expand Down
5 changes: 0 additions & 5 deletions src/components/MenuEnvelope.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,14 +499,9 @@ export default {
logger.info(`snoozing message ${this.envelope.databaseId}`)

try {
// Adds DB entry
await this.$store.dispatch('snoozeMessage', {
id: this.envelope.databaseId,
unixTimestamp: timestamp / 1000,
})
// Moves message
await this.$store.dispatch('moveMessage', {
id: this.envelope.databaseId,
destMailboxId: this.account.snoozeMailboxId,
})
showSuccess(t('mail', 'Message was snoozed'))
Expand Down
2 changes: 1 addition & 1 deletion src/components/NavigationMailbox.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
:size="20" />
<IconJunk v-else-if="mailbox.databaseId === account.junkMailboxId"
:size="20" />
<AlarmIcon v-else-if="mailbox.name === 'Snoozed'"
<AlarmIcon v-else-if="mailbox.databaseId === account.snoozeMailboxId"
:size="20" />
<IconFolderShared v-else-if="mailbox.shared"
:size="20" />
Expand Down
Loading
Loading