Skip to content

Add a Low Priority room filter behind a feature flag. #4394

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 8, 2025

This PR implements a new "Low Priority" room filter (hidden behind a feature flag), with the key requirement that low priority rooms are hidden by default in the room list (when the flag is enabled).

Core Filter Implementation

  • Added lowPriority case to the RoomListFilter enum
  • Configured incompatibility rules: Low Priority cannot be combined with Favourites or Invites filters

Default Hiding Behavior

The implementation handles the new SDK filter architecture where:

  • Default state: NonLowPriority filter will be included to hide low priority rooms by default
  • When Low Priority filter is activated: Switches to LowPriority filter to show only low priority rooms

This logic is implemented in RoomSummaryProvider.setFilter()

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-08-13.at.10.31.16.mp4

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Copilot Copilot AI changed the title [WIP] Please add a new room list filter called Low Priority. It cannot be combined with the Favourites or Invites filter. What's different about this filter is that in the default room list state (when not searching), low priority rooms should be hidden so p... Add Low Priority room filter with default hiding behavior Aug 8, 2025
@Copilot Copilot AI requested a review from pixlwave August 8, 2025 07:46
Copilot finished work on behalf of pixlwave August 8, 2025 07:46
@pixlwave pixlwave changed the base branch from doug/agent-test to develop August 8, 2025 11:54
@pixlwave
Copy link
Member

pixlwave commented Aug 8, 2025

@copilot There's a PR in the SDK now with the required filters: matrix-org/matrix-rust-sdk#5508

I'll integrate it in a bit, but you can update this PR in preparation. Unfortunately we needed 2 new filter types LowPriority and NonLowPriority. What we're going to need is to have the latter always part of the filters whenever your new RoomListFilter.lowPriority isn't set, and when it is, we switch it out for the former.

Copy link
Contributor Author

Copilot AI commented Aug 8, 2025

@copilot There's a PR in the SDK now with the required filters: matrix-org/matrix-rust-sdk#5508

I'll integrate it in a bit, but you can update this PR in preparation. Unfort...

I've updated the implementation to prepare for the new LowPriority and NonLowPriority filters from SDK PR #5508. The logic now:

  • Uses NonLowPriority filter by default (to hide low priority rooms)
  • Switches to LowPriority filter when the Low Priority filter is activated
  • Has clear TODO comments showing exactly where the SDK integration will happen

Ready for when you integrate the SDK update! (commit ea1f6b3)

@Copilot Copilot AI changed the title Add Low Priority room filter with default hiding behavior Add Low Priority room filter with SDK preparation for LowPriority and NonLowPriority filters Aug 8, 2025
Copilot finished work on behalf of pixlwave August 8, 2025 13:15
@pixlwave pixlwave force-pushed the copilot/fix-34d8af23-d479-453c-87a9-470e070fd31e branch from ea1f6b3 to a08bb84 Compare August 13, 2025 08:36
@pixlwave pixlwave added the pr-wip for anything that isn't ready to ship and will be enabled at a later date label Aug 13, 2025
@pixlwave pixlwave changed the title Add Low Priority room filter with SDK preparation for LowPriority and NonLowPriority filters Add a Low Priority room filter behind a feature flag. Aug 13, 2025
Copilot AI and others added 2 commits August 13, 2025 10:26
Co-authored-by: pixlwave <6060466+pixlwave@users.noreply.github.com>
@pixlwave pixlwave force-pushed the copilot/fix-34d8af23-d479-453c-87a9-470e070fd31e branch from bf29595 to 8eedd43 Compare August 13, 2025 09:27
@pixlwave pixlwave marked this pull request as ready for review August 13, 2025 09:28
@pixlwave pixlwave requested a review from a team as a code owner August 13, 2025 09:28
@pixlwave pixlwave requested review from stefanceriu and removed request for pixlwave and a team August 13, 2025 09:28
@pixlwave pixlwave force-pushed the copilot/fix-34d8af23-d479-453c-87a9-470e070fd31e branch from 8eedd43 to a69b221 Compare August 13, 2025 09:39
Comment on lines 236 to 237
// Apply active filters - the low priority exclusion logic will be handled
// at the SDK level when the low priority filter is implemented
Copy link
Member

Choose a reason for hiding this comment

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

This comment feels unnecessary, perhaps we should name the app side filter better e.g. onlyLowPriority

Copy link
Member

Choose a reason for hiding this comment

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

Lol oh yeah that one is very random, removing it.

I get what you're saying about the name, but technically they're all only, e.g. onlyFavourites, onlyRooms etc so I'm not sure how much that would help. I think it's clear from the comment on .lowPriority's rustFilter value.

Copy link

codecov bot commented Aug 13, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
1048 1 1047 0
View the full list of 1 ❄️ flaky tests
DeferredFulfillmentTests::testObservableMultipleUpdates()

Flake rate in main: 18.75% (Passed 143 times, Failed 33 times)

Stack Traces | 12.2s run time
Asynchronous wait failed: Exceeded timeout of 10 seconds, with unfulfilled expectations: "Awaiting stream". (.../Other/Extensions/XCTestCase.swift:76)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-wip for anything that isn't ready to ship and will be enabled at a later date
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants