Refactor in modules and classes

diff --git a/Gemfile.lock b/Gemfile.lock
index 79170978150584121170043bbb663d3f6edd0112..b8a6262db02768187ba67ade912e7182c8e88b25 100644 (file)
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -23,6 +23,7 @@ GEM
     ffi (1.8.1)
     gherkin (2.12.0)
       multi_json (~> 1.3)
     ffi (1.8.1)
     gherkin (2.12.0)
       multi_json (~> 1.3)

+    json (1.7.7)

     multi_json (1.7.4)
     rspec-expectations (2.13.0)
       diff-lcs (>= 1.1.3, < 2.0)
     multi_json (1.7.4)
     rspec-expectations (2.13.0)
       diff-lcs (>= 1.1.3, < 2.0)


@@ -34,4 +35,5 @@ PLATFORMS
 DEPENDENCIES
   aruba
   cucumber
 DEPENDENCIES
   aruba
   cucumber

+  json (~> 1.7.7)

   nos_oignons!
   nos_oignons!

diff --git a/bin/list-emails b/bin/list-emails
index 935bcd021a4b25043690fe547f64e7c8b6deda44..6b13304b764262dd851848b068d1b879cff43c97 100755 (executable)
--- a/bin/list-emails
+++ b/bin/list-emails
@@ -7,11 +7,8 @@ Bundler.setup
 
 require 'nos_oignons/subscriptions'
 
 
 require 'nos_oignons/subscriptions'
 

-subscriptions = read_subscriptions
-now = Time.now
-last_year = Time.new(now.year - 1, now.month, now.day).to_date
-subscriptions.sort.each do |subscription_id, data|
-  if data['membership_fee_paid_on'] && last_year < data['membership_fee_paid_on']
-    puts data['email']
+NosOignons::Subscription.all.each do |subscription|
+  if subscription.up_to_date?
+    puts subscription.email

   end
 end
   end
 end

diff --git a/bin/pre-commit-hook b/bin/pre-commit-hook
index 4571599b499014fa351c2c32c044dddee9b323f3..921c39c79f1c3f0c17eb410737c77482095c31fd 100755 (executable)
--- a/bin/pre-commit-hook
+++ b/bin/pre-commit-hook
@@ -16,10 +16,12 @@ else
 end
 
 IO.popen(['git', 'diff-index', '--cached', '--name-status', against]) do |io|
 end
 
 IO.popen(['git', 'diff-index', '--cached', '--name-status', against]) do |io|

-  handle_modified_files(io) do |file|
-    next unless file.start_with?("#{SUBSCRIPTIONS_ROOT}/")
-    # Use empty ref to get the index
-    if !is_valid_subscription_file?('', file)
+  NosOignons::Git.handle_modified_files(io) do |file|
+    next unless file.start_with?("#{NosOignons::SUBSCRIPTIONS_ROOT}/")
+    begin
+      # Use empty ref to get the index
+      NosOignons::Subscription.read_from_git('', file)
+    rescue ArgumentError

       $stderr.puts "Désolé : #{file} n'a pas le bon format !"
       exit 1
     end
       $stderr.puts "Désolé : #{file} n'a pas le bon format !"
       exit 1
     end

diff --git a/bin/pre-receive-hook b/bin/pre-receive-hook
index 46500b09da328e2817faa51ecc63761c0539a7ed..c86b3259ad66712b49154853748362d91ab3dfa6 100755 (executable)
--- a/bin/pre-receive-hook
+++ b/bin/pre-receive-hook
@@ -11,9 +11,11 @@ require 'nos_oignons/subscriptions'
 $stdin.readlines.each do |ref_line|
   old_value, new_value, ref_name = ref_line.rstrip.split(' ', 3)
   IO.popen(['git', 'diff', '--name-status', "#{old_value}..#{new_value}"]) do |io|
 $stdin.readlines.each do |ref_line|
   old_value, new_value, ref_name = ref_line.rstrip.split(' ', 3)
   IO.popen(['git', 'diff', '--name-status', "#{old_value}..#{new_value}"]) do |io|

