diff --git a/app/models/user.rb b/app/models/user.rb index 87d29f6d..d413c14d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -10,10 +10,6 @@ # remember_created_at :datetime # admin :boolean default(FALSE) # dagschotel_id :integer -# avatar_file_name :string -# avatar_content_type :string -# avatar_file_size :integer -# avatar_updated_at :datetime # orders_count :integer default(0) # koelkast :boolean default(FALSE) # name :string @@ -21,11 +17,11 @@ # frecency :integer default(0), not null # quickpay_hidden :boolean default(FALSE) # userkey :string +# zauth_id :integer # class User < ApplicationRecord include FriendlyId - include Avatarable include Statistics friendly_id :name, use: :finders @@ -40,12 +36,28 @@ class User < ApplicationRecord scope :publik, -> { where private: false } def self.from_omniauth(auth) - db_user = find_or_create_by!(name: auth.uid) do |user| - user.avatar = Paperclip.io_adapters.for(Identicon.data_url_for(auth.uid)) - user.generate_key! - user.private = true + zauth_id = auth.extra.raw_info["id"] + unless zauth_id.is_a? Integer + raise( + "zauth id is not valid, this is not good, what is happening? what did you do? i am confused and will give up" + ) + + end + + db_user = find_by(zauth_id: zauth_id) + + if db_user.nil? + db_user = find_or_create_by!(name: auth.uid) do |user| + user.generate_key! + user.private = true + end + + db_user.zauth_id = zauth_id end + # overwrite name (for if name changed) + db_user.name = auth.uid + # get roles info roles = auth.dig(:extra, :raw_info, :roles) || [] @@ -107,12 +119,14 @@ def balance bal - total_pending end + def avatar(*) + "https://zpi.zeus.gent/image/#{zauth_id || 0}" + end + # Static Users def self.guest - @guest || find_or_create_by(name: "Guest") do |user| - user.avatar = File.new(File.join("app", "assets", "images", "guest.png")) - end + @guest || find_or_create_by(name: "Guest") end def guest? @@ -121,7 +135,6 @@ def guest? def self.koelkast @koelkast || find_or_create_by(name: "Koelkast") do |user| - user.avatar = File.new(File.join("app", "assets", "images", "logo.png")) user.koelkast = true end end diff --git a/db/migrate/20251223210834_remove_avatar_from_users.rb b/db/migrate/20251223210834_remove_avatar_from_users.rb new file mode 100644 index 00000000..8328a2f1 --- /dev/null +++ b/db/migrate/20251223210834_remove_avatar_from_users.rb @@ -0,0 +1,8 @@ +class RemoveAvatarFromUsers < ActiveRecord::Migration[8.1] + def change + remove_column :users, :avatar_file_name, :string + remove_column :users, :avatar_content_type, :string + remove_column :users, :avatar_file_size, :integer + remove_column :users, :avatar_updated_at, :datetime + end +end diff --git a/db/migrate/20251223211742_add_zauth_id_to_users.rb b/db/migrate/20251223211742_add_zauth_id_to_users.rb new file mode 100644 index 00000000..173a4c26 --- /dev/null +++ b/db/migrate/20251223211742_add_zauth_id_to_users.rb @@ -0,0 +1,5 @@ +class AddZauthIdToUsers < ActiveRecord::Migration[8.1] + def change + add_column :users, :zauth_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 7967252e..0a982322 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2025_12_16_022119) do +ActiveRecord::Schema[8.1].define(version: 2025_12_23_211742) do create_table "barcodes", force: :cascade do |t| t.string "code", default: "", null: false t.datetime "created_at", precision: nil @@ -68,10 +68,6 @@ create_table "users", force: :cascade do |t| t.boolean "admin", default: false - t.string "avatar_content_type" - t.string "avatar_file_name" - t.bigint "avatar_file_size" - t.datetime "avatar_updated_at", precision: nil t.datetime "created_at", precision: nil t.integer "dagschotel_id" t.integer "frecency", default: 0, null: false @@ -83,6 +79,7 @@ t.datetime "remember_created_at", precision: nil t.datetime "updated_at", precision: nil t.string "userkey" + t.integer "zauth_id" t.index ["koelkast"], name: "index_users_on_koelkast" t.index ["orders_count"], name: "index_users_on_orders_count" end diff --git a/db/seeds.rb b/db/seeds.rb index 511c360e..cf58ec8f 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -21,7 +21,6 @@ User.create!( name: name, private: false, - avatar: Paperclip.io_adapters.for(Identicon.data_url_for(name)) ) end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index d751c3d5..fe1c5098 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -10,16 +10,13 @@ # remember_created_at :datetime # admin :boolean default(FALSE) # dagschotel_id :integer -# avatar_file_name :string -# avatar_content_type :string -# avatar_file_size :integer -# avatar_updated_at :datetime # orders_count :integer default(0) # koelkast :boolean default(FALSE) # name :string # private :boolean default(FALSE) # frecency :integer default(0), not null # quickpay_hidden :boolean +# zauth_id :integer # # quickpay_user GET /users/:id/quickpay(.:format) users#quickpay @@ -65,7 +62,7 @@ describe "PUT update" do it "loads the correct user" do - put :update, params: { id: user, user: attributes_for(:user).except(:avatar) } + put :update, params: { id: user, user: attributes_for(:user) } expect(assigns(:user)).to eq(user) end @@ -82,12 +79,6 @@ put :update, params: { id: user, user: { dagschotel_id: product.id } } expect(user.reload.dagschotel).to eq(product) end - - it "accepts real images" do - file = fixture_file_upload("real-image.png", "image/png") - put :update, params: { id: user, user: { avatar: file } } - expect(flash[:success]).to be_present - end end context "danger zone" do @@ -96,13 +87,6 @@ put :update, params: { id: user, user: {} } end.to raise_error(ActionController::ParameterMissing) end - - it "does not accept unreal images" do - file = fixture_file_upload("unreal-image.svg", "image/svg+xml") - expect do - put :update, params: { id: user, user: { avatar: file } } - end.to raise_error(ActiveRecord::RecordInvalid) - end end end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 2e746fea..2f820788 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -10,10 +10,6 @@ # remember_created_at :datetime # admin :boolean default(FALSE) # dagschotel_id :integer -# avatar_file_name :string -# avatar_content_type :string -# avatar_file_size :integer -# avatar_updated_at :datetime # orders_count :integer default(0) # koelkast :boolean default(FALSE) # name :string @@ -21,6 +17,7 @@ # frecency :integer default(0), not null # quickpay_hidden :boolean default(FALSE) # userkey :string +# zauth_id :integer # require "faker" @@ -29,7 +26,6 @@ FactoryBot.define do factory :user do name { Faker::Internet.user_name } - avatar { File.new(Rails.root.join("spec/fixtures/files/real-image.png")) } private { false } factory :admin do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2ab06555..bb124a47 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -10,10 +10,6 @@ # remember_created_at :datetime # admin :boolean default(FALSE) # dagschotel_id :integer -# avatar_file_name :string -# avatar_content_type :string -# avatar_file_size :integer -# avatar_updated_at :datetime # orders_count :integer default(0) # koelkast :boolean default(FALSE) # name :string @@ -21,6 +17,7 @@ # frecency :integer default(0), not null # quickpay_hidden :boolean default(FALSE) # userkey :string +# zauth_id :integer # require "webmock/rspec" @@ -37,13 +34,6 @@ ############ describe "fields" do - describe "avatar" do - it "is present" do - user.avatar = nil - expect(user).not_to be_valid - end - end - describe "orders_count" do it "automaticallies cache the number of orders" do balance = 5 @@ -79,7 +69,7 @@ { uid: "yet-another-test-user", extra: { - raw_info: { roles: [] } + raw_info: { roles: [], id: 7 } } } ) @@ -88,6 +78,7 @@ it "creates a new user with the correct name" do user = described_class.from_omniauth(auth_hash) expect(user.name).to eq("yet-another-test-user") + expect(user.zauth_id).to eq(7) expect(user.admin).to be(false) end end @@ -99,7 +90,30 @@ { uid: existing_user.name, extra: { - raw_info: { roles: [] } + raw_info: { roles: [], id: 7 } + } + } + ) + end + + it "finds the existing user" do + existing_user.zauth_id = 7 + existing_user.save + + user = described_class.from_omniauth(auth_hash) + expect(user).to eq(existing_user) + expect(user.admin).to be(false) + end + end + + describe "when the user already exists but without zauth id" do + let!(:existing_user) { create(:user) } + let(:auth_hash) do + OmniAuth::AuthHash.new( + { + uid: existing_user.name, + extra: { + raw_info: { roles: [], id: 7 } } } ) @@ -112,6 +126,29 @@ end end + describe "when the user exists but has changed their name" do + let!(:existing_user) { create(:user) } + let(:auth_hash) do + OmniAuth::AuthHash.new( + { + uid: "new-username-from-zauth", + extra: { + raw_info: { roles: [], id: existing_user.zauth_id } + } + } + ) + end + + it "updated the username" do + existing_user.zauth_id = 7 + existing_user.save + + user = described_class.from_omniauth(auth_hash) + expect(user.id).to eq(existing_user.id) + expect(user.name).to eq("new-username-from-zauth") + end + end + describe "when the user already exists and now has bestuur role" do let!(:existing_user) { create(:user) } let(:auth_hash) do @@ -119,7 +156,7 @@ { uid: existing_user.name, extra: { - raw_info: { roles: ["bestuur"] } + raw_info: { roles: ["bestuur"], id: 7 } } } ) @@ -130,6 +167,9 @@ end it "gets admin permissions" do + existing_user.zauth_id = 7 + existing_user.save + user = described_class.from_omniauth(auth_hash) expect(user).to eq(existing_user) expect(user.admin).to be(true) @@ -142,7 +182,7 @@ { uid: "a-test-admin-user-bestuur", extra: { - raw_info: { roles: ["bestuur"] } + raw_info: { roles: ["bestuur"], id: 7 } } } ) @@ -161,7 +201,7 @@ { uid: "a-test-admin-user-tap_admin", extra: { - raw_info: { roles: ["tap_admin"] } + raw_info: { roles: ["tap_admin"], id: 7 } } } ) @@ -173,6 +213,27 @@ expect(user.admin).to be(true) end end + + describe "when the user has an invalid zauth id" do + let(:auth_hash) do + OmniAuth::AuthHash.new( + { + uid: "a-test-user", + extra: { + raw_info: { roles: ["tap_admin"], id: "7" } + } + } + ) + end + + it "fails" do + expect do + described_class.from_omniauth(auth_hash) + end.to raise_error( + "zauth id is not valid, this is not good, what is happening? what did you do? i am confused and will give up" + ) + end + end end describe "static users" do