Screenshot upload process fixed and tested

app/controllers/packages_controller.rb

@@ -56,7 +56,9 @@ class PackagesController < ApplicationController
         new_screenshot.version = @package.version
         new_screenshot.user = current_user
         
-        # TODO: must it be moderated first?
+        # Package is not approved by default. Approve automatically
+        # for authenticated users.
+        new_screenshot.approve! if current_user and not current_user.is_anonymous?
         
         new_screenshot.save
         Log.log "Screenshot #{new_screenshot.id} uploaded successfully from #{session[:ip]}. User has token #{session[:token]} or is #{current_user}"
@@ -145,7 +147,7 @@ class PackagesController < ApplicationController
   
   def approve_screenshot
     @screenshot = Screenshot.find(params[:id])
-    @screenshot.approve_screenshot!
+    @screenshot.approve!
     flash['notice'] = "Screenshot approved."
     redirect_back(fallback_location: package_path(name: @screenshot.package.name))
   end

app/models/screenshot.rb

@@ -91,7 +91,7 @@ class Screenshot < ApplicationRecord
   end
 
   # Publish a screenshot from the moderation queue
-  def approve_screenshot!
+  def approve!
     self.delete_reason = nil
     self.markedfordelete = false
     self.approved = true

app/models/user.rb

@@ -43,6 +43,13 @@ class User < ApplicationRecord
     self.admin == 1
   end
 
+  # Check if a user has been created on-the-fly and is just an
+  # anonymous user who uploaded a screenshot. They can turn this
+  # user record into a registered account though.
+  def is_anonymous?
+    self.provider == nil
+  end
+
   def self.from_omniauth(auth)
     where(provider: auth.provider, email: auth.info.email).first_or_create do |user|
       user.provider = auth.provider

test/functional/upload_test.rb

@@ -0,0 +1,45 @@
+require 'test_helper'
+
+class BrowserBrowseTest < ActionDispatch::IntegrationTest
+  test "upload broken screenshot anonymously" do
+    visit package_path(name: 'firefox')
+    click_on 'Upload a screenshot'
+    attach_file 'file[]', Rails.root.join('test/fixtures/files/large-broken.png')
+    click_on 'Start upload'
+    page.must_have_content 'The image large-broken.png is not valid'
+  end
+
+  test "upload png screenshot anonymously" do
+    visit package_path(name: 'firefox')
+    click_on 'Upload a screenshot'
+    img_count_before = page.find_all('img').count
+    attach_file 'file[]', Rails.root.join('test/fixtures/files/large1.png')
+    click_on 'Start upload'
+    img_count_after = page.find_all('img').count
+    assert_equal img_count_after, img_count_before + 1
+
+    # Screenshot must be moderated
+    newest_screenshot = Package.find_by_name(:firefox).screenshots.order(id: :desc).first
+    assert_equal newest_screenshot.approved, false
+
+    # Clean up
+    newest_screenshot.destroy
+  end
+
+  test "upload screenshot as logged-in user" do
+    sign_in users(:normal)
+    visit upload_path(name: 'firefox')
+    img_count_before = page.find_all('img').count
+    attach_file 'file[]', Rails.root.join('test/fixtures/files/large1.png')
+    click_on 'Start upload'
+    img_count_after = page.find_all('img').count
+    assert_equal img_count_after, img_count_before + 1
+
+    # Screenshot is instantly public
+    newest_screenshot = Package.find_by_name(:firefox).screenshots.order(id: :desc).first
+    assert_equal newest_screenshot.approved, true
+
+    # Clean up
+    newest_screenshot.destroy
+  end
+end

test/test_helper.rb

@@ -33,6 +33,9 @@ class ActionDispatch::IntegrationTest
   # Make `assert_*` methods behave like Minitest assertions
   include Capybara::Minitest::Assertions
 
+  # Help simulate logins
+  include Devise::Test::IntegrationHelpers
+
   # Reset sessions and driver between tests
   # Use super wherever this method is redefined in your individual test classes
   def teardown