From 2ec80b4bded1fa1cd0eca81f396a43b3d862516b Mon Sep 17 00:00:00 2001 From: Christoph Haas Date: Mon, 3 Feb 2025 01:08:08 +0100 Subject: [PATCH] Automatic rubocop cleanups --- .rubocop.yml | 6 +- app/controllers/packages_controller.rb | 182 ++++++++++++------------- app/controllers/welcome_controller.rb | 5 +- 3 files changed, 96 insertions(+), 97 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 0780d38..0771bf2 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,5 @@ -# Style/Encoding: -# Enabled: false +Style/Encoding: + Enabled: false Layout/LineLength: Max: 99 @@ -11,4 +11,4 @@ Metrics/MethodLength: Max: 30 Style/PerlBackrefs: - Enabled: false \ No newline at end of file + Enabled: false diff --git a/app/controllers/packages_controller.rb b/app/controllers/packages_controller.rb index 69b09f3..add8d0a 100644 --- a/app/controllers/packages_controller.rb +++ b/app/controllers/packages_controller.rb @@ -1,28 +1,25 @@ class PackagesController < ApplicationController - # Allow legacy /uploadfile URL without CSRF protection - protect_from_forgery :except => :legacy_uploadfile - + protect_from_forgery except: :legacy_uploadfile + def list @packages = query_packages.paginate(page: params[:page], per_page: 6) @view_style = :list render :browse end - + def grid @packages = query_packages.paginate(page: params[:page], per_page: 24) @view_style = :grid render :browse end - + def details @package = Package.find_by(name: params[:name]) # Highlight a certain screenshot (by ID). This is used by the logs.slim # view to link to a certain screenshot of a package. - if params[:highlight] - @highlight_id = params[:highlight].to_i - end + @highlight_id = params[:highlight].to_i if params[:highlight] if @package.nil? @packagename = params[:name] @@ -32,12 +29,12 @@ class PackagesController < ApplicationController @screenshots = screenshots_visible_to_user(@package).paginate(page: @page, per_page: 6) end end - + # Show upload form for new images def upload @package = Package.find_by!(name: params[:name]) end - + # POST target of the screenshots upload form. # Receives uploaded images. Checks if they are valid. Asks for description. # This action saves the screenshots already if they are valid. The user is @@ -46,10 +43,10 @@ class PackagesController < ApplicationController @package = Package.find_by!(name: params[:name]) @valid_images = [] @invalid_images = [] - + # If Javascript is disabled a user may submit an empty selection of images. - if params[:file] == nil - flash[:error] = "You have not selected any images." + if params[:file].nil? + flash[:error] = 'You have not selected any images.' redirect_to upload_path return end @@ -59,7 +56,7 @@ class PackagesController < ApplicationController files = params[:file] # Turn into array if a single image was uploaded through AJAX - files=[files] if files.class != Array + files = [files] if files.class != Array files.each do |img| new_screenshot = @package.screenshots.new(simage: img) @@ -71,12 +68,12 @@ class PackagesController < ApplicationController new_screenshot.version = @package.version # ActiveStorage does not yet create a file checksum automatically. # Let's do that. It helps detect duplicate uploads later. - new_screenshot.image_fingerprint = Digest::MD5.hexdigest(File.read img.path) + new_screenshot.image_fingerprint = Digest::MD5.hexdigest(File.read(img.path)) # Check that this screenshot is not a duplicate for this package if @package.screenshots.where(image_fingerprint: new_screenshot.image_fingerprint).any? auditlog "Duplicate image with fingerprint #{new_screenshot.image_fingerprint} found. Rejecting.", - package: @package + package: @package all_errors << "Your file #{img.original_filename} is a duplicate. Sorry." else # TODO: Can the upload get approved automatically? @@ -86,8 +83,8 @@ class PackagesController < ApplicationController # But this makes it easier to track who screenshots belong to. unless user_signed_in? sign_in User.create_pseudo_user - auditlog "New pseudo user for anonymous upload created and logged in.", - package: @package, screenshot: new_screenshot + auditlog 'New pseudo user for anonymous upload created and logged in.', + package: @package, screenshot: new_screenshot end new_screenshot.user = current_user new_screenshot.approve! if can?(:approve, new_screenshot) @@ -97,54 +94,50 @@ class PackagesController < ApplicationController new_screenshot.save! auditlog "Screenshot #{new_screenshot.id} uploaded successfully.", - screenshot: new_screenshot, package: @package + screenshot: new_screenshot, package: @package @valid_images.push new_screenshot end else errors = new_screenshot.errors[:simage] auditlog "Screenshot #{img.original_filename} invalid (#{errors}).", - package: @package - #@invalid_images.push img.original_filename - #raise + package: @package + # @invalid_images.push img.original_filename + # raise all_errors << "Your file #{img.original_filename} #{errors.join(' and ')}." end end - - if all_errors.any? - flash[:error] = all_errors - end - + + flash[:error] = all_errors if all_errors.any? + # # Redirect back to upload form if all uploads were invalid # unless @valid_images.any? # auditlog "No valid images uploaded. Back to upload form." # redirect_to(upload_path, error: all_errors) and return # end - + # Show a list of invalid uploads by default. Or redirect to the review page # if all uploads were okay. # redirect_to upload_review_path unless @invalid_images - + # TODO # if @invalid_images… # ' #{image.image_file_name} (#{image.errors[:image].join(' and ')}) # Inform the admins about the upload - if @valid_images.any? - AdminMailer.with(package: @package).new_uploads_email.deliver_now - end + AdminMailer.with(package: @package).new_uploads_email.deliver_now if @valid_images.any? # Rails does not allow dots in the URL. So we cannot use the 'respond_to' # and 'format' ways to handle parameters. Instead the 'returns' parameters # is set in routes.rb to signal that this method was called by AJAX. if params[:returns] == :json # TODO: send all_errors back as JSON and make Javascript display it in #messages - render :json => {errors: all_errors.join(' ')} + render json: { errors: all_errors.join(' ') } else redirect_to package_path end end - + # Legacy action to upload an image along with metadata. # This was used in Debshots 1.x as the default upload method. # This method allows that old-style way to upload screenshots. @@ -157,9 +150,9 @@ class PackagesController < ApplicationController # - file def legacy_uploadfile @package = Package.find_by!(name: params[:packagename]) - + new_screenshot = @package.screenshots.new(image: params[:file]) - + # Check if the image was valid if new_screenshot.valid? # new_screenshot.uploaderhash = session[:token] @@ -167,93 +160,93 @@ class PackagesController < ApplicationController new_screenshot.version = @package.version new_screenshot.save auditlog "Screenshot #{new_screenshot.id} uploaded successfully from legacy upload form.", - screenshot: new_screenshot, package: @package + screenshot: new_screenshot, package: @package redirect_to package_path(params[:packagename]) else auditlog "Screenshot upload rejected. Errors: #{new_screenshot.errors.to_a}", - package: @package + package: @package head :not_acceptable end end - + def hide_screenshot # Is the user allowed to hide this screenshot? @screenshot = Screenshot.find(params[:id]) - + # Check if the user is allowed to change this screenshot # - Is this the user's own screenshot? (anonymous) if can? :hide, @screenshot auditlog "Screenshot #{@screenshot.id} hidden", - package: @screenshot.package + package: @screenshot.package @screenshot.hide! - flash['notice'] = "Screenshot hidden." + flash['notice'] = 'Screenshot hidden.' redirect_back(fallback_location: package_path(name: @screenshot.package.name)) else head :forbidden end end - + def unhide_screenshot # Is the user allowed to unhide this screenshot? @screenshot = Screenshot.find(params[:id]) - + # Check if the user is allowed to change this screenshot # - Is this the user's own screenshot? (anonymous) if can? :unhide, @screenshot auditlog "Screenshot #{@screenshot.id} un-hidden", - package: @screenshot.package + package: @screenshot.package @screenshot.unhide! - flash['notice'] = "Screenshot un-hidden." + flash['notice'] = 'Screenshot un-hidden.' redirect_back(fallback_location: package_path(name: @screenshot.package.name)) else head :forbidden end end - + def delete_screenshot # Is the user allowed to delete the screenshot? @screenshot = Screenshot.find(params[:id]) - + # Check if the user is allowed to change this screenshot # - Is this the user's own screenshot? (anonymous) if can? :destroy, @screenshot auditlog "Screenshot #{@screenshot.id} deleted", - package: @screenshot.package + package: @screenshot.package @screenshot.destroy # Increase the rejection counter for the user (social scoring) # if the screenshot is new and pending approval - if !@screenshot.approved + unless @screenshot.approved @screenshot.user.rejected_screenshots += 1 @screenshot.user.save! end - flash['notice'] = "Screenshot deleted." + flash['notice'] = 'Screenshot deleted.' redirect_back(fallback_location: package_path(name: @screenshot.package.name)) else head :forbidden end end - + def approve_screenshot @screenshot = Screenshot.find(params[:id]) if can? :approve, @screenshot @screenshot.approve! - auditlog "Screenshot approved", - package: @screenshot.package, screenshot: @screenshot + auditlog 'Screenshot approved', + package: @screenshot.package, screenshot: @screenshot # Increase the approval counter for the user (social scoring) @screenshot.user.approved_screenshots += 1 @screenshot.user.save! - flash['notice'] = "Screenshot approved." + flash['notice'] = 'Screenshot approved.' redirect_back(fallback_location: package_path(name: @screenshot.package.name)) else head :forbidden end end - + # Returns a 160x120 thumbnail image if posssible. # If the package is not found it returns a dummy image along with status 404. # If the package is found but has no screenshots then it also returns a @@ -261,27 +254,29 @@ class PackagesController < ApplicationController def thumbnail @package = Package.find_by(name: params[:name]) unless @package + logger.debug "Cannot render thumbnail for packages #{params[:name]} – no such package" thumbnail404 return end - + # Called as /thumbnail-with-version/:name/:version - if params[:version] - @screenshot = @package.best_screenshot_for_version(params[:version]) - # Called as /thumbnail/:name - else - @screenshot = @package.screenshots.approved.first - end - + @screenshot = if params[:version] + @package.best_screenshot_for_version(params[:version]) + # Called as /thumbnail/:name + else + @package.screenshots.approved.first + end + # Return a 404 if the package has no screenshots or the image was not found unless @screenshot + logger.debug "Cannot render thumbnail for packages #{params[:name]} – no screenshot found" thumbnail404 return end - + redirect_to @screenshot.simage_url(:thumb) end - + # Returns a large screenshot image if posssible. # If the package is not found it returns a dummy image along with status 404. # If the package is found but has no screenshots then it also returns a @@ -292,24 +287,24 @@ class PackagesController < ApplicationController screenshot404 return end - + # Called as /screenshot-with-version/:name/:version - if params[:version] - @screenshot = @package.best_screenshot_for_version(params[:version]) - # Called as /screenshot/:name - else - @screenshot = @package.screenshots.approved.first - end - + @screenshot = if params[:version] + @package.best_screenshot_for_version(params[:version]) + # Called as /screenshot/:name + else + @package.screenshots.approved.first + end + # Return a 404 if the package has no screenshots or the image was not found unless @screenshot screenshot404 return end - + redirect_to @screenshot.simage_url(:large) end - + # Receives a form with a simple text field 'description' so that users can update # the description of their screenshot. def update_screenshot_description @@ -318,10 +313,10 @@ class PackagesController < ApplicationController # @screenshot.description = params[:description] @screenshot.update params_screenshot_description @screenshot.save! - flash['notice'] = "Description updated." + flash['notice'] = 'Description updated.' redirect_back(fallback_location: package_path(name: @screenshot.package.name)) end - + # Show an HTML partial with reviews of this package from the Ubuntu API # def reviews # expires_in 1.day, public: true @@ -329,19 +324,21 @@ class PackagesController < ApplicationController # @reviews = get_ubuntu_reviews params[:name] # render '_reviews', layout: false # end - + private - + # Send a dummy thumbnail reading "No screenshot available. Sorry." def thumbnail404 - send_file Rails.root.join('public/images/dummy/thumbnail404.png'), type: 'image/png', disposition: 'inline', status: 404 + send_file Rails.root.join('public/images/dummy/thumbnail404.png'), type: 'image/png', + disposition: 'inline', status: 404 end - + # Send a dummy screenshot reading "No screenshot available. Sorry." def screenshot404 - send_file Rails.root.join('public/images/dummy/screenshot404.png'), type: 'image/png', disposition: 'inline', status: 404 + send_file Rails.root.join('public/images/dummy/screenshot404.png'), type: 'image/png', + disposition: 'inline', status: 404 end - + # Return packages matching the criteria given by parameters def query_packages packages = Package.order(visits: :desc) @@ -351,7 +348,7 @@ class PackagesController < ApplicationController logger.debug "Searching for #{params[:search]}" packages = packages.general_search(params[:search]) end - + case params[:show] when 'with' # Enrich the result with the screenshots readable by the current user (CanCanCan) @@ -362,33 +359,32 @@ class PackagesController < ApplicationController logger.debug 'Limiting packages to those without screenshots' end - return packages + packages end - + # Store a random identifier and the client's IP address in the session # for later identification. # def create_user_token # session[:token] ||= SecureRandom.hex # session[:ip] ||= request.remote_ip # end - + # Get reviews of this package from the Ubuntu API def get_ubuntu_reviews(packagename) # Use the URL defined in the configuration to get a JSON string url = Rails.configuration.ubuntu_reviews_api_url % packagename logger.debug "Loading Ubuntu reviews for package #{packagename} from #{url}" - + body = open(url).read # Turn JSON into a Ruby data structure json = JSON.parse(body) # Only show english reviews # TODO: Support further languages - json = json.select {|x| x['language']=='en'} + json = json.select { |x| x['language'] == 'en' } # Sort by 'usefulness_total' (how many people found this review useful) - json = json.sort { |x,y| y['usefulness_total'].to_i <=> x['usefulness_total'].to_i} - return json + json.sort { |x, y| y['usefulness_total'].to_i <=> x['usefulness_total'].to_i } end - + def params_screenshot_description params.require(:screenshot).permit(:description) end diff --git a/app/controllers/welcome_controller.rb b/app/controllers/welcome_controller.rb index 4310eee..d2dca58 100644 --- a/app/controllers/welcome_controller.rb +++ b/app/controllers/welcome_controller.rb @@ -1,10 +1,12 @@ +# This controller creates the main page and the about page. class WelcomeController < ApplicationController + # Render the main page. def home @newest_upload = Screenshot.where(approved: true).first @most_popular_package = Package.with_screenshots.order(visits: :desc).first - @package_count = Package.count + @package_count = Package.count # Get up 100 screenshots where the packages have the most visits query = Package.without_screenshots.order(visits: :desc) @@ -13,6 +15,7 @@ class WelcomeController < ApplicationController @most_wanted_package = query.offset(random_offset).first end + # Render the about page. def about @package_sources = Rails.configuration.package_sources @package_count = Package.count