Anonymous uploads replaced by creation of ad-hoc users

app/controllers/application_controller.rb

@@ -15,8 +15,6 @@ class ApplicationController < ActionController::Base
   def get_current_users_screenshots
     if user_signed_in?
       @current_users_screenshots = current_user.screenshots
-    else
-      @current_users_screenshots = Screenshot.uploaded_by(session[:token])
     end
   end
 

app/controllers/packages_controller.rb

@@ -29,14 +29,16 @@ class PackagesController < ApplicationController
   # This action saves the screenshots already if they are valid. The user is
   # then given the chance to comment on and delete the screenshots again.
   def upload_receive
-    # Send a cookie to remember the user by the cookie session.
+    # Create a pseudo user account for the user.
-    create_user_token unless user_signed_in?
+    # The user won't know that an account is created.
+    # But this makes it easier to track who screenshots belong to.
+    create_pseudo_user unless user_signed_in?
 
     @package = Package.find_by!(name: params[:name])
     @valid_images = []
     @invalid_images = []
 
-      params[:file].each do |img|
+    params[:file].each do |img|
       new_screenshot = @package.screenshots.new(image: img)
 
       # Check if the image was valid
@@ -44,14 +46,22 @@ class PackagesController < ApplicationController
         new_screenshot.uploaderhash = session[:token]
         new_screenshot.uploaderip = session[:ip]
         new_screenshot.version = @package.version
+
+        # TODO: must it be moderated first?
+
         new_screenshot.save
-        Log.log "Screenshot #{new_screenshot.id} uploaded successfully from #{session[:ip]}. User has token #{session[:token]}"
+        Log.log "Screenshot #{new_screenshot.id} uploaded successfully from #{session[:ip]}. User has token #{session[:token]} or is #{current_user}"
         @valid_images.push new_screenshot
       else
+        Log.log "Screenshot #{new_screenshot.image_file_name} invalid (#{new_screenshot.errors[:image]})."
         @invalid_images.push new_screenshot
       end
     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 ')})
@@ -92,7 +102,10 @@ class PackagesController < ApplicationController
     # Is the user allowed to delete the screenshot?
     @screenshot = Screenshot.find(params[:id])
 
-    if user_can_alter_screenshot?
+    # Check if the user is allowed to change this screenshot
+    # - Is this the user's own screenshot? (anonymous)
+    if self.user = current_user or current_user.is_admin?
+      logger.debug "User #{current_user} deletes screenshot #{@screenshot}"
       @screenshot.destroy
       flash['notice'] = "Screenshot deleted."
       redirect_to :back
@@ -203,6 +216,17 @@ class PackagesController < ApplicationController
 
   private
 
+  # Seamlessly create a user account for the current client.
+  # It helps track uploads.
+  def create_pseudo_user
+    generated_password = Devise.friendly_token.first(8)
+    new_user = User.create(
+      name: 'Anonymous',
+      password: generated_password)
+    Log.log "New pseudo user for anonymous upload created: #{new_user}"
+    sign_in(new_user)
+  end
+
   # Send a dummy thumbnail reading "No screenshot available. Sorry."
   def thumbnail404
     send_file Rails.root.join('public/images/dummy/thumbnail404.png'),
@@ -240,13 +264,6 @@ class PackagesController < ApplicationController
     return packages
   end
 
-  # Check if the user is allowed to do changed to a screenshot
-  def user_can_alter_screenshot?
-    # - Is this the user's own screenshot?
-    # - Is the user an admin (=logged in)?
-    @screenshot.uploaderhash == session[:token] or user_signed_in?
-  end
-
   # Store a random identifier and the client's IP address in the session
   # for later identification.
   def create_user_token
@@ -254,7 +271,6 @@ class PackagesController < ApplicationController
     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

app/models/package.rb

@@ -48,7 +48,7 @@ class Package < ApplicationRecord
 
   # Return a query of all approved/public screenshots of this package
   def screenshots_approved
-    self.screenshots.find_by(approved: true)
+    self.screenshots.where(approved: true)
   end
 
   # Return a query of all approved/public screenshots of this package

app/models/screenshot.rb

@@ -90,33 +90,8 @@ class Screenshot < ApplicationRecord
     end
   end
 
-  # Check if the user is allowed to change this screenshot
-  def user_can_alter?
-    # - Is this the user's own screenshot? (anonymous)
-    if self.uploaderhash == session[:token]
-      logger.debug "Screenshot belongs to current anonymous user"
-      return true
-    end
-
-    # - Is this the user's own screenshot? (anonymous)
-    if self.user = current_user
-      logger.debug "Screenshot belongs to user who is currently logged in"
-      return true
-    end
-
-    # - Is the user an admin (=logged in)?
-    if current_user.can_admin?
-      logger.debug "User is an administrator and has super powers"
-      return true
-    end
-
-    return false
-  end
-
   # Publish a screenshot from the moderation queue
   def approve_screenshot!
-
-
     self.delete_reason = nil
     self.markedfordelete = false
     self.approved = true
@@ -131,11 +106,6 @@ class Screenshot < ApplicationRecord
     self.find_by(approved: true)
   end
 
-  # Query for screenshots being uploaded by a certain user (by their token)
-  def self.uploaded_by(token)
-    self.where(approved: false, uploaderhash: token)
-  end
-
   # Check whether the user has administrative permissions
   def can_admin?
     self.admin == 1

app/views/packages/_grid_thumbnail.slim

@@ -1,7 +1,7 @@
 // TODO: Handle packages with multiple screenshots
 a.black href=package_path(name: pkg.name)
   div.grid-thumbnail
-    - if pkg.screenshots.any?
+    - if pkg.screenshots_approved.any?
       // TODO: smarter selection of the most useful screenshot instead of taking the first one
       - screenshot = pkg.screenshots.first
       = image_tag(screenshot.image.url(:thumb, timestamp: false), alt: screenshot.caption, class: 'thumbnail')

app/views/packages/list.slim

@@ -19,7 +19,7 @@
               .pkgname
                 =pkg.name
                 ' >
-              - if pkg.screenshots.any?
+              - if pkg.screenshots_approved.any?
                 // TODO: smarter search for the best screenshot instead of taking the first one
                 - screenshot = pkg.screenshots.first
                 a.black title=screenshot.caption href=package_path(name: pkg.name)

doc/TODO

@@ -0,0 +1,46 @@
+Next tasks
+
+> Before next deployment.
+
+Why does weboob-qt show an uploaded image in grid mode if no screenshot is approved?
+
+Fix upload process.
+
+Fix my/my uploads.
+
+Make sure that the moderation workflow works.
+
+Any upload will trigger the creation of an ad-hoc account.
+Screenshots will always be assigned to a user.
+
+After an upload ask the user to add descriptions to the
+screenshots. And allow them to delete the screenshots.
+
+Check all code with "current_user" and "user_signed_in?" to
+make sure it matches the new concept.
+
+Create a user account called "Anonymous" - via a migration.
+All existing screenshots will become uploaded by that user.
+
+Turn moderation into a seperate page showing just the one
+screenshot and aks the user for a deletion reason.
+
+Make sure that users can delete their own screenshots but
+no other.
+
+Fix formatting of http://localhost:3000/packages/list?search=weboob&show=
+(Enumerations etc.)
+
+> After next deployment
+
+Make sure that ad-hoc users are recognized when they visit again.
+Devise should recognize them by their session cookie.
+
+Offer anonymous users to become real users to track their uploads.
+The screenshots will have to be moved to the new account
+and the ad-hoc account can be deleted.
+
+Move ad-hoc/anonymous uploads to the main anonymous user after
+a week.
+
+Add tests.