Started to implement dates in links. [#255]

TODO

@@ -0,0 +1,19 @@
+== Parts that need an urgent cleanup ==
+
+Some parts of zena have become really messy and need an URGENT cleanup. These parts are mostly
+related to nested attributes and the cleanup should be made during the move to rails 2.3+.
+
+Some really ugly parts:
+
+=== links ===
+
+We support many different ways to alter links and it's becoming hard to maintain:
+
+node[hot_id] = ZIP
+node[link][hot][other_id] = ZIP
+node[link][1][other_id] = ZIP along with node[link][1][role] = 'hot'
+link[other_id]  = ZIP
+link[other_zip] = ZIP
+
+We need 'other_zip' because select_id helper needs to read the value... "select_id" is really bad code. yuk.
+

app/models/link.rb

@@ -6,11 +6,12 @@ class << self
     def find_through(node, link_id)
       return nil unless link = Link.find(:first, :conditions => ['(source_id = ? OR target_id = ?) AND id = ?', node[:id], node[:id], link_id])
       link.start = node
-      node.link  = link
+      node.set_link(link)
       link
     end
   end
 
+  # TODO: is this used ?
   def update_attributes_with_transformations(attrs)
     return false unless @node
 
@@ -21,13 +22,14 @@ def update_attributes_with_transformations(attrs)
     rel = @node.relation_proxy_from_link(self)
     rel.other_link = self
 
-    ['status', 'comment'].each do |k|
+    Zena::Relations::LINK_ATTRIBUTES.each do |k|
+      k = k.to_s # TODO: use only strings or symbols but avoid this mess
       rel.send("other_#{k}=", attrs[k]) if attrs.has_key?(k)
       self[k] = attrs[k]
     end
 
-    if attrs['other_zip']
-      other_id = secure(Node) { Node.translate_pseudo_id(attrs['other_zip'], :id, @node) }
+    if other_id = attrs['other_id'] || attrs['other_zip']
+      other_id = secure(Node) { Node.translate_pseudo_id(other_id, :id, @node) }
       rel.other_id = other_id
       if @side == :source
         self[:target_id] = other_id

app/models/node.rb

@@ -127,10 +127,10 @@
 Setting 'custom_base' on a node should be done with caution as the node's zip is on longer in the url and when you move the node around, there is no way to find the new location from the old url. Custom_base should therefore only be used for nodes that are not going to move.
 =end
 class Node < ActiveRecord::Base
-  attr_accessor      :link, :old_title
+  attr_accessor      :old_title
   zafu_readable      :name, :created_at, :updated_at, :event_at, :log_at, :kpath, :user_zip, :parent_zip, :project_zip,
                      :section_zip, :skin, :ref_lang, :fullpath, :rootpath, :position, :publish_from, :max_status, :rgroup_id, 
-                     :wgroup_id, :pgroup_id, :basepath, :custom_base, :klass, :zip, :score, :comments_count, :l_status, :l_comment,
+                     :wgroup_id, :pgroup_id, :basepath, :custom_base, :klass, :zip, :score, :comments_count,
                      :custom_a, :custom_b, :title, :text,
                      :m_text, :m_title, :m_author   
   zafu_context       :author => "Contact", :parent => "Node", 
@@ -677,7 +677,7 @@ def transform_attributes(new_attributes, base_node = nil)
           res["#{key}_id"] = Group.translate_pseudo_id(attributes[key], :id) || attributes[key]
         elsif ['user_id'].include?(key)
           res[key] = User.translate_pseudo_id(attributes[key], :id) || attributes[key]
-        elsif ['v_publish_from', 'log_at', 'event_at'].include?(key)
+        elsif ['v_publish_from', 'log_at', 'event_at', 'date'].include?(key)
           if attributes[key].kind_of?(Time)
             res[key] = attributes[key]
           elsif attributes[key]
@@ -703,6 +703,8 @@ def transform_attributes(new_attributes, base_node = nil)
           unless attributes[key].blank?
             res[key] = attributes[key]
           end
+        elsif attributes[key].kind_of?(Hash)
+          res[key] = transform_attributes(attributes[key])
         else
           # translate zazen
           value = attributes[key]

app/models/relation_proxy.rb

@@ -1,5 +1,8 @@
 class RelationProxy < Relation
   attr_accessor   :side, :link_errors, :start, :other_link
+  LINK_ATTRIBUTES = Zena::Relations::LINK_ATTRIBUTES
+  LINK_ATTRIBUTES_SQL = LINK_ATTRIBUTES.map {|sym| "`#{sym}`"}.join(',')
+  LINK_SELECT     = "nodes.*,links.id AS link_id,#{LINK_ATTRIBUTES.map {|l| "links.#{l} AS l_#{l}"}.join(',')}"
 
   class << self
 
@@ -32,7 +35,7 @@ def get_proxy(node, role)
   # Used by relation_links
   def records(options={})
     return @records if defined? @records
-    opts = { :select     => "nodes.*,links.id AS link_id, links.status AS l_status, links.comment AS l_comment", 
+    opts = { :select     => LINK_SELECT, 
              :joins      => "INNER JOIN links ON nodes.id=links.#{other_side} AND links.relation_id = #{self[:id]} AND links.#{link_side} = #{@start[:id]}",
              :group      => 'nodes.id'}
 
@@ -46,7 +49,7 @@ def records(options={})
   # I do not think this method is used anymore (all is done by @node.find(...)).
   def record(options={})
     return @record if defined?(@record) || @start.new_record?
-    opts = { :select     => "nodes.*,links.id AS link_id, links.status AS l_status, links.comment AS l_comment", 
+    opts = { :select     => LINK_SELECT, 
              :joins      => "INNER JOIN links ON nodes.id=links.#{other_side} AND links.relation_id = #{self[:id]} AND links.#{link_side} = #{@start[:id]}",
              :group      => 'nodes.id'}
 
@@ -100,12 +103,10 @@ def other_zips
     (records || []).map { |r| r[:zip] }
   end
 
-  def other_status
-    other_link ? other_link[:status] : nil
-  end
-
-  def other_comment
-    other_link ? other_link[:comment] : nil
+  LINK_ATTRIBUTES.each do |sym|
+    define_method(sym) do
+      other_link ? other_link[sym] : nil
+    end
   end
 
   def other_role
@@ -135,12 +136,10 @@ def remove_link(link)
     @links_to_delete << link
   end
 
-  def other_status=(v)
-    attributes_to_update[:status] = v.blank? ? nil : v
-  end
-
-  def other_comment=(v)
-    attributes_to_update[:comment] = v.blank? ? nil : v
+  LINK_ATTRIBUTES.each do |sym|
+    define_method("other_#{sym}=") do |v|
+      attributes_to_update[sym] = v.blank? ? nil : v
+    end
   end
 
   def this_kpath
@@ -160,7 +159,6 @@ def other_links
   # 1. can remove old link
   # 2. can write in new target
   def attributes_to_update_valid?
-    debugger if $debbbb
     return true unless @attributes_to_update || @links_to_delete
 
     @link_errors  = []
@@ -173,8 +171,9 @@ def attributes_to_update_valid?
       @del_links = @links_to_delete
       @attributes_to_update = {}
     else
+
       # check if we have an update/create
-      unless @attributes_to_update[:id]
+      unless @attributes_to_update.has_key?(:id)
         # try to find current id/ids
         if @other_link
           @attributes_to_update[:id] = @other_link[other_side]
@@ -198,13 +197,13 @@ def attributes_to_update_valid?
       if @attributes_to_update[:id].kind_of?(Array) 
         if unique?
           @link_errors << "Cannot set multiple targets on #{as_unique? ? 'one' : 'many'}-to-one relation '#{this_role}'."
-        elsif @attributes_to_update.has_key?(:status) || @attributes_to_update.has_key?(:comment)
+        elsif (@attributes_to_update.keys & LINK_ATTRIBUTES) != []
           keys = @attributes_to_update.keys
           keys.delete(:id)
           @link_errors << "Cannot set attributes #{keys.join(', ')} on multiple targets."
         end
       end
-
+      
       return false if @link_errors != []
 
       # 1. find what changed
@@ -236,14 +235,14 @@ def attributes_to_update_valid?
         else
           # other target: replace
           @del_links = [other_link] if other_link
-          @add_links << @attributes_to_update
+          @add_links << @attributes_to_update unless @attributes_to_update[:id].blank?
         end
       else
         # ..-to-many
         # add/update a link
         if other_ids.include?(@attributes_to_update[:id])
           # update
-          if @attributes_to_update.has_key?(:status) || @attributes_to_update.has_key?(:comment)
+          if (@attributes_to_update.keys & LINK_ATTRIBUTES) != []
             other_links.each do |link|
               if link[other_side] == @attributes_to_update[:id]
                 @update_links << changed_link(link, @attributes_to_update)
@@ -286,7 +285,7 @@ def attributes_to_update_valid?
   # Return updated link if changed or nil when nothing changed
   def changed_link(link, attrs)
     changed = false
-    [:status, :comment].each do |sym|
+    LINK_ATTRIBUTES.each do |sym|
       next unless attrs.has_key?(sym)
       if attrs[sym] != link[sym]
         changed = true
@@ -305,10 +304,10 @@ def update_links!
 
     list = []
     @add_links.each do |hash|
-      next unless hash[:id]
-      list << "(#{self[:id]},#{@start[:id]},#{hash[:id]},#{Link.connection.quote(hash[:status])},#{Link.connection.quote(hash[:comment])})"
+      next if hash[:id].blank?
+      list << "(#{self[:id]},#{@start[:id]},#{hash[:id]},#{LINK_ATTRIBUTES.map{|sym| Link.connection.quote(hash[sym])}.join(',')})"
     end
-    Link.connection.execute "INSERT INTO links (`relation_id`,`#{link_side}`,`#{other_side}`,`status`,`comment`) VALUES #{list.join(',')}"
+    Link.connection.execute "INSERT INTO links (`relation_id`,`#{link_side}`,`#{other_side}`,#{LINK_ATTRIBUTES_SQL}) VALUES #{list.join(',')}"
     @attributes_to_update = nil
     @links_to_delete      = nil
     remove_instance_variable(:@records)     if defined?(@records)

db/migrate/050_date_in_links.rb

@@ -0,0 +1,9 @@
+class DateInLinks < ActiveRecord::Migration
+  def self.up
+    add_column    :links, :date, :datetime
+  end
+
+  def self.down
+    remove_column :links, :date
+  end
+end

db/schema.rb

@@ -156,7 +156,8 @@
     t.column "target_id",   :integer
     t.column "relation_id", :integer
     t.column "status",      :integer
-    t.column "comment",     :string,  :limit => 60
+    t.column "comment",     :string,   :limit => 60
+    t.column "date",        :datetime
   end
 
   create_table "nodes", :force => true do |t|

lib/has_relations.rb

@@ -1,5 +1,8 @@
 module Zena
   module Relations
+    LINK_ATTRIBUTES = [:status, :comment, :date]
+    LINK_REGEXP = /^([\w_]+)_(ids?|zips?|#{LINK_ATTRIBUTES.join('|')})(=?)$/
+
     module HasRelations
       # this is called when the module is included into the 'base' module
       def self.included(base)
@@ -12,9 +15,12 @@ def has_relations
         validate      :relations_valid
         after_save    :update_relations
         after_destroy :destroy_links
+        zafu_readable :link
+        zafu_readable LINK_ATTRIBUTES.map {|k| "l_#{k}".to_sym}
+
+        include Zena::Relations::InstanceMethods
 
         class_eval <<-END
-          include Zena::Relations::InstanceMethods
           class << self
             include Zena::Relations::ClassMethods
           end
@@ -49,6 +55,9 @@ def split_kpath
     end
 
     module InstanceMethods
+      def set_link(link)
+        @link = link
+      end
 
       # status defined through loading link
       def l_status
@@ -57,12 +66,20 @@ def l_status
         val ? val.to_i : nil
       end
 
+      # TODO: could we use LINK_ATTRIBUTES and 'define_method' here ?
+
       # comment defined through loading link
       def l_comment
         return @l_comment if defined? @l_comment
         @link ? @link[:comment] : self['l_comment']
       end
 
+      # date defined through loading link
+      def l_date
+        return @l_date if defined? @l_date
+        @l_date = @link ? @link[:date] : (self['l_date'] ? Time.parse(self['l_date']) : nil)
+      end
+
       def link_id
         @link ? @link[:id] : (self[:link_id] == -1 ? nil : self[:link_id]) # -1 == dummy link
       end
@@ -81,11 +98,13 @@ def link_id=(v)
         end 
       end
 
+      # FIXME: this method does an 'update' not only 'add'
       def add_link(role, hash)
         if rel = relation_proxy(role)
-          [:status, :comment, :id].each do |k|
+          LINK_ATTRIBUTES.each do |k|
             rel.send("other_#{k}=", hash[k]) if hash.has_key?(k)
           end
+          rel.other_id = hash[:other_id] if hash.has_key?(:other_id)
         else
           errors.add(role, 'invalid relation')
         end
@@ -104,6 +123,24 @@ def remove_link(link)
         end
       end
 
+      # TODO: could use rails native nested attributes !!!
+      def link=(hash)
+        return unless hash.kind_of?(Hash)
+        hash.each do |role, definition|
+          if role =~ /\A\d+\Z/
+            # key used as array
+          else
+            # key used as role
+            definition['role'] ||= role
+          end
+          add_link(definition.delete('role'), definition.symbolize_keys)  # TODO: only use string keys
+        end 
+      end
+
+      def link
+        @link
+      end
+
       def l_comment=(v)
         @l_comment = v.blank? ? nil : v
         if rel = relation_proxy_from_link
@@ -118,6 +155,13 @@ def l_status=(v)
         end
       end
 
+      def l_date=(v)
+        @l_date = v.blank? ? nil : v
+        if rel = relation_proxy_from_link
+          rel.other_date = @l_date
+        end
+      end
+
       def all_relations
         @all_relations ||= self.vclass.all_relations(self)
       end
@@ -180,7 +224,7 @@ def method_missing(meth, *args)
           super
         rescue NoMethodError => err
           # 1. is this a method related to a relation ?
-          if meth.to_s =~ /^([\w_]+)_(ids?|zips?|status|comment)(=?)$/
+          if meth.to_s =~ LINK_REGEXP
             role  = $1
             field = $2
             mode  = $3

lib/node_query.rb

@@ -58,7 +58,8 @@ def default_context_filter
   def after_parse
     @where.unshift "(\#{#{@node_name}.secure_scope('#{table}')})"
     if @tables.include?('links')
-      @select << "#{table('links')}.id AS link_id, links.status AS l_status, links.comment AS l_comment"
+      link_table = table('links')
+      @select << "#{link_table}.id AS link_id,#{Zena::Relations::LINK_ATTRIBUTES.map {|l| "#{link_table}.#{l} AS l_#{l}"}.join(',')}"
     elsif @errors_unless_safe_links
       @errors += @errors_unless_safe_links
     end
@@ -236,7 +237,7 @@ def map_field(field, table_name = table, context = nil)
         end
       when 'l_'  
         key, function = parse_sql_function_in_field(field)
-        if key == 'l_status' || key == 'l_comment' || (key == 'l_id' && [:order, :group].include?(context))
+        if key == 'l_status' || key == 'l_comment' || key == 'l_date' || (key == 'l_id' && [:order, :group].include?(context))
           @errors_unless_safe_links ||= []
           @errors_unless_safe_links << "cannot use link field '#{key}' in this query" unless (key == 'l_id' && context == :order)
           # ok