Auto-approval and upload fixed

app/controllers/packages_controller.rb

@@ -53,17 +53,22 @@ class PackagesController < ApplicationController
         new_screenshot.uploaderhash = session[:token]
         new_screenshot.uploaderip = session[:ip]
         new_screenshot.version = @package.version
-        new_screenshot.user = current_user
         
         # Can the upload get approved automatically?
-        if current_user
+        if user_signed_in?
-          new_screenshot.approve! if current_user.auto_approve?
+          new_screenshot.user = current_user
+          new_screenshot.approve! if auto_approve?
 
           # For anonymous uploads remember which screenshots have been
           # uploaded. If the user logs in later the screenshots will
           # be linked to their account.
         else
+          # Save the screenshot database record to get an ID
+          new_screenshot.save!
           (session[:uploaded_screenshots] ||= []) << new_screenshot.id
+          puts "-------------------"
+          puts session[:uploaded_screenshots]
+          puts "-------------------"
           Log.log "Anonymously uploaded screenshots #{new_screenshot.id} remembered in cookie session to assign it to a user later."
         end
         
@@ -315,4 +320,23 @@ class PackagesController < ApplicationController
   def params_screenshot_description
     params.require(:screenshot).permit(:description)
   end
+
+  # Do uploads from this user get approved automatically?
+  def auto_approve?
+    # Anonymous users need to go through moderation
+    return false unless user_signed_in?
+
+    # Admins do not need moderation
+    return true if current_user.is_admin?
+
+    # Debian developers do not need moderation
+    return true if current_user.provider == 'debian-sso'
+
+    # After one successfully approved screenshot users do not need moderation
+    return true if current_user.approved_screenshots.count > 0
+
+    # Any other user's upload must be moderated
+    return false
+  end
+
 end

app/controllers/users/my_sessions_controller.rb

@@ -1,23 +1,19 @@
 class Users::MySessionsController < Devise::SessionsController
-  # before_filter :before_login, :only => :create
+  after_action :after_login, :only => :create
-  after_filter :after_login, :only => :create
-
-  # def before_login
-  # end
 
   # Hook after successful login
   def after_login
     Log.log "User #{current_user} logged in."
     old_screenshots = session[:uploaded_screenshots]
-    raise
+    if old_screenshots.to_a.length > 0
-    if count = old_screenshots.to_a.length > 0
       old_screenshots.each do |id|
         ss = Screenshot.find(id)
         ss.user = @user
         ss.save!
       end
-      flash[:info] = "#{count} uploads have been added to your account"
+      flash[:info] = "#{old_screenshots.to_a.length} uploads have been added to your account"
       Log.log "Anonymously uploaded screenshots #{old_screenshots} added to account."
+      session[:uploaded_screenshots] = nil
     end
   end
 end

app/models/user.rb

@@ -50,24 +50,6 @@ class User < ApplicationRecord
     self.provider == nil
   end
 
-  # Do uploads from this user get approved automatically?
-  def auto_approve?
-    # Anonymous users need to go through moderation
-    return false unless user_signed_in?
-
-    # Admins do not need moderation
-    return true if current_user.is_admin?
-
-    # Debian developers do not need moderation
-    return true if current_user.provider == 'debian-sso'
-
-    # After one successfully approved screenshot users do not need moderation
-    return true if current_user.approved_screenshots.count > 0
-
-    # Any other user's upload must be moderated
-    return false
-  end
-
   def self.from_omniauth(auth)
     where(provider: auth.provider, email: auth.info.email).first_or_create do |user|
       user.provider = auth.provider
@@ -79,6 +61,10 @@ class User < ApplicationRecord
     end
   end
 
+  def approved_screenshots
+    self.screenshots.where(approved: true)
+  end
+
   # Seamlessly create a user account for the current client.
   # It helps track uploads because uploaded screenshots get assigned
   # to this user record. The user can later decide to use a real

test/fixtures/users.yml

@@ -18,3 +18,12 @@ normal:
   admin: 0
   encrypted_password: <%= Devise::Encryptor.digest(User, 'normalsecret') %>
   
+debian:
+  name: Detlef Debian
+  created_at: 2018-01-01
+  updated_at: 2018-01-01
+  email: detlef@debian.ork
+  sign_in_count: 0
+  provider: debian-sso
+  admin: 0
+  encrypted_password: <%= Devise::Encryptor.digest(User, 'normalsecret') %>

test/functional/upload_test.rb

@@ -1,7 +1,7 @@
 require 'test_helper'
 
 class BrowserBrowseTest < ActionDispatch::IntegrationTest
-  test "upload broken screenshot anonymously" do
+  test "upload broken screenshot anonymously and get error message" do
     visit package_path(name: 'firefox')
     click_on 'Upload a screenshot'
     attach_file 'file[]', Rails.root.join('test/fixtures/files/large-broken.png')
@@ -9,7 +9,7 @@ class BrowserBrowseTest < ActionDispatch::IntegrationTest
     page.must_have_content 'The image large-broken.png is not valid'
   end
 
-  test "upload png screenshot anonymously" do
+  test "upload png screenshot anonymously and await moderation" do
     visit package_path(name: 'firefox')
     click_on 'Upload a screenshot'
     img_count_before = page.find_all('img').count
@@ -26,8 +26,8 @@ class BrowserBrowseTest < ActionDispatch::IntegrationTest
     newest_screenshot.destroy
   end
 
-  test "upload screenshot as logged-in user" do
+  test "upload screenshot as Debian SSO user and get auto-approval" do
-    sign_in users(:normal)
+    sign_in users(:debian)
     visit upload_path(name: 'firefox')
     img_count_before = page.find_all('img').count
     attach_file 'file[]', Rails.root.join('test/fixtures/files/large1.png')