Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade carrierwave to version 2.1 #7213

Merged
merged 47 commits into from Jan 29, 2021
Merged
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
09968d4
Merge pull request #7 from decidim/develop
alecslupu Oct 13, 2020
9fde202
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Oct 15, 2020
0cffe2c
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Oct 19, 2020
f9f5580
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Oct 21, 2020
2c3c667
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Oct 23, 2020
cdb057e
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Oct 28, 2020
bb451bb
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Nov 3, 2020
54d8b3d
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Nov 3, 2020
7b0309a
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Nov 4, 2020
dfdacd4
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Nov 8, 2020
146c831
Merge branch 'develop' of github.com:tremend-cofe/decidim into develop
alecslupu Nov 18, 2020
e2f0469
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Nov 19, 2020
40dbd49
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Nov 23, 2020
d37135b
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Nov 24, 2020
b9c97ae
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Nov 27, 2020
2377490
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Dec 4, 2020
0c3952a
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Dec 7, 2020
9909b41
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Dec 15, 2020
ed02079
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Dec 15, 2020
254f104
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Dec 17, 2020
92a20a8
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Dec 28, 2020
e6ffa44
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Dec 31, 2020
9210013
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Jan 12, 2021
28b3243
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Jan 12, 2021
6fadaa3
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Jan 13, 2021
6fba042
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Jan 14, 2021
ce89b55
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Jan 14, 2021
c584d0d
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Jan 14, 2021
8a6e875
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Jan 17, 2021
6a777f2
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Jan 20, 2021
0d4fe50
Merge branch 'develop' of github.com:decidim/decidim into develop
alecslupu Jan 22, 2021
e91b463
Updating a Partner with a invalid logo raises a 500 error
alecslupu Jan 23, 2021
3897b19
Update the Carrierwave and make sure that some of the models are not …
alecslupu Jan 23, 2021
a644a8a
Bulk change the command handling method in forms
alecslupu Jan 23, 2021
12082f9
Running Linters, and fix tests
alecslupu Jan 24, 2021
bb1f7af
Implement own Uploader provider method
alecslupu Jan 24, 2021
8a41cd4
Carrierwave 2.1
alecslupu Jan 24, 2021
eedf2e0
Fix failing spec
alecslupu Jan 24, 2021
e01b10a
Merge branch 'feature/upgrade-carierwave' into feature/upgrade-carier…
alecslupu Jan 24, 2021
965c9db
Applying changes requested by review
alecslupu Jan 25, 2021
f3f944b
Merge branch 'develop' of github.com:decidim/decidim into feature/upg…
alecslupu Jan 25, 2021
3e1b6a7
Merge branch 'develop' of github.com:decidim/decidim into feature/upg…
alecslupu Jan 26, 2021
ff2d91a
Refactor the update voting form
alecslupu Jan 26, 2021
8c60795
Merge branch 'develop' of github.com:decidim/decidim into feature/upg…
alecslupu Jan 26, 2021
4f7e969
Merge branch 'develop' of github.com:decidim/decidim into feature/upg…
alecslupu Jan 27, 2021
4a33186
Merge branch 'develop' of github.com:decidim/decidim into feature/upg…
alecslupu Jan 27, 2021
b2e97d5
Merge branch 'develop' of github.com:decidim/decidim into feature/upg…
alecslupu Jan 28, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 36 additions & 17 deletions Gemfile.lock
Expand Up @@ -74,7 +74,7 @@ PATH
autoprefixer-rails (~> 8.0)
batch-loader (~> 1.2)
browser (~> 2.7)
carrierwave (~> 1.3)
carrierwave (~> 2.1)
cells-erb (~> 0.1.0)
cells-rails (~> 0.0.9)
charlock_holmes (~> 0.7)
Expand All @@ -87,6 +87,7 @@ PATH
doorkeeper-i18n (~> 4.0)
etherpad-lite (~> 0.3)
file_validators (~> 2.1)
fog-local
foundation-rails (~> 6.6, < 6.7)
foundation_rails_helper (~> 3.0)
geocoder (>= 1.5)
Expand Down Expand Up @@ -315,10 +316,13 @@ GEM
rack-test (>= 0.6.3)
regexp_parser (~> 1.5)
xpath (~> 3.2)
carrierwave (1.3.1)
activemodel (>= 4.0.0)
activesupport (>= 4.0.0)
mime-types (>= 1.16)
carrierwave (2.1.0)
activemodel (>= 5.0.0)
activesupport (>= 5.0.0)
addressable (~> 2.6)
image_processing (~> 1.1)
mimemagic (>= 0.3.0)
mini_mime (>= 0.1.3)
cells (4.1.7)
declarative-builder (< 0.2.0)
declarative-option (< 0.2.0)
Expand Down Expand Up @@ -346,7 +350,7 @@ GEM
concurrent-ruby (1.1.7)
crack (0.4.4)
crass (1.0.6)
css_parser (1.7.1)
css_parser (1.8.0)
addressable
date_validator (0.9.0)
activemodel
Expand Down Expand Up @@ -402,6 +406,7 @@ GEM
erubi (1.9.0)
etherpad-lite (0.3.0)
rest-client (>= 1.6)
excon (0.78.1)
execjs (2.7.0)
factory_bot (4.11.1)
activesupport (>= 3.0.0)
Expand All @@ -418,6 +423,14 @@ GEM
file_validators (2.3.0)
activemodel (>= 3.2)
mime-types (>= 1.0)
fog-core (2.2.3)
builder
excon (~> 0.71)
formatador (~> 0.2)
mime-types
fog-local (0.6.0)
fog-core (>= 1.27, < 3.0)
formatador (0.2.5)
foundation-rails (6.6.2.0)
railties (>= 3.1.0)
sass (>= 3.3.0)
Expand Down Expand Up @@ -466,6 +479,9 @@ GEM
ice_cube (~> 0.16)
ice_cube (0.16.3)
ice_nine (0.11.2)
image_processing (1.12.1)
mini_magick (>= 4.9.5, < 5)
ruby-vips (>= 2.0.17, < 3)
invisible_captcha (0.13.0)
rails (>= 3.2.0)
jquery-rails (4.4.0)
Expand Down Expand Up @@ -518,9 +534,9 @@ GEM
method_source (1.0.0)
mime-types (3.3.1)
mime-types-data (~> 3.2015)
mime-types-data (3.2020.0512)
mime-types-data (3.2020.1104)
mimemagic (0.3.5)
mini_magick (4.10.1)
mini_magick (4.11.0)
mini_mime (1.0.2)
mini_portile2 (2.5.0)
minitest (5.14.2)
Expand All @@ -540,7 +556,7 @@ GEM
nokogiri (1.11.1)
mini_portile2 (~> 2.5.0)
racc (~> 1.4)
oauth (0.5.4)
oauth (0.5.5)
oauth2 (1.4.4)
faraday (>= 0.8, < 2.0)
jwt (>= 1.0, < 3.0)
Expand All @@ -552,16 +568,17 @@ GEM
rack (>= 1.6.2, < 3)
omniauth-facebook (5.0.0)
omniauth-oauth2 (~> 1.2)
omniauth-google-oauth2 (0.8.0)
omniauth-google-oauth2 (0.8.1)
jwt (>= 2.0)
oauth2 (~> 1.1)
omniauth (>= 1.1.1)
omniauth-oauth2 (>= 1.6)
omniauth-oauth (1.1.0)
oauth
omniauth (~> 1.0)
omniauth-oauth2 (1.7.0)
omniauth-oauth2 (1.7.1)
oauth2 (~> 1.4)
omniauth (~> 1.9)
omniauth (>= 1.9, < 3)
omniauth-rails_csrf_protection (0.1.2)
actionpack (>= 4.2)
omniauth (>= 1.3.1)
Expand All @@ -578,7 +595,7 @@ GEM
parser (2.7.2.0)
ast (~> 2.4.1)
pg (1.1.4)
pg_search (2.3.3)
pg_search (2.3.5)
activerecord (>= 5.2)
activesupport (>= 5.2)
premailer (1.14.2)
Expand All @@ -593,7 +610,7 @@ GEM
nio4r (~> 2.0)
racc (1.5.2)
rack (2.2.3)
rack-attack (6.3.1)
rack-attack (6.4.0)
rack (>= 1.0, < 3)
rack-cors (1.1.1)
rack (>= 2.0.0)
Expand Down Expand Up @@ -647,7 +664,7 @@ GEM
virtus (~> 1.0.5)
wisper (>= 1.6.1)
redcarpet (3.5.1)
redis (4.2.2)
redis (4.2.5)
regexp_parser (1.8.1)
request_store (1.5.0)
rack (>= 1.4)
Expand Down Expand Up @@ -714,6 +731,8 @@ GEM
rubocop (~> 0.87)
ruby-ole (1.2.12.2)
ruby-progressbar (1.10.1)
ruby-vips (2.0.17)
ffi (~> 1.9)
rubyzip (2.3.0)
sass (3.7.4)
sass-listen (~> 4.0.0)
Expand Down Expand Up @@ -742,8 +761,8 @@ GEM
smart_properties (1.15.0)
social-share-button (1.2.3)
coffee-rails
spreadsheet (1.2.6)
ruby-ole (>= 1.0)
spreadsheet (1.2.7)
ruby-ole
spring (2.1.1)
spring-watcher-listen (2.0.1)
listen (>= 2.7, < 4.0)
Expand Down
Expand Up @@ -10,6 +10,9 @@ class UpdateOrganizationAppearance < Rectify::Command
# organization - The Organization that will be updated.
# form - A form object with the params.
def initialize(organization, form)
image_fields.each do |field|
form.send("#{field}=".to_sym, organization.send(field)) if form.send(field).blank?
end
@organization = organization
@form = form
end
Expand All @@ -27,14 +30,19 @@ def call
update_organization
broadcast(:ok, organization)
rescue ActiveRecord::RecordInvalid
form.errors.add(:official_img_header, organization.errors[:official_img_header]) if organization.errors.include? :official_img_header
form.errors.add(:official_img_footer, organization.errors[:official_img_footer]) if organization.errors.include? :official_img_footer
image_fields.each do |field|
form.errors.add(field, organization.errors[field]) if organization.errors.include? field
end
broadcast(:invalid)
end
end

