Merge pull request #149 from loftwah/dl/default-avatar-bug

Dl/default avatar bug

app/models/user.rb

@@ -1,5 +1,8 @@
 # app/models/user.rb
 class User < ApplicationRecord
+
+  FALLBACK_AVATAR_URL = 'https://pbs.twimg.com/profile_images/1581014308397502464/NPogKMyk_400x400.jpg'
+
   attr_accessor :invite_code
   devise :database_authenticatable, :registerable,
          :recoverable, :rememberable, :validatable
@@ -40,33 +43,37 @@ def generate_open_graph_image
   end
 
   def download_and_store_avatar
-    return if avatar.blank?
-
+    if avatar.blank?
+      self.avatar = FALLBACK_AVATAR_URL
+      save(validate: false)
+      return
+    end
+
     begin
       avatar_dir = Rails.root.join('public', 'avatars')
       FileUtils.mkdir_p(avatar_dir) unless File.directory?(avatar_dir)
-
+  
       uri = URI.parse(avatar)
       filename = "#{username}_avatar#{File.extname(avatar)}"
       filepath = File.join(avatar_dir, filename)
-
+  
       response = Net::HTTP.get_response(uri)
       if response.is_a?(Net::HTTPSuccess)
         File.open(filepath, 'wb') do |local_file|
           local_file.write(response.body)
         end
         Rails.logger.info "Avatar downloaded for user #{username}"
       else
-        Rails.logger.error "Failed to download avatar for user #{username}. HTTP Error: #{response.code} #{response.message}. Using default avatar."
-        self.avatar = 'greg.jpg'  # Set to default avatar
-        save(validate: false)  # Save without triggering validations
+        Rails.logger.error "Failed to download avatar for user #{username}. HTTP Error: #{response.code} #{response.message}. Using fallback avatar."
+        self.avatar = FALLBACK_AVATAR_URL
+        save(validate: false)
       end
     rescue StandardError => e
-      Rails.logger.error "Failed to download avatar for user #{username}: #{e.message}"
-      self.avatar = 'greg.jpg'  # Set to default avatar
-      save(validate: false)  # Save without triggering validations
+      Rails.logger.error "Failed to download avatar for user #{username}: #{e.message}. Using fallback avatar."
+      self.avatar = FALLBACK_AVATAR_URL
+      save(validate: false)
     end
-  end
+  end  
 
   private
 

app/services/open_graph_image_generator.rb

@@ -1,9 +1,6 @@
-# app/services/open_graph_image_generator.rb
 class OpenGraphImageGenerator
-  IMAGE_WIDTH = 1200
-  IMAGE_HEIGHT = 630
-  AVATAR_SIZE = 200
-  BORDER_SIZE = 10
+  # Constants for sizes and paths
+  FALLBACK_AVATAR_URL = 'https://pbs.twimg.com/profile_images/1581014308397502464/NPogKMyk_400x400.jpg'
 
   def initialize(user)
     @user = user
@@ -14,7 +11,7 @@ def generate
     output_path = Rails.root.join('public', 'uploads', 'og_images', "#{@user.username}_og.png")
     image = MiniMagick::Image.open(template_path)
 
-    avatar = @user.avatar.present? ? download_image(@user.avatar) : default_avatar
+    avatar = @user.avatar.present? ? download_image(@user.avatar) : download_image(FALLBACK_AVATAR_URL)
 
     # Resize avatar and add a white square border
     avatar.resize "#{AVATAR_SIZE}x#{AVATAR_SIZE}"
@@ -64,23 +61,15 @@ def download_image(url)
         tempfile.rewind
         MiniMagick::Image.open(tempfile.path)
       else
-        Rails.logger.error("Failed to download image from URL: #{url}. HTTP Error: #{response.code} #{response.message}. Using default avatar.")
-        MiniMagick::Image.open(default_avatar_path)
+        Rails.logger.error("Failed to download image from URL: #{url}. HTTP Error: #{response.code} #{response.message}.")
+        MiniMagick::Image.open(FALLBACK_AVATAR_URL)
       end
     rescue SocketError, Errno::ENOENT => e
