FEAT(client): Add separate notification types for image messages#7159
FEAT(client): Add separate notification types for image messages#715928arnab wants to merge 1 commit intomumble-voip:masterfrom
Conversation
WalkthroughThis pull request introduces two new message types— 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/mumble/Messages.cpp`:
- Around line 1040-1046: The current substring check in Messages.cpp
(messageContent/hasImage/msgType selection) is too loose and case-sensitive;
include QRegularExpression and replace the contains(QLatin1String("<img")) check
with a case-insensitive regex match for an actual <img tag boundary (e.g.
pattern "<img\\b") using QRegularExpression::CaseInsensitiveOption so tags like
"<IMG>" match but tokens like "<imgur>" do not, then compute hasImage from that
regex match and keep the existing msgType selection logic (use privateMessage,
Log::PrivateImageMessage, Log::ImageMessage, etc.).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71342437-65a2-48d7-8766-5f90cd12c5b4
📒 Files selected for processing (5)
src/mumble/EnumStringConversions.cppsrc/mumble/Log.cppsrc/mumble/Log.hsrc/mumble/Messages.cppsrc/mumble/Settings.cpp
|
@Hartmnt can you check ? my last pr for this issue got removed by me because i followed ai to fix a git issue. |
Hartmnt
left a comment
There was a problem hiding this comment.
You can start by squashing the two relevant commits and then dropping the merge commit. Then rebase against the current master branch. We want to have PRs without merge commits. (force push is the correct way to update this PR)
I have not tested if this works, yet. But this PR will most likely not make the cut for 1.6 anyway.
Signed-off-by: 28arnab <68094959+28arnab@users.noreply.github.com>
The client currently treats messages containing embedded images as regular text messages for logging and notification purposes. That makes it impossible to disable notifications for image posts without also disabling notifications for normal text messages.
This commit keeps image delivery on the existing TextMessage protocol path but classifies incoming messages with embedded content as image messages in the client. It also adds dedicated log/settings entries for image messages and private image messages so their notifications can be configured independently.
Implements #6970
Checks