From 3af0a45b27f65cf83f5895d78b1d0b6f32f8db34 Mon Sep 17 00:00:00 2001 From: Tim Cowlishaw Date: Thu, 4 Jul 2024 08:50:26 +0200 Subject: [PATCH] Remove deprecated avatars and uploads implementation --- app/controllers/v0/me_controller.rb | 2 - app/controllers/v0/static_controller.rb | 1 - app/controllers/v0/uploads_controller.rb | 66 ----------- app/controllers/v0/users_controller.rb | 2 - app/models/avatar.rb | 5 - app/models/upload.rb | 31 ----- app/models/user.rb | 15 +-- app/views/v0/devices/_device.jbuilder | 1 - app/views/v0/users/_user.jbuilder | 1 - config/routes.rb | 6 - ...240704062854_remove_avatars_and_uploads.rb | 4 + db/schema.rb | 1 - public/examples/image_uploads/form.html | 108 ------------------ spec/controllers/v0/me_controller_spec.rb | 2 +- spec/controllers/v0/users_controller_spec.rb | 2 +- spec/factories/uploads.rb | 9 -- spec/models/upload_spec.rb | 32 ------ spec/models/user_spec.rb | 9 -- spec/requests/v0/uploads_spec.rb | 57 --------- 19 files changed, 7 insertions(+), 347 deletions(-) delete mode 100644 app/controllers/v0/uploads_controller.rb delete mode 100644 app/models/avatar.rb delete mode 100644 app/models/upload.rb create mode 100644 db/migrate/20240704062854_remove_avatars_and_uploads.rb delete mode 100644 public/examples/image_uploads/form.html delete mode 100644 spec/factories/uploads.rb delete mode 100644 spec/models/upload_spec.rb delete mode 100644 spec/requests/v0/uploads_spec.rb diff --git a/app/controllers/v0/me_controller.rb b/app/controllers/v0/me_controller.rb index f11616ea..f235edb1 100644 --- a/app/controllers/v0/me_controller.rb +++ b/app/controllers/v0/me_controller.rb @@ -39,8 +39,6 @@ def user_params :country_code, :profile_picture, :url, - :avatar, - :avatar_url ) end diff --git a/app/controllers/v0/static_controller.rb b/app/controllers/v0/static_controller.rb index b9d24667..c64e923b 100644 --- a/app/controllers/v0/static_controller.rb +++ b/app/controllers/v0/static_controller.rb @@ -116,7 +116,6 @@ def search h['url'] = v0_device_url(s.searchable_id) elsif s.searchable_type == 'User' h['username'] = s.searchable.username - h['avatar'] = s.searchable.avatar h['profile_picture'] = profile_picture_url(s.searchable) h['city'] = s.searchable.city h['url'] = v0_user_url(s.searchable_id) diff --git a/app/controllers/v0/uploads_controller.rb b/app/controllers/v0/uploads_controller.rb deleted file mode 100644 index 7d66d051..00000000 --- a/app/controllers/v0/uploads_controller.rb +++ /dev/null @@ -1,66 +0,0 @@ -module V0 -class UploadsController < ApplicationController - - before_action :check_if_authorized!, only: [:create] - after_action :verify_authorized, only: [:create] - - # create the document in rails, then send json back to our javascript to populate the form that will be - # going to amazon. - def create - check_missing_params 'filename' - @avatar = current_user.uploads.create(original_filename: params[:filename]) - authorize current_user, :update? - response.headers.except! 'X-Frame-Options' - render :json => { - :policy => s3_upload_policy_document, - :signature => s3_upload_signature, - :key => @avatar.key - # :success_action_redirect => devices_url - } - end - - def uploaded - check_missing_params 'key' - upload = Upload.find_by(key: CGI.unescape(params[:key]) ) - upload.user.update_attribute(:avatar_url, upload.full_path) - # head :ok - render json: {message: 'OK'}, status: :ok - end - - # # just in case you need to do anything after the document gets uploaded to amazon. - # # but since we are sending our docs via a hidden iframe, we don't need to show the user a - # # thank-you page. - # def s3_confirm - # head :ok - # end - -private - - # generate the policy document that amazon is expecting. - def s3_upload_policy_document - return @policy if @policy - ret = {"expiration" => 5.minutes.from_now.utc.xmlschema, - "conditions" => [ - {"bucket" => ENV['s3_bucket']}, - ["starts-with", "$key", @avatar.key], - {"acl" => "public-read"}, - {"success_action_status" => "200"}, - ["content-length-range", 0, 1073741824] # 100 MB - ] - } - @policy = Base64.encode64(ret.to_json).gsub(/\n/,'') - end - - # sign our request by Base64 encoding the policy document. - def s3_upload_signature - signature = Base64.encode64( - OpenSSL::HMAC.digest( - OpenSSL::Digest::Digest.new('sha1'), - (ENV['aws_secret_key'] || 'key_not_found'), - s3_upload_policy_document - ) - ).gsub("\n","") - end - - end -end diff --git a/app/controllers/v0/users_controller.rb b/app/controllers/v0/users_controller.rb index 1ec0f11c..056b1968 100644 --- a/app/controllers/v0/users_controller.rb +++ b/app/controllers/v0/users_controller.rb @@ -62,8 +62,6 @@ def user_params :city, :country_code, :url, - :avatar, - :avatar_url ) end diff --git a/app/models/avatar.rb b/app/models/avatar.rb deleted file mode 100644 index 5ef98838..00000000 --- a/app/models/avatar.rb +++ /dev/null @@ -1,5 +0,0 @@ -# This is the user's profile picture, it's stored in AWS S3 and is automatically -# resized using AWS Lambda. - -class Avatar < Upload -end diff --git a/app/models/upload.rb b/app/models/upload.rb deleted file mode 100644 index 15083b02..00000000 --- a/app/models/upload.rb +++ /dev/null @@ -1,31 +0,0 @@ -# Only Avatars are uploaded right now, this is the base class that they inherit. -# Some of its methods are specific to Avatar, this needs to be fixed. - -require 'digest' - -class Upload < ActiveRecord::Base - - belongs_to :user - - before_create :generate_key - - def new_filename - [created_at.to_i.to_s(32), original_filename].join('.') - end - - def full_path - "https://images.smartcitizen.me/s100/#{key}" - end - - def self.uploaded _key - upload = Upload.find_by(key: _key) - upload.user.update_attribute(:avatar_url, upload.key) - end - -private - - def generate_key - self.key = ['avatars', user.uuid[0..2], new_filename].join('/') - end - -end diff --git a/app/models/user.rb b/app/models/user.rb index 49cb1992..ebeb2f00 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -33,7 +33,6 @@ class User < ActiveRecord::Base has_many :devices, foreign_key: 'owner_id', after_add: :update_cached_device_ids!, after_remove: :update_cached_device_ids! has_many :sensors, through: :devices - has_many :uploads, dependent: :destroy has_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner has_one_attached :profile_picture @@ -50,7 +49,7 @@ def self.ransackable_attributes(auth_object = nil) end def self.ransackable_associations(auth_object = nil) - ["devices", "sensors", "uploads"] + ["devices", "sensors"] end def archive @@ -69,10 +68,6 @@ def access_token Doorkeeper::AccessToken.find_or_initialize_by(application_id: 4, resource_owner_id: id) end - def avatar - avatar_url || "https://smartcitizen.s3.amazonaws.com/avatars/default.svg" - end - def access_token! access_token.expires_in = 2.days.from_now access_token.save @@ -142,14 +137,6 @@ def update_all_device_ids! update_column(:cached_device_ids, device_ids.try(:sort)) end - def self.check_bad_avatar_urls - User.where.not(avatar_url: nil).each do |user| - unless `curl -I #{user.avatar_url} 2>/dev/null | head -n 1`.split(' ')[1] == "200" - puts [user.id, user.avatar_url].join(' - ') - end - end - end - def forward_device_readings? !!forwarding_token end diff --git a/app/views/v0/devices/_device.jbuilder b/app/views/v0/devices/_device.jbuilder index 0bd9cc9d..e3158b31 100644 --- a/app/views/v0/devices/_device.jbuilder +++ b/app/views/v0/devices/_device.jbuilder @@ -45,7 +45,6 @@ if local_assigns[:with_owner] && device.owner json.url device.owner.url unless local_assigns[:slim_owner] - json.avatar device.owner.avatar json.profile_picture profile_picture_url(device.owner) json.location device.owner.location json.device_ids device.owner.cached_device_ids diff --git a/app/views/v0/users/_user.jbuilder b/app/views/v0/users/_user.jbuilder index 7543208e..934dc4e2 100644 --- a/app/views/v0/users/_user.jbuilder +++ b/app/views/v0/users/_user.jbuilder @@ -3,7 +3,6 @@ json.(user, :uuid, :role, :username, - :avatar, :profile_picture, :url, :location, diff --git a/config/routes.rb b/config/routes.rb index f7f5ca2e..5aed44b4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -27,10 +27,6 @@ resources :sessions, only: :create resources :experiments - resources :uploads, path: 'avatars' do - post 'uploaded' => 'uploads#uploaded', on: :collection - end - resources :tag_sensors resources :tags resources :measurements @@ -40,8 +36,6 @@ get :forward, to: "forwarding#authorize" resources :me, only: [:index] do - patch 'avatar' => 'uploads#create', on: :collection - post 'avatar' => 'uploads#create', on: :collection patch '/' => 'me#update', on: :collection put '/' => 'me#update', on: :collection delete '/' => 'me#destroy', on: :collection diff --git a/db/migrate/20240704062854_remove_avatars_and_uploads.rb b/db/migrate/20240704062854_remove_avatars_and_uploads.rb new file mode 100644 index 00000000..2f47f75f --- /dev/null +++ b/db/migrate/20240704062854_remove_avatars_and_uploads.rb @@ -0,0 +1,4 @@ +class RemoveAvatarsAndUploads < ActiveRecord::Migration[6.1] + def change + end +end diff --git a/db/schema.rb b/db/schema.rb index 6c7c9344..7eef4fe3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,6 @@ # It's strongly recommended that you check this file into your version control system. ActiveRecord::Schema.define(version: 2024_08_12_081108) do - # These are extensions that must be enabled in order to support this database enable_extension "adminpack" enable_extension "hstore" diff --git a/public/examples/image_uploads/form.html b/public/examples/image_uploads/form.html deleted file mode 100644 index 263a6b8d..00000000 --- a/public/examples/image_uploads/form.html +++ /dev/null @@ -1,108 +0,0 @@ - - - - -Example Uploader - - - - -
- - - - - - - -
-
-
-
- -
-
-
- - - - - - - - - - \ No newline at end of file diff --git a/spec/controllers/v0/me_controller_spec.rb b/spec/controllers/v0/me_controller_spec.rb index 0ddfeaa9..b63ce9bc 100644 --- a/spec/controllers/v0/me_controller_spec.rb +++ b/spec/controllers/v0/me_controller_spec.rb @@ -1,5 +1,5 @@ require 'rails_helper' RSpec.describe V0::MeController do - skip { is_expected.to permit(:email,:username,:password,:city,:country_code,:url,:avatar,:avatar_url).for(:update) } + skip { is_expected.to permit(:email,:username,:password,:city,:country_code,:url).for(:update) } end diff --git a/spec/controllers/v0/users_controller_spec.rb b/spec/controllers/v0/users_controller_spec.rb index 324af31f..641ded0d 100644 --- a/spec/controllers/v0/users_controller_spec.rb +++ b/spec/controllers/v0/users_controller_spec.rb @@ -1,5 +1,5 @@ require 'rails_helper' RSpec.describe V0::UsersController do - it { is_expected.to permit(:email,:username,:password,:city,:country_code,:url,:avatar,:avatar_url).for(:create) } + it { is_expected.to permit(:email,:username,:password,:city,:country_code,:url).for(:create) } end diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb deleted file mode 100644 index 67a4556f..00000000 --- a/spec/factories/uploads.rb +++ /dev/null @@ -1,9 +0,0 @@ -FactoryBot.define do - - factory :upload do - type { "Avatar" } - association :user - original_filename { "testing.jpg" } - end - -end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb deleted file mode 100644 index 8581d395..00000000 --- a/spec/models/upload_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'rails_helper' - -RSpec.describe Upload, type: :model do - - it { is_expected.to belong_to(:user) } - - let(:user) { create(:user) } - let(:upload) { create(:upload, user: user, original_filename: 'testing.jpg') } - - it "works with filenames with spaces" - - it "works with filenames with special characters" - - it "has a new_filename" do - expect(upload.new_filename).to eq("#{upload.created_at.to_i.to_s(32)}.testing.jpg") - end - - it "has self.uploaded method" do - expect(user.avatar_url).to be_nil - Upload.uploaded( upload.key ) - user.reload - expect(user.avatar_url).to eq(upload.key) - end - - it "generates key on create and has full_path" do - Timecop.freeze do - expect(upload.key).to eq("avatars/#{upload.user.uuid[0..2]}/#{upload.created_at.to_i.to_s(32)}.testing.jpg") - expect(upload.full_path).to eq("https://images.smartcitizen.me/s100/#{upload.key}") - end - end - -end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a764acbf..f2a6b69c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -18,8 +18,6 @@ let(:user) { create(:user) } let(:homer) { build_stubbed(:user, username: 'homersimpson', email: 'homer@springfieldnuclear.com') } - it "has an avatar" - it "validates url" do expect(build(:user, url: nil)).to be_valid expect(build(:user, url: 'not a url')).to be_invalid @@ -27,13 +25,6 @@ expect(build(:user, url: 'https://www.facebook.com')).to be_valid end - skip "validates avatar" do - expect(build(:user, avatar: nil)).to be_valid - expect(build(:user, avatar: 'not a url')).to be_invalid - expect(build(:user, avatar: 'https://i.imgur.com/SZD8ADL.png')).to be_valid - expect(build(:user, avatar: 'http://i.imgur.com/SZD8ADL.JPEG')).to be_valid - end - it "is invalid with an email which isn't an email adddress" do expect(build(:user, email: "not an email")).to be_invalid end diff --git a/spec/requests/v0/uploads_spec.rb b/spec/requests/v0/uploads_spec.rb deleted file mode 100644 index 49f0b1fd..00000000 --- a/spec/requests/v0/uploads_spec.rb +++ /dev/null @@ -1,57 +0,0 @@ -require 'rails_helper' - -describe V0::UploadsController do - - let(:application) { create :application } - let(:user) { create :user } - let(:token) { create :access_token, application: application, resource_owner_id: user.id } - - describe "/POST" do - - it "creates an upload" do - j = api_post "avatars", - access_token: token.token, - filename: "test.jpg" - expect(response.status).to eq(200) - user.reload - expect(j['policy']).to_not be_blank - expect(j['signature']).to_not be_blank - expect(j['key']).to eq(user.uploads.last.key) - end - - it "requires filename" do - j = api_post "avatars", - access_token: token.token - expect(response.status).to eq(400) - end - - it "requires access_token" do - j = api_post "avatars", - filename: "test.jpg" - expect(response.status).to eq(401) - end - - end - - describe "amazon s3 callback" do - - let(:upload) { create(:upload, user: user) } - - it "responds to uploaded" do - # i know this doesn't belong here, but needed to check - expect(user.avatar_url).to be_blank - j = api_post "avatars/uploaded", key: upload.key - user.reload - expect(user.avatar_url).to eq(upload.full_path) - expect(response.status).to eq(200) - end - - it "requires key" do - j = api_post "avatars/uploaded" - expect(j['id']).to eq('parameter_missing') - expect(response.status).to eq(400) - end - - end - -end