[feature] Uncategorized access should allow API Namespace creation (#…

…1316) (#1319)

Addresses: #1315

**Demo**

https://user-images.githubusercontent.com/25191509/209044452-9db5a0f7-8037-4561-bdf9-0f06c99a6508.mov

Co-authored-by: Prashant <[email protected]>

app/controllers/comfy/admin/api_actions_controller.rb

@@ -1,7 +1,7 @@
 class Comfy::Admin::ApiActionsController < Comfy::Admin::Cms::BaseController
   before_action :set_api_action
-  before_action :ensure_authority_for_read_api_actions_only_in_api, only: %i[ show index ]
-  before_action :ensure_authority_for_full_access_for_api_actions_only_in_api, only: %i[ new action_workflow ]
+  before_action :ensure_authority_for_read_api_actions_only_in_api, only: %i[ show index action_workflow ]
+  before_action :ensure_authority_for_full_access_for_api_actions_only_in_api, only: %i[ new ]
   before_action :set_current_user_and_visit
 
   def new

app/controllers/subdomains/base_controller.rb

@@ -193,19 +193,17 @@ def user_authorized_for_api_accessibility?(api_permissions, check_categories: tr
       is_user_authorized = api_permissions.any? do |access_name|
         user_api_accessibility.dig('all_namespaces', access_name).present? && user_api_accessibility.dig('all_namespaces', access_name) == 'true'
       end
-    elsif check_categories && user_api_accessibility.keys.include?('namespaces_by_category')
-      categories = @api_namespace.categories.pluck(:label)
-
-      if categories.blank? && user_api_accessibility.dig('namespaces_by_category', 'uncategorized').present?
-        is_user_authorized = api_permissions.any? do |access_name|
-          user_api_accessibility.dig('namespaces_by_category', 'uncategorized', access_name).present? && user_api_accessibility.dig('namespaces_by_category', 'uncategorized', access_name) == 'true'
-        end
-      else
+    elsif user_api_accessibility.keys.include?('namespaces_by_category')
+      if check_categories && (categories = @api_namespace.categories.pluck(:label)) && categories.present?
         categories.any? do |category|
           is_user_authorized = api_permissions.any? do |access_name|
             user_api_accessibility.dig('namespaces_by_category', category, access_name).present? && user_api_accessibility.dig('namespaces_by_category', category, access_name) == 'true'
           end
         end
+      else
+        is_user_authorized = api_permissions.any? do |access_name|
+          user_api_accessibility.dig('namespaces_by_category', 'uncategorized', access_name).present? && user_api_accessibility.dig('namespaces_by_category', 'uncategorized', access_name) == 'true'
+        end
       end
     end
 

app/helpers/api_accessibility_helper.rb

@@ -52,4 +52,24 @@ def has_access_to_api_accessibility?(api_permissions, user, api_namespace)
 
     is_user_authorized
   end
+
+  def has_only_uncategorized_access?(api_accessibility)
+    return false if api_accessibility.keys.include?('all_namespaces')
+    if api_accessibility.keys.include?('namespaces_by_category')
+      categories = api_accessibility['namespaces_by_category'].keys
+      return true if categories.size == 1 && categories[0] == 'uncategorized'
+    end
+
+    false
+  end
+
+  def filter_categories_by_api_accessibility(api_accessibility, categories)
+    if api_accessibility.keys.include?('all_namespaces')
+      categories
+    elsif api_accessibility.keys.include?('namespaces_by_category')
+      accessible_categories = api_accessibility['namespaces_by_category'].keys - ['uncategorized']
+
+      categories.where(label: accessible_categories)
+    end
+  end
 end

app/views/comfy/admin/api_actions/_form.html.haml

@@ -1,7 +1,8 @@
 .card.form-container.font-size-sm.position-relative.p-3.mb-4{id: "api_action_form_#{type}_#{index}", style: "font-size: 13px; border-color: #3883fa"}
-  .position-absolute{style: "top: 6px; right: 6px"}
-    %a.text-danger{style: "cursor: pointer;", onclick: "removeForm('api_action_form_#{type}_#{index}', 'api_action_form_destroy_field_#{type}_#{index}')"}
-      %i.fas.fa-times
+  - if has_access_to_api_accessibility?(ApiNamespace::API_ACCESSIBILITIES[:full_access_for_api_actions_only], current_user, @api_namespace)
+    .position-absolute{style: "top: 6px; right: 6px"}
+      %a.text-danger{style: "cursor: pointer;", onclick: "removeForm('api_action_form_#{type}_#{index}', 'api_action_form_destroy_field_#{type}_#{index}')", 'data-action-workflow': 'remove-api-action'}
+        %i.fas.fa-times
   .row
     .form-group.col-12
       = label_tag "Action"

app/views/comfy/admin/api_actions/action_workflow.html.haml

@@ -20,8 +20,9 @@
         .form-group{id: "#{subclass}_forms", class: "js-sortable"}
           = f.fields_for subclass, @api_namespace.send(subclass) do |ff|
             = render partial: 'comfy/admin/api_actions/form', locals: {index: ff.index, resource: ff.object, type: subclass }
-        %a.btn.btn-primary.text-white{onclick: "appendApiActionForm('#{subclass}')"}
-          %i.fa.fa-plus
+        - if has_access_to_api_accessibility?(ApiNamespace::API_ACCESSIBILITIES[:full_access_for_api_actions_only], current_user, @api_namespace)
+          %a.btn.btn-primary.text-white{onclick: "appendApiActionForm('#{subclass}')", 'data-action-workflow': 'add-new-api-action-form'}
+            %i.fa.fa-plus
 
   .actions
     = f.submit 'Save'

app/views/comfy/admin/api_namespaces/_form.html.haml

@@ -32,7 +32,8 @@
     = f.label :has_form
     = f.check_box :has_form, checked: @api_namespace.api_form.present?
 
-  = render "comfy/admin/cms/categories/form", form: f
+  - unless has_only_uncategorized_access?(current_user.api_accessibility) 
+    = render "comfy/admin/cms/categories/form", form: f
 
   .actions
     = f.submit 'Save'

app/views/comfy/admin/cms/categories/_form.html.haml

@@ -5,9 +5,10 @@
 
 - if categories.present?
   - if non_site_entity
-    .field
+    .field.categories-form-partial
       %label.mr-1 
         Categories:
+      - categories = filter_categories_by_api_accessibility(current_user.api_accessibility, categories) if object_type == 'ApiNamespace'
       = form.collection_check_boxes(:category_ids, categories, :id, :label) do |f|
         = f.label(class: 'mr-3') {f.check_box(class: 'mr-1') + f.text}
   - else

test/controllers/admin/comfy/api_actions_controller_test.rb

@@ -419,6 +419,10 @@ class Comfy::Admin::ApiActionsControllerTest < ActionDispatch::IntegrationTest
     sign_in(@user)
     get action_workflow_api_namespace_api_actions_url(api_namespace_id: @api_action.api_namespace_id)
     assert_response :success
+    # '+' button should be shown to add new api-action form
+    assert_select "a[data-action-workflow='add-new-api-action-form']"
+    # 'x' button should be shown to remove existing api-action
+    assert_select "a[data-action-workflow='remove-api-action']"
   end
 
   test 'should get action_workflow if user has full_access_for_api_actions_only for all namespace' do
@@ -427,16 +431,32 @@ class Comfy::Admin::ApiActionsControllerTest < ActionDispatch::IntegrationTest
     sign_in(@user)
     get action_workflow_api_namespace_api_actions_url(api_namespace_id: @api_action.api_namespace_id)
     assert_response :success
+    # '+' button should be shown to add new api-action form
+    assert_select "a[data-action-workflow='add-new-api-action-form']"
+    # 'x' button should be shown to remove existing api-action
+    assert_select "a[data-action-workflow='remove-api-action']"
   end
 
-  test 'should not get action_workflow if user has other access for all namespace' do
+  test 'should get action_workflow if user has read_api_actions_only for all namespace' do
     @user.update(api_accessibility: {all_namespaces: {read_api_actions_only: 'true'}})
 
+    sign_in(@user)
+    get action_workflow_api_namespace_api_actions_url(api_namespace_id: @api_action.api_namespace_id)
+    assert_response :success
+    # '+' button should not be shown to add new api-action form
+    assert_select "a[data-action-workflow='add-new-api-action-form']", {count: 0}
+    # 'x' button should not be shown to remove existing api-action
+    assert_select "a[data-action-workflow='remove-api-action']", {count: 0}
+  end
+
+  test 'should not get action_workflow if user has other access for all namespace' do
+    @user.update(api_accessibility: {all_namespaces: {read_api_resources_only: 'true'}})
+
     sign_in(@user)
     get action_workflow_api_namespace_api_actions_url(api_namespace_id: @api_action.api_namespace_id)
     assert_response :redirect
 
-    expected_message = "You do not have the permission to do that. Only users with full_access or full_access_for_api_actions_only are allowed to perform that action."
+    expected_message = "You do not have the permission to do that. Only users with full_access or full_read_access or full_access_for_api_actions_only or read_api_actions_only are allowed to perform that action."
     assert_equal expected_message, flash[:alert]
   end
 
@@ -561,6 +581,10 @@ class Comfy::Admin::ApiActionsControllerTest < ActionDispatch::IntegrationTest
     sign_in(@user)
     get action_workflow_api_namespace_api_actions_url(api_namespace_id: @api_action.api_namespace_id)
     assert_response :success
+    # '+' button should be shown to add new api-action form
+    assert_select "a[data-action-workflow='add-new-api-action-form']"
+    # 'x' button should be shown to remove existing api-action
+    assert_select "a[data-action-workflow='remove-api-action']"
   end
 
   test 'should get action_workflow if user has full_access_for_api_actions_only for the namespace' do
@@ -571,26 +595,48 @@ class Comfy::Admin::ApiActionsControllerTest < ActionDispatch::IntegrationTest
     sign_in(@user)
     get action_workflow_api_namespace_api_actions_url(api_namespace_id: @api_action.api_namespace_id)
     assert_response :success
+    # '+' button should be shown to add new api-action form
+    assert_select "a[data-action-workflow='add-new-api-action-form']"
+    # 'x' button should be shown to remove existing api-action
+    assert_select "a[data-action-workflow='remove-api-action']"
   end
 
-  test 'should get action_workflow if user has read_api_actions_only for the uncategorized namespace' do
+  test 'should get action_workflow if user has full_access_for_api_actions_only for the uncategorized namespace' do
     @user.update(api_accessibility: {namespaces_by_category: {uncategorized: {full_access_for_api_actions_only: 'true'}}})
 
     sign_in(@user)
     get action_workflow_api_namespace_api_actions_url(api_namespace_id: @api_action.api_namespace_id)
     assert_response :success
+    # '+' button should be shown to add new api-action form
+    assert_select "a[data-action-workflow='add-new-api-action-form']"
+    # 'x' button should be shown to remove existing api-action
+    assert_select "a[data-action-workflow='remove-api-action']"
   end
 
-  test 'should not get action_workflow if user has other access for the namespace' do
+  test 'should get action_workflow if user has read_api_actions_only for the namespace' do
     category = comfy_cms_categories(:api_namespace_1)
     @api_namespace.update(category_ids: [category.id])
     @user.update(api_accessibility: {namespaces_by_category: {"#{category.label}": {read_api_actions_only: 'true'}}})
 
+    sign_in(@user)
+    get action_workflow_api_namespace_api_actions_url(api_namespace_id: @api_action.api_namespace_id)
+    assert_response :success
+    # '+' button should not be shown to add new api-action form
+    assert_select "a[data-action-workflow='add-new-api-action-form']", {count: 0}
+    # 'x' button should not be shown to remove existing api-action
+    assert_select "a[data-action-workflow='remove-api-action']", {count: 0}
+  end
+
+  test 'should not get action_workflow if user has other access for the namespace' do
+    category = comfy_cms_categories(:api_namespace_1)
+    @api_namespace.update(category_ids: [category.id])
+    @user.update(api_accessibility: {namespaces_by_category: {"#{category.label}": {read_api_resources_only: 'true'}}})
+
     sign_in(@user)
     get action_workflow_api_namespace_api_actions_url(api_namespace_id: @api_action.api_namespace_id)
     assert_response :redirect
 
-    expected_message = "You do not have the permission to do that. Only users with full_access or full_access_for_api_actions_only are allowed to perform that action."
+    expected_message = "You do not have the permission to do that. Only users with full_access or full_read_access or full_access_for_api_actions_only or read_api_actions_only are allowed to perform that action."
     assert_equal expected_message, flash[:alert]
   end