Skip to content

📌 Pinned messages - API #16239

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
Merged
merged 19 commits into from
Nov 4, 2025
Merged

📌 Pinned messages - API #16239

merged 19 commits into from
Nov 4, 2025

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Oct 30, 2025 •
edited
Loading

🛠️ API Checklist

🚧 Tasks

  • Pinned messages are added as a new type to "shared items"
  • A conversation property gives a short cut whether the conversation has any pinned conversation and whether the request should be made
  • An attendee property is used to "hide" the last pinned message (convo + attendee property have same value => hide)
  • On hide we should a message that you can still see them in shared items unless unpinned
  • There will be system messages (like in other messengers) for pinning at least
  • Currently limited to moderators
  • X on the pinned message to hide for "self"
  • Moderators see a second "Unpin" option to revert a pin
  • Pin action is in message … menu
    • Most likely the chat message needs the info in the meta data as well, so that the action can be changed to "Unpin" in there as well?
  • Reset "hide pinned id" field on pinning, to show again on "re"-pinning
  • Reset "last pinned id" field on unpinning
  • Pin information is exposed on message data $message['metaData']['pinnedByType'] and so on
  • Federation support
    • Getting
    • Getting overview
    • Pin
    • Unpin
    • Hide

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • 📘 API documentation in docs/ has been updated or is not required
  • 🔖 Capability is added or not needed

@nickvergessen nickvergessen added this to the 🍏 Next Major (33) milestone Oct 30, 2025
@nickvergessen nickvergessen self-assigned this Oct 30, 2025
@nickvergessen nickvergessen added 2. developing enhancement feature: chat 💬 Chat and system messages feature: api 🛠️ OCS API for conversations, chats and participants labels Oct 30, 2025
@nickvergessen nickvergessen moved this to 🏗️ In progress in 💬 Talk team Oct 30, 2025
SystemKeeper
SystemKeeper reviewed Oct 30, 2025
View reviewed changes
@SystemKeeper
Copy link
Contributor

SystemKeeper commented Oct 30, 2025

If I understand it correctly:

  • Pin message 1
  • User hides the pinned messages
  • Pin message 2
  • User hides the pinned message
  • Unpin message 2
  • User see's message 1 pinned again

right? Expected?

@SystemKeeper
Copy link
Contributor

SystemKeeper commented Oct 30, 2025

Should the pin be reset when the message expires ?

@nickvergessen
Copy link
Member Author

nickvergessen commented Oct 31, 2025

If I understand it correctly:

  • Pin message 1
  • User hides the pinned messages
  • Pin message 2
  • User hides the pinned message
  • Unpin message 2
  • User see's message 1 pinned again

right?

yes

Expected?

Most likely not, but the additional effort needed to cover this, storing Users×PinnedMessages in a new database table is not worth the effort from my POV.

SystemKeeper
SystemKeeper reviewed Nov 2, 2025
View reviewed changes
miaulalala
miaulalala reviewed Nov 3, 2025
View reviewed changes
@nickvergessen nickvergessen force-pushed the feat/3390/pinned-messages branch from c0f74ad to 134b096 Compare November 3, 2025 15:52
@nickvergessen nickvergessen force-pushed the feat/3390/pinned-messages branch from 134b096 to 4449271 Compare November 3, 2025 15:53
SystemKeeper
SystemKeeper reviewed Nov 3, 2025
View reviewed changes
lib/BackgroundJob/UnpinMessage.php
Comment on lines +50 to +58
// Message most likely expired, reset the last_pinned_id if matching
if ($room->getLastPinnedId() === $messageId) {
$newLastPinned = 0;
$attachments = $this->attachmentService->getAttachmentsByType($room, Attachment::TYPE_PINNED, 0, 1);
if (isset($attachments[0])) {
$newLastPinned = $attachments[0]->getMessageId();
}
$this->roomService->setLastPinnedId($room, $newLastPinned);
}
Copy link
Contributor

@SystemKeeper SystemKeeper Nov 3, 2025

Choose a reason for hiding this comment

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

Hm, that only works for messages that have a unpin set, i.e. have a unpin job. I would have expected it to be in https://github.com/nextcloud/spreed/blob/main/lib/BackgroundJob/ExpireChatMessages.php, but I see that it's a db operation there. So we might end up in a situation were it's set on room, but still expired?

Also, just to mention it, this code part is identical to the chatmanager.

Copy link
Member Author

@nickvergessen nickvergessen Nov 3, 2025

Choose a reason for hiding this comment

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

that only works for messages that have a unpin set, i.e. have a unpin job.

Which will be the case going forward. At the moment of pinning I will set pinUntil when the message expires at some point

Also, just to mention it, this code part is identical to the chatmanager.

Yeah just renamed variables…

@SystemKeeper SystemKeeper mentioned this pull request Nov 3, 2025
Merged
SystemKeeper
SystemKeeper approved these changes Nov 3, 2025
View reviewed changes
Copy link
Contributor

@SystemKeeper SystemKeeper left a comment

Choose a reason for hiding this comment

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

Looks good from my side, besides the failing test.

@nickvergessen nickvergessen force-pushed the feat/3390/pinned-messages branch from aad45a7 to 1918a39 Compare November 4, 2025 00:25
@nickvergessen nickvergessen mentioned this pull request Nov 4, 2025
Merged
4 tasks
miaulalala
miaulalala approved these changes Nov 4, 2025
View reviewed changes
@nickvergessen nickvergessen force-pushed the feat/3390/pinned-messages branch from 67f536f to f9ef8f7 Compare November 4, 2025 14:13
@nickvergessen nickvergessen merged commit bc0e6d5 into main Nov 4, 2025
100 of 102 checks passed
@nickvergessen nickvergessen deleted the feat/3390/pinned-messages branch November 4, 2025 14:35
@github-project-automation github-project-automation bot moved this from 🏗️ In progress to ☑️ Done in 💬 Talk team Nov 4, 2025
@rapterjet2004 rapterjet2004 mentioned this pull request Nov 4, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SystemKeeper SystemKeeper SystemKeeper approved these changes

@miaulalala miaulalala miaulalala approved these changes

Assignees

@nickvergessen nickvergessen

Labels

3. to review enhancement feature: api 🛠️ OCS API for conversations, chats and participants feature: chat 💬 Chat and system messages

Projects

💬 Talk team
Status: ☑️ Done

Milestone

🍏 Next Major (33)

Development

Successfully merging this pull request may close these issues.

4 participants

@nickvergessen @SystemKeeper @miaulalala