Skip to content

feat: add snooze mvp - #8694

Merged
JohannesGGE merged 1 commit into
mainfrom
enh/2048/snooze-mvp
Aug 10, 2023
Merged

feat: add snooze mvp#8694
JohannesGGE merged 1 commit into
mainfrom
enh/2048/snooze-mvp

Conversation

@JohannesGGE

@JohannesGGE JohannesGGE commented Aug 9, 2023

Copy link
Copy Markdown
Contributor

fix: #2048

Comment thread lib/BackgroundJob/UnSnoozeJob.php Outdated
Comment thread lib/BackgroundJob/UnSnoozeJob.php

@ChristophWurst ChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Early architectural nitpicking 鉁旓笍

Comment thread lib/Controller/MessagesController.php Outdated
Comment on lines +408 to +412
$now = $this->timeFactory->getTime();
$snooze = new MessageSnooze();
$snooze->setMessageId($message->getMessageId());
$snooze->setSnoozedUntil($now);
$this->messageSnoozeMapper->insert($snooze);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this

Comment thread lib/BackgroundJob/UnSnoozeJob.php
return new JSONResponse([], Http::STATUS_FORBIDDEN);
}

$this->snoozeService->snoozeThread($selectedMessage, $unixTimestamp);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pretty cool that a whole thread can be snoozed as well :)

@ChristophWurst ChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks super clean! Will also give this a test shortly

Comment thread lib/Db/ThreadMapper.php
Comment on lines +86 to +90
$rows = array_map(static function (array $row) {
return [
'messageId' => (string)$row[0]
];
}, $result->fetchAll(\PDO::FETCH_NUM));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

class Version3300Date20230807300513 extends SimpleMigrationStep {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
class Version3300Date20230807300513 extends SimpleMigrationStep {
class Version3400Date20230807300513 extends SimpleMigrationStep {

because the next minor will be 3.4. 3.3 has already been branched off

@ChristophWurst ChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

bump the version in info.xml so that the backrground job is inserted and the migration run automatically

@ChristophWurst ChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Works!

Message can be snoozed (moved) 鉁旓笍
When timestamp expires (updated the database) the message move back 鉁旓笍

@ChristophWurst

Copy link
Copy Markdown
Member

馃槵 needs rebase onto latest main to get rid of the conflicts

@JohannesGGE
JohannesGGE force-pushed the enh/2048/snooze-mvp branch 5 times, most recently from f882dd2 to 26de4f8 Compare August 10, 2023 16:41
@JohannesGGE
JohannesGGE marked this pull request as ready for review August 10, 2023 16:52
@ChristophWurst ChristophWurst changed the title WIP: snooze mvp feat: add snooze mvp Aug 10, 2023
'length' => 4,
]);
$messagesRetentionTable->setPrimaryKey(['id'], 'mail_msg_snoozed_id_idx');
$messagesRetentionTable->addIndex(['message_id'], 'mail_msg_snoozed_msgid_idx');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we make it unique? two entries don't make any sense

@ChristophWurst ChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit: Otherwise

Signed-off-by: Johannes Merkel <mail@johannesgge.de>
@JohannesGGE
JohannesGGE merged commit abf1d94 into main Aug 10, 2023
@JohannesGGE
JohannesGGE deleted the enh/2048/snooze-mvp branch August 10, 2023 17:28
@hamza221 hamza221 mentioned this pull request Aug 16, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Snooze mails

2 participants