diff --git a/app/controllers/entries_controller.rb b/app/controllers/entries_controller.rb index 2ed577e8e..58fa774f1 100644 --- a/app/controllers/entries_controller.rb +++ b/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 diff --git a/app/models/entry.rb b/app/models/entry.rb index c7364ca7c..39197844f 100644 --- a/app/models/entry.rb +++ b/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 diff --git a/app/models/entry_maker.rb b/app/models/entry_maker.rb new file mode 100644 index 000000000..5439f97bf --- /dev/null +++ b/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 diff --git a/app/views/entries/_form.html.erb b/app/views/entries/_form.html.erb index 401acd785..fc7d07dca 100644 --- a/app/views/entries/_form.html.erb +++ b/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 %> diff --git a/test/integration/entries_integration_test.rb b/test/integration/entries_integration_test.rb index b955a7d95..692eb1efb 100644 --- a/test/integration/entries_integration_test.rb +++ b/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 diff --git a/test/models/entry_test.rb b/test/models/entry_test.rb index ba77bbc28..c1e021233 100644 --- a/test/models/entry_test.rb +++ b/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: "foo@example.com, bar@example.com", from: "qux@example.com", body: "#test #right @foobar\n\nhello!\n\ntest")