Skip to content

Commit 61c46fd

Browse files
committed
Refactor user management and enhance email confirmation process
- Added `users` to the list of accessible resources in the `ApplicationController`. - Removed super admin assignment logic from the `UsersController` to streamline user updates. - Updated email confirmation link generation in the confirmation instructions template for better URL handling. - Adjusted test cases to reflect changes in email confirmation paths and improved request specs for user actions. - Enhanced test setup to ensure proper routing and CSRF protection handling in request specs.
1 parent ad0a67f commit 61c46fd

File tree

13 files changed

+100
-58
lines changed

13 files changed

+100
-58
lines changed

app/controllers/application_controller.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ class ApplicationController < ActionController::Base
1212
subscriptions
1313
account_settings
1414
settings
15+
users
1516
].freeze
1617

1718
# Include Pagy backend for pagination

app/controllers/users_controller.rb

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,6 @@ def update
6161

6262
update_params = permitted_attributes(@user)
6363

64-
# Only allow super_admin assignment if current user is super_admin and not editing themselves
65-
if current_user.super_admin? && @user != current_user && params.dig(:user, :super_admin).present?
66-
@user.super_admin = params.dig(:user, :super_admin) == "1"
67-
end
68-
6964
if @user == current_user
7065
if update_params[:password].present?
7166
if update_params[:current_password].blank?

app/views/devise/mailer/confirmation_instructions.html.erb

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,17 @@
127127
<p>Please confirm your email address (<strong><%= @email %></strong>) to get started with error monitoring.</p>
128128

129129
<div class="button-wrapper">
130-
<a href="<%= confirmation_url(@resource, confirmation_token: @token) %>" class="button">Confirm Email Address</a>
130+
<%
131+
app_host = ENV.fetch('APP_HOST') { Rails.env.production? ? 'https://activerabbit.ai' : 'http://localhost:3000' }
132+
# Strip trailing slash and ensure we have a proper base URL
133+
base_url = app_host.sub(/\/+$/, '')
134+
# Add protocol if missing
135+
unless base_url.start_with?('http://', 'https://')
136+
base_url = (Rails.env.production? ? 'https://' : 'http://') + base_url
137+
end
138+
confirmation_link = "#{base_url}/confirmation?confirmation_token=#{@token}"
139+
%>
140+
<a href="<%= confirmation_link %>" class="button">Confirm Email Address</a>
131141
</div>
132142

133143
<div class="note">

config/environments/development.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,5 +100,4 @@
100100
config.hosts << "127.0.0.1"
101101
config.hosts << "host.docker.internal"
102102
config.hosts << "host.docker.internal:3000"
103-
config.hosts << "160ec4227c78.ngrok-free.app"
104103
end

config/environments/test.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,7 @@
5353

5454
# Ensure Rack::Attack is disabled in test environment
5555
config.middleware.delete Rack::Attack
56+
57+
# Disable host authorization in test environment
58+
config.host_authorization = { exclude: ->(request) { true } }
5659
end

spec/features/email_confirmation_spec.rb

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@
2929
expect(user.confirmed?).to be false
3030

3131
# Confirm using raw token
32-
get user_confirmation_path(confirmation_token: raw_token)
32+
get "/confirmation?confirmation_token=#{raw_token}"
3333

3434
user.reload
3535
expect(user.confirmed?).to be true
3636
end
3737
end
3838

39-
describe "GET /users/confirmation" do
39+
describe "GET /confirmation" do
4040
let(:account) { create(:account) }
4141

4242
context "with valid token" do
@@ -51,27 +51,27 @@
5151
it "confirms the user" do
5252
expect(user.confirmed?).to be false
5353

54-
get user_confirmation_path(confirmation_token: @raw_token)
54+
get "/confirmation?confirmation_token=#{@raw_token}"
5555

5656
user.reload
5757
expect(user.confirmed?).to be true
5858
end
5959

6060
it "redirects to sign in" do
61-
get user_confirmation_path(confirmation_token: @raw_token)
61+
get "/confirmation?confirmation_token=#{@raw_token}"
6262

63-
expect(response).to redirect_to(new_user_session_path)
63+
expect(response).to redirect_to("/signin")
6464
end
6565
end
6666

6767
context "with invalid token" do
6868
it "does not confirm and shows error or redirects" do
69-
get user_confirmation_path(confirmation_token: "invalid_token")
69+
get "/confirmation?confirmation_token=invalid_token"
7070

7171
# Devise either renders a page with errors or redirects
7272
# Check that it either has error content or redirects
7373
if response.redirect?
74-
expect(response).to redirect_to(new_user_confirmation_path).or redirect_to(new_user_session_path)
74+
expect(response).to redirect_to("/confirmation/new").or redirect_to("/signin")
7575
else
7676
expect(response.body.downcase).to include("invalid").or include("token")
7777
end
@@ -89,7 +89,7 @@
8989
end
9090

9191
it "does not confirm user if token expired" do
92-
get user_confirmation_path(confirmation_token: @raw_token)
92+
get "/confirmation?confirmation_token=#{@raw_token}"
9393

9494
user.reload
9595
# User should not be confirmed with expired token (unless confirm_within is not set)
@@ -108,7 +108,7 @@
108108
end
109109

110110
it "handles already confirmed user" do
111-
get user_confirmation_path(confirmation_token: @raw_token)
111+
get "/confirmation?confirmation_token=#{@raw_token}"
112112

113113
# User should remain confirmed
114114
user.reload

spec/jobs/weekly_report_job_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
end
1414

1515
before do
16+
# Update account stats without changing name (to avoid uniqueness conflicts)
1617
account.update!(
17-
name: "Test Account",
1818
cached_events_used: 100,
1919
usage_cached_at: Time.current
2020
)

spec/mailers/devise_mailer_spec.rb

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,19 @@
55
RSpec.describe Devise::Mailer, type: :mailer do
66
describe "#confirmation_instructions" do
77
let(:account) { create(:account) }
8-
let(:user) { create(:user, :unconfirmed, account: account, email: "test@example.com") }
8+
let(:user) { create(:user, :unconfirmed, account: account) }
99
let(:token) { "test_confirmation_token_123" }
1010

11+
before do
12+
# Ensure routes and Devise mappings are properly loaded
13+
Rails.application.reload_routes!
14+
Rails.application.routes.default_url_options[:host] = 'localhost'
15+
end
16+
1117
subject(:mail) { described_class.confirmation_instructions(user, token) }
1218

1319
it "renders the headers" do
14-
expect(mail.to).to eq(["test@example.com"])
20+
expect(mail.to).to eq([user.email])
1521
expect(mail.from.first).to include("activerabbit")
1622
end
1723

@@ -28,7 +34,7 @@
2834
end
2935

3036
it "includes user email" do
31-
expect(mail.body.encoded).to include("test@example.com")
37+
expect(mail.body.encoded).to include(user.email)
3238
end
3339

3440
it "includes confirm button" do

spec/rails_helper.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,9 @@
8181
config.before(:each) do
8282
WebMock.disable_net_connect!(allow_localhost: true)
8383
end
84+
85+
# Use localhost as default host for request specs to avoid host authorization issues
86+
config.before(:each, type: :request) do
87+
host! 'localhost'
88+
end
8489
end

spec/requests/api_deploys_spec.rb

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,17 @@
33
RSpec.describe 'API::V1::Deploys', type: :request, api: true do
44
let(:account) { create(:account) }
55
let(:user) { create(:user, account: account) }
6-
let(:project) { create(:project, user: user, account: account) }
7-
let(:token) { create(:api_token, project: project, account: account) }
8-
let(:headers) { { 'CONTENT_TYPE' => 'application/json', 'X-Project-Token' => token.token } }
9-
10-
before do
11-
ActsAsTenant.current_tenant = account
6+
let(:project) do
7+
ActsAsTenant.with_tenant(account) do
8+
create(:project, user: user, account: account)
9+
end
1210
end
11+
let(:token) do
12+
ActsAsTenant.with_tenant(account) do
13+
create(:api_token, project: project, account: account)
14+
end
15+
end
16+
let(:headers) { { 'CONTENT_TYPE' => 'application/json', 'X-Project-Token' => token.token } }
1317

1418
describe 'POST /api/v1/deploys' do
1519
it 'creates a deploy and associated release' do
@@ -25,8 +29,8 @@
2529

2630
expect {
2731
post '/api/v1/deploys', params: body, headers: headers
28-
}.to change { Deploy.count }.by(1)
29-
.and change { Release.count }.by(1)
32+
}.to change { ActsAsTenant.with_tenant(account) { Deploy.count } }.by(1)
33+
.and change { ActsAsTenant.with_tenant(account) { Release.count } }.by(1)
3034

3135
expect(response).to have_http_status(:ok)
3236
json = JSON.parse(response.body)
@@ -36,13 +40,15 @@
3640

3741
it 'reuses existing release for same version/environment' do
3842
# Pre-create a release for this project/version/environment
39-
existing_release = create(
40-
:release,
41-
project: project,
42-
account: account,
43-
version: 'v1.0.1',
44-
environment: 'staging'
45-
)
43+
existing_release = ActsAsTenant.with_tenant(account) do
44+
create(
45+
:release,
46+
project: project,
47+
account: account,
48+
version: 'v1.0.1',
49+
environment: 'staging'
50+
)
51+
end
4652

4753
body = {
4854
project_slug: project.slug,
@@ -54,17 +60,17 @@
5460
finished_at: Time.current.iso8601
5561
}.to_json
5662

57-
deploy_count_before = Deploy.count
58-
release_count_before = Release.count
63+
deploy_count_before = ActsAsTenant.with_tenant(account) { Deploy.count }
64+
release_count_before = ActsAsTenant.with_tenant(account) { Release.count }
5965

6066
post '/api/v1/deploys', params: body, headers: headers
6167

6268
expect(response).to have_http_status(:ok)
6369
json = JSON.parse(response.body)
6470
expect(json['ok']).to eq(true)
6571

66-
expect(Deploy.count).to eq(deploy_count_before + 1)
67-
expect(Release.count).to eq(release_count_before)
72+
expect(ActsAsTenant.with_tenant(account) { Deploy.count }).to eq(deploy_count_before + 1)
73+
expect(ActsAsTenant.with_tenant(account) { Release.count }).to eq(release_count_before)
6874
end
6975

7076
it 'returns not_found for unknown project slug' do

0 commit comments

Comments
 (0)