diff --git a/Gemfile b/Gemfile index da601b4cc..504934edd 100644 --- a/Gemfile +++ b/Gemfile @@ -27,7 +27,7 @@ gem "redis", ">= 4.0.1" gem "bcrypt", "~> 3.1.7" # Windows does not include zoneinfo files, so bundle the tzinfo-data gem -gem "tzinfo-data", platforms: %i[windows jruby] +gem "tzinfo-data", platforms: %i[mswin mswin64 mingw x64_mingw jruby] # Reduces boot times through caching; required in config/boot.rb gem "bootsnap", require: false @@ -57,7 +57,7 @@ gem "omniauth-rails_csrf_protection", "~> 1.0.2" group :development, :test do # See https://guides.rubyonrails.org/debugging_rails_applications.html#debugging-with-the-debug-gem - gem "debug", platforms: %i[mri windows] + gem "debug", platforms: %i[mri mswin mswin64 mingw x64_mingw] gem "timecop" end diff --git a/app/controllers/settings/people_controller.rb b/app/controllers/settings/people_controller.rb index 75d660c26..a0f3c2b9b 100644 --- a/app/controllers/settings/people_controller.rb +++ b/app/controllers/settings/people_controller.rb @@ -16,7 +16,7 @@ def update def person_params h = params.require(:person).permit(:email, personable_attributes: [ - :id, :first_name, :last_name, :password, preferences: [:dark_mode], + :id, :first_name, :last_name, :password, :profile_picture, :remove_profile_picture, preferences: [:dark_mode], credentials_attributes: [ :id, :type, :password ] ]).to_h format_and_strip_all_but_first_valid_credential(h) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index a088b0899..adf434d00 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -94,4 +94,21 @@ def n_a_if_blank(value, n_a = "Not Available") def to_dollars(cents, precision: 2) number_to_currency(cents / 100.0, precision:) end + + # Profile picture helper methods + def user_avatar_image_tag(user, variant: :small, **options) + return nil unless user&.has_profile_picture? + + default_options = { + alt: "#{user.name.full}'s profile picture", + class: "w-full h-full object-cover" + } + + image_tag(user.profile_picture_url(variant), **default_options.merge(options)) + end + + def user_avatar_url(user, variant: :small, fallback: nil) + return fallback unless user&.has_profile_picture? + user.profile_picture_url(variant) || fallback + end end diff --git a/app/javascript/stimulus/profile_picture_upload_controller.js b/app/javascript/stimulus/profile_picture_upload_controller.js new file mode 100644 index 000000000..28a06b46b --- /dev/null +++ b/app/javascript/stimulus/profile_picture_upload_controller.js @@ -0,0 +1,66 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = ["fileInput", "currentAvatar", "previewContainer", "previewImage"] + + connect() { + // Initialize the controller + } + + chooseFile() { + this.fileInputTarget.click() + } + + previewImage(event) { + const file = event.target.files[0] + if (!file) return + + // Validate file type + if (!file.type.match(/^image\/(jpeg|jpg|png|gif|webp)$/)) { + alert("Please select a valid image file (JPEG, PNG, GIF, or WebP)") + this.clearFileInput() + return + } + + // Validate file size (5MB limit) + if (file.size > 5 * 1024 * 1024) { + alert("File size must be less than 5MB") + this.clearFileInput() + return + } + + // Create preview + const reader = new FileReader() + reader.onload = (e) => { + this.previewImageTarget.src = e.target.result + this.showPreview() + } + reader.readAsDataURL(file) + } + + removeImage() { + if (confirm("Are you sure you want to remove your profile picture?")) { + this.clearFileInput() + this.hidePreview() + + // Create a hidden input to signal removal + const removeInput = document.createElement('input') + removeInput.type = 'hidden' + removeInput.name = 'person[personable_attributes][remove_profile_picture]' + removeInput.value = '1' + this.element.appendChild(removeInput) + } + } + + showPreview() { + this.previewContainerTarget.classList.remove("hidden") + } + + hidePreview() { + this.previewContainerTarget.classList.add("hidden") + } + + clearFileInput() { + this.fileInputTarget.value = "" + } +} diff --git a/app/models/user.rb b/app/models/user.rb index c822d839b..07d076383 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,6 +4,13 @@ class User < ApplicationRecord has_secure_password validations: false has_person_name + # Profile picture attachment + has_one_attached :profile_picture do |attachable| + attachable.variant :thumbnail, resize_to_limit: [50, 50], preprocessed: true + attachable.variant :small, resize_to_limit: [100, 100], preprocessed: true + attachable.variant :medium, resize_to_limit: [200, 200], preprocessed: true + end + has_many :assistants, -> { not_deleted } has_many :assistants_including_deleted, class_name: "Assistant", inverse_of: :user, dependent: :destroy has_many :language_models, -> { not_deleted } @@ -26,6 +33,9 @@ class User < ApplicationRecord validates :first_name, presence: true validates :last_name, presence: true, on: :create, unless: :creating_google_credential? + # Profile picture validations + validate :profile_picture_validation + accepts_nested_attributes_for :credentials serialize :preferences, coder: JsonSerializer @@ -33,8 +43,45 @@ def preferences attributes["preferences"].with_defaults(dark_mode: "system") end + # Profile picture helper methods + def has_profile_picture? + profile_picture.attached? + end + + def profile_picture_url(variant = :small) + return nil unless has_profile_picture? + + if Rails.application.config.x.app_url.blank? + # For development/test environments without configured app URL + Rails.application.routes.url_helpers.rails_blob_url(profile_picture.variant(variant), only_path: true) + else + profile_picture.variant(variant).url + end + end + + # Virtual attribute for removing profile picture + def remove_profile_picture=(value) + if value.to_s == '1' && profile_picture.attached? + profile_picture.purge + end + end + private + def profile_picture_validation + return unless profile_picture.attached? + + # Validate content type + unless profile_picture.content_type.in?(%w[image/jpeg image/jpg image/png image/gif image/webp]) + errors.add(:profile_picture, "must be a valid image format (JPEG, PNG, GIF, or WebP)") + end + + # Validate file size + if profile_picture.byte_size > 5.megabytes + errors.add(:profile_picture, "must be less than 5MB") + end + end + def creating_google_credential? return false unless credential = credentials.first diff --git a/app/views/layouts/_user_avatar.erb b/app/views/layouts/_user_avatar.erb index 8a74e979c..f814c45ee 100644 --- a/app/views/layouts/_user_avatar.erb +++ b/app/views/layouts/_user_avatar.erb @@ -1,15 +1,31 @@ <%# locals: (user:, size:, classes: nil) -%> <% text_size = size > 6 ? "text-sm" : "text-xs" %> - - <%= at_most_two_initials(user.name.initials)&.upcase %> - +<% if user.has_profile_picture? %> + + <%= image_tag user.profile_picture_url(:small), + alt: "#{user.name.full}'s profile picture", + class: "w-full h-full object-cover" %> + +<% else %> + + <%= at_most_two_initials(user.name.initials)&.upcase %> + +<% end %> diff --git a/app/views/settings/people/_form.html.erb b/app/views/settings/people/_form.html.erb index adc60f593..ec27393ae 100644 --- a/app/views/settings/people/_form.html.erb +++ b/app/views/settings/people/_form.html.erb @@ -30,6 +30,63 @@ <%= user_fields.text_field :last_name, class: "block shadow rounded-md border border-gray-200 outline-none px-3 py-2 mt-2 w-full dark:text-black" %> + +
+ <%= user_fields.label :profile_picture, "Profile Picture" %> +
+
+ +
+ <%= render partial: "layouts/user_avatar", locals: { user: person.personable, size: 16, classes: "" } %> +
+ + +
+ <%= user_fields.file_field :profile_picture, + accept: "image/*", + class: "hidden", + data: { + profile_picture_upload_target: "fileInput", + action: "change->profile-picture-upload#previewImage" + } %> + +
+ <%= button_tag "Choose Photo", + type: "button", + class: "inline-block font-medium bg-gray-100 dark:bg-gray-900 dark:text-white border border-gray-400 rounded-lg py-2 px-4 cursor-pointer text-sm", + data: { action: "profile-picture-upload#chooseFile" } %> + + <% if person.personable.has_profile_picture? %> + <%= button_tag "Remove", + type: "button", + class: "inline-block font-medium bg-red-100 dark:bg-red-900 dark:text-white border border-red-400 rounded-lg py-2 px-4 cursor-pointer text-sm", + data: { action: "profile-picture-upload#removeImage" } %> + <% end %> +
+ +