private

def image_fields
[:highlighted_content_banner_image, :logo, :favicon, :official_img_header, :official_img_footer]
end

attr_reader :form, :organization

def update_organization
Expand All @@ -50,6 +58,7 @@ def attributes
.merge(highlighted_content_banner_attributes)
.merge(omnipresent_banner_attributes)
.merge(colors_attributes)
.delete_if { |_k, val| val.is_a?(Decidim::ApplicationUploader) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because in the class initializer we assign the object logo to the form. We assign it, as if we do not perform that step, the images in the form will be lost when updating other fields.
In this particular case we delete the file to prevent multiple processing of the same file. This is a workaround that i have found in decidim codebase, added via: #7026

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks!

.tap do |attributes|
attributes[:header_snippets] = form.header_snippets if Decidim.enable_html_header_snippets
end
Expand Down
Expand Up @@ -56,7 +56,7 @@ module Decidim::Admin

context "when the image does not exist" do
it "creates the related image" do
expect(content_block.images).to be_empty
expect(content_block.images).to eq({ "background_image" => nil })

subject.call
content_block.reload
Expand Down
Expand Up @@ -11,6 +11,7 @@ class UpdateConferenceSpeaker < Rectify::Command
# form - A form object with the params.
# conference_speaker - The ConferenceSpeaker to update
def initialize(form, conference_speaker)
form.avatar = conference_speaker.avatar if form.avatar.blank?
@form = form
@conference_speaker = conference_speaker
end
Expand Down Expand Up @@ -56,14 +57,19 @@ def attributes
:full_name,
:twitter_handle,
:personal_url,
:avatar,
:remove_avatar,
:position,
:affiliation,
:short_bio
).merge(
user: form.user
)
).merge(uploader_attributes)
end

