Merge pull request #1350 from rubymonsters/fix-profiles-controller-index

Fix profiles controller index

.tool-versions

@@ -0,0 +1 @@
+ruby 3.1.1

app/controllers/profiles_controller.rb

@@ -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
@@ -223,7 +223,7 @@ def profiles_for_index
         .by_region(current_region)
         .includes(:translations)
         .main_topic_translated_in(I18n.locale)
-        .random
+        .order(created_at: :desc)
       )
   end
 

spec/controllers/profiles_controller_spec.rb

@@ -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')
@@ -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' }
@@ -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 }
@@ -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 }
@@ -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 }
@@ -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
@@ -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' } }
@@ -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: ' ' } }
@@ -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
@@ -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
@@ -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
 
@@ -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 []
+    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