+ JPEG, PNG, GIF, or WebP. Max 5MB. +

+
+
+ + + +
+
+ <% person.personable.credentials.type_is("PasswordCredential").each do |credential| %> <%= user_fields.fields_for :credentials_attributes, credential, index: credential.id do |credential_fields| %> <%= credential_fields.hidden_field :type, value: credential.type %> diff --git a/test/controllers/settings/people_controller_test.rb b/test/controllers/settings/people_controller_test.rb index 48ca42093..f6af6a4cf 100644 --- a/test/controllers/settings/people_controller_test.rb +++ b/test/controllers/settings/people_controller_test.rb @@ -94,6 +94,47 @@ class Settings::PeopleControllerTest < ActionDispatch::IntegrationTest assert @user.password_credential.authenticate("secret") end + # Profile picture tests + test "should upload profile picture" do + refute @user.has_profile_picture? + + params = person_params + params["personable_attributes"]["profile_picture"] = fixture_file_upload("test_image.jpg", "image/jpeg") + + patch settings_person_url, params: { person: params } + assert_redirected_to edit_settings_person_url + + assert @user.reload.has_profile_picture? + end + + test "should remove profile picture" do + # First attach a profile picture + @user.profile_picture.attach( + io: StringIO.new("fake image data"), + filename: "test.jpg", + content_type: "image/jpeg" + ) + assert @user.has_profile_picture? + + params = person_params + params["personable_attributes"]["remove_profile_picture"] = "1" + + patch settings_person_url, params: { person: params } + assert_redirected_to edit_settings_person_url + + refute @user.reload.has_profile_picture? + end + + test "should handle invalid profile picture gracefully" do + params = person_params + params["personable_attributes"]["profile_picture"] = fixture_file_upload("test_document.txt", "text/plain") + + patch settings_person_url, params: { person: params } + assert_response :unprocessable_entity + + refute @user.reload.has_profile_picture? + end + private def person_params diff --git a/test/fixtures/files/test_document.txt b/test/fixtures/files/test_document.txt new file mode 100644 index 000000000..9b659c313 --- /dev/null +++ b/test/fixtures/files/test_document.txt @@ -0,0 +1 @@ +This is a text document for testing invalid file uploads diff --git a/test/fixtures/files/test_image.jpg b/test/fixtures/files/test_image.jpg new file mode 100644 index 000000000..ce9947a55 --- /dev/null +++ b/test/fixtures/files/test_image.jpg @@ -0,0 +1 @@ +fake_image_data_for_testing diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb index 1497ec26f..682a0a463 100644 --- a/test/helpers/application_helper_test.rb +++ b/test/helpers/application_helper_test.rb @@ -30,4 +30,44 @@ class ApplicationHelperTest < ActionView::TestCase assert_equal "pQ", at_most_two_initials("p v Q") end + # Profile picture helper tests + test "user_avatar_image_tag returns nil when user has no profile picture" do + user = users(:keith) + assert_nil user_avatar_image_tag(user) + end + + test "user_avatar_image_tag returns image tag when user has profile picture" do + user = users(:keith) + user.profile_picture.attach( + io: StringIO.new("fake image data"), + filename: "test.jpg", + content_type: "image/jpeg" + ) + + result = user_avatar_image_tag(user) + assert_not_nil result + assert_includes result, "img" + assert_includes result, "test.jpg" + end + + test "user_avatar_url returns fallback when user has no profile picture" do + user = users(:keith) + fallback_url = "http://example.com/default.jpg" + + assert_equal fallback_url, user_avatar_url(user, fallback: fallback_url) + end + + test "user_avatar_url returns profile picture URL when user has profile picture" do + user = users(:keith) + user.profile_picture.attach( + io: StringIO.new("fake image data"), + filename: "test.jpg", + content_type: "image/jpeg" + ) + + result = user_avatar_url(user) + assert_not_nil result + assert_includes result, "test.jpg" + end + end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index b64cf217a..bd8a32883 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -91,4 +91,96 @@ class UserTest < ActiveSupport::TestCase end # Tests for creating_google_credential? are in person_test + + # Profile picture tests + test "has_profile_picture? returns false when no profile picture attached" do + user = users(:keith) + refute user.has_profile_picture? + end + + test "has_profile_picture? returns true when profile picture is attached" do + user = users(:keith) + user.profile_picture.attach( + io: StringIO.new("fake image data"), + filename: "test.jpg", + content_type: "image/jpeg" + ) + assert user.has_profile_picture? + end + + test "profile_picture_url returns nil when no profile picture attached" do + user = users(:keith) + assert_nil user.profile_picture_url + end + + test "profile_picture_url returns URL when profile picture is attached" do + user = users(:keith) + user.profile_picture.attach( + io: StringIO.new("fake image data"), + filename: "test.jpg", + content_type: "image/jpeg" + ) + assert_not_nil user.profile_picture_url + assert user.profile_picture_url.include?("test.jpg") + end + + test "remove_profile_picture= removes attached profile picture" do + user = users(:keith) + user.profile_picture.attach( + io: StringIO.new("fake image data"), + filename: "test.jpg", + content_type: "image/jpeg" + ) + assert user.has_profile_picture? + + user.remove_profile_picture = "1" + refute user.has_profile_picture? + end + + test "remove_profile_picture= does nothing when no profile picture attached" do + user = users(:keith) + refute user.has_profile_picture? + + assert_nothing_raised do + user.remove_profile_picture = "1" + end + refute user.has_profile_picture? + end + + test "profile picture validates content type" do + user = users(:keith) + user.profile_picture.attach( + io: StringIO.new("fake file data"), + filename: "test.txt", + content_type: "text/plain" + ) + refute user.valid? + assert_includes user.errors[:profile_picture], "must be a valid image format (JPEG, PNG, GIF, or WebP)" + end + + test "profile picture validates file size" do + user = users(:keith) + # Create a large fake file (6MB) + large_data = "x" * (6 * 1024 * 1024) + user.profile_picture.attach( + io: StringIO.new(large_data), + filename: "large.jpg", + content_type: "image/jpeg" + ) + refute user.valid? + assert_includes user.errors[:profile_picture], "must be less than 5MB" + end + + test "profile picture accepts valid image formats" do + user = users(:keith) + %w[image/jpeg image/jpg image/png image/gif image/webp].each do |content_type| + user.profile_picture.attach( + io: StringIO.new("fake image data"), + filename: "test.#{content_type.split('/').last}", + content_type: content_type + ) + assert user.valid?, "Should accept #{content_type}" + user.profile_picture.purge + end + end end