From 26c39f855c40039c954476f771c1299f2ec20011 Mon Sep 17 00:00:00 2001 From: Hannes Date: Mon, 15 Dec 2025 21:33:33 +0100 Subject: [PATCH 1/7] remove user avatar columns --- app/models/user.rb | 11 +---------- db/migrate/20251223210834_remove_avatar_from_users.rb | 8 ++++++++ db/schema.rb | 6 +----- db/seeds.rb | 1 - 4 files changed, 10 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20251223210834_remove_avatar_from_users.rb diff --git a/app/models/user.rb b/app/models/user.rb index 87d29f6d..81965031 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 @@ -25,7 +21,6 @@ class User < ApplicationRecord include FriendlyId - include Avatarable include Statistics friendly_id :name, use: :finders @@ -41,7 +36,6 @@ class User < ApplicationRecord 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 end @@ -110,9 +104,7 @@ def balance # 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 +113,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/schema.rb b/db/schema.rb index 7967252e..5571058f 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_210834) 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 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 From 708d2a452b92941a757ee28d60556aacd1ee74f1 Mon Sep 17 00:00:00 2001 From: Hannes Date: Mon, 15 Dec 2025 21:53:44 +0100 Subject: [PATCH 2/7] return zpi url as avatar --- app/models/user.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index 81965031..0c1b1fad 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -101,6 +101,10 @@ def balance bal - total_pending end + def avatar(kind = nil) + "https://zpi.zeus.gent/image/#{id}" + end + # Static Users def self.guest From f44bc8cfa6d03d7257c57d6f5f21ddc4e0632dd9 Mon Sep 17 00:00:00 2001 From: Hannes Date: Mon, 15 Dec 2025 22:11:26 +0100 Subject: [PATCH 3/7] store and use zauth id for avatar --- app/models/user.rb | 5 ++++- db/migrate/20251223211742_add_zauth_id_to_users.rb | 5 +++++ db/schema.rb | 3 ++- 3 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20251223211742_add_zauth_id_to_users.rb diff --git a/app/models/user.rb b/app/models/user.rb index 0c1b1fad..277f2a13 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -17,6 +17,7 @@ # frecency :integer default(0), not null # quickpay_hidden :boolean default(FALSE) # userkey :string +# zauth_id :integer # class User < ApplicationRecord @@ -40,6 +41,8 @@ def self.from_omniauth(auth) user.private = true end + db_user.zauth_id = auth.extra.raw_info["id"] + # get roles info roles = auth.dig(:extra, :raw_info, :roles) || [] @@ -102,7 +105,7 @@ def balance end def avatar(kind = nil) - "https://zpi.zeus.gent/image/#{id}" + "https://zpi.zeus.gent/image/#{zauth_id || 0}" end # Static Users 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 5571058f..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_23_210834) 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 @@ -79,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 From cf4ac7077ffc136d76ac0adc19575b9d377e3249 Mon Sep 17 00:00:00 2001 From: Hannes Date: Mon, 15 Dec 2025 22:38:12 +0100 Subject: [PATCH 4/7] get users using their zauth_id --- app/models/user.rb | 23 ++++++++++++++++++----- spec/controllers/users_controller_spec.rb | 20 ++------------------ spec/factories/users.rb | 6 +----- spec/models/user_spec.rb | 12 +----------- 4 files changed, 22 insertions(+), 39 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 277f2a13..869e2317 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -36,12 +36,25 @@ 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.generate_key! - user.private = true + zauth_id = auth.extra.raw_info["id"] + if !zauth_id.is_a? Integer + puts "help" + return end - db_user.zauth_id = auth.extra.raw_info["id"] + 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) || [] @@ -104,7 +117,7 @@ def balance bal - total_pending end - def avatar(kind = nil) + def avatar(*) "https://zpi.zeus.gent/image/#{zauth_id || 0}" 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..063d3ccb 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 From d9c3afdb027f43e47d2a5046672cbe0c04a86c13 Mon Sep 17 00:00:00 2001 From: Hannes Date: Mon, 15 Dec 2025 23:11:07 +0100 Subject: [PATCH 5/7] add tests for user with zauth id --- spec/models/user_spec.rb | 76 +++++++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 13 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 063d3ccb..3e1bf4d3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -69,7 +69,7 @@ { uid: "yet-another-test-user", extra: { - raw_info: { roles: [] } + raw_info: { roles: [], id: 7 } } } ) @@ -78,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 @@ -89,40 +90,89 @@ { uid: existing_user.name, extra: { - raw_info: { roles: [] } + raw_info: { roles: [], id: 7 } } } - ) - end - + ) + 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 and now has bestuur role" do + 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: ["bestuur"] } + raw_info: { roles: [], id: 7 } } } ) end - it "does not start with admin" do + it "finds the existing user" do + 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 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 - it "gets admin permissions" do user = described_class.from_omniauth(auth_hash) - expect(user).to eq(existing_user) - expect(user.admin).to be(true) + 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 + OmniAuth::AuthHash.new( + { + uid: existing_user.name, + extra: { + raw_info: { roles: ["bestuur"], id: 7 } + } + } + ) + end + + it "does not start with admin" do + expect(user.admin).to be(false) + 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) end end @@ -132,7 +182,7 @@ { uid: "a-test-admin-user-bestuur", extra: { - raw_info: { roles: ["bestuur"] } + raw_info: { roles: ["bestuur"], id: 7 } } } ) @@ -151,7 +201,7 @@ { uid: "a-test-admin-user-tap_admin", extra: { - raw_info: { roles: ["tap_admin"] } + raw_info: { roles: ["tap_admin"], id: 7 } } } ) From 9aa249c331f9a82ce7b9632c002bfd61b7166eb1 Mon Sep 17 00:00:00 2001 From: Hannes Date: Mon, 15 Dec 2025 23:15:08 +0100 Subject: [PATCH 6/7] rubocopped --- app/models/user.rb | 4 ++-- spec/models/user_spec.rb | 36 ++++++++++++++++++------------------ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 869e2317..57f80c82 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -37,14 +37,14 @@ class User < ApplicationRecord def self.from_omniauth(auth) zauth_id = auth.extra.raw_info["id"] - if !zauth_id.is_a? Integer + unless zauth_id.is_a? Integer puts "help" return end db_user = find_by(zauth_id: zauth_id) - if db_user == nil + if db_user.nil? db_user = find_or_create_by!(name: auth.uid) do |user| user.generate_key! user.private = true diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3e1bf4d3..32795c25 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -93,13 +93,13 @@ raw_info: { roles: [], id: 7 } } } - ) - end - + ) + 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) @@ -159,20 +159,20 @@ raw_info: { roles: ["bestuur"], id: 7 } } } - ) - end - - it "does not start with admin" do - expect(user.admin).to be(false) - 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) + ) + end + + it "does not start with admin" do + expect(user.admin).to be(false) + 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) end end From c61c6a63339abf932440a12519a728bba87009b4 Mon Sep 17 00:00:00 2001 From: Hannes Date: Mon, 15 Dec 2025 23:25:22 +0100 Subject: [PATCH 7/7] check if the zauth id is valid --- app/models/user.rb | 6 ++++-- spec/models/user_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 57f80c82..d413c14d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -38,8 +38,10 @@ class User < ApplicationRecord def self.from_omniauth(auth) zauth_id = auth.extra.raw_info["id"] unless zauth_id.is_a? Integer - puts "help" - return + 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) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 32795c25..bb124a47 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -213,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