diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a42cdc2ca..107016e16 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,8 +5,8 @@ ¿Has encontrado algún bug? ¿Quieres colaborar? Proceso: -* Abrir un hilo en [discourse](http://community.coopdevs.org/) para aclarar si es un bug o una feature :wink: -* Una vez aclarado el bug y detectado los pasos para reproducirlos se crearia una issue en GH con enlaze al hilo original en discourse. +* Abrir un hilo en [discourse](https://community.coopdevs.org/c/timeoverflow) para aclarar si es un bug o una feature :wink: +* Una vez aclarado, si se trata de un bug, abre una issue en Github indicando los pasos para reproducirlo y cómo debería funncionar correctamente, enlaza al hilo original en discourse. * Pull request que resuelve la issue. * Reportar en el hilo de discourse que el bug ha sido resuelto con enlace a PR. diff --git a/Gemfile.lock b/Gemfile.lock index 8dcf90423..bb95fa0ac 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,21 +1,21 @@ GEM remote: https://rubygems.org/ specs: - actionmailer (4.2.10) - actionpack (= 4.2.10) - actionview (= 4.2.10) - activejob (= 4.2.10) + actionmailer (4.2.11) + actionpack (= 4.2.11) + actionview (= 4.2.11) + activejob (= 4.2.11) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 1.0, >= 1.0.5) - actionpack (4.2.10) - actionview (= 4.2.10) - activesupport (= 4.2.10) + actionpack (4.2.11) + actionview (= 4.2.11) + activesupport (= 4.2.11) rack (~> 1.6) rack-test (~> 0.6.2) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (4.2.10) - activesupport (= 4.2.10) + actionview (4.2.11) + activesupport (= 4.2.11) builder (~> 3.1) erubis (~> 2.7.0) rails-dom-testing (~> 1.0, >= 1.0.5) @@ -32,17 +32,17 @@ GEM ransack (~> 1.3) sass (~> 3.1) sprockets (< 4.1) - activejob (4.2.10) - activesupport (= 4.2.10) + activejob (4.2.11) + activesupport (= 4.2.11) globalid (>= 0.3.0) - activemodel (4.2.10) - activesupport (= 4.2.10) + activemodel (4.2.11) + activesupport (= 4.2.11) builder (~> 3.1) - activerecord (4.2.10) - activemodel (= 4.2.10) - activesupport (= 4.2.10) + activerecord (4.2.11) + activemodel (= 4.2.11) + activesupport (= 4.2.11) arel (~> 6.0) - activesupport (4.2.10) + activesupport (4.2.11) i18n (~> 0.7) minitest (~> 5.1) thread_safe (~> 0.3, >= 0.3.4) @@ -111,7 +111,7 @@ GEM execjs coffee-script-source (1.8.0) columnize (0.9.0) - concurrent-ruby (1.0.5) + concurrent-ruby (1.1.3) connection_pool (2.2.1) crass (1.0.4) dalli (2.7.2) @@ -204,15 +204,15 @@ GEM i18n (~> 0.4) json rest-client - loofah (2.2.2) + loofah (2.2.3) crass (~> 1.0.2) nokogiri (>= 1.5.9) - mail (2.7.0) + mail (2.7.1) mini_mime (>= 0.1.1) mime-types (3.1) mime-types-data (~> 3.2015) mime-types-data (3.2016.0521) - mini_mime (1.0.0) + mini_mime (1.0.1) mini_portile2 (2.3.0) minitest (5.11.3) multi_json (1.11.2) @@ -221,7 +221,7 @@ GEM net-ssh (>= 2.6.5) net-ssh (2.9.2) netrc (0.11.0) - nokogiri (1.8.4) + nokogiri (1.8.5) mini_portile2 (~> 2.3.0) orm_adapter (0.5.0) parallel (1.12.1) @@ -240,21 +240,21 @@ GEM public_suffix (2.0.5) pundit (2.0.0) activesupport (>= 3.0.0) - rack (1.6.10) + rack (1.6.11) rack-protection (2.0.1) rack rack-test (0.6.3) rack (>= 1.0) - rails (4.2.10) - actionmailer (= 4.2.10) - actionpack (= 4.2.10) - actionview (= 4.2.10) - activejob (= 4.2.10) - activemodel (= 4.2.10) - activerecord (= 4.2.10) - activesupport (= 4.2.10) + rails (4.2.11) + actionmailer (= 4.2.11) + actionpack (= 4.2.11) + actionview (= 4.2.11) + activejob (= 4.2.11) + activemodel (= 4.2.11) + activerecord (= 4.2.11) + activesupport (= 4.2.11) bundler (>= 1.3.0, < 2.0) - railties (= 4.2.10) + railties (= 4.2.11) sprockets-rails rails-deprecated_sanitizer (1.0.3) activesupport (>= 4.2.0.alpha) @@ -272,9 +272,9 @@ GEM rails_stdout_logging rails_serve_static_assets (0.0.3) rails_stdout_logging (0.0.3) - railties (4.2.10) - actionpack (= 4.2.10) - activesupport (= 4.2.10) + railties (4.2.11) + actionpack (= 4.2.11) + activesupport (= 4.2.11) rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) rainbow (3.0.0) @@ -364,7 +364,7 @@ GEM sshkit (1.8.1) net-scp (>= 1.1.2) net-ssh (>= 2.8.0) - thor (0.20.0) + thor (0.20.3) thread_safe (0.3.6) tilt (2.0.8) ttfunk (1.5.1) diff --git a/app/assets/javascripts/give_time.js.coffee b/app/assets/javascripts/give_time.js.coffee index fe067d4ec..0465c0dc2 100644 --- a/app/assets/javascripts/give_time.js.coffee +++ b/app/assets/javascripts/give_time.js.coffee @@ -5,8 +5,13 @@ jQuery.validator.addMethod "either-hours-minutes-informed", ((value, element) -> giveTimeReadyFn = () -> config = submitHandler: (form) -> - $(" #transfer_amount ").val($(" #transfer_hours ").val() * 3600 + $(" #transfer_minutes ").val() * 60) - form.submit() + amount = $("#transfer_hours").val() * 3600 + $("#transfer_minutes").val() * 60 + $("#transfer_amount").val(amount) + + if amount > 0 + form.submit() + else + $(form).find('.form-actions .error').removeClass('invisible').show() $( "#new_transfer" ).validate(config) diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index b322f59ab..bd3ae9580 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -12,7 +12,7 @@ def create if persister.save redirect_to redirect_target else - flash[:error] = transfer.errors.full_messages.to_sentence + redirect_to :back, alert: transfer.errors.full_messages.to_sentence end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6c198475c..260f6da31 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -3,6 +3,10 @@ module ApplicationHelper TEXT_SUCCESS = 'text-success'.freeze TEXT_DANGER = 'text-danger'.freeze + def page_title + current_organization || 'TimeOverflow' + end + # from gravatar def avatar_url(user, size = 32) gravatar_id = Digest::MD5::hexdigest(user.email).downcase diff --git a/app/models/movement.rb b/app/models/movement.rb index 959c45757..3408de8cc 100644 --- a/app/models/movement.rb +++ b/app/models/movement.rb @@ -18,6 +18,8 @@ class Movement < ActiveRecord::Base where(created_at: month.beginning_of_month..month.end_of_month) } + validates :amount, numericality: { other_than: 0 } + after_create do account.update_balance end diff --git a/app/models/transfer.rb b/app/models/transfer.rb index b841cce45..c70968485 100644 --- a/app/models/transfer.rb +++ b/app/models/transfer.rb @@ -17,6 +17,7 @@ class Transfer < ActiveRecord::Base has_many :movements, dependent: :destroy has_many :events, dependent: :destroy + validates :amount, numericality: { greater_than: 0 } validate :different_source_and_destination after_create :make_movements @@ -26,14 +27,6 @@ def make_movements movements.create(account: Account.find(destination_id), amount: amount.to_i) end - def movement_from - movements.detect {|m| m.amount < 0 } - end - - def movement_to - movements.detect {|m| m.amount > 0 } - end - def source_id source.respond_to?(:id) ? source.id : source end diff --git a/app/services/push_notifications/creator/post.rb b/app/services/push_notifications/creator/post.rb index 4579aa961..66e047d6e 100644 --- a/app/services/push_notifications/creator/post.rb +++ b/app/services/push_notifications/creator/post.rb @@ -8,7 +8,13 @@ def title end def body - event.post.description&.truncate(20) || 'No description' + description = event.post.description + + if description.blank? + 'No description' + else + description.truncate(20) + end end def data diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 0e02f748b..333227399 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -4,26 +4,17 @@ - - <% if content_for? :window_title %> - <%= content_for :window_title %> - <% else %> - <% if current_organization %> - <%= current_organization %> — - <% end %> - <%= yield :title %> - <% end %> - + <%= page_title %> <%= csrf_meta_tags %> <%= stylesheet_link_tag 'libs', media: 'all' %> <%= stylesheet_link_tag 'application', media: 'all' %> - <%= stylesheet_link_tag 'application', 'https://fonts.googleapis.com/css?family=Work+Sans:400,500,600,700' %> - <%= stylesheet_link_tag 'application', 'https://fonts.googleapis.com/icon?family=Material+Icons' %> + <%= stylesheet_link_tag 'https://fonts.googleapis.com/css?family=Work+Sans:400,500,600,700' %> + <%= stylesheet_link_tag 'https://fonts.googleapis.com/icon?family=Material+Icons' %> <%= javascript_include_tag 'libs' %> <%= javascript_include_tag 'application' %> - + <%= render 'navbar' %>
<%= render 'layouts/messages' unless devise_controller? %> @@ -38,19 +29,5 @@ <%= render 'application/footer' %>
- - - diff --git a/app/views/shared/_post.html.erb b/app/views/shared/_post.html.erb index efa554f84..3751e5de3 100644 --- a/app/views/shared/_post.html.erb +++ b/app/views/shared/_post.html.erb @@ -43,12 +43,16 @@ <% if post.user.has_valid_email? %>
<%= User.human_attribute_name :email %>
-
<%= post.user.email %>
+
<%= mail_to post.user.email %>
<% end %> <% phones = [post.user.phone, post.user.alt_phone].select(&:present?) %> <% if phones.present? %>
<%= User.human_attribute_name(:phone) %>
-
<%= phones.map(&:to_s).join(" — ") %>
+
+ <% phones.each do |phone| %> + <%= phone_to phone%> + <% end %> +
<% end %>
diff --git a/app/views/transfers/new.html.erb b/app/views/transfers/new.html.erb index 66c74131a..3f349f5f9 100644 --- a/app/views/transfers/new.html.erb +++ b/app/views/transfers/new.html.erb @@ -36,7 +36,8 @@
- <%= f.button :submit, class: "btn btn-default", style: "margin-bottom: 20px;" %> + <%= f.button :submit, class: "btn btn-default" %> +
<% end %> diff --git a/app/views/users/_member_card.html.erb b/app/views/users/_member_card.html.erb index 0b61420b3..d4e556498 100644 --- a/app/views/users/_member_card.html.erb +++ b/app/views/users/_member_card.html.erb @@ -19,13 +19,13 @@ <%= member.description&.truncate(124) %>
- <% if member.phone.present? && !member.phone.blank? %> + <% if member.phone.present? %>
<%= phone_to member.phone %>
<% end %> - <% if member.mail_to.present? && !member.mail_to.blank? %> + <% if member.mail_to.present? %>
<%= member.mail_to %> diff --git a/config/locales/ca.yml b/config/locales/ca.yml index 46ba2cd5f..201011210 100644 --- a/config/locales/ca.yml +++ b/config/locales/ca.yml @@ -494,6 +494,8 @@ ca: minute: one: "%{count} minut" other: "%{count} minuts" + new: + error_amount: El temps ha de ser més gran que 0 users: edit: edit_user: Canviar usuari diff --git a/config/locales/en.yml b/config/locales/en.yml index 8c5b4361b..199e837fc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -494,6 +494,8 @@ en: minute: one: "%{count} minute" other: "%{count} minutes" + new: + error_amount: Time must be greater than 0 users: edit: edit_user: Update user diff --git a/config/locales/es.yml b/config/locales/es.yml index 47a045be9..b9654ca8f 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -494,6 +494,8 @@ es: minute: one: "%{count} minuto" other: "%{count} minutos" + new: + error_amount: 'El tiempo debe ser mayor que 0 ' users: edit: edit_user: Cambiar usuario diff --git a/config/locales/eu.yml b/config/locales/eu.yml index ee6060db4..b2cf066e8 100644 --- a/config/locales/eu.yml +++ b/config/locales/eu.yml @@ -500,6 +500,8 @@ eu: minute: one: "%{count}minutu" other: "%{count} minutu" + new: + error_amount: users: edit: edit_user: Erabiltzailea aldatu diff --git a/config/locales/pt-BR.yml b/config/locales/pt-BR.yml index a1b31f8c6..b3c5fde3f 100644 --- a/config/locales/pt-BR.yml +++ b/config/locales/pt-BR.yml @@ -494,6 +494,8 @@ pt-BR: minute: one: "%{count} minuto" other: "%{count} minutos" + new: + error_amount: users: edit: edit_user: Trocar usuário diff --git a/db/migrate/20181004200104_add_amount_constraint_to_movements.rb b/db/migrate/20181004200104_add_amount_constraint_to_movements.rb new file mode 100644 index 000000000..978eb0269 --- /dev/null +++ b/db/migrate/20181004200104_add_amount_constraint_to_movements.rb @@ -0,0 +1,10 @@ +class AddAmountConstraintToMovements < ActiveRecord::Migration + def change + # Destroy movements (and parent transfer) with amount equal to 0 + Movement.includes(:transfer).where(amount: 0).find_each do |movement| + movement.transfer&.destroy + end + + execute 'ALTER TABLE movements ADD CONSTRAINT non_zero_amount CHECK(amount != 0)' + end +end diff --git a/db/schema.rb b/db/schema.rb index 151afa5d2..38963d039 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180924164456) do +ActiveRecord::Schema.define(version: 20181004200104) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/docs/CONTRIBUTING.en.md b/docs/CONTRIBUTING.en.md index 9ed140e25..3e168d3ca 100644 --- a/docs/CONTRIBUTING.en.md +++ b/docs/CONTRIBUTING.en.md @@ -3,7 +3,7 @@ Did you find a bug? Do you want to collaborate? Process: -* Open a thread in [discourse](http://community.coopdevs.org/) to make sure whether it's a bug or a feature :wink: +* Open a thread in [discourse](https://community.coopdevs.org/c/timeoverflow) to make sure whether it's a bug or a feature :wink: * Once we understand what the bug is about and know the steps to reproduce we will create an issue in GitHub linking to the Discourse thread. * Create a pull request to solve the issue. * Report back to the discourse thread the bug has been solved linking to the pull request. diff --git a/spec/controllers/transfers_controller_spec.rb b/spec/controllers/transfers_controller_spec.rb index 7eff15bb6..c075091ec 100644 --- a/spec/controllers/transfers_controller_spec.rb +++ b/spec/controllers/transfers_controller_spec.rb @@ -207,5 +207,22 @@ end end end + + context 'with invalid params' do + let(:user) { member_giver.user } + let(:referer) { "/transfers/new?destination_account_id=#{member_taker.account.id}" } + + before do + request.env["HTTP_REFERER"] = referer + end + + it 'does not create any Transfer and redirects to :back if the amount is 0' do + expect { + post(:create, transfer: { amount: 0, destination: member_taker.account.id }) + }.not_to change(Transfer, :count) + + expect(response).to redirect_to(referer) + end + end end end diff --git a/spec/services/push_notifications/post_spec.rb b/spec/services/push_notifications/post_spec.rb index 43d2b6165..d4b312818 100644 --- a/spec/services/push_notifications/post_spec.rb +++ b/spec/services/push_notifications/post_spec.rb @@ -13,10 +13,71 @@ end describe '#create!' do - it 'creates as many PushNotification resources as needed' do - expect { - creator.create! - }.to change{PushNotification.count}.by(1) + context 'integration' do + it 'creates as many PushNotification resources as needed' do + expect { creator.create! }.to change { PushNotification.count }.by(1) + end + end + + context 'unit' do + let(:post) do + Fabricate( + :post, + organization: organization, + user: user, + description: description + ) + end + + before { allow(PushNotification).to receive(:create!) } + + context 'when the post description is empty' do + let(:description) { '' } + + it 'creates a PushNotification with a default body' do + creator.create! + + expect(PushNotification) + .to have_received(:create!) + .with(include(body: 'No description')) + end + end + + context 'when the post description is nil' do + let(:description) { nil } + + it 'creates a PushNotification with a default body' do + creator.create! + + expect(PushNotification) + .to have_received(:create!) + .with(include(body: 'No description')) + end + end + + context 'when the post description is shorter than 20 chars' do + let(:description) { 'description' } + + it 'creates a PushNotification with the post body' do + creator.create! + + expect(PushNotification) + .to have_received(:create!) + .with(include(body: post.description)) + end + end + + context 'when the post description is larger than 20 chars' do + let(:description) { 'this is a very long description' } + + it 'creates a PushNotification with the post body truncated' do + creator.create! + + expect(PushNotification) + .to have_received(:create!) + .with(include(body: post.description.truncate(20))) + end + end end end end