Repository navigation
Conversation
…er is negative last_mention_message is 0 when there is no mention, but last_read_message can be -2 (UNREAD_FIRST_MESSAGE) or -1 (UNREAD_MIGRATION), so `0 > -2` counted the conversation as an unread mention. Since nextcloud#19654 prepareRoom() then loads message "0", OC\Comments\Manager::get() throws an InvalidArgumentException and the widget-items request fails with HTTP 500. Assisted-by: ClaudeCode:claude-sonnet-5-5 Signed-off-by: Volodymyr Fesko <vladimir.fesko@gmail.com>
This file contains hidden or 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
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.
☑️ Resolves
AI (if applicable)
The patch and the test were written with the help of Claude Code and reviewed by me; the commit carries an
Assisted-bytrailer.The problem
last_mention_messageis0when there is no mention, whilelast_read_messagecan be negative:-2(ChatManager::UNREAD_FIRST_MESSAGE) or-1(UNREAD_MIGRATION). The dashboard widget compared them withgetLastMentionMessage() > getLastReadMessage(), and0 > -2is true.Since #19654
TalkWidget::prepareRoom()then callsChatManager::getComment($room, "0"), whereOC\Comments\Manager::get("0")throwsInvalidArgumentException: IDs must be translatable to a number in this implementation.OnlyNotFoundExceptionis caught, soGET /ocs/v2.php/apps/dashboard/api/v2/widget-items?widgets[]=spreedanswers500. The dashboard then stops loading other widgets too (the "Recommended files" widget kept its spinner).-2is not rare: it is set when the chat history is cleared (ParticipantService::resetChatDetails), by "mark as unread", and by theFixLastReadMessageZerorepair step (a new attendee of an empty room starts with0, which becomes-2after an update).Seen on a real instance (25.0.6): the per-user changelog conversation Talk creates after an update, not opened yet (
last_read_message = -2,last_mention_message = 0). Opening the conversation made the 500 go away.To reproduce without the changelog: two users in a group conversation, a moderator clears the history, the other user opens the dashboard.
The fix
hasUnreadMention(Attendee)requires an actual mention (> 0) before comparing it with the read marker, and is used at the three places that had the comparison (getItems(),getItemsV2(),prepareRoom()). In API v2 such a conversation is also no longer listed among the "mentions".prepareRoom()does not catchInvalidArgumentExceptionon purpose: after the guard the ID is always positive, andICommentsManager::get()documents onlyNotFoundException.On stable32–34 there is no 500 (the
getComment()call is not there), only the same misclassification ("You were mentioned"); the code differs there, so a backport to those is up to you.Tests
New
tests/php/Dashboard/TalkWidgetTest.php: a data provider forhasUnreadMention()and agetItemsV2()case withlast_read_message = -2andlast_mention_message = 0. Run against the servermaster(sqlite): 9 tests pass; without the fix thegetItemsV2()test fails withgetComment(..., '0') was not expected to be called. I did not runphp-cs-fixerandpsalmlocally, CI will.🛠️ API Checklist
docs/has been updated or is not requiredCould you add the
regressionlabel and/backport to stable35after merging? I can't set labels from a fork.