From 614ff79ba11364727b055751667e49f562a215fa Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 27 Jun 2017 17:57:35 +0200 Subject: [PATCH] WIP --- .../budgets/investments_controller.rb | 6 +-- app/helpers/investments_helper.rb | 21 +++----- app/models/budget/investment.rb | 2 +- app/models/concerns/imageable.rb | 14 ++++++ app/models/image.rb | 48 +++++++++++++++++++ app/views/budgets/investments/_form.html.erb | 32 +++++++------ .../budgets/investments/_image_form.html.erb | 24 +++++----- .../budgets/investments/_investment.html.erb | 13 ++--- .../investments/_investment_show.html.erb | 7 ++- config/locales/en/activerecord.yml | 5 ++ config/locales/es/activerecord.yml | 5 ++ config/routes.rb | 8 +++- spec/factories.rb | 8 +++- spec/features/budgets/investments_spec.rb | 8 ++-- spec/models/budget/investment_spec.rb | 2 +- 15 files changed, 135 insertions(+), 68 deletions(-) create mode 100644 app/models/concerns/imageable.rb create mode 100644 app/models/image.rb diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index a755daecd..739c6da71 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -93,9 +93,7 @@ module Budgets end def remove_image - @investment.image.destroy - @investment.image_title = nil - @investment.save + @investment.image.destroy! redirect_to budget_investment_path(@investment.budget, @investment), notice: t("flash.actions.remove_image.budget_investment") end @@ -128,7 +126,7 @@ module Budgets params.require(:budget_investment) .permit(:title, :description, :external_url, :heading_id, :tag_list, :organization_name, :location, :terms_of_service, - :image, :image_title, + image_attributes: [:title, :attachment], documents_attributes: [:id, :title, :attachment, :cached_attachment, :user_id]) end diff --git a/app/helpers/investments_helper.rb b/app/helpers/investments_helper.rb index b7d29bf85..2adccaa5c 100644 --- a/app/helpers/investments_helper.rb +++ b/app/helpers/investments_helper.rb @@ -1,32 +1,23 @@ module InvestmentsHelper def investment_image_full_url(investment, version) - URI(request.url) + investment.image.url(version) - end - - def investment_image_file_name(investment) - investment.image.url.split('/').last.split("?")[0] if investment.image.present? + URI(request.url) + investment.image_url(version) end def investment_image_advice_note(investment) - title = investment.title if investment.image.exists? - t "budgets.investments.edit_image.edit_note", title: title + t("budgets.investments.edit_image.edit_note", title: investment.title) else - t "budgets.investments.edit_image.add_note", title: title + t("budgets.investments.edit_image.add_note", title: investment.title) end end def investment_image_button_text(investment) - if investment.image.exists? - t "budgets.investments.show.edit_image" - else - t "budgets.investments.show.add_image" - end + investment.image.exists? ? t("budgets.investments.show.edit_image") : t("budgets.investments.show.add_image") end def errors_on_image(investment) - investment.errors[:image].join(', ') if investment.errors.key?(:image) + investment.errors[:attachment].join(', ') if investment.errors.key?(:attachment) end -end \ No newline at end of file +end diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 640274cc6..e99bbca9a 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -8,6 +8,7 @@ class Budget include Followable include Communitable include Documentable + include Imageable documentable max_documents_allowed: 3, max_file_size: 3.megabytes, accepted_content_types: [ "application/pdf" ] @@ -17,7 +18,6 @@ class Budget acts_as_paranoid column: :hidden_at include ActsAsParanoidAliases - belongs_to :author, -> { with_hidden }, class_name: 'User', foreign_key: 'author_id' belongs_to :heading belongs_to :group diff --git a/app/models/concerns/imageable.rb b/app/models/concerns/imageable.rb new file mode 100644 index 000000000..04a4ba3a3 --- /dev/null +++ b/app/models/concerns/imageable.rb @@ -0,0 +1,14 @@ +# can [:update, :destroy ], Image, :imageable_id => user.id, :imageable_type => 'User' +# and add a feature like forbidden/without_role_images_spec.rb to test it +module Imageable + extend ActiveSupport::Concern + + included do + has_one :image, as: :imageable, dependent: :destroy + accepts_nested_attributes_for :image, allow_destroy: true + + def image_url(style) + image.attachment.url(style) if image && image.attachment.exists? + end + end +end diff --git a/app/models/image.rb b/app/models/image.rb new file mode 100644 index 000000000..089ec9856 --- /dev/null +++ b/app/models/image.rb @@ -0,0 +1,48 @@ +class Image < ActiveRecord::Base + TITLE_LEGHT_RANGE = 4..80 + MIN_SIZE = 475 + + attr_accessor :content_type, :original_filename, :attachment_data, :attachment_urls + belongs_to :imageable, polymorphic: true + before_validation :set_styles + has_attached_file :attachment, styles: { large: "x475", medium: "300x300#", thumb: "140x245#" }, + url: "/system/:class/:attachment/:imageable_name_path/:style/:hash.:extension", + hash_secret: Rails.application.secrets.secret_key_base + validates_attachment :attachment, presence: true, content_type: { content_type: %w(image/jpeg image/jpg) }, + size: { less_than: 1.megabytes } + validates :title, presence: true, length: { in: TITLE_LEGHT_RANGE } + validate :check_image_dimensions + + after_create :redimension_using_origin_styles + accepts_nested_attributes_for :imageable + + # # overwrite default styles for Image class + # def set_image_styles + # { large: "x#{MIN_SIZE}", medium: "300x300#", thumb: "140x245#" } + # end + def set_styles + if imageable + imageable.set_styles if imageable.respond_to? :set_styles + else + { large: "x#{MIN_SIZE}", medium: "300x300#", thumb: "140x245#" } + end + end + + Paperclip.interpolates :imageable_name_path do |attachment, _style| + attachment.instance.imageable.class.to_s.downcase.split('::').map(&:pluralize).join('/') + end + + private + + def redimension_using_origin_styles + attachment.reprocess! + end + + def check_image_dimensions + return unless attachment? + + dimensions = Paperclip::Geometry.from_file(attachment.queued_for_write[:original].path) + errors.add(:attachment, :min_image_width, required_min_width: MIN_SIZE) if dimensions.width < MIN_SIZE + errors.add(:attachment, :min_image_height, required_min_height: MIN_SIZE) if dimensions.height < MIN_SIZE + end +end diff --git a/app/views/budgets/investments/_form.html.erb b/app/views/budgets/investments/_form.html.erb index 6a9237b32..7d612a79e 100644 --- a/app/views/budgets/investments/_form.html.erb +++ b/app/views/budgets/investments/_form.html.erb @@ -53,26 +53,28 @@ data: {js_url: suggest_tags_path} %> -
-
- <%= f.file_field :image, accept: 'image/jpeg', label: false, class:'show-for-sr' %> -
- <%= f.label :image, t("budgets.investments.form.image_label"), class:'button' %> -

