Prevent tasks from inheriting a group chatType and showing the Not here page#97019
Draft
MobileMage wants to merge 4 commits into
Draft
Prevent tasks from inheriting a group chatType and showing the Not here page#97019MobileMage wants to merge 4 commits into
MobileMage wants to merge 4 commits into
Conversation
A task inside a group chat is returned by the backend carrying the parent's chatType (e.g. group). getChatType now drops that inherited value for non-#admins tasks, so isGroupChat/isRootGroupChat/canLeaveChat no longer misclassify the task, the bogus Leave row and destructive leaveGroupChat path disappear, and the details page stops flashing the Not-here page. Keeps the intentional POLICY_ADMINS inheritance. Fixes Expensify#96421.
7 tasks
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Tasks no longer classify as their parent's chat type, so a wiped group-inherited task stopped matching isGroupChat in isRemovalExpectedForReportType and the user would stay on the Not-here page instead of redirecting. Treat task removals as expected for the task type itself. Also pin canLeaveChat(task) === false and add a policy-room-inherited task case to the classification tests.
Give the fixture a visible participant so the assertion exercises the classification path rather than passing through hidden-membership.
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.
Explanation of Change
When an assignee opens a task created inside a group chat, the backend returns the task report carrying the parent group chat's
chatType(for examplegroup).getChatTypepassed that value straight through, so every chatType-based classifier misfired for the task.isRootGroupChatreturned true, because its thread exclusion usesisChatThread(which needstype === CHAT, while a task's type isTASK), so a task thread slips past the root check.Once the task counts as a root group chat,
canLeaveChatreturns true, so a Leave row shows on the task details page, andleaveChatruns the destructiveleaveGroupChatinstead ofleaveRoom.leaveGroupChatnulls the report'sreportID; the focused details RHP wraps inwithReportOrNotFound, whose gate flips toshouldShowNotFoundPagethe momentreportIDgoes null, so the page renders the full "Not here" screen before the deferred dismissal moves the user away.This PR fixes the misclassification at its one choke point,
getChatType: a task no longer reports an inherited chat type unless it is the one the client supports (POLICY_ADMINS, used for #admins task routing).isGroupChat,isRootGroupChat, andcanLeaveChatthen return false for the task, the Leave row disappears, the destructive path becomes unreachable, and the details page renders the task layout. Real group chats (type: CHAT) and #admins tasks stay unchanged. Unit tests cover this intests/actions/TaskTest.ts.Fixed Issues
$ #96421
PROPOSAL: #96421 (comment)
Tests
Preconditions: User A, User B, and User C are members of a group chat.
Offline tests
No offline-specific behavior. The classification reads local Onyx data only.
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Videos will be added in a few hours.
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari