Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
module BetterTogether
class HostDashboardController < ApplicationController # rubocop:todo Style/Documentation
def index # rubocop:todo Metrics/MethodLength
authorize :host_dashboard, :index?
root_classes = [
Community, NavigationArea, Page, Platform, Person, Role, ResourcePermission, User,
Conversation, Message, Category
Expand Down
11 changes: 11 additions & 0 deletions app/policies/better_together/host_dashboard_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

# app/policies/better_together/host_dashboard_policy.rb

Comment on lines +3 to +4
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

The file path comment on line 3 is redundant and violates the DRY principle. The file path is already evident from the actual file location, and this type of comment is not used consistently across the codebase. It should be removed to maintain consistency with other policy files like HubPolicy, GuestAccessPolicy, etc.

Suggested change
# app/policies/better_together/host_dashboard_policy.rb

Copilot uses AI. Check for mistakes.
module BetterTogether
class HostDashboardPolicy < ApplicationPolicy # rubocop:todo Style/Documentation
def index?
user.present? && user.permitted_to?('manage_platform')
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

The policy implementation is checking user.permitted_to? directly on the user object, but this is inconsistent with the codebase pattern. According to ApplicationPolicy (lines 96-98), policies should use the protected permitted_to? method which delegates to agent (the user's person).

The current implementation will fail if user is nil, even though the policy already has a nil check. The pattern used in HubPolicy (line 6) shows the correct approach:

def index?
  permitted_to?('manage_platform')
end

This change aligns with the authorization architecture where permissions are associated with Person records, not User records directly.

Suggested change
user.present? && user.permitted_to?('manage_platform')
permitted_to?('manage_platform')

Copilot uses AI. Check for mistakes.
end
end
end
40 changes: 40 additions & 0 deletions spec/controllers/better_together/host_dashboard_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe BetterTogether::HostDashboardController, type: :controller do
include Devise::Test::ControllerHelpers
include BetterTogether::DeviseSessionHelpers

routes { BetterTogether::Engine.routes }

before do
configure_host_platform
@request.env['devise.mapping'] = Devise.mappings[:user]
end

describe 'GET #index' do
context 'when user can manage platform' do
let(:user) { BetterTogether::User.find_by(email: '[email protected]') }

before { sign_in user }

it 'returns http success' do
get :index, params: { locale: I18n.default_locale }
expect(response).to be_successful
end
end

context 'when user cannot manage platform' do
let(:user) { create(:user, :confirmed) }

before { sign_in user }

it 'raises Pundit::NotAuthorizedError' do
expect do
get :index, params: { locale: I18n.default_locale }
end.to raise_error(Pundit::NotAuthorizedError)
end
end
end
end
Comment on lines +1 to +40
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

According to the project's testing architecture standards (guideline 1000000), this should be a request spec (type: :request) instead of a controller spec. The project follows a consistent pattern of using request specs for all controller testing to maintain architectural consistency and handle Rails engine routing automatically.

Please convert this to a request spec similar to spec/requests/better_together/communities_controller_spec.rb:

# frozen_string_literal: true

require 'rails_helper'

RSpec.describe 'BetterTogether::HostDashboard', :as_platform_manager do
  let(:locale) { I18n.default_locale }

  describe 'GET /:locale/host' do
    it 'renders index for platform manager' do
      get better_together.host_dashboard_path(locale:)
      expect(response).to have_http_status(:ok)
    end
  end

  describe 'authorization', :skip_host_setup do
    before { configure_host_platform }

    context 'when user cannot manage platform', :as_user do
      it 'returns forbidden' do
        get better_together.host_dashboard_path(locale:)
        expect(response).to have_http_status(:forbidden)
      end
    end
  end
end

Copilot generated this review using guidance from repository custom instructions.
31 changes: 31 additions & 0 deletions spec/policies/better_together/host_dashboard_policy_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe BetterTogether::HostDashboardPolicy, type: :policy do
subject(:policy) { described_class.new(user, nil) }

context 'when user can manage platform' do
let(:user) { create(:user, :confirmed, :platform_manager) }

it 'permits access' do
expect(policy.index?).to be(true)
end
end

context 'when user cannot manage platform' do
let(:user) { create(:user, :confirmed) }

it 'denies access' do
expect(policy.index?).to be(false)
end
end

context 'when no user is present' do
let(:user) { nil }

it 'denies access' do
expect(policy.index?).to be(false)
end
end
end
Loading