-      Rails.logger.error("Failed to download image from URL: #{url}. Error: #{e.message}. Using default avatar.")
-      MiniMagick::Image.open(default_avatar_path)
+      Rails.logger.error("Failed to download image from URL: #{url}. Error: #{e.message}. Using fallback URL.")
+      MiniMagick::Image.open(FALLBACK_AVATAR_URL)
     ensure
       tempfile.close
       tempfile.unlink  # Unlink after we've processed the image
     end
   end  
-
-  def default_avatar
-    MiniMagick::Image.open(default_avatar_path)
-  end
-
-  def default_avatar_path
-    ActionController::Base.helpers.asset_path('greg.jpg')
-  end  
 end

app/views/analytics/index.html.erb

@@ -59,7 +59,7 @@
     <div class="bg-gray-800 rounded-lg shadow p-4">
       <h2 class="text-xl font-semibold mb-4">Daily Views (Last 30 Days)</h2>
       <%= line_chart @daily_views, 
-                     colors: ["#84CC16"],
+                     colors: ["#3B82F6"],
                      library: {
                        backgroundColor: 'transparent',
                        legend: { display: false },
@@ -100,30 +100,32 @@
     </div>
   </div>
 
-  <!-- Top Visitor Locations (New Section) -->
-  <div class="bg-gray-800 rounded-lg shadow p-4 mb-6">
-    <h2 class="text-xl font-semibold mb-4">Top Visitor Locations</h2>
-    <div class="overflow-x-auto">
-      <table class="w-full text-sm text-center table-auto">
-        <thead class="text-xs uppercase bg-gray-700">
-          <tr>
-            <th scope="col" class="px-4 py-3 rounded-tl-lg">City</th>
-            <th scope="col" class="px-4 py-3">Country</th>
-            <th scope="col" class="px-4 py-3 rounded-tr-lg">Views</th>
-          </tr>
-        </thead>
-        <tbody>
-          <% @location_data.each_with_index do |location, index| %>
+ <!-- Top Visitor Locations (New Section) -->
+<div class="bg-gray-800 rounded-lg shadow p-4 mb-6">
+  <h2 class="text-xl font-semibold mb-4">Top Visitor Locations</h2>
+  <div class="overflow-x-auto">
+    <table class="w-full text-sm text-center table-auto">
+      <thead class="text-xs uppercase bg-gray-700">
+        <tr>
+          <th scope="col" class="px-4 py-3 rounded-tl-lg">City</th>
+          <th scope="col" class="px-4 py-3">Country</th>
+          <th scope="col" class="px-4 py-3 rounded-tr-lg">Views</th>
+        </tr>
+      </thead>
+      <tbody>
+        <% @location_data.each_with_index do |location, index| %>
+          <% unless location[:city] == 'Unknown' && location[:country] == 'Unknown' %>
             <tr class="<%= index.even? ? 'bg-gray-800' : 'bg-gray-900' %> border-b border-gray-700">
-              <td class="px-4 py-3"><%= location[:city] || 'Unknown' %></td>
-              <td class="px-4 py-3"><%= location[:country] || 'Unknown' %></td>
+              <td class="px-4 py-3"><%= location[:city].present? && location[:city] != 'Unknown' ? location[:city] : '' %></td>
+              <td class="px-4 py-3"><%= location[:country].present? && location[:country] != 'Unknown' ? location[:country] : '' %></td>
               <td class="px-4 py-3"><%= number_with_delimiter(location[:count]) %></td>
             </tr>
           <% end %>
-        </tbody>
-      </table>
-    </div>
+        <% end %>
+      </tbody>
+    </table>
   </div>
+</div>
 
   <!-- Link Analytics Table -->
   <div class="bg-gray-800 rounded-lg shadow p-4 mb-6">

spec/controllers/users/registrations_controller_spec.rb

@@ -6,55 +6,35 @@
   end
 
   describe "POST #create" do
-    let(:valid_attributes) {
-      { email: "[email protected]", password: "password", password_confirmation: "password",
-        username: "testuser", full_name: "Test User", tags: "tag1,tag2", avatar_border: "white", invite_code: "POWEROVERWHELMING" }
-    }
+  let(:valid_attributes) {
+    { email: "[email protected]", password: "password", password_confirmation: "password",
+      username: "testuser", full_name: "Test User", tags: "tag1,tag2", avatar_border: "white", invite_code: "POWEROVERWHELMING" }
+  }
 
-    context "when sign-ups are enabled" do
-      before do
-        allow(Rails.application.config).to receive(:sign_ups_open).and_return(true)
-      end
-
-      it "creates a new User" do
-        expect {
-          post :create, params: { user: valid_attributes }
-        }.to change(User, :count).by(1)
-      end
+  context "when sign-ups are enabled" do
+    before do
+      allow(Rails.application.config).to receive(:sign_ups_open).and_return(true)
+    end
 
-      it "correctly processes tags" do
+    it "creates a new User" do
+      expect {
         post :create, params: { user: valid_attributes }
-        user = User.last
-        tags = user.tags.is_a?(String) ? JSON.parse(user.tags) : user.tags
-        expect(tags).to eq(["tag1", "tag2"])
-      end
+      }.to change(User, :count).by(1)
     end
 
-    context "when sign-ups are disabled" do
-      before do
-        allow(Rails.application.config).to receive(:sign_ups_open).and_return(false)
-      end
-
-      it "does not create a new User without a valid invite code and redirects to root path" do
-        invalid_attributes = valid_attributes.merge(invite_code: "INVALIDCODE")
-        expect {
-          post :create, params: { user: invalid_attributes }
-        }.not_to change(User, :count)
-        expect(response).to redirect_to(root_path)
-        expect(flash[:alert]).to eq("Sign-ups are currently disabled.")
-      end
-
-      it "creates a new User with a valid invite code" do
-        expect(controller).to receive(:after_sign_up_path_for).with(instance_of(User)).and_return("/path/to/redirect")
+    it "sets the fallback avatar URL if none is provided" do
+      post :create, params: { user: valid_attributes.merge(avatar: nil) }
+      user = User.last
+      expect(user.avatar).to eq('https://pbs.twimg.com/profile_images/1581014308397502464/NPogKMyk_400x400.jpg')
+    end
 
-        expect {
-          post :create, params: { user: valid_attributes }
-        }.to change(User, :count).by(1)
-
-        expect(response).to redirect_to("/path/to/redirect")
-      end
+    it "handles invalid avatar URLs and sets the fallback URL" do
+      post :create, params: { user: valid_attributes.merge(avatar: 'http://invalid-url.com/avatar.jpg') }
+      user = User.last
+      expect(user.avatar).to eq('https://pbs.twimg.com/profile_images/1581014308397502464/NPogKMyk_400x400.jpg')
     end
   end
+end
 
   describe "PUT #update" do
     let(:user) { create(:user, tags: ["old_tag1", "old_tag2"].to_json) }

spec/models/user_spec.rb

@@ -38,18 +38,18 @@
   end
 
   describe 'callbacks' do
-    it 'does not generate open graph image in test environment' do
-      user = build(:user)
-      expect(OpenGraphImageGenerator).not_to receive(:new)
+    it 'uses the fallback avatar URL when no avatar is provided' do
+      user = build(:user, avatar: nil)
       user.save
+      expect(user.avatar).to eq(User::FALLBACK_AVATAR_URL)
     end
-
-    it 'downloads and stores avatar after save' do
-      user = build(:user, avatar: 'http://example.com/avatar.jpg')
-      expect(user).to receive(:download_and_store_avatar)
+
+    it 'handles invalid avatar URLs and falls back to default' do
+      user = build(:user, avatar: 'http://invalid-url.com/avatar.jpg')
       user.save
+      expect(user.avatar).to eq(User::FALLBACK_AVATAR_URL)
     end
-  end
+  end  
 
   describe '#parsed_tags' do
     it 'returns parsed JSON when tags is a valid JSON string' do

spec/rails_helper.rb

@@ -34,8 +34,12 @@
     Rake::Task['assets:precompile'].invoke
   end
 
-  # Clean up uploaded files after each test
+  # Clean up uploaded files and generated avatars after each test
   config.after(:each) do
+    # Clean up the avatars generated during tests
+    FileUtils.rm_rf(Dir["#{Rails.root}/public/avatars"])
+
+    # Clean up other uploaded files
     FileUtils.rm_rf(Dir["#{Rails.root}/spec/support/uploads"])
   end