From 5c75c265af78c7ee3e186d0cdd22bced5c2729bf Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Thu, 4 Apr 2024 11:26:44 -0400 Subject: [PATCH] fix(permissions): fix hang when all channel perm check return (#14259) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(permissions): fix hang when all channel perm check return Fixes #14234 The problem was that we updated **all** the models from **all** the channels of a community each time the channel requirement checks returned. The fix is to first of all, make sure we don't call that check too often. It sometimes got called twice in a row by accident. The other better fix is to check if anything actually changed before updating. This solves the issue almost entirely. Since the permissions almost never change, the updates now take only a second. * fix(permisisons): never run permission checks for privileged users Also fixes #14234 but for admins, TMs and Owners. Admins+ were still getting the hang, because the permission checks always returned something different than the models, because the models knew that admins have access to everything, but the permission check was running as if it were a normal user (I think, un-tested). Anyway, the solution is more simple, we never need to run the permission checks on admins+, because they always have access to everything! * fix(Communities): prevent channels model from emitting unnecessary signals Closes: #14274 * chore(Communities): improve channels metadata lookup performance ChannelsSelectionModel is removed, replaced with plain LeftJoinModel. Transformations of left-side model are done in a single place, not in every delegate making the join. * only call update functions when there is something to update + move permission model creation when needed --------- Co-authored-by: Michał Cieślak --- .../modules/main/chat_section/controller.nim | 13 ++++- src/app/modules/main/chat_section/model.nim | 16 ++++++ src/app/modules/main/chat_section/module.nim | 40 +++++++++++--- src/app/modules/main/communities/module.nim | 7 +++ .../panels/PermissionsSettingsPanel.qml | 44 ++++++++++++++- .../views/ChannelsSelectionModel.qml | 55 ------------------- .../Communities/views/CommunityColumnView.qml | 10 ++-- .../Communities/views/EditPermissionView.qml | 8 ++- .../Communities/views/PermissionsView.qml | 17 +++--- ui/app/AppLayouts/Communities/views/qmldir | 5 +- 10 files changed, 128 insertions(+), 87 deletions(-) delete mode 100644 ui/app/AppLayouts/Communities/views/ChannelsSelectionModel.qml diff --git a/src/app/modules/main/chat_section/controller.nim b/src/app/modules/main/chat_section/controller.nim index 55d8d7a4fa..fe682b60fc 100644 --- a/src/app/modules/main/chat_section/controller.nim +++ b/src/app/modules/main/chat_section/controller.nim @@ -30,6 +30,7 @@ type isCommunitySection: bool activeItemId: string isCurrentSectionActive: bool + allChannelsPermissionCheckOngoing: bool events: UniqueUUIDEventEmitter settingsService: settings_service.Service nodeConfigurationService: node_configuration_service.Service @@ -45,6 +46,8 @@ type sharedUrlsService: shared_urls_service.Service networkService: network_service.Service +# Forward declarations +proc getMyCommunity*(self: Controller): CommunityDto proc newController*(delegate: io_interface.AccessInterface, sectionId: string, isCommunity: bool, events: EventEmitter, settingsService: settings_service.Service, nodeConfigurationService: node_configuration_service.Service, @@ -96,8 +99,9 @@ proc asyncCheckPermissionsToJoin*(self: Controller) = self.delegate.setPermissionsToJoinCheckOngoing(true) proc asyncCheckAllChannelsPermissions*(self: Controller) = - if self.delegate.getPermissionsToJoinCheckOngoing(): + if self.allChannelsPermissionCheckOngoing: return + self.allChannelsPermissionCheckOngoing = true self.chatService.asyncCheckAllChannelsPermissions(self.getMySectionId(), addresses = @[]) self.delegate.setChannelsPermissionsCheckOngoing(true) @@ -105,6 +109,9 @@ proc asyncCheckChannelPermissions*(self: Controller, communityId: string, chatId self.chatService.asyncCheckChannelPermissions(communityId, chatId) proc asyncCheckPermissions*(self: Controller) = + let community = self.getMyCommunity() + if community.isPrivilegedUser: + return self.asyncCheckPermissionsToJoin() self.asyncCheckAllChannelsPermissions() @@ -309,12 +316,14 @@ proc init*(self: Controller) = self.events.on(SIGNAL_CHECK_ALL_CHANNELS_PERMISSIONS_RESPONSE) do(e: Args): let args = CheckAllChannelsPermissionsResponseArgs(e) if args.communityId == self.sectionId: + self.allChannelsPermissionCheckOngoing = false self.delegate.onCommunityCheckAllChannelsPermissionsResponse(args.checkAllChannelsPermissionsResponse) self.events.on(SIGNAL_CHECK_ALL_CHANNELS_PERMISSIONS_FAILED) do(e: Args): let args = CheckChannelsPermissionsErrorArgs(e) if args.communityId == self.sectionId: - self.delegate.setPermissionsToJoinCheckOngoing(false) + self.allChannelsPermissionCheckOngoing = false + self.delegate.setPermissionsToJoinCheckOngoing(false) self.events.on(SIGNAL_WAITING_ON_NEW_COMMUNITY_OWNER_TO_CONFIRM_REQUEST_TO_REJOIN) do(e: Args): let args = CommunityIdArgs(e) diff --git a/src/app/modules/main/chat_section/model.nim b/src/app/modules/main/chat_section/model.nim index 79c38991b4..9b6bed5ed0 100644 --- a/src/app/modules/main/chat_section/model.nim +++ b/src/app/modules/main/chat_section/model.nim @@ -361,6 +361,10 @@ QtObject: let index = self.getItemIdxById(id) if index == -1: return + + if (self.items[index].isLocked == locked): + return + self.items[index].locked = locked let modelIndex = self.createIndex(index, 0, nil) defer: modelIndex.delete @@ -370,6 +374,10 @@ QtObject: let index = self.getItemIdxById(id) if index == -1: return + + if (self.items[index].viewOnlyPermissionsSatisfied == satisfied): + return + self.items[index].viewOnlyPermissionsSatisfied = satisfied let modelIndex = self.createIndex(index, 0, nil) defer: modelIndex.delete @@ -381,6 +389,10 @@ QtObject: let index = self.getItemIdxById(id) if index == -1: return + + if (self.items[index].viewAndPostPermissionsSatisfied == satisfied): + return + self.items[index].viewAndPostPermissionsSatisfied = satisfied let modelIndex = self.createIndex(index, 0, nil) defer: modelIndex.delete @@ -392,6 +404,10 @@ QtObject: let index = self.getItemIdxById(id) if index == -1: return + + if (self.items[index].requiresPermissions == value): + return + self.items[index].requiresPermissions = value let modelIndex = self.createIndex(index, 0, nil) defer: modelIndex.delete diff --git a/src/app/modules/main/chat_section/module.nim b/src/app/modules/main/chat_section/module.nim index 6fd395c020..060ab72290 100644 --- a/src/app/modules/main/chat_section/module.nim +++ b/src/app/modules/main/chat_section/module.nim @@ -604,7 +604,9 @@ method onActiveSectionChange*(self: Module, sectionId: string) = self.controller.setIsCurrentSectionActive(false) return + var firstLoad = false if not self.view.getChatsLoaded: + firstLoad = true self.controller.getChatsAndBuildUI() self.controller.setIsCurrentSectionActive(true) @@ -618,7 +620,8 @@ method onActiveSectionChange*(self: Module, sectionId: string) = let community = self.controller.getMyCommunity() if not community.isPrivilegedUser: self.controller.asyncCheckPermissionsToJoin() - self.controller.asyncCheckAllChannelsPermissions() + if firstLoad: + self.controller.asyncCheckAllChannelsPermissions() self.delegate.onActiveChatChange(self.controller.getMySectionId(), self.controller.getActiveChatId()) @@ -924,7 +927,9 @@ method onCommunityTokenPermissionCreated*(self: Module, communityId: string, tok if tokenPermission.state == TokenPermissionState.Approved: singletonInstance.globalEvents.showCommunityTokenPermissionCreatedNotification(communityId, "Community permission created", "A token permission has been added") -proc updateTokenPermissionModel*(self: Module, permissions: Table[string, CheckPermissionsResultDto], community: CommunityDto) = +# Returns true if there was an update +proc updateTokenPermissionModel*(self: Module, permissions: Table[string, CheckPermissionsResultDto], community: CommunityDto): bool = + var thereWasAnUpdate = false for id, criteriaResult in permissions: if community.tokenPermissions.hasKey(id): let tokenPermissionItem = self.view.tokenPermissionsModel.getItemById(id) @@ -935,6 +940,10 @@ proc updateTokenPermissionModel*(self: Module, permissions: Table[string, CheckP var permissionSatisfied = true for index, tokenCriteriaItem in tokenPermissionItem.getTokenCriteria().getItems(): + let criteriaMet = criteriaResult.criteria[index] + + if tokenCriteriaItem.criteriaMet == criteriaMet: + continue let updatedTokenCriteriaItem = initTokenCriteriaItem( tokenCriteriaItem.symbol, @@ -950,6 +959,10 @@ proc updateTokenPermissionModel*(self: Module, permissions: Table[string, CheckP updatedTokenCriteriaItems.add(updatedTokenCriteriaItem) + if updatedTokenCriteriaItems.len == 0: + continue + + thereWasAnUpdate = true let updatedTokenPermissionItem = initTokenPermissionItem( tokenPermissionItem.id, tokenPermissionItem.`type`, @@ -961,6 +974,8 @@ proc updateTokenPermissionModel*(self: Module, permissions: Table[string, CheckP ) self.view.tokenPermissionsModel().updateItem(id, updatedTokenPermissionItem) + return thereWasAnUpdate + proc updateCommunityPermissionsView*(self: Module) = let tokenPermissionsItems = self.view.tokenPermissionsModel().getItems() @@ -994,11 +1009,20 @@ proc updateCommunityPermissionsView*(self: Module) = self.view.setAllTokenRequirementsMet(tokenRequirementsMet) self.view.setRequiresTokenPermissionToJoin(requiresPermissionToJoin) -proc updateChannelPermissionViewData*(self: Module, chatId: string, viewOnlyPermissions: ViewOnlyOrViewAndPostPermissionsResponseDto, viewAndPostPermissions: ViewOnlyOrViewAndPostPermissionsResponseDto, community: CommunityDto) = - self.updateTokenPermissionModel(viewOnlyPermissions.permissions, community) - self.updateTokenPermissionModel(viewAndPostPermissions.permissions, community) - self.updateChatRequiresPermissions(chatId) - self.updateChatLocked(chatId) +proc updateChannelPermissionViewData*( + self: Module, + chatId: string, + viewOnlyPermissions: ViewOnlyOrViewAndPostPermissionsResponseDto, + viewAndPostPermissions: ViewOnlyOrViewAndPostPermissionsResponseDto, + community: CommunityDto + ) = + + let viewOnlyUpdated = self.updateTokenPermissionModel(viewOnlyPermissions.permissions, community) + let viewAndPostUpdated = self.updateTokenPermissionModel(viewAndPostPermissions.permissions, community) + if viewOnlyUpdated or viewAndPostUpdated: + self.updateChatRequiresPermissions(chatId) + self.updateChatLocked(chatId) + if self.chatContentModules.hasKey(chatId): self.chatContentModules[chatId].onUpdateViewOnlyPermissionsSatisfied(viewOnlyPermissions.satisfied) self.chatContentModules[chatId].onUpdateViewAndPostPermissionsSatisfied(viewAndPostPermissions.satisfied) @@ -1010,7 +1034,7 @@ proc updateChannelPermissionViewData*(self: Module, chatId: string, viewOnlyPerm method onCommunityCheckPermissionsToJoinResponse*(self: Module, checkPermissionsToJoinResponse: CheckPermissionsToJoinResponseDto) = let community = self.controller.getMyCommunity() self.view.setAllTokenRequirementsMet(checkPermissionsToJoinResponse.satisfied) - self.updateTokenPermissionModel(checkPermissionsToJoinResponse.permissions, community) + discard self.updateTokenPermissionModel(checkPermissionsToJoinResponse.permissions, community) self.updateCommunityPermissionsView() self.setPermissionsToJoinCheckOngoing(false) diff --git a/src/app/modules/main/communities/module.nim b/src/app/modules/main/communities/module.nim index 2d6f946035..5bc9b1cc6f 100644 --- a/src/app/modules/main/communities/module.nim +++ b/src/app/modules/main/communities/module.nim @@ -905,6 +905,11 @@ proc applyPermissionResponse*(self: Module, communityId: string, permissions: Ta var permissionSatisfied = true for index, tokenCriteriaItem in tokenPermissionItem.getTokenCriteria().getItems(): + let criteriaMet = criteriaResult.criteria[index] + + if tokenCriteriaItem.criteriaMet == criteriaMet: + continue + let updatedTokenCriteriaItem = initTokenCriteriaItem( tokenCriteriaItem.symbol, @@ -920,6 +925,8 @@ proc applyPermissionResponse*(self: Module, communityId: string, permissions: Ta updatedTokenCriteriaItems.add(updatedTokenCriteriaItem) + if updatedTokenCriteriaItems.len == 0: + continue let updatedTokenPermissionItem = initTokenPermissionItem( tokenPermissionItem.id, tokenPermissionItem.`type`, diff --git a/ui/app/AppLayouts/Communities/panels/PermissionsSettingsPanel.qml b/ui/app/AppLayouts/Communities/panels/PermissionsSettingsPanel.qml index 1d339e8111..9646e00f1f 100644 --- a/ui/app/AppLayouts/Communities/panels/PermissionsSettingsPanel.qml +++ b/ui/app/AppLayouts/Communities/panels/PermissionsSettingsPanel.qml @@ -5,13 +5,16 @@ import AppLayouts.Communities.controls 1.0 import AppLayouts.Communities.layouts 1.0 import AppLayouts.Communities.views 1.0 -import StatusQ.Core 0.1 +import StatusQ 0.1 import StatusQ.Controls 0.1 +import StatusQ.Core 0.1 import StatusQ.Core.Utils 0.1 import utils 1.0 import shared.popups 1.0 +import SortFilterProxyModel 0.2 + StackView { id: root @@ -46,6 +49,40 @@ StackView { root.push(newPermissionView, properties, StackView.Immediate); } + SortFilterProxyModel { + id: allChannelsTransformed + + sourceModel: root.channelsModel + + proxyRoles: [ + FastExpressionRole { + name: "key" + expression: model.itemId ?? "" + expectedRoles: ["itemId"] + }, + FastExpressionRole { + name: "text" + expression: "#" + model.name + expectedRoles: ["name"] + }, + FastExpressionRole { + name: "imageSource" + expression: model.icon + expectedRoles: ["icon"] + }, + FastExpressionRole { + name: "operator" + + // Direct call for singleton enum is not handled properly by SortFilterProxyModel. + readonly property int none: OperatorsUtils.Operators.None + + expression: none + expectedRoles: [] + } + ] + } + + // Community Permissions possible view contents: initialItem: SettingsPage { id: initialItem @@ -70,7 +107,8 @@ StackView { permissionsModel: root.permissionsModel assetsModel: root.assetsModel collectiblesModel: root.collectiblesModel - channelsModel: root.channelsModel + channelsModel: allChannelsTransformed + communityDetails: root.communityDetails viewWidth: root.viewWidth @@ -148,7 +186,7 @@ StackView { assetsModel: root.assetsModel collectiblesModel: root.collectiblesModel - channelsModel: root.channelsModel + channelsModel: allChannelsTransformed communityDetails: root.communityDetails showChannelSelector: root.showChannelSelector isEditState: newPermissionViewPage.isEditState diff --git a/ui/app/AppLayouts/Communities/views/ChannelsSelectionModel.qml b/ui/app/AppLayouts/Communities/views/ChannelsSelectionModel.qml deleted file mode 100644 index 8059f87e47..0000000000 --- a/ui/app/AppLayouts/Communities/views/ChannelsSelectionModel.qml +++ /dev/null @@ -1,55 +0,0 @@ -import QtQml 2.15 -import SortFilterProxyModel 0.2 - -import StatusQ.Core.Utils 0.1 -import StatusQ.Core.Theme 0.1 -import StatusQ 0.1 - -import utils 1.0 - -SortFilterProxyModel { - id: root - - property var selectedChannels - property var allChannels - - property var jointModel: LeftJoinModel { - readonly property var channelsModelAlignedKey: SortFilterProxyModel { - sourceModel: root.allChannels - proxyRoles: [ - FastExpressionRole { - name: "key" - expression: model.itemId ?? "" - expectedRoles: ["itemId"] - } - ] - } - leftModel: root.selectedChannels - rightModel: channelsModelAlignedKey - joinRole: "key" - } - - sourceModel: jointModel - - proxyRoles: [ - FastExpressionRole { - name: "text" - expression: "#" + model.name - expectedRoles: ["name"] - }, - FastExpressionRole { - name: "imageSource" - expression: model.icon - expectedRoles: ["icon"] - }, - FastExpressionRole { - name: "operator" - - // Direct call for singleton enum is not handled properly by SortFilterProxyModel. - readonly property int none: OperatorsUtils.Operators.None - - expression: none - expectedRoles: [] - } - ] -} diff --git a/ui/app/AppLayouts/Communities/views/CommunityColumnView.qml b/ui/app/AppLayouts/Communities/views/CommunityColumnView.qml index d23cca066b..b91d0c1eab 100644 --- a/ui/app/AppLayouts/Communities/views/CommunityColumnView.qml +++ b/ui/app/AppLayouts/Communities/views/CommunityColumnView.qml @@ -55,11 +55,6 @@ Item { communityData.memberRole === Constants.memberRole.admin || communityData.memberRole === Constants.memberRole.tokenMaster - readonly property var permissionsModel: { - root.store.prepareTokenModelForCommunity(communityData.id) - return root.store.permissionsModel - } - signal infoButtonClicked signal manageButtonClicked @@ -544,7 +539,10 @@ Item { communitiesStore: root.communitiesStore assetsModel: root.store.assetsModel collectiblesModel: root.store.collectiblesModel - permissionsModel: root.store.permissionsModel + permissionsModel: { + root.store.prepareTokenModelForCommunity(communityData.id) + return root.store.permissionsModel + } channelsModel: root.store.chatCommunitySectionModule.model emojiPopup: root.emojiPopup activeCommunity: root.communityData diff --git a/ui/app/AppLayouts/Communities/views/EditPermissionView.qml b/ui/app/AppLayouts/Communities/views/EditPermissionView.qml index 4738d75ea9..f6ad334113 100644 --- a/ui/app/AppLayouts/Communities/views/EditPermissionView.qml +++ b/ui/app/AppLayouts/Communities/views/EditPermissionView.qml @@ -2,6 +2,7 @@ import QtQuick 2.15 import QtQuick.Layouts 1.15 import QtQml 2.15 +import StatusQ 0.1 import StatusQ.Core 0.1 import StatusQ.Core.Theme 0.1 import StatusQ.Components 0.1 @@ -516,11 +517,12 @@ StatusScrollView { } } - ChannelsSelectionModel { + LeftJoinModel { id: channelsSelectionModel - selectedChannels: d.dirtyValues.selectedChannelsModel - allChannels: root.channelsModel + leftModel: d.dirtyValues.selectedChannelsModel + rightModel: root.channelsModel + joinRole: "key" } InDropdown { diff --git a/ui/app/AppLayouts/Communities/views/PermissionsView.qml b/ui/app/AppLayouts/Communities/views/PermissionsView.qml index ce945ee0c2..30433c7e15 100644 --- a/ui/app/AppLayouts/Communities/views/PermissionsView.qml +++ b/ui/app/AppLayouts/Communities/views/PermissionsView.qml @@ -1,8 +1,9 @@ -import QtQuick 2.14 -import QtQuick.Layouts 1.14 +import QtQuick 2.15 +import QtQuick.Layouts 1.15 -import StatusQ.Core 0.1 +import StatusQ 0.1 import StatusQ.Controls 0.1 +import StatusQ.Core 0.1 import SortFilterProxyModel 0.2 import shared.status 1.0 @@ -70,6 +71,7 @@ ColumnLayout { ] } + Repeater { id: repeater @@ -87,14 +89,15 @@ ColumnLayout { permissionType: model.permissionType permissionState: model.permissionState // TODO: Backend! - ChannelsSelectionModel { + LeftJoinModel { id: channelsSelectionModel - selectedChannels: model.channelsListModel ?? null - allChannels: root.channelsModel + leftModel: model.channelsListModel ?? null + rightModel: root.channelsModel + joinRole: "key" } - channelsListModel: channelsSelectionModel.count + channelsListModel: model.channelsListModel.rowCount() ? channelsSelectionModel : communityItemModel isPrivate: model.isPrivate diff --git a/ui/app/AppLayouts/Communities/views/qmldir b/ui/app/AppLayouts/Communities/views/qmldir index 8168429827..f02eed61f6 100644 --- a/ui/app/AppLayouts/Communities/views/qmldir +++ b/ui/app/AppLayouts/Communities/views/qmldir @@ -1,14 +1,13 @@ BannedMemberCommunityView 1.0 BannedMemberCommunityView.qml -ChannelsSelectionModel 1.0 ChannelsSelectionModel.qml -CommunityColumnView 1.0 CommunityColumnView.qml CommunitiesGridView 1.0 CommunitiesGridView.qml +CommunityColumnView 1.0 CommunityColumnView.qml CommunitySettingsView 1.0 CommunitySettingsView.qml CommunityTokenView 1.0 CommunityTokenView.qml ControlNodeOfflineCommunityView 1.0 ControlNodeOfflineCommunityView.qml EditAirdropView 1.0 EditAirdropView.qml -EditPermissionView 1.0 EditPermissionView.qml EditCommunityTokenView 1.0 EditCommunityTokenView.qml EditOwnerTokenView 1.0 EditOwnerTokenView.qml +EditPermissionView 1.0 EditPermissionView.qml HoldingsSelectionModel 1.0 HoldingsSelectionModel.qml JoinCommunityView 1.0 JoinCommunityView.qml MintedTokensView 1.0 MintedTokensView.qml