Merge pull request #158 from loftwah/dl/user-model-fixes

Dl/awful-changes-too-many

Author: loftwah

app/controllers/analytics_controller.rb

@@ -26,9 +26,9 @@ def set_user
   end
 
   def check_analytics_visibility
-    unless @user == current_user || @user.public_analytics?
+    unless current_user == @user || @user.public_analytics?
       flash[:alert] = "This user's analytics are not public."
-      redirect_to root_path
+      redirect_to root_path and return
     end
   end
 

app/controllers/users/registrations_controller.rb

@@ -42,14 +42,14 @@ def update
     @user = current_user
     @user.tags = JSON.parse(@user.tags) if @user.tags.is_a?(String)
 
-    is_password_change = params[:user][:password].present? || params[:user][:password_confirmation].present?
-    is_email_change = params[:user][:email].present? && params[:user][:email] != @user.email
-
-    if is_password_change || is_email_change
+    # Check if the user is trying to change their password
+    if params[:user][:password].present? || params[:user][:password_confirmation].present?
+      # If password change is requested, use Devise's `update_with_password`
       successfully_updated = @user.update_with_password(account_update_params)
     else
+      # If password change is not requested, remove the current_password requirement
       params[:user].delete(:current_password)
-      successfully_updated = @user.update_without_password(account_update_params)
+      successfully_updated = @user.update(account_update_params)
     end
 
     if successfully_updated
@@ -75,7 +75,7 @@ def valid_invite_code?(invite_code)
 
     # Check if the provided invite code matches any of the valid codes, case-insensitive
     valid_codes.any? { |code| code.casecmp(invite_code).zero? }
-  end  
+  end
 
   protected
 

app/helpers/open_graph_helper.rb

@@ -2,16 +2,22 @@ module OpenGraphHelper
   include Rails.application.routes.url_helpers
 
   def set_open_graph_tags(user)
-    twitter_handle = user.username.downcase
+    # Fallback values for Open Graph
+    default_title = 'Linkarooie - Simplify Your Online Presence'
+    default_description = 'Manage all your links in one place with Linkarooie. Create a central hub for your social and professional profiles.'
+    default_image = image_url('default_og_image.png')
+    default_image_alt = 'Linkarooie logo'
+    default_url = root_url
+    twitter_handle = user.username&.downcase || '@loftwah'
 
-    # Open Graph tags
-    content_for :og_title, user.full_name
-    content_for :og_description, user.description.truncate(160)
-    content_for :og_image, url_for("/uploads/og_images/#{user.username}_og.png")
-    content_for :og_image_alt, "#{user.full_name}'s profile image"
-    content_for :og_url, user_links_url(user.username)
+    # Open Graph tags with fallback values
+    content_for :og_title, user.full_name || default_title
+    content_for :og_description, (user.description || default_description).truncate(160)
+    content_for :og_image, user.username.present? ? url_for("/uploads/og_images/#{user.username}_og.png") : default_image
+    content_for :og_image_alt, user.full_name.present? ? "#{user.full_name}'s profile image" : default_image_alt
+    content_for :og_url, user_links_url(user.username || default_url)
 
-    # Twitter Card tags
+    # Twitter Card tags with fallback values
     content_for :twitter_card, 'summary_large_image'
     content_for :twitter_site, "@#{twitter_handle}"
     content_for :twitter_creator, "@#{twitter_handle}"

app/models/user.rb

@@ -1,6 +1,6 @@
 class User < ApplicationRecord
-
-  FALLBACK_AVATAR_URL = 'https://pbs.twimg.com/profile_images/1581014308397502464/NPogKMyk_400x400.jpg'
+  FALLBACK_AVATAR_URL = '/avatars/default_avatar.jpg'
+  FALLBACK_BANNER_URL = '/banners/default_banner.jpg'
 
   attr_accessor :invite_code
   devise :database_authenticatable, :registerable,
@@ -16,15 +16,16 @@ class User < ApplicationRecord
   VALID_USERNAME_REGEX = /\A[a-zA-Z0-9_]+\z/
 
   validates :username, presence: true, uniqueness: true, format: { with: VALID_USERNAME_REGEX, message: 'can only contain letters, numbers, and underscores' }
-
-  validates :username, uniqueness: true, allow_blank: true
   validates :full_name, presence: true
-  validate :ensure_username_presence
   validates :avatar_border, inclusion: { in: ['white', 'black', 'none', 'rainbow', 'rainbow-overlay'] }
   validates :avatar, format: { with: /\A(https?:\/\/).*\z/i, message: "must be a valid URL" }, allow_blank: true
+  validates :banner, format: { with: /\A(https?:\/\/).*\z/i, message: "must be a valid URL" }, allow_blank: true
 