-    handle_modified_files(io) do |file|
-      next unless file.start_with?("#{SUBSCRIPTIONS_ROOT}/")
-      if !is_valid_subscription_file?(new_value, file)
+    NosOignons::Git.handle_modified_files(io) do |file|
+      next unless file.start_with?("#{NosOignons::SUBSCRIPTIONS_ROOT}/")
+      begin
+        NosOignons::Subscription.read_from_git(new_value, file)
+      rescue ArgumentError

         $stderr.puts "Désolé : #{file} n'a pas le bon format !"
         exit 1
       end
         $stderr.puts "Désolé : #{file} n'a pas le bon format !"
         exit 1
       end

diff --git a/features/step_definitions/commands.rb b/features/step_definitions/commands.rb
index 54bc747ed69826d35fae7a1b23bc5d8cc4658665..0aec8b608430a5bd6a97ff4a5d831eddaee82190 100644 (file)
--- a/features/step_definitions/commands.rb
+++ b/features/step_definitions/commands.rb
@@ -1,3 +1,5 @@
+#-*- coding: utf-8 -*-
+

 When /^j'exécute list\-emails$/ do
   run_simple 'list-emails'
 end
 When /^j'exécute list\-emails$/ do
   run_simple 'list-emails'
 end

diff --git a/features/step_definitions/git.rb b/features/step_definitions/git.rb
index f2ba85e2d7a19315dc154a1fb3b21a9b872b400a..062181f068eaf1c74149c4c00e26b51299f6a2f1 100644 (file)
--- a/features/step_definitions/git.rb
+++ b/features/step_definitions/git.rb
@@ -1,3 +1,5 @@
+#-*- coding: utf-8 -*-
+

 require 'fileutils'
 
 Given /^un clone du Git contenant les adhésions$/ do
 require 'fileutils'
 
 Given /^un clone du Git contenant les adhésions$/ do

diff --git a/lib/nos_oignons/git.rb b/lib/nos_oignons/git.rb
index 4a2d04df463d3a90689ca1e4733d629ff989e76d..53399bd0e469b1bd3a2131b5ee92033bfe5b53ed 100644 (file)
--- a/lib/nos_oignons/git.rb
+++ b/lib/nos_oignons/git.rb
@@ -1,9 +1,15 @@
-def handle_modified_files(io)
-  io.readlines.each do |line|
-    status, file = line.strip.split("\t", 2)
-    # Has file been added or modified?
-    if ['A', 'M'].include?(status)
-      yield file
+module NosOignons
+  module Git
+    class << self
+      def handle_modified_files(io)
+        io.readlines.each do |line|
+          status, file = line.strip.split("\t", 2)
+          # Has file been added or modified?
+          if ['A', 'M'].include?(status)
+            yield file
+          end
+        end
+      end

     end
   end
 end
     end
   end
 end

diff --git a/lib/nos_oignons/subscriptions.rb b/lib/nos_oignons/subscriptions.rb
index ca5cafe5265f745b80d6e1fcf261803f8fa8fbfa..de4ddf0607c2a47f80108b8346ceb7f70ffb7784 100644 (file)
--- a/lib/nos_oignons/subscriptions.rb
+++ b/lib/nos_oignons/subscriptions.rb
@@ -1,37 +1,65 @@
 require 'safe_yaml'
 SafeYAML::OPTIONS[:default_mode] = :safe
 
 require 'safe_yaml'
 SafeYAML::OPTIONS[:default_mode] = :safe
 

-SUBSCRIPTIONS_ROOT = 'Membres'
+module NosOignons
+  SUBSCRIPTIONS_ROOT = 'Membres'
+  SUBSCRIPTION_FIELDS = [:name, :address, :email, :membership_fee_paid_on]
+  SUBSCRIPTION_MANDATORY_FIELDS = [:name, :email]

 
 

