MM-24455: Fix role responses. (#17385)

Automatic Merge

app/app_iface.go

@@ -541,7 +541,6 @@ type AppIface interface {
 	GetAllPrivateTeams() ([]*model.Team, *model.AppError)
 	GetAllPublicTeams() ([]*model.Team, *model.AppError)
 	GetAllRemoteClusters(filter model.RemoteClusterQueryFilter) ([]*model.RemoteCluster, *model.AppError)
-	GetAllRoles() ([]*model.Role, *model.AppError)
 	GetAllStatuses() map[string]*model.Status
 	GetAllTeams() ([]*model.Team, *model.AppError)
 	GetAllTeamsPage(offset int, limit int, opts *model.TeamSearch) ([]*model.Team, *model.AppError)

app/opentracing/opentracing_layer.go

@@ -4412,28 +4412,6 @@ func (a *OpenTracingAppLayer) GetAllRemoteClusters(filter model.RemoteClusterQue
 	return resultVar0, resultVar1
 }
 
-func (a *OpenTracingAppLayer) GetAllRoles() ([]*model.Role, *model.AppError) {
-	origCtx := a.ctx
-	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetAllRoles")
-
-	a.ctx = newCtx
-	a.app.Srv().Store.SetContext(newCtx)
-	defer func() {
-		a.app.Srv().Store.SetContext(origCtx)
-		a.ctx = origCtx
-	}()
-
-	defer span.Finish()
-	resultVar0, resultVar1 := a.app.GetAllRoles()
-
-	if resultVar1 != nil {
-		span.LogFields(spanlog.Error(resultVar1))
-		ext.Error.Set(span, true)
-	}
-
-	return resultVar0, resultVar1
-}
-
 func (a *OpenTracingAppLayer) GetAllStatuses() map[string]*model.Status {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetAllStatuses")

app/permissions_migrations.go

@@ -924,7 +924,7 @@ func (a *App) DoPermissionsMigrations() error {
 		{Key: model.MIGRATION_KEY_ADD_REPORTING_SUBSECTION_PERMISSIONS, Migration: a.getAddReportingSubsectionPermissions},
 	}
 
-	roles, err := a.GetAllRoles()
+	roles, err := a.srv.Store.Role().GetAll()
 	if err != nil {
 		return err
 	}

app/role.go

@@ -27,16 +27,12 @@ func (a *App) GetRole(id string) (*model.Role, *model.AppError) {
 		}
 	}
 
-	return role, nil
+	appErr := a.Srv().mergeChannelHigherScopedPermissions([]*model.Role{role})
-}
+	if appErr != nil {
-
+		return nil, appErr
-func (a *App) GetAllRoles() ([]*model.Role, *model.AppError) {
-	roles, err := a.Srv().Store.Role().GetAll()
-	if err != nil {
-		return nil, model.NewAppError("GetAllRoles", "app.role.get_all.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
-	return roles, nil
+	return role, nil
 }
 
 func (s *Server) GetRoleByName(ctx context.Context, name string) (*model.Role, *model.AppError) {

app/role_test.go

@@ -51,6 +51,16 @@ func TestGetRoleByName(t *testing.T) {
 	})
 }
 
+func TestGetRoleByID(t *testing.T) {
+	testPermissionInheritance(t, func(t *testing.T, th *TestHelper, testData permissionInheritanceTestData) {
+		actualRole, err := th.App.GetRole(testData.channelRole.Id)
+		require.Nil(t, err)
+		require.NotNil(t, actualRole)
+		require.Equal(t, testData.channelRole.Id, actualRole.Id)
+		require.Equal(t, testData.shouldHavePermission, utils.StringInSlice(testData.permission.Id, actualRole.Permissions), "row: %+v", testData.truthTableRow)
+	})
+}
+
 // testPermissionInheritance tests 48 combinations of scheme, permission, role data.
 func testPermissionInheritance(t *testing.T, testCallback func(t *testing.T, th *TestHelper, testData permissionInheritanceTestData)) {
 	th := Setup(t).InitBasic()

i18n/en.json

@@ -5938,10 +5938,6 @@
     "id": "app.role.get.app_error",
     "translation": "Unable to get role."
   },
-  {
-    "id": "app.role.get_all.app_error",
-    "translation": "Unable to get all the roles."
-  },
   {
     "id": "app.role.get_by_name.app_error",
     "translation": "Unable to get role."