Read about our upcoming Code of Conduct on this issue

Commit bf4b4384 authored by Stan Hu's avatar Stan Hu
Browse files

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
parent a5a55e51a228
...@@ -33,7 +33,7 @@ v 7.11.0 (unreleased) ...@@ -33,7 +33,7 @@ v 7.11.0 (unreleased)
- Show incompatible projects in Google Code import status (Stan Hu) - Show incompatible projects in Google Code import status (Stan Hu)
- 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)
- - 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 @@ def set_last_activity_at ...@@ -144,7 +144,7 @@ def set_last_activity_at
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
......
...@@ -137,7 +137,7 @@ class User < ActiveRecord::Base ...@@ -137,7 +137,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? }
...@@ -298,7 +298,7 @@ def update_emails_with_primary_email ...@@ -298,7 +298,7 @@ def update_emails_with_primary_email
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