From 99f51549b277aa73e3cb393326dfe612c09bb81d Mon Sep 17 00:00:00 2001 From: Bruce Li Date: Mon, 22 Sep 2014 23:45:30 +0800 Subject: [PATCH 1/5] Solution for FIXME: 1 Intention Revealing Method --- app/controllers/projects_controller.rb | 35 ++++++++++++++++++-------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 2ed0470..0099abf 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -6,20 +6,11 @@ class ProjectsController < ApplicationController def index # FIXME: 1. Intention Revealing Method - # When user is admitted we show it's active projects - if current_user && current_user.projects.exists? + if user_is_admitted? @projects = current_user.active_projects else - # If not admitted we show some featured projects @projects = Project.featured - - # set a marketing flash if user is new, - # and a special price for user who signed up less than a week ago - if current_user && current_user.created_at >= 1.week.ago - flash.now[:notice] = 'Upgrade and get 20% off for having your own projects!' - elsif current_user && current_user.created_at < 1.week.ago - flash.now[:notice] = 'Upgrade for having your own projects!' - end + set_flash_for_not_admitted_user end end @@ -80,4 +71,26 @@ def set_project def project_params params.require(:project).permit(:title, :description, :is_featured) end + + private + + def user_is_admitted? + current_user && current_user.projects.exists? + end + + def set_flash_for_not_admitted_user + if new_free_user? + flash.now[:notice] = 'Upgrade and get 20% off for having your own projects!' + elsif old_free_user? + flash.now[:notice] = 'Upgrade for having your own projects!' + end + end + + def new_free_user? + current_user && current_user.created_at >= 1.week.ago + end + + def old_free_user? + current_user && current_user.created_at < 1.week.ago + end end From db10b0cbeb522a7763bb63b1888758c44d0ef747 Mon Sep 17 00:00:00 2001 From: Bruce Li Date: Mon, 22 Sep 2014 23:47:31 +0800 Subject: [PATCH 2/5] Solution for FIXME: 2 Move controller logic into model --- app/controllers/projects_controller.rb | 11 +---------- app/models/project.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 0099abf..87c87b6 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -28,16 +28,7 @@ def update if @project.update(project_params) # FIXME: 2 Move controller logic into model - if @project.is_featured? - if @project.created_at > 1.week.ago - @project.label = "new featured" - else - @project.label = "featured" - end - else - @project.label = "normal" - end - @project.save + @project.update_label redirect_to @project, notice: 'Project was successfully updated.' else diff --git a/app/models/project.rb b/app/models/project.rb index 65f6164..b2fca53 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -4,4 +4,17 @@ class Project < ActiveRecord::Base belongs_to :user + def update_label + if is_featured? + if created_at > 1.week.ago + self.label = "new featured" + else + self.label = "featured" + end + else + self.label = "normal" + end + save + end + end From 5a4de41901151490b7be03e64ab2c5260b49111a Mon Sep 17 00:00:00 2001 From: Bruce Li Date: Tue, 23 Sep 2014 08:18:20 +0800 Subject: [PATCH 3/5] Solution for FIXME: 3 Replace Method with Method Object --- lib/import.rb | 105 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 68 insertions(+), 37 deletions(-) diff --git a/lib/import.rb b/lib/import.rb index 99638a5..d63fdce 100644 --- a/lib/import.rb +++ b/lib/import.rb @@ -4,52 +4,83 @@ # 2. Copy & Paste the method's body in the new class, with no arguments # 3. Replace original method with a call to the new class # 4. Apply "Intention Revealing Method" to the new class. Woot! -class Formatter - # More code, methods, and stuff in this big class - def row_per_day_format(file_name) - # FIXME: 3 Replace Method with Method Object +class FormatterNew + def initialize(file_name) + @file_name = file_name + @hash = { '1' => {}, '2' => {} } + @dates = [] + end - file = File.open file_name, 'r:ISO-8859-1' - # hash[NivelConsistencia][date] = [[value, status]] - hash = { '1' => {}, '2' => {} } - dates = [] - str = '' - CSV.parse(file, col_sep: ';').each do |row| - next if row.empty? - next if row[0] =~ /^\/\// - date = Date.parse(row[2]) - (13..43).each do |i| - measurement_date = date + (i-13) + def perform + load_values_from_csv + turn_into_b_format + end - # If NumDiasDeChuva is empty it means no data - value = row[7].nil? ? -99.9 : row[i] - status = row[i + 31] - hash_value = [value, status] + private - dates << measurement_date - hash[row[1]][measurement_date] = hash_value - end + def load_values_from_csv + file = File.open @file_name, 'r:ISO-8859-1' + CSV.parse(file, col_sep: ';').each do |row| + next if row.empty? || row[0] =~ /^\/\// + load_a_month(row) end + end - dates.uniq.each do |date| - if !hash['1'][date].nil? && hash['2'][date].nil? - # Only 'bruto' (good) - value = hash['1'][date] - str << "#{date}\t#{value[0]}\t#{value[1]}\n" - elsif hash['1'][date].nil? && !hash['2'][date].nil? - # Only 'consistido' (kind of good) - value = hash['2'][date] - str << "#{date}\t#{value[0]}\t#{value[1]}\n" - else - # 'bruto' y 'consistido' (has new and old data) - old_value = hash['1'][date] - new_value = hash['2'][date] - str << "#{date}\t#{new_value[0]}\t#{old_value[1]}\t#{old_value[0]}\n" + def load_a_month(row) + @beginning_of_the_month = Date.parse(row[2]) + (13..43).each do |i| + load_a_day(row, i) end + end + + def load_a_day(row, i) + measurement_date = @beginning_of_the_month + (i-13) + + # If NumDiasDeChuva is empty it means no data + value = row[7].nil? ? -99.9 : row[i] + status = row[i + 31] + hash_value = [value, status] + + @dates << measurement_date + @hash[row[1]][measurement_date] = hash_value + end + + def turn_into_b_format + array_of_day = @dates.uniq.map { |date| format_for_day(date) } + array_of_day.join("\n") + "\n" + end + + def format_for_day(date) + old_value = @hash['1'][date] + new_value = @hash['2'][date] + if only_bruto?(date) + result = [date, old_value[0], old_value[1]] + elsif only_consistido?(date) + result = [date, new_value[0], new_value[1]] + else # 'bruto' y 'consistido' (has new and old data) + result = [date, new_value[0], old_value[1], old_value[0]] end + result.join("\t") + end + + def only_bruto?(date) + !@hash['1'][date].nil? && @hash['2'][date].nil? + end + + def only_consistido?(date) + @hash['1'][date].nil? && !@hash['2'][date].nil? + end + +end + +class Formatter + # More code, methods, and stuff in this big class + + def row_per_day_format(file_name) + # FIXME: 3 Replace Method with Method Object - str + FormatterNew.new(file_name).perform end # More code, methods, and stuff in this big class From 883d65be76b80ecf38d909426e2ee23db57ff0f6 Mon Sep 17 00:00:00 2001 From: Bruce Li Date: Tue, 23 Sep 2014 23:08:00 +0800 Subject: [PATCH 4/5] Solution for FIXME: 4 Service Object --- app/models/subscribe_service.rb | 30 ++++++++++++++++++++++++++++++ app/models/user.rb | 20 +++++--------------- 2 files changed, 35 insertions(+), 15 deletions(-) create mode 100644 app/models/subscribe_service.rb diff --git a/app/models/subscribe_service.rb b/app/models/subscribe_service.rb new file mode 100644 index 0000000..0a7f8ca --- /dev/null +++ b/app/models/subscribe_service.rb @@ -0,0 +1,30 @@ +class SubscribeService + def initialize(user) + @user = user + end + + def subscribe + api_result = try_api { PaymentGateway.subscribe } + if api_result == :success + @user.update_attributes(subscription_plan: "monthly") + end + api_result + end + + def unsubscribe + api_result = try_api { PaymentGateway.unsubscribe } + if api_result == :success + @user.update_attributes(subscription_plan: nil) + end + api_result + end + + private + + def try_api + yield + rescue => e + Rails.logger.error "API call failed. message: #{e.message}" + return :network_error + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 27b111d..2927030 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -18,27 +18,17 @@ def active_projects # FIXME: 4. Service Object def subscribe - api_result = try_api { PaymentGateway.subscribe } - if api_result == :success - update_attributes(subscription_plan: "monthly") - end - api_result + subscribe_service.subscribe end def unsubscribe - api_result = try_api { PaymentGateway.unsubscribe } - if api_result == :success - update_attributes(subscription_plan: nil) - end - api_result + subscribe_service.unsubscribe end private - def try_api - yield - rescue => e - Rails.logger.error "API call failed. message: #{e.message}" - return :network_error + def subscribe_service + @subscribe_service ||= SubscribeService.new(self) end + end From ecd2e78f00d53368c77c35f9af33c7235ec89c5d Mon Sep 17 00:00:00 2001 From: Bruce Li Date: Wed, 24 Sep 2014 20:58:37 +0800 Subject: [PATCH 5/5] Solution for FIXME: 5 Form Object --- app/controllers/users_controller.rb | 12 +++-- app/models/user_update_form.rb | 80 +++++++++++++++++++++++++++++ app/views/users/edit.html.erb | 32 +++++------- 3 files changed, 101 insertions(+), 23 deletions(-) create mode 100644 app/models/user_update_form.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index df8aaa6..a9f00da 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -25,13 +25,15 @@ def create end def edit - @user = User.find(params[:id]) - @user.profile ||= @user.build_profile + @user_update_form = UserUpdateForm.new(params[:id]) + # @user = User.find(params[:id]) + # @user.profile ||= @user.build_profile end def update - if @user.update(user_params) - redirect_to @user, notice: 'User was successfully updated.' + @user_update_form = UserUpdateForm.new(params[:id]) + if @user_update_form.update(user_params) + redirect_to user_path(@user_update_form.user), notice: 'User was successfully updated.' else render :edit end @@ -52,6 +54,6 @@ def set_user # Never trust parameters from the scary internet, only allow the white list through. def user_params - params.require(:user).permit(:email, profile_attributes: [:id, :nickname, :bio] ) + params.require(:user_update_form).permit(:email,:id, :nickname, :bio) end end diff --git a/app/models/user_update_form.rb b/app/models/user_update_form.rb new file mode 100644 index 0000000..f375a38 --- /dev/null +++ b/app/models/user_update_form.rb @@ -0,0 +1,80 @@ +class UserUpdateForm + include ActiveModel::Model + + attr_reader :user + attr_reader :profile + + attr_accessor :email # from user + attr_accessor :nickname # from profile + attr_accessor :bio # from profile + + + validates_presence_of :email + validates_format_of :email, with: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i + validates_presence_of :nickname, :bio + validates_length_of :nickname, in: 3..20 + + def initialize(user_id) + @user = User.find_by_id(user_id) + @profile = @user.profile || @user.build_profile + end + + def persisted? + @user.persisted? && @profile.persisted? + end + + def update(params) + self.email = params[:email] + self.nickname = params[:nickname] + self.bio = params[:bio] + # binding.pry + if valid? + @user.update_attributes(email: email) + @profile.update_attributes(nickname: nickname, bio: bio) + true + else + false + end + end +end + + + +# class SignupForm +# include ActiveModel::Model + +# attr_reader :company +# attr_reader :user + +# attr_accessor :company_name +# attr_accessor :name +# attr_accessor :email + +# validates :company_name, presence: true +# validates :name, presence: true +# validates :email, presence: true +# validate :email_uniqueness + +# # Forms are never themselves persisted +# def persisted? +# false +# end + +# def save +# if valid? +# @company = Company.create!(name: company_name) +# @user = @company.users.create!(name: name, email: email) +# true +# else +# false +# end +# end + +# private + +# def email_uniqueness +# if User.where(email: email).exists? +# errors.add :email, "has already been taken" +# end +# end +# end diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 52d18e0..d5035e1 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -1,12 +1,12 @@

Editing user

-<%= form_for(@user) do |f| %> - <% if @user.errors.any? %> +<%= form_for(@user_update_form, url: user_path(@user_update_form.user)) do |f| %> + <% if @user_update_form.errors.any? %>
-

<%= pluralize(@user.errors.count, "error") %> prohibited this user from being saved:

+

<%= pluralize(@user_update_form.errors.count, "error") %> prohibited this user from being saved:

    - <% @user.errors.full_messages.each do |message| %> + <% @user_update_form.errors.full_messages.each do |message| %>
  • <%= message %>
    • <% end %>
@@ -18,24 +18,20 @@ <%= f.text_field :email %>
- <%= f.fields_for :profile, @user.profile do |ff| %> +

Profile

-

Profile

- -
- <%= ff.label :nickname %>
- <%= ff.text_field :nickname %> -
- -
- <%= ff.label :bio %>
- <%= ff.text_area :bio, rows: 5, cols: 40 %> -
+
+ <%= f.label :nickname %>
+ <%= f.text_field :nickname %> +
- <% end %> +
+ <%= f.label :bio %>
+ <%= f.text_area :bio, rows: 5, cols: 40 %> +
- <%= f.submit %> + <%= f.submit "Update User" %>
<% end %>