-
Notifications
You must be signed in to change notification settings - Fork 570
fix(conversations): system messages won't bump activity in room / thread order anymore #17977
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
Changes from 5 commits
682cd0d
6f7f684
d028ad8
739857c
c08c7aa
f0cb33a
50d3fe3
3ad92d6
d8c57dc
6c80235
319cfb6
c5f50d2
225116b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -476,6 +476,7 @@ | |
| } | ||
|
|
||
| protected function attendeesAddedEvent(AttendeesAddedEvent $event): void { | ||
| $event->shouldSkipLastMessageUpdate = true; | ||
|
Check failure on line 479 in lib/Chat/SystemMessage/Listener.php
|
||
| foreach ($event->getAttendees() as $attendee) { | ||
| $this->logger->debug($attendee->getActorType() . ' "' . $attendee->getActorId() . '" added to room "' . $event->getRoom()->getToken() . '"', ['app' => 'spreed-bfp']); | ||
| if ($attendee->getActorType() === Attendee::ACTOR_GROUPS) { | ||
|
|
@@ -493,6 +494,7 @@ | |
| } | ||
|
|
||
| protected function attendeesRemovedEvent(AttendeesRemovedEvent $event): void { | ||
| $event->shouldSkipLastMessageUpdate = true; | ||
|
Check failure on line 497 in lib/Chat/SystemMessage/Listener.php
|
||
| foreach ($event->getAttendees() as $attendee) { | ||
| $this->logger->debug($attendee->getActorType() . ' "' . $attendee->getActorId() . '" removed from room "' . $event->getRoom()->getToken() . '"', ['app' => 'spreed-bfp']); | ||
| if ($attendee->getActorType() === Attendee::ACTOR_GROUPS) { | ||
|
|
@@ -512,7 +514,7 @@ | |
| string $message, | ||
| array $parameters = [], | ||
| ?Participant $participant = null, | ||
| bool $shouldSkipLastMessageUpdate = false, | ||
| bool $shouldSkipLastMessageUpdate = true, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we keep this we need to check all the calls in this file. E.g. currently posting a file does not update the last message anymore. |
||
| bool $silent = false, | ||
| bool $forceSystemAsActor = false, | ||
| ?int $replyTo = null, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| namespace OCA\Talk\Migration; | ||
|
|
||
| use Closure; | ||
| use OCP\DB\ISchemaWrapper; | ||
| use OCP\DB\Types; | ||
| use OCP\Migration\IOutput; | ||
| use OCP\Migration\SimpleMigrationStep; | ||
| use OCP\DB\QueryBuilder\IQueryBuilder; | ||
| use OCP\IDBConnection; | ||
| use Override; | ||
|
|
||
| class Version24000Date20260510193300 extends SimpleMigrationStep { | ||
|
|
||
| public function __construct( | ||
| private readonly IDBConnection $connection, | ||
| ) { | ||
| } | ||
|
|
||
| /** | ||
| * @param IOutput $output | ||
| * @param Closure(): ISchemaWrapper $schemaClosure | ||
| * @param array $options | ||
| * @return null|ISchemaWrapper | ||
| */ | ||
| #[Override] | ||
| public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { | ||
| /** @var ISchemaWrapper $schema */ | ||
| $schema = $schemaClosure(); | ||
|
|
||
| $table = $schema->getTable('talk_rooms'); | ||
|
|
||
| if (!$table->hasColumn('last_metadata_activity')) { | ||
| $table->addColumn('last_metadata_activity', Types::DATETIME, [ | ||
| 'notnull' => false, | ||
| ]); | ||
| <<<<<<< HEAD | ||
|
Check failure on line 45 in lib/Migration/Version24000Date20260510193300.php
|
||
| <<<<<<< HEAD | ||
|
sudormant marked this conversation as resolved.
Outdated
|
||
| $table->addIndex(['last_metadata_activity'], 'talkroom_lastmetadataactive'); | ||
|
|
||
| } | ||
|
|
||
| $table = $schema->getTable('talk_threads'); | ||
|
|
||
| if (!$table->hasColumn('last_metadata_activity')) { | ||
| $table->addColumn('last_metadata_activity', Types::DATETIME, [ | ||
| 'notnull' => false, | ||
| ]); | ||
| $table->addIndex(['last_metadata_activity'], 'talkthread_lastmetadataactive'); | ||
| ======= | ||
| $table->addIndex(['last_metadata_activity'], 'talkthread_lastmetadataactive'); | ||
| ======= | ||
| $table->addIndex(['last_metadata_activity'], 'talkroom_lastmetadataactive'); | ||
| >>>>>>> a798ce9366 (change(conversations): change behavior in all necessary places for Rooms and Threads to use / set lastMetadataActivity instaed of lastActivity where appropriate, i.e. where system messages are signalled and not real chat messages.) | ||
|
|
||
| >>>>>>> 2ed52550f0 (feature(api): Add a new field and corresponding functionality for lastMetadaActivity for Rooms and Threads, similar to lastActivity. This also adds a database migration for the oc_talk_rooms and oc_talk_threads tables as well. Functions are introduced and prepared for later use. The use of the field lastActivity to signal when the last real message in a conversation appeared remains unchanged to keep the API stable. The intended use of this feature is to better distinguish between real messages (lastActivity) to notify and bump conversations in the thread list to the top, and other status / metadata related messages (lastMetadataActivity) like room state and participant list changes that shall get synced and be updated in the clients, but not trigger an activity bump of its conversations in the thread list.) | ||
| } | ||
|
|
||
| $table = $schema->getTable('talk_threads'); | ||
|
|
||
| if (!$table->hasColumn('last_metadata_activity')) { | ||
| $table->addColumn('last_metadata_activity', Types::DATETIME, [ | ||
| 'notnull' => false, | ||
| ]); | ||
| $table->addIndex(['last_metadata_activity'], 'talkthread_lastmetadataactive'); | ||
| } | ||
|
|
||
| return $schema; | ||
| } | ||
|
|
||
| /** | ||
| * @param IOutput $output | ||
| * @param Closure(): ISchemaWrapper $schemaClosure | ||
| * @param array $options | ||
| */ | ||
| #[Override] | ||
| public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) : void { | ||
| <<<<<<< HEAD | ||
| <<<<<<< HEAD | ||
| ======= | ||
| >>>>>>> a798ce9366 (change(conversations): change behavior in all necessary places for Rooms and Threads to use / set lastMetadataActivity instaed of lastActivity where appropriate, i.e. where system messages are signalled and not real chat messages.) | ||
| $update = $this->connection->getQueryBuilder(); | ||
| $update->update('talk_rooms') | ||
| ->set('last_metadata_activity', 'last_activity'); | ||
| $update->executeStatement(); | ||
| $update = $this->connection->getQueryBuilder(); | ||
| $update->update('talk_threads') | ||
| ->set('last_metadata_activity', 'last_activity'); | ||
| $update->executeStatement(); | ||
| <<<<<<< HEAD | ||
| ======= | ||
| $update = $this->connection->getQueryBuilder(); | ||
| $update->update('talk_rooms') | ||
| ->set('last_metadata_activity', 'last_activity'); | ||
| $update->executeStatement(); | ||
| >>>>>>> 2ed52550f0 (feature(api): Add a new field and corresponding functionality for lastMetadaActivity for Rooms and Threads, similar to lastActivity. This also adds a database migration for the oc_talk_rooms and oc_talk_threads tables as well. Functions are introduced and prepared for later use. The use of the field lastActivity to signal when the last real message in a conversation appeared remains unchanged to keep the API stable. The intended use of this feature is to better distinguish between real messages (lastActivity) to notify and bump conversations in the thread list to the top, and other status / metadata related messages (lastMetadataActivity) like room state and participant list changes that shall get synced and be updated in the clients, but not trigger an activity bump of its conversations in the thread list.) | ||
| ======= | ||
| >>>>>>> a798ce9366 (change(conversations): change behavior in all necessary places for Rooms and Threads to use / set lastMetadataActivity instaed of lastActivity where appropriate, i.e. where system messages are signalled and not real chat messages.) | ||
| } | ||
|
|
||
| } | ||
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.
A parallel PR made this read-only and private, so I guess we need to drop read-only and add a setter
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.
I committed another change to use getter / setters in this place.
I removed readonly also but kept it private, so this should now be compatible to the other pull request?