WIP: if not set otherwise, identifiers are now generated from the subject

Author: phillmv

app/controllers/entries_controller.rb

@@ -109,11 +109,11 @@ def edit
   # POST /entries.json
   # TODO: why do we support create or update in this method vs the method above?
   def create
-    @entry = @current_notebook.entries.find_by(identifier: entry_params[:identifier])
-    @entry ||= @current_notebook.entries.new(entry_params)
+    maker = EntryMaker.new(current_notebook)
+    @entry, err = maker.create(entry_params)
 
     respond_to do |format|
-      if (@entry.new_record? && @entry.save) || @entry.update(entry_params)
+      if !err
         format.html do
           if request.referer =~ /agenda/
             redirect_back(fallback_location: timeline_path(current_notebook))
@@ -221,7 +221,7 @@ def find_or_build_entry
 
   # Never trust parameters from the scary internet, only allow the white list through.
   def entry_params
-    params.require(:entry).permit(:identifier, :body, :url, :subject, :occurred_at, :in_reply_to, :hide, :occurred_date, :occurred_time, files: [])
+    params.require(:entry).permit(:identifier, :generated_identifier, :body, :url, :subject, :occurred_at, :in_reply_to, :hide, :occurred_date, :occurred_time, files: [])
   end
 
   def bookmark_params

app/models/entry.rb

@@ -306,6 +306,15 @@ def copy_to!(notebook)
     notebook.entries.find_by(identifier: copy.identifier)
   end
 
+  ## little hack for tracking whether the identifier value returned from the
+  # form was set by the user, or was just the default generated identifier.
+  # this allows me to decide whether to replace the identifier with the
+  # parameterized subject
+  attr_accessor :generated_identifier
+  def generated_identifier?
+    self.generated_identifier == self.identifier
+  end
+
   # this is actually pretty complicated to do properly?
   # https://github.com/middleman/middleman/blob/master/middleman-core/lib/middleman-core/util/data.rb
   # https://github.com/middleman/middleman/blob/master/middleman-core/lib/middleman-core/core_extensions/front_matter.rb

app/models/entry_maker.rb

@@ -0,0 +1,39 @@
+# a little maker for encapsulating entry creation behaviour.
+# orig. created to handle setting identifiers from subjects, but there's no
+# reason why in the near future this shouldn't handle _all_ of the entry callbacks
+# i.e. - set_identifier, set_subject, set_thread_identifier
+# but also even like, syncing to git and processing tags, contacts, links, etc.
+class EntryMaker
+  def initialize(current_notebook)
+    @current_notebook = current_notebook
+  end
+
+  def create(entry_params)
+    Entry.transaction do
+      @entry = @current_notebook.entries.find_by(identifier: entry_params[:identifier])
+      @entry ||= @current_notebook.entries.new(entry_params)
+
+      # TODO: rename on update, but only if no other articles link to it yet.
+      if @entry.generated_identifier?
+        if @entry.set_subject.present?
+          @entry.identifier = @entry.subject.parameterize
+        end
+
+        # TODO: need to restrict this up to 100 times or w/e
+        while @current_notebook.entries.find_by(identifier: @entry.identifier)
+          dash_number = @entry.identifier.match(/-\d+$/).to_s
+
+          if dash_number.blank?
+            @entry.identifier = "#{@entry.identifier}-1"
+          else
+            new_number = dash_number.gsub("-", "").to_i + 1
+            @entry.identifier = @entry.identifier.gsub(dash_number, "-#{new_number}")
+
+          end
+        end
+      end
+
+      return [@entry, !((@entry.new_record? && @entry.save) || @entry.update(entry_params))]
+    end
+  end
+end

app/views/entries/_form.html.erb

@@ -19,6 +19,7 @@
 
 <%= form_with(model: entry, url: url, local: true) do |form| %>
   <%= form.hidden_field :identifier, value: @entry.set_identifier %>
+  <%= form.hidden_field :generated_identifier, value: @entry.identifier %>
   <% if @parent_entry %>
     <%= form.hidden_field :in_reply_to,  value: @parent_entry.identifier %>
   <% end %>

test/integration/entries_integration_test.rb

@@ -85,4 +85,30 @@ class EntriesIntegrationTest < ActionDispatch::IntegrationTest
     # put in Timecop right now.
     assert_equal textarea_text, "\n# #daily #{Date.today.to_s}"
   end
+
+  test "when saved thru a controller, an entry's identifier will be set from its subject" do
+    assert_equal 0, @current_notebook.entries.count
+    post create_entry_path(owner: @current_notebook.owner, notebook: @current_notebook), params: { entry: { body: "test mc test"} }
+
+    # ideally look this up from the redirect path or w/e
+    # here we can count on the most recent one having been just created
+    entry_with_no_subject = @current_notebook.entries.last
+
+    # no subject, no identifier tweak
+    assert_nil entry_with_no_subject.subject
+    # same as defined in entry_test.rb
+    assert entry_with_no_subject.identifier =~ /\d\d\d\d\d\d\d\d-[23456789cfghjmpqrvwx]{4}/
+
+    # with a subject it should set the identifier
+    post create_entry_path(owner: @current_notebook.owner, notebook: @current_notebook), params: { entry: { body: "# has a subject\n\ntest mc test"} }
+
+    entry_with_subject = @current_notebook.entries.last
+
+    # TODO:
+    # - create with generated_identifier
+    # - it replaces it with the subject
+    # - but not if the identifier was modified
+    # - then it sets the identifier value
+    # - also if the post already exists it will increment the -1 on the identifier
+  end
 end

test/models/entry_test.rb

@@ -6,7 +6,6 @@ class EntryTest < ActiveSupport::TestCase
     @file_path = File.join(Rails.root, "test", "fixtures", "test_image.jpg")
   end
 
-  # describe "subject & identifier are interrelated" do
   test "an identifier gets set by default, and it resembles a date with some random letters" do
     entry = @notebook.entries.new(body: "body")
     refute entry.identifier
@@ -31,7 +30,6 @@ class EntryTest < ActiveSupport::TestCase
     assert_equal "my subject", h2_subject.subject
 
     # if more than one heading is set it picks the first one
-    
     h2h1_subject = @notebook.entries.create(body: "## first line\n# second line\nhi")
 
     assert_equal "first line", h2h1_subject.subject
@@ -41,7 +39,6 @@ class EntryTest < ActiveSupport::TestCase
     gasp_no_subject = @notebook.entries.create(body: "line1\n\nline2\b\nline3\n\n## this won't get read\n# neither will this\nhi")
     assert_nil gasp_no_subject.subject
   end
-  # end
 
   test "#copy_parent" do
     parent_cal_entry = @notebook.entries.calendars.create(to: "[email protected], [email protected]", from: "[email protected]", body: "#test #right @foobar\n\nhello!\n\ntest")