Skip to content

Require a partner for the partner dashboard #4063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 23, 2024
Merged

Require a partner for the partner dashboard #4063

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a4b8777
Require a partner for the partner dashboard
awwaiid Jan 21, 2024
b5b799c
Validate that a non-partner user gets redirected
awwaiid Jan 28, 2024
f46bbae
Merge remote-tracking branch 'origin/main' into not-partner-error
awwaiid Aug 4, 2024
5caa4c0
Centralize partner-user requirement to partner base controller
awwaiid Aug 4, 2024
b7e0682
Merge remote-tracking branch 'origin/main' into not-partner-error
awwaiid Aug 19, 2024
c8c7905
Clean out the lint!
awwaiid Aug 19, 2024
221ed0e
Fix redirect spec
awwaiid Aug 19, 2024
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
9 changes: 9 additions & 0 deletions app/controllers/application_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ def authorize_admin
current_user.has_role?(Role::ORG_ADMIN, current_organization)
end

def require_partner
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this in ApplicationController if it's only used in the partner DashboardController?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Future proofing to make it look like the others -- authorize_user, current_role, etc. It looked like those so I put it with those -- wamme to move it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm... I think Partner::BaseController is a fine place to put it. It's still fairly centralized and makes it more obvious that it's only relevant to pages in that namespace.

unless current_partner
respond_to do |format|
format.html { redirect_to dashboard_path, flash: { error: "Logged in user is not set up as a 'partner'." } }
format.json { render body: nil, status: :forbidden }
end
end
end

def log_active_user
if current_user && should_update_last_request_at?
# we don't want the user record to validate or run callbacks when we're tracking activity
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/partners/dashboards_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ class DashboardsController < BaseController

protect_from_forgery with: :exception

before_action :require_partner
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be in the partner base controller? I feel like every page under there needs a current partner, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh, I didn't realize BaseController is Partners::BaseController. I'll move it!


def index; end

def show
Expand Down
10 changes: 10 additions & 0 deletions spec/requests/partners/dashboard_requests_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@
end
end

context "without a partner role" do
it "should redirect to the organization dashboard" do
partner_user.add_role(Role::ORG_USER, @organization)
partner_user.remove_role(Role::PARTNER_USER, partner)
allow(UsersRole).to receive(:current_role_for).and_return(partner_user.roles.find_by(name: "partner"))
get partners_dashboard_path
expect(response.body).to include("switch_to_role")
end
end

context "BroadcastAnnouncement card" do
it "displays announcements if there are valid ones" do
BroadcastAnnouncement.create(message: "test announcement", user_id: 1, organization_id: 1)
Expand Down