<%= investment_image_file_name(@investment) %>

-
-
+ <%= f.fields_for :image do |builder| %> - <% if @investment.errors.has_key?(:image) %>
-
- <%= errors_on_image(@investment)%> +
+ <%= f.file_field :attachment, accept: 'image/jpg,image/jpeg', label: false, class:'show-for-sr' %> +
+ <%= f.label :attachment, t("budgets.investments.form.image_label"), class:'button' %>
- <% end %> -
- <%= f.text_field :image_title %> -
+ <% if @investment.errors.has_key?(:attachment) %> +
+
+ <%= errors_on_image(@investment)%> +
+
+ <% end %> + +
+ <%= f.text_field :title %> +
+ <% end %> <% unless current_user.manager? %> diff --git a/app/views/budgets/investments/_image_form.html.erb b/app/views/budgets/investments/_image_form.html.erb index 616e4f4c1..d2cefe79a 100644 --- a/app/views/budgets/investments/_image_form.html.erb +++ b/app/views/budgets/investments/_image_form.html.erb @@ -10,16 +10,15 @@ <% if @investment.image.exists? %>
- <%= image_tag @investment.image.url(:large) %> + <%= image_tag @investment.image_url(:large) %>
<% end %>
- <%= f.file_field :image, accept: 'image/jpeg', label: false, class:'show-for-sr' %> + <%= f.file_field :attachment, accept: 'image/jpg,image/jpeg', label: false, class:'show-for-sr' %>
- <%= f.label :image, t("budgets.investments.edit_image.form.image_label"), class:'button' %> -

