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.

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

feat: add snooze mvp #8694

Merged
merged 1 commit into from
Aug 10, 2023
Merged

feat: add snooze mvp #8694

merged 1 commit into from
Aug 10, 2023

Conversation

JohannesGGE
Copy link
Contributor

@JohannesGGE JohannesGGE commented Aug 9, 2023

fix: #2048

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Early architectural nitpicking ✔️

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
Member

Choose a reason for hiding this comment

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

this

lib/BackgroundJob/UnSnoozeJob.php Outdated Show resolved Hide resolved
return new JSONResponse([], Http::STATUS_FORBIDDEN);
}

$this->snoozeService->snoozeThread($selectedMessage, $unixTimestamp);
Copy link
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 :)

Copy link
Member

@ChristophWurst ChristophWurst left a comment

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 on lines +86 to +90
$rows = array_map(static function (array $row) {
return [
'messageId' => (string)$row[0]
];
}, $result->fetchAll(\PDO::FETCH_NUM));
Copy link
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
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

Copy link
Member

@ChristophWurst ChristophWurst left a comment

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

Copy link
Member

@ChristophWurst ChristophWurst left a comment

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
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
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

Copy link
Member

@ChristophWurst ChristophWurst left a comment

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
28 of 29 checks passed
@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