-  after_save :generate_open_graph_image, unless: -> { Rails.env.test? }
-  after_save :download_and_store_avatar
+  before_validation :ensure_username_presence
+  before_create :set_default_images
+  after_create :generate_open_graph_image, unless: -> { Rails.env.test? }
+  after_save :download_and_store_avatar, if: -> { saved_change_to_avatar? && avatar.present? }
+  after_save :download_and_store_banner, if: -> { saved_change_to_banner? && banner.present? }
 
   serialize :tags, coder: JSON
 
@@ -36,7 +37,7 @@ def parsed_tags
         []
       end
     else
-      tags
+      tags || []
     end
   end
 
@@ -45,43 +46,93 @@ def generate_open_graph_image
   end
 
   def download_and_store_avatar
-    if avatar.blank?
-      self.avatar = FALLBACK_AVATAR_URL
-      save(validate: false)
+    download_and_store_image(:avatar, FALLBACK_AVATAR_URL)
+  end
+
+  def download_and_store_banner
+    download_and_store_image(:banner, FALLBACK_BANNER_URL)
+  end
+
+  def avatar_url
+    avatar_local_path.present? ? "/#{avatar_local_path}" : (avatar.presence || FALLBACK_AVATAR_URL)
+  end
+
+  def banner_url
+    banner_local_path.present? ? "/#{banner_local_path}" : (banner.presence || FALLBACK_BANNER_URL)
+  end
+
+  def valid_url?(url)
+    uri = URI.parse(url)
+    uri.is_a?(URI::HTTP) && !uri.host.nil?
+  rescue URI::InvalidURIError
+    false
+  end
+
+  private
+
+  def ensure_username_presence
+    if username.blank?
+      self.username = email.present? ? email.split('@').first : "user#{SecureRandom.hex(4)}"
+    end
+  end
+
+  def set_default_images
+    self.avatar ||= FALLBACK_AVATAR_URL
+    self.banner ||= FALLBACK_BANNER_URL
+  end
+
+  def download_and_store_image(type, fallback_url)
+    url = send(type)
+  
+    Rails.logger.info "Downloading #{type} from #{url}"
+  
+    if url.blank? || !valid_url?(url)
+      Rails.logger.warn "#{type.capitalize} URL invalid or blank. Using fallback."
+      update_column("#{type}_local_path", fallback_url)
       return
     end
 
     begin
-      avatar_dir = Rails.root.join('public', 'avatars')
-      FileUtils.mkdir_p(avatar_dir) unless File.directory?(avatar_dir)
+      uri = URI.parse(url)
+      Rails.logger.info "Attempting to download #{type} from #{uri}"
+      Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == 'https') do |http|
+        request = Net::HTTP::Get.new(uri)
+        response = http.request(request)
+        
+        if response.is_a?(Net::HTTPSuccess)
+          content_type = response['Content-Type']
+          Rails.logger.info "Downloaded #{type}, content type: #{content_type}"
+          
+          unless content_type.start_with?('image/')
+            raise "Invalid content type: #{content_type}"
+          end
+  
+          extension = case content_type
+                      when 'image/jpeg' then '.jpg'
+                      when 'image/png'  then '.png'
+                      when 'image/gif'  then '.gif'
+                      else ''
+                      end
+  
+          image_dir = Rails.root.join('public', type.to_s.pluralize)
+          FileUtils.mkdir_p(image_dir) unless File.directory?(image_dir)
+  
+          filename = "#{username}_#{type}#{extension}"
+          filepath = File.join(image_dir, filename)
 
-      uri = URI.parse(avatar)
-      filename = "#{username}_avatar#{File.extname(avatar)}"
-      filepath = File.join(avatar_dir, filename)
+          File.open(filepath, 'wb') { |file| file.write(response.body) }
 
-      response = Net::HTTP.get_response(uri)
-      if response.is_a?(Net::HTTPSuccess)
-        File.open(filepath, 'wb') do |local_file|
-          local_file.write(response.body)
+          update_column("#{type}_local_path", "#{type.to_s.pluralize}/#{filename}")
+          Rails.logger.info "#{type.capitalize} successfully downloaded for user #{username}"
+  
+        else
+          Rails.logger.warn "Failed to download #{type} for user #{username}: HTTP Error: #{response.code} #{response.message}. Using local fallback."
+          update_column(type, fallback_url)
         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 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}. Using fallback avatar."
-      self.avatar = FALLBACK_AVATAR_URL
-      save(validate: false)
+      Rails.logger.error "Failed to download #{type} for user #{username}: #{e.message}. Using fallback."
+      update_column(type, fallback_url)
     end
   end  
-
-  private
-
-  def ensure_username_presence
-    if username.blank?
-      self.username = email.present? ? email.split('@').first : "user#{SecureRandom.hex(4)}"
-    end
-  end
-end
+end