Skip to content

Conversation

@pixlwave
Copy link
Member

@pixlwave pixlwave commented Nov 25, 2025

This PR makes 2 changes:

  • Refactors the tests for Spaces to use a common add_space_rooms function instead of having 3 variants.
  • Adds a new method to the SpaceService to get all of the user's joined spaces where they have the power to edit the spaces' children.

@pixlwave pixlwave requested a review from a team as a code owner November 25, 2025 13:32
@pixlwave pixlwave requested review from poljar and removed request for a team November 25, 2025 13:32
Copy link
Member Author

@pixlwave pixlwave Nov 25, 2025

Choose a reason for hiding this comment

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

Note: I'm open to naming suggestions as I wasn't massively confident on what to call this (and the matrix_sdk) function.

@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

❌ Patch coverage is 99.63235% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.66%. Comparing base (ec44c74) to head (61dbc07).
⚠️ Report is 40 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
crates/matrix-sdk-ui/src/spaces/mod.rs 99.63% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5897      +/-   ##
==========================================
+ Coverage   88.65%   88.66%   +0.01%     
==========================================
  Files         362      362              
  Lines      103146   103292     +146     
  Branches   103146   103292     +146     
==========================================
+ Hits        91440    91584     +144     
- Misses       7458     7459       +1     
- Partials     4248     4249       +1     

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

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 25, 2025

CodSpeed Performance Report

Merging #5897 will not alter performance

Comparing doug/all-joined-spaces (61dbc07) with main (100a04a)

Summary

✅ 50 untouched

@pixlwave pixlwave force-pushed the doug/all-joined-spaces branch from 6807e50 to e4b17ad Compare November 25, 2025 14:40
Copy link
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

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

Looks mostly good, I think I found a bit of a better name for this function.

Let me know what you think.

///
/// Note: Unlike [`Self::joined_spaces()`], this method does not recompute
/// graph, nor does it notify subscribers about changes.
pub async fn all_editable_joined_spaces(&self) -> Vec<SpaceRoom> {
Copy link
Contributor

Choose a reason for hiding this comment

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

One way to make this name a bit more pleasant is to remove the all prefix, and the joined nominator.

I think it's a given that you can only edit joined spaces or rooms for that matter.

So editable_spaces() sounds more palatable to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's a given that you can only edit joined spaces or rooms for that matter.

Oh yeah good point, totally missed that fact 😆

Copy link
Member Author

Choose a reason for hiding this comment

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

@pixlwave pixlwave force-pushed the doug/all-joined-spaces branch from e4b17ad to 61dbc07 Compare November 26, 2025 11:49
@poljar poljar enabled auto-merge (rebase) November 26, 2025 11:52
@poljar poljar merged commit 9e842a5 into main Nov 26, 2025
78 of 81 checks passed
@poljar poljar deleted the doug/all-joined-spaces branch November 26, 2025 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants