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

Communication: Add ability to pin messenges #114

Merged
merged 20 commits into from
Dec 12, 2024

Conversation

julian-wls
Copy link
Contributor

@julian-wls julian-wls commented Nov 18, 2024

Description

This PR introduces pinning posts. A new post action has been created to allow users to pin standalone posts. Pinning posts is only available for channel creators and moderators, which is why the according flags are passed down to the post action.

With the pinning feature added, it becomes quite clear that the post items in the chat need an overhaul to make the whole view coherent again. This will happen as a follow-up PR as described here.

Changes

  • The database had to be migrated as there was no column for the display priority in the Posts table.
  • The PostItem was changed in order to give pinned posts a different look and show a pinned label on top of the post (see screenshot).
  • This PR includes multiple UI and E2E tests to ensure the correct behavior.
  • MetisModificationService was changed to handle the modification request

Steps for testing

  1. Make sure you have moderation rights or create a chat before testing
  2. Send a message and long click on it.
  3. The Pin Post button should be visible.
  4. Click the button and see how the message changes its appearance in the chat (A reload might be required because of ongoing issues with the websocket).
  5. Long click again, click Unpin Post and verify that the message returns to its initial state
  6. Verify that the Pin Post option is not available on answer posts in the thread view

Screenshots

@julian-wls julian-wls self-assigned this Nov 18, 2024
@julian-wls julian-wls added the ready for review This PR can be reviewed label Nov 28, 2024
@julian-wls julian-wls marked this pull request as ready for review November 28, 2024 12:13
@FelberMartin FelberMartin added the database changes This PR includes database changes and should be treated with extra care when merging label Nov 29, 2024
Copy link
Collaborator

@FelberMartin FelberMartin left a comment

Choose a reason for hiding this comment

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

Tested the functionality, works great!

Only thing that I noticed, is that the web app allows users to pin messages in DMs, but this is not possible currently in Android

See code comments below

FelberMartin added a commit that referenced this pull request Nov 29, 2024
Copy link
Collaborator

@FelberMartin FelberMartin left a comment

Choose a reason for hiding this comment

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

Pinning in DMs now also work, nice.

Just update the database version and its good to be merged from my side 👍

@FelberMartin FelberMartin added ready to merge This PR can be merged and removed ready for review This PR can be reviewed labels Dec 5, 2024
Copy link

@ahbitaqu ahbitaqu left a comment

Choose a reason for hiding this comment

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

Works perfectly over here!

@FelberMartin FelberMartin merged commit 0afbff8 into develop Dec 12, 2024
5 checks passed
@FelberMartin FelberMartin deleted the feature/communication/pin-posts branch December 12, 2024 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database changes This PR includes database changes and should be treated with extra care when merging ready to merge This PR can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants