Skip to content
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.

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

Fix profiles controller index #1350

Merged
merged 6 commits into from
Feb 22, 2024
Merged
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
1 change: 1 addition & 0 deletions .tool-versions
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ruby 3.1.1
6 changes: 3 additions & 3 deletions app/controllers/profiles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ def index
elsif params[:category_id]
search_with_category_id
elsif params[:tag_filter]
if params[:tag_filter].empty?
if params[:tag_filter].empty?
redirect_to profiles_url(anchor: "top"), notice: I18n.t('flash.profiles.no_tags_selected')
return
return
end
search_with_tags
else
Expand Down Expand Up @@ -223,7 +223,7 @@ def profiles_for_index
.by_region(current_region)
.includes(:translations)
.main_topic_translated_in(I18n.locale)
.random
tyranja marked this conversation as resolved.
Show resolved Hide resolved
.order(created_at: :desc)
)
end

Expand Down
84 changes: 56 additions & 28 deletions spec/controllers/profiles_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,17 @@

describe ProfilesController, type: :controller do
include AuthHelper

let!(:profile_published) { create(:published_profile, topic_list: %w[ruby algorithms]) }
let!(:profile_unpublished) { create(:unpublished_profile) }
let!(:ada) { create(:published_profile, email: "ada@mail.org", main_topic_en: 'math') }
let!(:admin) { create(:admin) }

describe 'test index action' do
describe 'index' do
before do
get :index
end

it 'displays index' do
it 'displays index template' do
expect(response).to be_successful
expect(response.response_code).to eq(200)
expect(response).to render_template('index')
Expand All @@ -28,22 +27,22 @@
end
end

describe 'category id search' do
describe '#search_with_category_id' do
it 'stores the correct category when params has category id' do
category = FactoryBot.create(:category, name: 'Seasons', name_en: 'Seasons')
get :index, params: { category_id: category.id }
expect(assigns(:category)).to eq(category)
end
end

describe 'tag filter search' do
describe '#search_with_tags' do
it 'redirects to profile index when tags filter is empty' do
get :index, params: { tag_filter: "" }
expect(response).to redirect_to("/#{I18n.locale}/profiles#top")
end
end

describe 'search action' do
describe '#search_with_search_params' do
it 'displays search results if search term is present' do
sleep 1
get :index, params: { search: 'ruby' }
Expand All @@ -59,7 +58,7 @@
end
end

describe 'show profile' do
describe 'show' do
describe 'of unpublished profile' do
it 'is not permitted for unauthorized not signed in profile' do
get :show, params: { id: profile_unpublished.id }
Expand Down Expand Up @@ -93,9 +92,8 @@
end
end

describe 'edit profile' do

context 'when editing own profile' do
describe '#edit' do
context 'own profile' do
before do
sign_in ada
get :edit, params: { locale: 'de', id: ada.id }
Expand All @@ -106,7 +104,7 @@
end
end

context 'when trying to edit a different profile' do
context "other people's profile" do
before do
sign_in ada
get :edit, params: { locale: 'de', id: profile_published.id }
Expand All @@ -121,7 +119,7 @@
end
end

context 'when trying edit profile if user is not signed in' do
context 'own profile but not signed in' do
before do
get :edit, params: { locale: 'de', id: ada.id }
end
Expand Down Expand Up @@ -162,9 +160,8 @@
end
end

describe 'update profile action' do

context 'when updating own profile with valid params' do
describe '#update' do
context 'with valid params' do
before do
sign_in ada
put :update, params: { id: ada.id, profile: { firstname: 'marie', lastname: 'curie' } }
Expand All @@ -180,7 +177,7 @@
end
end

context 'when invalid params are supplied' do
context 'with invalid params' do
before do
sign_in ada
put :update, params: { id: ada.id, profile: { email: ' ' } }
Expand All @@ -195,13 +192,13 @@
end
end

context 'when trying to update a different profile' do
context "other people's profile" do
before do
sign_in ada
put :update, params: { id: profile_published.id, profile: { firstname: 'marie', lastname: 'curie' } }
end

it 'does not update the requested profile' do
it 'does not update' do
profile_published.reload
expect(profile_published.firstname).to eq('Susi')
end
Expand All @@ -211,36 +208,35 @@
end
end

context 'when trying update profile if user is not signed in' do
context 'when user is not signed in' do
before do
put :update, params: { id: profile_published.id, profile: { firstname: 'marie', lastname: 'curie' } }
end

it 'does not update the requested profile' do
it 'does not update' do
profile_published.reload
expect(profile_published.firstname).to eq('Susi')
end

it 'should redirect to profiles overview' do
it 'redirects to profiles overview' do
expect(response).to redirect_to("/#{I18n.locale}/profiles")
end
end
end

describe 'destroy profile action' do

context 'when destroying own profile' do
describe '#destroy' do
context 'own profile' do
before do
sign_in ada
end

it 'should destroy requested profile' do
it 'should destroy profile' do
expect do
delete :destroy, params: { id: ada.id }
end.to change(Profile, :count).by(-1)
end

it 'should not find the destroyed user' do
it 'should not find the destroyed profile' do
delete :destroy, params: { id: ada.id }
expect { Profile.find(ada.id) }.to raise_exception(ActiveRecord::RecordNotFound)
end
Expand All @@ -251,13 +247,13 @@
end
end

context 'when trying to destroy a different profile' do
context "other people's profile" do
before do
sign_in ada
delete :destroy, params: { id: profile_published.id }
end

it 'should not destroy the requested profile' do
it 'should not destroy' do
expect(Profile.where(id: profile_published.id).count).to eq 1
end

Expand All @@ -280,4 +276,36 @@
end
end
end

context "pagination with multiple profiles" do
it 'displays no profiles on index page 2' do
published_profiles = create_list(:published_profile, 23, main_topic_en: 'math')
get :index, params: { page: 1 }
# we have with previous created profile 24 in total
expect(assigns(:records).count).to eq 24
expect(assigns(:records).to_a).to match_array(published_profiles << ada)
get :index, params: { page: 2 }
expect(assigns(:records).count).to eq 0
end

it 'displays no profiles twice on index page 2' do
create_list(:published_profile, 25, main_topic_en: 'math')

get :index, params: { page: 1 }
expect(assigns(:records).count).to eq 24
profiles_page_1 = assigns(:records)

get :index, params: { page: 2 }
expect(assigns(:records).count).to eq 2
profiles_page_2 = assigns(:records)
expect(profiles_page_2 & profiles_page_1).to eq []
Copy link
Member

Choose a reason for hiding this comment

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

Sieht elegant aus, aber eigentlich verstehe ich nicht genau, was & hier macht. Ist das sowas wie eine Schnittmenge?

Copy link
Member Author

Choose a reason for hiding this comment

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

jep it checks for an intersection. I wanted to check that we shall no profile twice.

end

it 'order by last created' do
published_profiles = create_list(:published_profile, 23, main_topic_en: 'math')
last_created_profile = published_profiles.last
get :index, params: { page: 1 }
expect(assigns(:records).first).to eq last_created_profile
end
end
end
Loading