<%= investment_image_file_name(@investment) %>

+ <%= f.label :attachment, t("budgets.investments.edit_image.form.image_label"), class:'button' %>
@@ -32,18 +31,21 @@ <% end %>
- <%= f.label :image_title, t("budgets.investments.edit_image.form.image_title") %> - <%= f.text_field :image_title, placeholder: t("budgets.investments.edit_image.form.image_title"), label: false %> + <%= f.label :title, t("budgets.investments.edit_image.form.image_title") %> + <%= f.text_field :title, placeholder: t("budgets.investments.edit_image.form.image_title"), label: false %>
<%= f.submit(class: "button", value: t("budgets.investments.edit_image.form.submit_button")) %> + <% if @investment.image.exists? %> - <%= link_to t("budgets.investments.edit_image.form.remove_button"), - remove_image_budget_investment_path(@investment.budget, @investment), - class: "button hollow alert", - method: :delete, - data: { confirm: t("budgets.investments.edit_image.form.remove_alert") } %> +
+ <%= link_to t("budgets.investments.edit_image.form.remove_button"), + remove_image_budget_investment_path(@investment.budget, @investment), + class: "button hollow alert", + method: :delete, + data: { confirm: t("budgets.investments.edit_image.form.remove_alert") } %> +
<% end %>
diff --git a/app/views/budgets/investments/_investment.html.erb b/app/views/budgets/investments/_investment.html.erb index 6867c04a4..ffe71f955 100644 --- a/app/views/budgets/investments/_investment.html.erb +++ b/app/views/budgets/investments/_investment.html.erb @@ -4,15 +4,10 @@
<% if investment.image.exists? %> - - - <%= investment.image_title %> - +
+ <%= image_tag investment.image_url(:thumb), alt: investment.image.title %> +
<%= investment.image.title %>
+
<% else %>
<% end %> diff --git a/app/views/budgets/investments/_investment_show.html.erb b/app/views/budgets/investments/_investment_show.html.erb index 4ce59188c..f200b1263 100644 --- a/app/views/budgets/investments/_investment_show.html.erb +++ b/app/views/budgets/investments/_investment_show.html.erb @@ -32,11 +32,10 @@
- <%= image_tag investment.image.url(:large), - title: investment.image_title, - alt: investment.image_title %> + <%= image_tag investment.image_url(:large), + alt: investment.image.title %>
- <%= investment.image_title %> + <%= investment.image.title %>
diff --git a/config/locales/en/activerecord.yml b/config/locales/en/activerecord.yml index 5070e27a7..27d9b10c5 100644 --- a/config/locales/en/activerecord.yml +++ b/config/locales/en/activerecord.yml @@ -219,6 +219,11 @@ en: attributes: max_per_day: invalid: "You have reached the maximum number of private messages per day" + image: + attributes: + attachment: + min_image_width: "Image Width must be at least %{required_min_width}px" + min_image_height: "Image Height must be at least %{required_min_height}px" poll/voter: attributes: document_number: diff --git a/config/locales/es/activerecord.yml b/config/locales/es/activerecord.yml index 238782276..a5ab18d9e 100644 --- a/config/locales/es/activerecord.yml +++ b/config/locales/es/activerecord.yml @@ -214,6 +214,11 @@ es: attributes: max_per_day: invalid: "Has llegado al número máximo de mensajes privados por día" + image: + attributes: + attachment: + min_image_width: "La imagen debe tener al menos %{required_min_width}px de largo" + min_image_height: "La imagen debe tener al menos %{required_min_height}px de alto" poll/voter: attributes: document_number: diff --git a/config/routes.rb b/config/routes.rb index 386a3fbf5..400c7bb7e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -36,6 +36,10 @@ Rails.application.routes.draw do get '/welcome', to: 'welcome#welcome' get '/cuentasegura', to: 'welcome#verification', as: :cuentasegura + concern :imageable do + resources :images + end + resources :debates do member do post :vote @@ -77,7 +81,7 @@ Rails.application.routes.draw do resources :budgets, only: [:show, :index] do resources :groups, controller: "budgets/groups", only: [:show] - resources :investments, controller: "budgets/investments", only: [:index, :new, :create, :show, :destroy] do + resources :investments, controller: "budgets/investments", only: [:index, :new, :create, :show, :destroy], concerns: :imageable do member do post :vote get :edit_image @@ -229,7 +233,7 @@ Rails.application.routes.draw do end end - resources :budget_investments, only: [:index, :show, :edit, :update] do + resources :budget_investments, only: [:index, :show, :edit, :update], concerns: :imageable do resources :budget_investment_milestones member { patch :toggle_selection } end diff --git a/spec/factories.rb b/spec/factories.rb index 4f44ccb32..fc4e41310 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -320,11 +320,15 @@ FactoryGirl.define do end trait :with_descriptive_image do - image { File.new("spec/fixtures/files/clippy.jpg") } - image_title "Lorem ipsum dolor sit amet" + association :image, factory: :image end end + factory :image, class: 'Image' do + attachment { File.new("spec/fixtures/files/clippy.jpg") } + title "Lorem ipsum dolor sit amet" + end + factory :budget_ballot, class: 'Budget::Ballot' do association :user, factory: :user budget diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index 5fbba0e59..3633f4b7f 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -29,8 +29,8 @@ feature 'Budget Investments' do end scenario 'Index should show investment descriptive image only when is defined' do - investment = create(:budget_investment, heading: heading) - investment_with_image = create(:budget_investment, :with_descriptive_image, heading: heading) + investment = FactoryGirl.create(:budget_investment, heading: heading) + investment_with_image = FactoryGirl.create(:budget_investment, :with_descriptive_image, heading: heading) visit budget_investments_path(budget, heading_id: heading.id) @@ -39,7 +39,7 @@ feature 'Budget Investments' do end within("#budget_investment_#{investment_with_image.id}") do - expect(page).to have_css("picture img[alt='#{investment_with_image.image_title}'][title='#{investment_with_image.image_title}']") + expect(page).to have_css("picture img[alt='#{investment_with_image.image.title}']") end end @@ -377,7 +377,7 @@ feature 'Budget Investments' do visit budget_investment_path(budget_id: budget.id, id: investment_with_image.id) - expect(page).to have_css("img[alt='#{investment_with_image.image_title}'][title='#{investment_with_image.image_title}']") + expect(page).to have_css("img[alt='#{investment_with_image.image.title}']") end scenario "Show back link contains heading id" do diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb index 5f0a519d9..8e78bb88d 100644 --- a/spec/models/budget/investment_spec.rb +++ b/spec/models/budget/investment_spec.rb @@ -29,7 +29,7 @@ describe Budget::Investment do end end - describe "#image and #image_title" do + describe "#image and #image_title", :investment_image do let(:investment_with_image) { build(:budget_investment, :with_descriptive_image) } it "should be valid" do