-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
OF-2918 Clear MUC Chat History #2627
Open
hqv126
wants to merge
24
commits into
igniterealtime:main
Choose a base branch
from
surevine:OF-2918_Clear-MUC-chat-history
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
OF-2918 Clear MUC Chat History #2627
hqv126
wants to merge
24
commits into
igniterealtime:main
from
surevine:OF-2918_Clear-MUC-chat-history
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add the string properties for the new clear chat history feature.
Add a new chat history command to the left-hand side admin bar.
Add a confirmation page before clearing chat history for a room.
Update the edit form to redirect users there after clearing the room’s chat history.
Add a function to clear chat history from a room by sending retraction messages. Do not store these messages, as they will be cleared immediately.
Add a function to clear chat history from the database. Only chat messages should be cleared, not messages that update a room’s topic.
guusdk
requested changes
Dec 2, 2024
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.
Thanks for this! I've left a bunch of suggestions. Most of them are pretty minor, but I believe some others are more important.
xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MUCPersistenceManager.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MUCPersistenceManager.java
Show resolved
Hide resolved
Changes from PR review 1. Add the chatManagement.bulkMsgRetractionEnabled configuration to allow administrators to skip sending bulk retraction messages. 2. Make the MucRoom.clearChatHistory() method asynchronous, as it can be a long-running process. 3. Iterate over the history from the end, sending a revocation for the most recently received message first. This message is much more likely to be currently displayed in a connected client than the oldest message in a chatroom. 4. Add a translatable muc.room.clear_chat.retraction_fallback_msg string.
Fix the error in setting room.setSavedToDB(false) when clearing chat history.
Changes from PR review 1. Updated copyright text to exclude Jive Software. 2. Improved webManager logging to include the full address of the room. 3. Added logging to indicate the before and after states of the asynchronous clear chat history process.
An improvement to SequenceManager has been made to better support its usage in plugins like the Monitoring plugin. To use SequenceManager to keep track of and generate new unique IDs, an explicit type number is required. One way to achieve this is to assign a JiveID to a class that uses SequenceManager, e.g. @JiveID(604). However, there is potential for undetected ID clashes if IDs are explicitly assigned in more than one plugin. This commit proposes a solution that uses unique names to assign a unique SequenceManager type. To obtain a SequenceManager for the roomIDs in the Monitoring plugin, you simply need to call: SequenceManager.getSequenceManagerByUniqueName("Monitoring Plugin - RoomID"). These changes support fixes for issues igniterealtime#369 and igniterealtime#370 in the Monitoring plugin.
Add the event dispatcher code for roomClearChatHistory events. This change supports fixes for issues igniterealtime/openfire-monitoring-plugin#369 and igniterealtime/openfire-monitoring-plugin#370 in the Monitoring plugin.
Add a method to dispatcher roomClearChatHistory events. This change supports fixes for issues igniterealtime/openfire-monitoring-plugin#369 and igniterealtime/openfire-monitoring-plugin#370 in the Monitoring plugin.
Support dispatching roomClearChatHistory events in the following scenarios: 1. When a clear chat room history command is issued. (Fixes issue igniterealtime#369 in the monitoring plugin.) 2. When a chat room is destroyed and its chat history is not preserved. (Fixes issue igniterealtime#370 in the monitoring plugin.)
Implement the roomClearChatHistory event handler. Nothing is needed to be done in the OccupantManager for this event.
guusdk
requested changes
Dec 10, 2024
xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/database/SequenceManager.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCEventDispatcher.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java
Outdated
Show resolved
Hide resolved
These files should remain unchanged after PR review feedback.
Add roomJID to exception log. Existing exception messages don’t log roomJID or other useful information, the same change can be applied for them in a separate PR.
- Changed `BULK_MSG_RETRACTION_ENABLED` to a `SystemProperty` with key `xmpp.muc.bulkretraction`, and default value `false`. - Updated the `clearChatHistory` method to use the new `SystemProperty` for checking if bulk message retraction is enabled. - Modified event dispatching in `destroyRoom` method to use `getJID()` instead of `getSelfRepresentation().getOccupantJID()` as recommended in PR feedback.
Removed unnecessary security log event.
Modified event dispatching in `clearChatHistory` method to use `getJID()` instead of `getSelfRepresentation().getOccupantJID()` as recommended in PR feedback.
guusdk
reviewed
Dec 12, 2024
Adding system_property.xmpp.muc.bulkretraction.
Correct system_property.xmpp.muc.bulkretraction.enabled to system_property.xmpp.muc.bulkretraction
Add an Issue369and370 class handle the migration of data for the monitoring plugin to obtain roomIDs so chat history can be cleared.
- Improved class description for Issue369and370. - Changed some logging to the more appropriate debug level. - Avoid out-of-memory issues by optimising SQL queries and resultset usage.
guusdk
reviewed
Dec 16, 2024
xmppserver/src/main/java/org/jivesoftware/database/bugfix/Issue369and370.java
Outdated
Show resolved
Hide resolved
Remove redundant and misplaced pstmt.setFetchSize(…) and pstmt.setFetchDirection(…) calls.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.