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.

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

Feature/bca/room caps restricted #3663

Merged
merged 11 commits into from
Jul 30, 2021
Merged

Conversation

BillCarsonFr
Copy link
Member

@BillCarsonFr BillCarsonFr commented Jul 12, 2021

Fixes #3509
Fixes #3665
Fixes #3508

New setting screen that allow to use the new restricted access rule
image

If the selected room is using a version that do not support restricted, a button will be displayed to migrate the room first (shown only if the server supports the restricted join rule as per Room caps MSC

image

image

@bmarty bmarty changed the title Feature/bca/room caps restriced Feature/bca/room caps restricted Jul 12, 2021
@BillCarsonFr BillCarsonFr marked this pull request as ready for review July 16, 2021 09:46
@BillCarsonFr BillCarsonFr requested a review from bmarty July 20, 2021 07:23
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Nice work.

I cannot test on matrix.org yet AFAIU.

Some remark in the mean time.

)

data class RoomVersionInfo(
val version: String,
val status: RoomVersionStatus
)

@JsonClass(generateAdapter = true)
Copy link
Member

Choose a reason for hiding this comment

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

This is not a Json class?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

data class RoomVersionCapabilities(
val defaultRoomVersion: String,
val supportedVersion: List<RoomVersionInfo>
val supportedVersion: List<RoomVersionInfo>,
val capabilities: Map<String, RoomCapabilitySupport>?
Copy link
Member

Choose a reason for hiding this comment

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

What are the keys?

Copy link
Member Author

Choose a reason for hiding this comment

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

added a comment

return info.preferred == byRoomVersion || info.support.contains(byRoomVersion)
}

fun versionOverrideForFeature(feature: String) : String? {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add some doc for this public APIs?

Copy link
Member Author

Choose a reason for hiding this comment

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

done


suspend fun setJoinRulePublic()
suspend fun setJoinRuleInviteOnly()
suspend fun setJoinRuleRestricted(allowList: List<String>)
Copy link
Member

Choose a reason for hiding this comment

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

These 3 fun should be moved to StateService and documented please.

Copy link
Member Author

Choose a reason for hiding this comment

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

Initially it was in state service, but the join restricted is doing more than just state, it is also computing the via parameters. It feels strange to inject he viaParameterFinder in state event as it requires roomGetter.
I have put it back in StateService

import org.matrix.android.sdk.api.session.room.model.RoomJoinRulesAllowEntry
import org.matrix.android.sdk.api.session.room.model.RoomJoinRulesContent

interface RoomFeaturePreset {
Copy link
Member

Choose a reason for hiding this comment

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

Interface and class must be moved to internal package and declared internal.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's public, used in CreateSpaceViewModelTask for example

android:layout_width="match_parent"
android:layout_height="wrap_content">

<com.google.android.material.appbar.CollapsingToolbarLayout
Copy link
Member

Choose a reason for hiding this comment

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

Is this used?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes it's snapping but no animation

<string name="room_create_member_of_space_name_can_join">Members of Space %s can find, preview and join.</string>
<string name="allow_space_member_to_find_and_access">Allow space members to find and access.</string>
<string name="spaces_which_can_access">Spaces which can access</string>
<string name="decide_which_spaces_can_access">Decide which spaces can access this room. If a space is selected its members will be able to find and join Room name.</string>
Copy link
Member

Choose a reason for hiding this comment

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

Too many spaces in room. If and find and

Copy link
Member Author

Choose a reason for hiding this comment

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

done


<string name="room_using_unstable_room_version">This room is running room version %s, which this homeserver has marked as unstable.</string>
<string name="room_upgrade_to_recommended_version">Upgrade to the recommended room version</string>

<string name="error_failed_to_join_room">Sorry, an error occurred while trying to join: %s</string>

<string name="upgrade_room_for_restricted">Anyone in %s will be able to find and join this room - no need to manually invite everyone. You’ll be able to change this in room settings anytime.</string>
Copy link
Member

Choose a reason for hiding this comment

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

dup spaces everyone. You’ll

Copy link
Member Author

Choose a reason for hiding this comment

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

done


<string name="room_using_unstable_room_version">This room is running room version %s, which this homeserver has marked as unstable.</string>
<string name="room_upgrade_to_recommended_version">Upgrade to the recommended room version</string>

<string name="error_failed_to_join_room">Sorry, an error occurred while trying to join: %s</string>

<string name="upgrade_room_for_restricted">Anyone in %s will be able to find and join this room - no need to manually invite everyone. You’ll be able to change this in room settings anytime.</string>
<string name="upgrade_room_for_restricted_no_param">Anyone in a parent space will be able to find and join this room - no need to manually invite everyone. You’ll be able to change this in room settings anytime.</string>
Copy link
Member

Choose a reason for hiding this comment

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

Dup everyone. You

Copy link
Member Author

Choose a reason for hiding this comment

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

done

<string name="upgrade_room_for_restricted">Anyone in %s will be able to find and join this room - no need to manually invite everyone. You’ll be able to change this in room settings anytime.</string>
<string name="upgrade_room_for_restricted_no_param">Anyone in a parent space will be able to find and join this room - no need to manually invite everyone. You’ll be able to change this in room settings anytime.</string>

<string name="upgrade_room_for_restricted_note">Please note upgrading will make a new version of the room. All current messages will stay in this archived room.</string>
Copy link
Member

Choose a reason for hiding this comment

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

room. All

Copy link
Member Author

Choose a reason for hiding this comment

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

done


<Button
android:id="@+id/cancelButton"
style="@style/Widget.Vector.Button.Outlined"
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@bmarty
Copy link
Member

bmarty commented Jul 20, 2021

  • On the Room access setting screen, we do not have the classical room setting header (see Figma)

Expected:

image

Actual:

image

@BillCarsonFr BillCarsonFr force-pushed the feature/bca/room_caps_restriced branch from eae2f73 to 85d3941 Compare July 27, 2021 12:58
@BillCarsonFr BillCarsonFr added the A-Spaces Spaces, groups, communities label Jul 30, 2021
@BillCarsonFr BillCarsonFr force-pushed the feature/bca/room_caps_restriced branch from 85d3941 to c475418 Compare July 30, 2021 16:48
@BillCarsonFr BillCarsonFr force-pushed the feature/bca/room_caps_restriced branch from c475418 to aaa9c7e Compare July 30, 2021 17:05
@BillCarsonFr BillCarsonFr merged commit 60caac4 into develop Jul 30, 2021
@BillCarsonFr BillCarsonFr deleted the feature/bca/room_caps_restriced branch July 30, 2021 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Spaces Spaces, groups, communities
Projects
None yet
2 participants