-
Notifications
You must be signed in to change notification settings - Fork 260
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
feat: add snooze mvp #8694
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Early architectural nitpicking ✔️
$now = $this->timeFactory->getTime(); | ||
$snooze = new MessageSnooze(); | ||
$snooze->setMessageId($message->getMessageId()); | ||
$snooze->setSnoozedUntil($now); | ||
$this->messageSnoozeMapper->insert($snooze); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this
return new JSONResponse([], Http::STATUS_FORBIDDEN); | ||
} | ||
|
||
$this->snoozeService->snoozeThread($selectedMessage, $unixTimestamp); |
There was a problem hiding this comment.
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 :)
There was a problem hiding this 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
$rows = array_map(static function (array $row) { | ||
return [ | ||
'messageId' => (string)$row[0] | ||
]; | ||
}, $result->fetchAll(\PDO::FETCH_NUM)); |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class Version3300Date20230807300513 extends SimpleMigrationStep { | |
class Version3400Date20230807300513 extends SimpleMigrationStep { |
because the next minor will be 3.4. 3.3 has already been branched off
There was a problem hiding this 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
There was a problem hiding this 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 ✔️
be3fbdd
to
bbda253
Compare
😬 needs rebase onto latest main to get rid of the conflicts |
f882dd2
to
26de4f8
Compare
'length' => 4, | ||
]); | ||
$messagesRetentionTable->setPrimaryKey(['id'], 'mail_msg_snoozed_id_idx'); | ||
$messagesRetentionTable->addIndex(['message_id'], 'mail_msg_snoozed_msgid_idx'); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise
Signed-off-by: Johannes Merkel <mail@johannesgge.de>
26de4f8
to
e16b6a7
Compare
fix: #2048