def uploader_attributes
{
avatar: form.avatar,
remove_avatar: form.remove_avatar
}.delete_if { |_k, val| val.is_a?(Decidim::ApplicationUploader) }
end

def update_conference_speaker!
Expand Down
Expand Up @@ -11,6 +11,8 @@ class UpdatePartner < Rectify::Command
# form - A form object with the params.
# conference_partner - The ConferencePartner to update
def initialize(form, conference_partner)
form.logo = conference_partner.logo if form.logo.blank?

@form = form
@conference_partner = conference_partner
end
Expand All @@ -25,14 +27,39 @@ def call
return broadcast(:invalid) if form.invalid?
return broadcast(:invalid) unless conference_partner

update_conference_partner!
broadcast(:ok)
# We are going to assign the attributes only to handle the validation of the avatar before accessing
# `update_conference_partner!` which uses `update!`. Without this step, the image validation may render
# an ActiveRecord::RecordInvalid error
# After we assign and check if the object is valid, we reload the model to let it be handled the old way
# If there is an error we add the error to the form
conference_partner.assign_attributes(attributes)
alecslupu marked this conversation as resolved.
Show resolved Hide resolved
if conference_partner.valid?
conference_partner.reload

update_conference_partner!
broadcast(:ok)
else
form.errors.add(:logo, conference_partner.errors[:logo]) if conference_partner.errors.include? :logo

