[ABAC - Table Editor] Improvements on table editor and review feedback (#31125)

* reflect review comments

* update table editor

* adjust test limits

* reflect review comments

* MM-64376

* resolve conflicts

* address review comments

* fix merge conflict error

server/channels/api4/access_control.go

@@ -337,7 +337,7 @@ func unassignAccessPolicy(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 
 	if len(assignments.ChannelIds) != 0 {
-		appErr := c.App.UnAssignPoliciesFromChannels(c.AppContext, policyID, assignments.ChannelIds)
+		appErr := c.App.UnassignPoliciesFromChannels(c.AppContext, policyID, assignments.ChannelIds)
 		if appErr != nil {
 			c.Err = appErr
 			return

server/channels/app/access_control.go

@@ -30,6 +30,7 @@ func (a *App) GetChannelsForPolicy(rctx request.CTX, policyID string, cursor mod
 		}
 		channelIDs := make([]string, 0, len(policies))
 
+		// channel IDs are the same as policy IDs
 		for _, p := range policies {
 			channelIDs = append(channelIDs, p.ID)
 		}
@@ -173,10 +174,10 @@ func (a *App) AssignAccessControlPolicyToChannels(rctx request.CTX, parentID str
 	return policies, nil
 }
 
-func (a *App) UnAssignPoliciesFromChannels(rctx request.CTX, policyID string, channelIDs []string) *model.AppError {
+func (a *App) UnassignPoliciesFromChannels(rctx request.CTX, policyID string, channelIDs []string) *model.AppError {
 	acs := a.Srv().ch.AccessControl
 	if acs == nil {
-		return model.NewAppError("UnAssignPoliciesFromChannels", "app.pap.unassign_access_control_policy_from_channels.app_error", nil, "Policy Administration Point is not initialized", http.StatusNotImplemented)
+		return model.NewAppError("UnassignPoliciesFromChannels", "app.pap.unassign_access_control_policy_from_channels.app_error", nil, "Policy Administration Point is not initialized", http.StatusNotImplemented)
 	}
 
 	cps, _, err := a.Srv().Store().AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{
@@ -184,7 +185,7 @@ func (a *App) UnAssignPoliciesFromChannels(rctx request.CTX, policyID string, ch
 		ParentID: policyID,
 	})
 	if err != nil {
-		return model.NewAppError("UnAssignPoliciesFromChannels", "app.pap.unassign_access_control_policy_from_channels.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return model.NewAppError("UnassignPoliciesFromChannels", "app.pap.unassign_access_control_policy_from_channels.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	childPolicies := make(map[string]bool)

server/channels/app/access_control_test.go

@@ -399,7 +399,7 @@ func TestUnAssignPoliciesFromChannels(t *testing.T) {
 
 	t.Run("Feature not enabled", func(t *testing.T) {
 		th.App.Srv().ch.AccessControl = nil
-		appErr := th.App.UnAssignPoliciesFromChannels(rctx, parentPolicy.ID, []string{ch1.Id, ch2.Id})
+		appErr := th.App.UnassignPoliciesFromChannels(rctx, parentPolicy.ID, []string{ch1.Id, ch2.Id})
 		require.NotNil(t, appErr)
 		assert.Equal(t, "app.pap.unassign_access_control_policy_from_channels.app_error", appErr.Id)
 	})
@@ -412,7 +412,7 @@ func TestUnAssignPoliciesFromChannels(t *testing.T) {
 		mockAccessControl.On("DeletePolicy", rctx, ch1.Id).Return(expectedErr).Once()
 		mockAccessControl.On("DeletePolicy", rctx, ch2.Id).Return(nil).Maybe()
 
-		appErr := th.App.UnAssignPoliciesFromChannels(rctx, parentPolicy.ID, []string{ch1.Id, ch2.Id})
+		appErr := th.App.UnassignPoliciesFromChannels(rctx, parentPolicy.ID, []string{ch1.Id, ch2.Id})
 		require.NotNil(t, appErr)
 		assert.Equal(t, expectedErr.Id, appErr.Id)
 		assert.Equal(t, expectedErr.Message, appErr.Message)
@@ -438,7 +438,7 @@ func TestUnAssignPoliciesFromChannels(t *testing.T) {
 		mockAccessControl.On("DeletePolicy", rctx, ch1.Id).Return(nil).Once()
 		mockAccessControl.On("DeletePolicy", rctx, ch2.Id).Return(nil).Once()
 
-		appErr := th.App.UnAssignPoliciesFromChannels(rctx, parentPolicy.ID, []string{ch1.Id, ch2.Id, ch3.Id})
+		appErr := th.App.UnassignPoliciesFromChannels(rctx, parentPolicy.ID, []string{ch1.Id, ch2.Id, ch3.Id})
 		require.Nil(t, appErr)
 	})
 
@@ -449,7 +449,7 @@ func TestUnAssignPoliciesFromChannels(t *testing.T) {
 		mockAccessControl.On("DeletePolicy", rctx, ch1.Id).Return(nil).Once()
 		mockAccessControl.On("DeletePolicy", rctx, ch2.Id).Return(nil).Once()
 
-		appErr := th.App.UnAssignPoliciesFromChannels(rctx, parentPolicy.ID, []string{ch1.Id, ch2.Id})
+		appErr := th.App.UnassignPoliciesFromChannels(rctx, parentPolicy.ID, []string{ch1.Id, ch2.Id})
 		require.Nil(t, appErr)
 	})
 }

server/channels/app/channel.go

@@ -1614,7 +1614,7 @@ func (a *App) addUserToChannel(c request.CTX, user *model.User, channel *model.C
 				}
 			}
 		} else if appErr != nil {
-			c.Logger().Error("Error checking access control policy for channel", mlog.Err(appErr))
+			return nil, appErr
 		}
 	}
 

server/channels/app/channels.go

@@ -4,6 +4,7 @@
 package app
 
 import (
+	"net/http"
 	"os"
 	"os/signal"
 	"runtime"
@@ -138,7 +139,7 @@ func NewChannels(s *Server) (*Channels, error) {
 		ch.AccessControl = accessControlServiceInterface(app)
 
 		appErr := ch.AccessControl.Init(request.EmptyContext(s.Log()))
-		if appErr != nil {
+		if appErr != nil && appErr.StatusCode != http.StatusNotImplemented {
 			s.Log().Error("An error occurred while initializing Access Control", mlog.Err(appErr))
 		}
 

server/channels/store/sqlstore/access_control_policy_store.go

@@ -482,7 +482,7 @@ func (s *SqlAccessControlPolicyStore) GetAll(_ request.CTX, opts model.GetAccess
 
 	limit := uint64(opts.Limit)
 	if limit < 1 {
-		limit = 10
+		limit = 1
 	} else if limit > MaxPerPage {
 		limit = MaxPerPage
 	}
@@ -576,7 +576,7 @@ func (s *SqlAccessControlPolicyStore) SearchPolicies(rctx request.CTX, opts mode
 
 	limit := uint64(opts.Limit)
 	if limit < 1 {
-		limit = 10
+		limit = 1
 	} else if limit > MaxPerPage {
 		limit = MaxPerPage
 	}

server/channels/store/storetest/access_control_policy_store.go

@@ -319,14 +319,14 @@ func testAccessControlPolicyStoreGetAll(t *testing.T, rctx request.CTX, ss store
 	require.NoError(t, err)
 	require.NotNil(t, resourcePolicy)
 	t.Run("GetAll", func(t *testing.T) {
-		policies, _, err := ss.AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{})
+		policies, _, err := ss.AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{Limit: 10})
 		require.NoError(t, err)
 		require.NotNil(t, policies)
 		require.Len(t, policies, 3)
 	})
 
 	t.Run("GetAll by type", func(t *testing.T) {
-		policies, _, err := ss.AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{Type: model.AccessControlPolicyTypeParent, IncludeChildren: true})
+		policies, _, err := ss.AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{Type: model.AccessControlPolicyTypeParent, IncludeChildren: true, Limit: 10})
 		require.NoError(t, err)
 		require.NotNil(t, policies)
 		require.Len(t, policies, 2)

server/i18n/en.json

@@ -7920,6 +7920,10 @@
     "id": "common.parse_error_int64",
     "translation": "Failed to parse the value:{{.Value}} to int64"
   },
+  {
+    "id": "ent.access_control.job_data_conversion.app_error",
+    "translation": "Failed to extract data from previous job."
+  },
   {
     "id": "ent.access_control.sync_job.app_error",
     "translation": "Failed to run access control sync job."

server/public/model/client4.go

@@ -627,11 +627,11 @@ func (c *Client4) accessControlPoliciesRoute() string {
 }
 
 func (c *Client4) celRoute() string {
-	return "/access_control_policies/cel"
+	return fmt.Sprintf(c.accessControlPoliciesRoute() + "/cel")
 }
 
 func (c *Client4) accessControlPolicyRoute(policyID string) string {
-	return fmt.Sprintf(c.accessControlPoliciesRoute()+"/%v", policyID)
+	return fmt.Sprintf(c.accessControlPoliciesRoute()+"/%v", url.PathEscape(policyID))
 }
 
 func (c *Client4) GetServerLimits(ctx context.Context) (*ServerLimits, *Response, error) {