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

[Spaces] M10.8 Browsing users in a space #4682 #4742

Merged
merged 15 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Riot/Modules/Application/AppCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ final class AppCoordinator: NSObject, AppCoordinatorType {
private weak var splitViewCoordinator: SplitViewCoordinatorType?
fileprivate weak var sideMenuCoordinator: SideMenuCoordinatorType?

private let userSessionsService: UserSessionsService
let userSessionsService: UserSessionsService
Copy link
Contributor

Choose a reason for hiding this comment

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

We should try to keep it private and inject this instance from Coordinators to other Coordinators.
For places where we still can't inject instances such as UserSessionsService from side to side. The parent does not have the reference or the flow is started from ObjC class.
We can temporary define and use a shared instance like UserSessionsService.shared but we still continue to inject it from the outside like: CoordinatorParameters(userSessionsService: UserSessionsService.shared) to avoid having singleton inside Coordinators. And in AppCoordinator.init we can temporary use self.userSessionsService = UserSessionsService.shared until we can avoid singleton in the app.


/// Main user Matrix session
private var mainMatrixSession: MXSession? {
Expand Down Expand Up @@ -131,7 +131,7 @@ final class AppCoordinator: NSObject, AppCoordinatorType {

private func addSideMenu() {
let appInfo = AppInfo.current
let coordinatorParameters = SideMenuCoordinatorParameters(appNavigator: self.appNavigator, userSessionsService: self.userSessionsService, appInfo: appInfo)
let coordinatorParameters = SideMenuCoordinatorParameters(appNavigator: self.appNavigator, appCoordinator: self, userSessionsService: self.userSessionsService, appInfo: appInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

The AppCoordinator should not be injected, other coordinators should not know is existence. It's adding an unnecessary object relation and avoid reusability.


let coordinator = SideMenuCoordinator(parameters: coordinatorParameters)
coordinator.delegate = self
Expand Down
4 changes: 3 additions & 1 deletion Riot/Modules/People/PeopleViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ @interface PeopleViewController () <SpaceMembersCoordinatorBridgePresenterDelega
{
NSInteger directRoomsSectionNumber;
RecentsDataSource *recentsDataSource;
UserSessionsService *userSessionsService;
}

@property(nonatomic) SpaceMembersCoordinatorBridgePresenter *spaceMembersCoordinatorBridgePresenter;
Expand All @@ -50,6 +51,7 @@ - (void)finalizeInit
[super finalizeInit];

directRoomsSectionNumber = 0;
userSessionsService = [UserSessionsService new];

self.screenName = @"People";
}
Expand Down Expand Up @@ -123,7 +125,7 @@ - (void)onPlusButtonPressed
{
if (self.dataSource.currentSpace != nil)
{
self.spaceMembersCoordinatorBridgePresenter = [[SpaceMembersCoordinatorBridgePresenter alloc] initWithSession:self.mainSession spaceId:self.dataSource.currentSpace.spaceId];
self.spaceMembersCoordinatorBridgePresenter = [[SpaceMembersCoordinatorBridgePresenter alloc] initWithUserSessionsService:userSessionsService session:self.mainSession spaceId:self.dataSource.currentSpace.spaceId];
self.spaceMembersCoordinatorBridgePresenter.delegate = self;
[self.spaceMembersCoordinatorBridgePresenter presentFrom:self animated:YES];
}
Expand Down
6 changes: 5 additions & 1 deletion Riot/Modules/SideMenu/SideMenuCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@ import SafariServices

class SideMenuCoordinatorParameters {
let appNavigator: AppNavigatorProtocol
let appCoordinator: AppCoordinator
let userSessionsService: UserSessionsService
let appInfo: AppInfo

init(appNavigator: AppNavigatorProtocol,
appCoordinator: AppCoordinator,
userSessionsService: UserSessionsService,
appInfo: AppInfo) {
self.appNavigator = appNavigator
self.appCoordinator = appCoordinator
self.userSessionsService = userSessionsService
self.appInfo = appInfo
}
Expand Down Expand Up @@ -218,7 +221,8 @@ final class SideMenuCoordinator: NSObject, SideMenuCoordinatorType {
}

private func showMembers(spaceId: String, session: MXSession) {
let spaceMembersCoordinator = SpaceMembersCoordinator(session: session, spaceId: spaceId)
let parameters = SpaceMembersCoordinatorParameters(userSessionsService: parameters.appCoordinator.userSessionsService, session: session, spaceId: spaceId)
let spaceMembersCoordinator = SpaceMembersCoordinator(parameters: parameters)
spaceMembersCoordinator.delegate = self
let presentable = spaceMembersCoordinator.toPresentable()
presentable.presentationController?.delegate = self
Copy link
Contributor

Choose a reason for hiding this comment

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

Even if it's fine to let it here I think it can be interesting to put presentable.presentationController?.delegate = self inside SpaceMembersCoordinator. We will add this to the file template. And add a delegate method for this case. It will centralise this edge case and avoid to forget it for other screens.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,20 @@
import Foundation
import UIKit

struct SpaceMemberDetailCoordinatorParameters {
let userSessionsService: UserSessionsService
let member: MXRoomMember
let session: MXSession
let spaceId: String
}

final class SpaceMemberDetailCoordinator: NSObject, SpaceMemberDetailCoordinatorType {

// MARK: - Properties

// MARK: Private

private let session: MXSession
private let member: MXRoomMember
private let spaceId: String
private let parameters: SpaceMemberDetailCoordinatorParameters

private var spaceMemberDetailViewModel: SpaceMemberDetailViewModelType
private let spaceMemberDetailViewController: SpaceMemberDetailViewController
Expand All @@ -41,12 +46,10 @@ final class SpaceMemberDetailCoordinator: NSObject, SpaceMemberDetailCoordinator

// MARK: - Setup

init(session: MXSession, member: MXRoomMember, spaceId: String) {
self.session = session
self.member = member
self.spaceId = spaceId
init(parameters: SpaceMemberDetailCoordinatorParameters) {
self.parameters = parameters

let spaceMemberDetailViewModel = SpaceMemberDetailViewModel(session: self.session, member: self.member)
let spaceMemberDetailViewModel = SpaceMemberDetailViewModel(userSessionsService: parameters.userSessionsService, session: parameters.session, member: parameters.member, spaceId: parameters.spaceId)
let spaceMemberDetailViewController = SpaceMemberDetailViewController.instantiate(with: spaceMemberDetailViewModel)
spaceMemberDetailViewController.enableMention = true
spaceMemberDetailViewController.enableVoipCall = false
Expand All @@ -60,11 +63,6 @@ final class SpaceMemberDetailCoordinator: NSObject, SpaceMemberDetailCoordinator

func start() {
self.spaceMemberDetailViewModel.coordinatorDelegate = self
if let space = self.session.spaceService.getSpace(withId: spaceId) {
// Set delegate to handle action on member (start chat, mention)
self.spaceMemberDetailViewController.delegate = self
self.spaceMemberDetailViewController.display(self.member, withMatrixRoom: space.room)
}
}

func toPresentable() -> UIViewController {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import Foundation

/// SpaceMemberDetailViewController view actions exposed to view model
enum SpaceMemberDetailViewAction {
case loadData
case openRoom(_ roomId: String)
case createRoom(_ memberId: String)
case cancel
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ final class SpaceMemberDetailViewController: RoomMemberDetailsViewController {
return UINib(nibName: "RoomMemberDetailsViewController", bundle: Bundle(for: RoomMemberDetailsViewController.self))
}

private enum Constants {
static let aConstant: Int = 666
}

// MARK: - Properties

// MARK: Outlets
Expand Down Expand Up @@ -67,6 +63,9 @@ final class SpaceMemberDetailViewController: RoomMemberDetailsViewController {
self.update(theme: self.theme)

self.viewModel.viewDelegate = self
self.viewModel.process(viewAction: .loadData)

self.delegate = self
}

override func viewWillAppear(_ animated: Bool) {
Expand Down Expand Up @@ -118,8 +117,8 @@ final class SpaceMemberDetailViewController: RoomMemberDetailsViewController {
switch viewState {
case .loading:
self.renderLoading()
case .loaded:
self.renderLoaded()
case .loaded(let member, let space):
self.renderLoaded(member: member, space: space)
case .error(let error):
self.render(error: error)
}
Expand All @@ -129,7 +128,8 @@ final class SpaceMemberDetailViewController: RoomMemberDetailsViewController {
self.activityPresenter.presentActivityIndicator(on: self.view, animated: true)
}

private func renderLoaded() {
private func renderLoaded(member: MXRoomMember, space: MXRoom?) {
self.display(member, withMatrixRoom: space)
self.activityPresenter.removeCurrentActivityIndicator(animated: true)
}

Expand Down Expand Up @@ -159,3 +159,13 @@ extension SpaceMemberDetailViewController: SpaceMemberDetailViewModelViewDelegat
self.render(viewState: viewSate)
}
}

// MARK: - MXKRoomMemberDetailsViewControllerDelegate
extension SpaceMemberDetailViewController: MXKRoomMemberDetailsViewControllerDelegate {

func roomMemberDetailsViewController(_ roomMemberDetailsViewController: MXKRoomMemberDetailsViewController!, startChatWithMemberId memberId: String!, completion: (() -> Void)!) {
completion()
self.viewModel.process(viewAction: .createRoom(memberId))
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@

import Foundation

final class SpaceMemberDetailViewModel: SpaceMemberDetailViewModelType {
final class SpaceMemberDetailViewModel: NSObject, SpaceMemberDetailViewModelType {

// MARK: - Properties

// MARK: Private

private let userSessionsService: UserSessionsService
private let session: MXSession
private let member: MXRoomMember
private let spaceId: String
private var space: MXSpace?

private var currentOperation: MXHTTPOperation?

Expand All @@ -36,9 +39,11 @@ final class SpaceMemberDetailViewModel: SpaceMemberDetailViewModelType {

// MARK: - Setup

init(session: MXSession, member: MXRoomMember) {
init(userSessionsService: UserSessionsService, session: MXSession, member: MXRoomMember, spaceId: String) {
self.userSessionsService = userSessionsService
self.session = session
self.member = member
self.spaceId = spaceId
}

deinit {
Expand All @@ -49,6 +54,8 @@ final class SpaceMemberDetailViewModel: SpaceMemberDetailViewModelType {

func process(viewAction: SpaceMemberDetailViewAction) {
switch viewAction {
case .loadData:
self.loadData()
case .openRoom(let roomId):
self.coordinatorDelegate?.spaceMemberDetailViewModel(self, showRoomWithId: roomId)
case .createRoom(let memberId):
Expand All @@ -61,47 +68,50 @@ final class SpaceMemberDetailViewModel: SpaceMemberDetailViewModelType {

// MARK: - Private

private func loadData() {
self.space = self.session.spaceService.getSpace(withId: self.spaceId)
self.update(viewState: .loaded(self.member, self.space?.room))
}

private func update(viewState: SpaceMemberDetailViewState) {
self.viewDelegate?.spaceMemberDetailViewModel(self, didUpdateViewState: viewState)
}

private func createDirectRoom(forMemberWithId memberId: String) {
self.update(viewState: .loading)
AppDelegate.theDelegate().selectMatrixAccount { account in
guard let account = account, let session = account.mxSession else {
self.update(viewState: .loaded)
return
guard let account = self.userSessionsService.mainUserSession?.account, let session = account.mxSession else {
self.update(viewState: .loaded(self.member, self.space?.room))
return
}

let invite: [String]? = (session.myUserId != memberId) ? [memberId] : nil
self.currentOperation = session.vc_canEnableE2EByDefaultInNewRoom(withUsers: invite) { canEnableE2E in
self.currentOperation = nil
let roomCreationParameters = MXRoomCreationParameters()
roomCreationParameters.visibility = kMXRoomDirectoryVisibilityPrivate
roomCreationParameters.inviteArray = invite
roomCreationParameters.isDirect = !(invite?.isEmpty ?? true)
roomCreationParameters.preset = kMXRoomPresetTrustedPrivateChat

if canEnableE2E {
roomCreationParameters.initialStateEvents = [MXRoomCreationParameters.initialStateEventForEncryption(withAlgorithm: kMXCryptoMegolmAlgorithm)]
}

let invite: [String]? = (session.myUserId != memberId) ? [memberId] : nil
self.currentOperation = session.vc_canEnableE2EByDefaultInNewRoom(withUsers: invite) { canEnableE2E in
self.currentOperation = session.createRoom(parameters: roomCreationParameters) { response in
self.currentOperation = nil
let roomCreationParameters = MXRoomCreationParameters()
roomCreationParameters.visibility = kMXRoomDirectoryVisibilityPrivate
roomCreationParameters.inviteArray = invite
roomCreationParameters.isDirect = !(invite?.isEmpty ?? true)
roomCreationParameters.preset = kMXRoomPresetTrustedPrivateChat

if canEnableE2E {
roomCreationParameters.initialStateEvents = [MXRoomCreationParameters.initialStateEventForEncryption(withAlgorithm: kMXCryptoMegolmAlgorithm)]
}

self.currentOperation = session.createRoom(parameters: roomCreationParameters) { response in
self.currentOperation = nil
self.update(viewState: .loaded)
guard response.isSuccess, let room = response.value else {
if let error = response.error {
self.update(viewState: .error(error))
}
return
self.update(viewState: .loaded(self.member, self.space?.room))
guard response.isSuccess, let room = response.value else {
if let error = response.error {
self.update(viewState: .error(error))
}
self.coordinatorDelegate?.spaceMemberDetailViewModel(self, showRoomWithId: room.roomId)
}
} failure: { error in
self.update(viewState: .loaded)
if let error = error {
self.update(viewState: .error(error))
return
}
self.coordinatorDelegate?.spaceMemberDetailViewModel(self, showRoomWithId: room.roomId)
}
} failure: { error in
self.update(viewState: .loaded(self.member, self.space?.room))
if let error = error {
self.update(viewState: .error(error))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ import Foundation
/// SpaceMemberDetailViewController view state
enum SpaceMemberDetailViewState {
case loading
case loaded
case loaded(MXRoomMember, MXRoom?)
case error(Error)
}
23 changes: 14 additions & 9 deletions Riot/Modules/Spaces/SpaceMembers/SpaceMembersCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,21 @@

import UIKit

struct SpaceMembersCoordinatorParameters {
let userSessionsService: UserSessionsService
let session: MXSession
let spaceId: String
}

@objcMembers
final class SpaceMembersCoordinator: SpaceMembersCoordinatorType {

// MARK: - Properties

// MARK: Private

private let parameters: SpaceMembersCoordinatorParameters
private let navigationRouter: NavigationRouterType
private let session: MXSession
private let spaceId: String
private weak var memberDetailCoordinator: SpaceMemberDetailCoordinator?

// MARK: Public
Expand All @@ -39,11 +44,10 @@ final class SpaceMembersCoordinator: SpaceMembersCoordinatorType {

// MARK: - Setup

init(session: MXSession, spaceId: String) {
init(parameters: SpaceMembersCoordinatorParameters) {
self.parameters = parameters
self.navigationRouter = NavigationRouter(navigationController: RiotNavigationController())
self.session = session
self.spaceId = spaceId
}
}

// MARK: - Public methods

Expand Down Expand Up @@ -89,19 +93,20 @@ final class SpaceMembersCoordinator: SpaceMembersCoordinatorType {
// MARK: - Private methods

private func createSpaceMemberListCoordinator() -> SpaceMemberListCoordinator {
let coordinator = SpaceMemberListCoordinator(session: self.session, spaceId: self.spaceId)
let coordinator = SpaceMemberListCoordinator(session: self.parameters.session, spaceId: self.parameters.spaceId)
coordinator.delegate = self
return coordinator
}

private func createSpaceMemberDetailCoordinator(with member: MXRoomMember) -> SpaceMemberDetailCoordinator {
let coordinator = SpaceMemberDetailCoordinator(session: self.session, member: member, spaceId: self.spaceId)
let parameters = SpaceMemberDetailCoordinatorParameters(userSessionsService: self.parameters.userSessionsService, member: member, session: self.parameters.session, spaceId: self.parameters.spaceId)
let coordinator = SpaceMemberDetailCoordinator(parameters: parameters)
coordinator.delegate = self
return coordinator
}

private func navigateTo(roomWith roomId: String) {
let roomDataSourceManager = MXKRoomDataSourceManager.sharedManager(forMatrixSession: self.session)
let roomDataSourceManager = MXKRoomDataSourceManager.sharedManager(forMatrixSession: self.parameters.session)
roomDataSourceManager?.roomDataSource(forRoom: roomId, create: true, onComplete: { [weak self] roomDataSource in

let storyboard = UIStoryboard(name: "Main", bundle: Bundle.main)
Expand Down
Loading