Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use a static room summary provider to resolve room summaries through id and aliases #3863

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

Use a static room summary provider to resolve room summaries through id and aliases #3863

wants to merge 3 commits into from

Conversation

Velin92
Copy link
Member

@Velin92 Velin92 commented Mar 3, 2025 •
edited
Loading

This fixes two issues:

  • We clean the state of the alternateRoomSummaryProvider when we are done with it to avoid the various views to clash with each others
  • We made a new staticRoomSummaryProvider to which we can't update its filtering through the protocol making its usage just for the sake of finding a specific in the room list, independent of the filtering or query state, also we set a max page size of .max over it

fixes #3852

@Velin92
trying to debug why sometimes the value
473c8fa 
is missing even if it appears in the room list
@Velin92 Velin92 requested a review from a team as a code owner March 3, 2025 15:49
@Velin92 Velin92 requested review from stefanceriu and removed request for a team March 3, 2025 15:49
@Velin92 Velin92 added the pr-misc for other changes label Mar 3, 2025
@codecov Codecov
Copy link

codecov bot commented Mar 3, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 4.34783% with 22 lines in your changes missing coverage. Please review.

Project coverage is 78.99%. Comparing base (4e6ad03) to head (2210ecc).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...obalSearchScreen/GlobalSearchScreenViewModel.swift 0.00% 4 Missing ⚠️
...rdingScreen/MessageForwardingScreenViewModel.swift 0.00% 4 Missing ⚠️
...SelectionScreen/RoomSelectionScreenViewModel.swift 0.00% 4 Missing ⚠️
...alSearchScreen/GlobalSearchScreenCoordinator.swift 0.00% 3 Missing ⚠️
...ingScreen/MessageForwardingScreenCoordinator.swift 0.00% 3 Missing ⚠️
...lectionScreen/RoomSelectionScreenCoordinator.swift 0.00% 3 Missing ⚠️
...ervices/Room/RoomSummary/RoomSummaryProvider.swift 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #3863      +/-   ##
===========================================
- Coverage    79.04%   78.99%   -0.06%     
===========================================
  Files          796      796              
  Lines        70584    70605      +21     
===========================================
- Hits         55795    55771      -24     
- Misses       14789    14834      +45
Flag Coverage Δ
unittests 70.94% <4.34%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pixlwave
Copy link
Member

pixlwave commented Mar 3, 2025

We figured it out - using the forwarding screen or global search leaves the alternate provider in a filtered state.

@Velin92 Velin92 changed the title Logs to understand why sometimes the pills don't render properly Use a static room summary provider to resolve room summaries through id and aliases Mar 3, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 3, 2025 •
edited
Loading

Warnings
⚠️

ElementX/Sources/Services/Client/ClientProxyProtocol.swift#L79 - TODOs should be resolved (This is a temporary value, in ...) (todo)

Generated by 🚫 Danger Swift against 2210ecc

@Velin92 Velin92 requested a review from pixlwave March 3, 2025 17:20
@Velin92 Velin92 added pr-bugfix for bug fix and removed pr-misc for other changes labels Mar 3, 2025
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Mar 3, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stefanceriu stefanceriu Awaiting requested review from stefanceriu stefanceriu is a code owner automatically assigned from element-hq/element-x-ios-reviewers

@pixlwave pixlwave Awaiting requested review from pixlwave

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
pr-bugfix for bug fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Raw room ID exposed in message permalink
2 participants
@Velin92 @pixlwave