broadcast(:invalid)
end
end

private

attr_reader :form, :conference_partner

def attributes
form.attributes.slice(
:name,
:weight,
:partner_type,
:link,
:logo,
:remove_logo
)
end

def update_conference_partner!
log_info = {
resource: {
Expand All @@ -46,14 +73,7 @@ def update_conference_partner!
Decidim.traceability.update!(
conference_partner,
form.current_user,
form.attributes.slice(
:name,
:weight,
:partner_type,
:link,
:logo,
:remove_logo
),
attributes,
log_info
)
end
Expand Down
Expand Up @@ -47,7 +47,7 @@ def update
@partner = collection.find(params[:id])
enforce_permission_to :update, :partner, partner: @partner
@form = form(Decidim::Conferences::Admin::PartnerForm).from_params(params)
@form.logo = @partner.logo if @form.logo.blank?

UpdatePartner.call(@form, @partner) do
on(:ok) do
flash[:notice] = I18n.t("partners.update.success", scope: "decidim.admin")
Expand Down
Expand Up @@ -29,6 +29,8 @@ module Decidim::Conferences
current_user: current_user,
full_name: "New name",
user: user,
avatar: Decidim::Dev.test_file("avatar.jpg", "image/jpeg"),
remove_avatar: false,
attributes: {
full_name: "New name",
position: { en: "new position" },
Expand Down
5 changes: 3 additions & 2 deletions decidim-conferences/spec/commands/update_partner_spec.rb
Expand Up @@ -15,13 +15,14 @@ module Decidim::Conferences
invalid?: invalid,
current_user: current_user,
full_name: "New name",
logo: Decidim::Dev.test_file("avatar.jpg", "image/jpeg"),
remove_logo: false,
attributes: {
name: "New name",
weight: 2,
partner_type: "collaborator",
logo: Decidim::Dev.test_file("avatar.jpg", "image/jpeg"),
link: Faker::Internet.url,
remove_logo: false
link: Faker::Internet.url
}
)
end
Expand Down
Expand Up @@ -11,6 +11,9 @@ class UpdateConsultation < Rectify::Command
# consultation - the Consultation to update
# form - A form object with the params.
def initialize(consultation, form)
form.banner_image = consultation.banner_image if form.banner_image.blank?
form.introductory_image = consultation.introductory_image if form.introductory_image.blank?

@consultation = consultation
@form = form
end
Expand Down
Expand Up @@ -11,6 +11,9 @@ class UpdateQuestion < Rectify::Command
# question - the Question to update
# form - A form object with the params.
def initialize(question, form)
form.hero_image = question.hero_image if form.hero_image.blank?
form.banner_image = question.banner_image if form.banner_image.blank?

@question = question
@form = form
end
Expand Down Expand Up @@ -55,17 +58,22 @@ def attributes
participatory_scope: form.participatory_scope,
question_context: form.question_context,
hashtag: form.hashtag,
hero_image: form.hero_image,
remove_hero_image: form.remove_hero_image,
banner_image: form.banner_image,
remove_banner_image: form.remove_banner_image,
origin_scope: form.origin_scope,
origin_title: form.origin_title,
origin_url: form.origin_url,
external_voting: form.external_voting,
i_frame_url: form.i_frame_url,
order: form.order
}
}.merge(uploader_attributes)
end

def uploader_attributes
{
hero_image: form.hero_image,
remove_hero_image: form.remove_hero_image,
banner_image: form.banner_image,
remove_banner_image: form.remove_banner_image
}.delete_if { |_k, val| val.is_a?(Decidim::ApplicationUploader) }
end
end
end
Expand Down