-def is_valid_subscription?(content)
-  return false if content.length == 0
-  return false unless content.start_with?("---\n")
-  begin
-    data = YAML.load(content)
-  rescue ArgumentError
-    # Parse error
-    return false
-  end
-  ['name', 'email'].each do |key|
-    return false unless data.include?(key)
-  end
-  if data.include?('membership_fee_paid_on')
-    return false unless data['membership_fee_paid_on'].is_a?(Date)
-  end
-  true
-end
+  class Subscription < Struct.new(*SUBSCRIPTION_FIELDS)
+    class << self
+      def all
+        Dir.glob("#{SUBSCRIPTIONS_ROOT}/*.mdwn").sort.collect do |file|
+          subscription_id = File.basename(file).gsub(/\.mdwn$/, '')
+          Subscription.new(subscription_id)
+        end
+      end

 
 

-def is_valid_subscription_file?(ref, file)
-  IO.popen(['git', 'show', "#{ref}:#{file}"]) do |f|
-    is_valid_subscription?(f.read)
-  end
-end
+      def filename_for_id(subscription_id)
+        "Membres/%06d.mdwn" % subscription_id
+      end
+
+      def read_from_git(ref, file)
+        IO.popen(['git', 'show', "#{ref}:#{file}"]) do |f|
+          subscription_id = File.basename(file).gsub(/\.mdwn$/, '')
+          Subscription.new(subscription_id, f.read)
+        end
+      end
+    end
+
+    attr_reader :subscription_id
+
+    def initialize(subscription_id, page_content=nil)
+      unless subscription_id =~ /\A\d{6}\z/
+        raise ArgumentError.new('bad subscription id format')
+      end
+      @subscription_id = subscription_id
+      unless page_content
+        page_content = File.open(Subscription.filename_for_id(subscription_id)).read
+      end
+      unless page_content.start_with?("---\n")
+        raise ArgumentError.new('content is not a proper YAML document')
+      end
+      data = YAML.load(page_content)
+      SUBSCRIPTION_FIELDS.each do |field|
+        self[field] = data[field.to_s]
+      end
+      # Immutability for the win
+      SUBSCRIPTION_FIELDS.each do |field|
+        instance_eval{ undef :"#{field}=" }
+      end
+      SUBSCRIPTION_MANDATORY_FIELDS.each do |sym|
+        raise ArgumentError.new('missing mandatory fields') unless self[sym]
+      end
+      if membership_fee_paid_on && !membership_fee_paid_on.is_a?(Date)
+        raise ArgumentError.new('membership_fee_paid_on is not a date')
+      end
+    end

 
 

-def read_subscriptions
-  subscriptions = {}
-  Dir.glob("#{SUBSCRIPTIONS_ROOT}/*.mdwn") do |file|
-    subscription_id = File.basename(file).gsub(/\.mdwn$/, '')
-    subscriptions[subscription_id] = YAML.load_file(file)
+    def up_to_date?
+      now = Time.now
+      last_year = Time.new(now.year - 1, now.month, now.day).to_date
+      membership_fee_paid_on && last_year < membership_fee_paid_on
+    end

   end
   end

-  subscriptions

 end
 end

diff --git a/nos_oignons.gemspec b/nos_oignons.gemspec
index 585576f7531bfaa463d150d090c73054f6dc87c3..e32ccd1fddf9d623cb043892e67b57dc37574c85 100644 (file)
--- a/nos_oignons.gemspec
+++ b/nos_oignons.gemspec
@@ -5,5 +5,6 @@ Gem::Specification.new do |s|
 
   s.add_development_dependency 'cucumber'
   s.add_development_dependency 'aruba'
 
   s.add_development_dependency 'cucumber'
   s.add_development_dependency 'aruba'

+  s.add_development_dependency 'json', '~> 1.7.7'

   s.add_runtime_dependency 'safe_yaml'
 end
   s.add_runtime_dependency 'safe_yaml'
 end