Commit 1dd0caf4 authored by Hannes Rosenögger's avatar Hannes Rosenögger

Merge branch 'fix-avatar-removal' into 'master'

Fix bug where avatar filenames were not actually deleted from the database during removal

This would result in a 404 error in certain views.

The `save` call was being rolled back due to an error in the validation step.
Relax the validation step so that this works.

Closes #1570

See merge request !620
parents 8b9e3af8 bf4b4384
...@@ -40,6 +40,7 @@ v 7.11.0 (unreleased) ...@@ -40,6 +40,7 @@ v 7.11.0 (unreleased)
- Fix bug where commit data would not appear in some subdirectories (Stan Hu) - Fix bug where commit data would not appear in some subdirectories (Stan Hu)
- Unescape branch names in compare commit (Stan Hu) - Unescape branch names in compare commit (Stan Hu)
- Task lists are now usable in comments, and will show up in Markdown previews. - Task lists are now usable in comments, and will show up in Markdown previews.
- Fix bug where avatar filenames were not actually deleted from the database during removal (Stan Hu)
- Fix bug where Slack service channel was not saved in admin template settings. (Stan Hu) - Fix bug where Slack service channel was not saved in admin template settings. (Stan Hu)
- Move snippets UI to fluid layout - Move snippets UI to fluid layout
- Improve UI for sidebar. Increase separation between navigation and content - Improve UI for sidebar. Increase separation between navigation and content
......
...@@ -20,7 +20,7 @@ class Group < Namespace ...@@ -20,7 +20,7 @@ class Group < Namespace
has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember' has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
has_many :users, through: :group_members has_many :users, through: :group_members
validate :avatar_type, if: ->(user) { user.avatar_changed? } validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
validates :avatar, file_size: { maximum: 200.kilobytes.to_i } validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
mount_uploader :avatar, AvatarUploader mount_uploader :avatar, AvatarUploader
......
...@@ -144,7 +144,7 @@ class Project < ActiveRecord::Base ...@@ -144,7 +144,7 @@ class Project < ActiveRecord::Base
validates :star_count, numericality: { greater_than_or_equal_to: 0 } validates :star_count, numericality: { greater_than_or_equal_to: 0 }
validate :check_limit, on: :create validate :check_limit, on: :create
validate :avatar_type, validate :avatar_type,
if: ->(project) { project.avatar && project.avatar_changed? } if: ->(project) { project.avatar.present? && project.avatar_changed? }
validates :avatar, file_size: { maximum: 200.kilobytes.to_i } validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
mount_uploader :avatar, AvatarUploader mount_uploader :avatar, AvatarUploader
......
...@@ -148,7 +148,7 @@ class User < ActiveRecord::Base ...@@ -148,7 +148,7 @@ class User < ActiveRecord::Base
validates :notification_level, inclusion: { in: Notification.notification_levels }, presence: true validates :notification_level, inclusion: { in: Notification.notification_levels }, presence: true
validate :namespace_uniq, if: ->(user) { user.username_changed? } validate :namespace_uniq, if: ->(user) { user.username_changed? }
validate :avatar_type, if: ->(user) { user.avatar_changed? } validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
validate :unique_email, if: ->(user) { user.email_changed? } validate :unique_email, if: ->(user) { user.email_changed? }
validate :owns_notification_email, if: ->(user) { user.notification_email_changed? } validate :owns_notification_email, if: ->(user) { user.notification_email_changed? }
validate :owns_public_email, if: ->(user) { user.public_email_changed? } validate :owns_public_email, if: ->(user) { user.public_email_changed? }
...@@ -309,7 +309,7 @@ class User < ActiveRecord::Base ...@@ -309,7 +309,7 @@ class User < ActiveRecord::Base
if primary_email_record if primary_email_record
primary_email_record.destroy primary_email_record.destroy
self.emails.create(email: self.email_was) self.emails.create(email: self.email_was)
self.update_secondary_emails! self.update_secondary_emails!
end end
end end
......
require 'spec_helper'
describe Groups::AvatarsController do
let(:user) { create(:user) }
let(:group) { create(:group, owner: user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) }
before do
sign_in(user)
end
it 'destroy should remove avatar from DB' do
delete :destroy, group_id: group.path
@group = assigns(:group)
expect(@group.avatar.present?).to be_falsey
expect(@group).to be_valid
end
end
require 'spec_helper'
describe Profiles::AvatarsController do
let(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png")) }
before do
sign_in(user)
controller.instance_variable_set(:@user, user)
end
it 'destroy should remove avatar from DB' do
delete :destroy
@user = assigns(:user)
expect(@user.avatar.present?).to be_falsey
expect(@user).to be_valid
end
end
require 'spec_helper'
describe Projects::AvatarsController do
let(:project) { create(:project, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) }
let(:user) { create(:user) }
before do
sign_in(user)
project.team << [user, :developer]
controller.instance_variable_set(:@project, project)
end
it 'destroy should remove avatar from DB' do
delete :destroy, namespace_id: project.namespace.id, project_id: project.id
expect(project.avatar.present?).to be_falsey
expect(project).to be_valid
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment