Skip to content

Commit

Permalink
require that resubmits start new discussions
Browse files Browse the repository at this point in the history
  • Loading branch information
pushcx committed Mar 3, 2024
1 parent b5dda51 commit 23c0c2d
Show file tree
Hide file tree
Showing 11 changed files with 109 additions and 38 deletions.
4 changes: 4 additions & 0 deletions app/assets/javascripts/application.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,16 @@ class _LobstersFunction {
checkStoryDuplicate(form) {
const formData = new FormData(form);
const action = '/stories/check_url_dupe';
console.log('checkStoryDuplicate', form);
fetchWithCSRF(action, {
method: 'post',
headers: new Headers({'X-Requested-With': 'XMLHttpRequest'}),
body: formData,
}).then (response => {
console.log('checkStoryDuplicate', response);
response.text().then(text => {
console.log('checkStoryDuplicate', text, qS('.form_errors_header'));
// FIXME on second click of 'fetch title', this doesn't run
qS('.form_errors_header').innerHTML = text;
});
});
Expand Down
5 changes: 5 additions & 0 deletions app/assets/stylesheets/application.css.erb
Original file line number Diff line number Diff line change
Expand Up @@ -1440,6 +1440,11 @@ div.flash-success a {
font-weight: bold;
color: var(--color-fg-contrast-13);
}
div.flash-error .similar a,
div.flash-notice .similar a,
div.flash-success .similar a {
font-weight: normal;
}
div.flash-error p,
div.flash-notice p,
div.flash-success p {
Expand Down
5 changes: 1 addition & 4 deletions app/controllers/comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ def create
comment = story.comments.build
comment.comment = params[:comment].to_s
comment.user = @user

if params[:hat_id] && @user.wearable_hats.where(id: params[:hat_id])
comment.hat_id = params[:hat_id]
end
comment.hat = @user.wearable_hats.find_by(id: params[:hat_id])

if params[:parent_comment_short_id].present?
# includes parent story_id to ensure this comment's story_id matches
Expand Down
32 changes: 25 additions & 7 deletions app/controllers/stories_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,25 @@ def create
@story = Story.new(user: @user)
@story.attributes = story_params

if @story.valid? && !(@story.already_posted_recently? && !@story.seen_previous)
if @story.save
ReadRibbon.where(user: @user, story: @story).first_or_create!
return redirect_to @story.comments_path
if @story.is_resubmit?
@comment = @story.comments.new(user: @user)
@comment.comment = params[:comment]
@comment.hat = @user.wearable_hats.find_by(id: params[:hat_id])
end

if @story.valid? &&
!@story.already_posted_recently? &&
(!@story.is_resubmit? || @comment.valid?)

Story.transaction do
if @story.save && (!@story.is_resubmit? || @comment.save)
ReadRibbon.where(user: @user, story: @story).first_or_create!
redirect_to @story.comments_path
else
raise ActiveRecord::Rollback
end
end
return if @story.persisted? # can't return out of transaction block
end

render action: "new"
Expand Down Expand Up @@ -97,6 +111,12 @@ def new
return redirect_to @story.most_recent_similar.comments_path
end

if @story.is_resubmit?
@comment = @story.comments.new(user: @user)
@comment.comment = params[:comment]
@comment.hat = @user.wearable_hats.find_by(id: params[:hat_id])
end

# ignore what the user brought unless we need it as a fallback
@story.title = sattrs[:title]
if @story.title.blank? && params[:title].present?
Expand All @@ -115,8 +135,6 @@ def preview

@story.valid?

@story.seen_previous = true

render action: "new", layout: false
end

Expand Down Expand Up @@ -416,7 +434,7 @@ def check_url_dupe

def story_params
p = params.require(:story).permit(
:title, :url, :description, :moderation_reason, :seen_previous,
:title, :url, :description, :moderation_reason,
:merge_story_short_id, :is_unavailable, :user_is_author, :user_is_following,
tags_a: []
)
Expand Down
7 changes: 6 additions & 1 deletion app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ def errors_for(object)
html << "<p>There were the problems with the following fields:</p>"
html << "<ul>"
object.errors.full_messages.each do |error|
html << "<li>#{error}</li>"
html << if error == "Comments is invalid"
# FIXME Ugly kludge, I don't know where this validation is defined to fix the wording
"<li>Comment is missing</li>"
else
"<li>#{error}</li>"
end
end
html << "</ul></div>"
end
Expand Down
13 changes: 10 additions & 3 deletions app/models/story.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ class Story < ApplicationRecord

attr_accessor :current_vote, :editing_from_suggestions, :editor, :fetching_ip,
:is_hidden_by_cur_user, :latest_comment_id,
:is_saved_by_cur_user, :moderation_reason, :previewing,
:seen_previous
:is_saved_by_cur_user, :moderation_reason, :previewing
attr_writer :fetched_response

before_validation :assign_short_id_and_score, on: :create
Expand Down Expand Up @@ -197,7 +196,7 @@ def already_posted_recently?
if most_recent_similar&.is_recent?
errors.add(:url, "has already been submitted within the past #{RECENT_DAYS} days")
true
elsif most_recent_similar && user&.is_new?
elsif user&.is_new? && most_recent_similar
errors.add(:url, "cannot be resubmitted by new users")
true
else
Expand Down Expand Up @@ -227,6 +226,10 @@ def check_not_banned_domain
end
end

def comments_closing_soon?
created_at && (created_at - 1.hour).before?(Story::COMMENTABLE_DAYS.days.ago)
end

# current_vote is the vote loaded for the currently-viewing user
def current_flagged?
current_vote.try(:[], :vote) == -1
Expand Down Expand Up @@ -265,6 +268,10 @@ def public_similar_stories(user)
@_public_similar_stories ||= similar_stories.base(user)
end

def is_resubmit?
!already_posted_recently? && similar_stories.any?
end

def most_recent_similar
similar_stories.first
end
Expand Down
6 changes: 3 additions & 3 deletions app/views/comments/_commentbox.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
<ol class="comments comments1">
<li class="comments_subtree">
<%= render partial: 'threads', locals: { thread: parents, story: story } %>
</li>
</ol>
</li>
</ol>
<% end %>

<div class="comment comment_form_container" data-shortid="<%= comment.short_id if comment.persisted? %>">
Expand All @@ -20,7 +20,7 @@
<%= f.hidden_field "parent_comment_short_id", value: comment.parent_comment.short_id %>
<% end %>
<% if (comment.story.created_at - 1.hour).before? Story::COMMENTABLE_DAYS.days.ago %>
<% if comment.story.comments_closing_soon? %>
<p class="hint">
This story is almost <%= Story::COMMENTABLE_DAYS %> days old
and will stop accepting comments in
Expand Down
29 changes: 16 additions & 13 deletions app/views/stories/_form_errors.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,28 @@
<div class="form_errors_header">
<% if story.errors.any? %>
<%= errors_for story %>
<% elsif !story.errors.any? && story.similar_stories.any? %>
<% end %>
<% if story.is_resubmit? %>
<div class="flash-notice">
<h2>Note: This story was already submitted <%= time_ago_in_words_label(story.most_recent_similar.created_at) %></h2>
<p>
You can submit it again if the content has changed or warrants new discussion.
Please post a comment about that to start.
This story has been submitted before. You can resubmit it with a comment to start a fresh discussion.
</p>

<%= render partial: "stories/similar", locals: { similar: story.similar_stories } %>
</div>

<% if f %>
<%= f.hidden_field :seen_previous %>
<% else %>
<%= form_with model: story do |f| %>
<%= f.hidden_field :seen_previous %>
<% end %>
<% end %>
<% end %>
<div>
<div class="d">
<strong>What has changed or warrants new discussion?</strong><br>
(Not just that you think it's still interesting or read it for the first time.)
<br><br>
</div>

<% if story.similar_stories.any? %>
<%= label_tag :comment, "Comment:" %>
<%= text_area_tag "comment", @comment&.comment, :rows => 5 %>
<br><br>
</div>
<% elsif story.similar_stories.any? %>
<p>Previous discussions for this story:</p>
<%= render partial: "stories/similar", locals: { similar: story.similar_stories } %>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/stories/_similar.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<%# locals: (similar:) -%>
<ol>
<ol class="similar">
<% similar.each do |story| %>
<li>
<a href="<%= story.url %>" target="_blank"><%= story.title %></a>
Expand Down
36 changes: 30 additions & 6 deletions spec/features/submit_story_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,41 @@
select :tag1, from: "Tags"
click_button "Submit"

# TODO: would be nice if this had a specific error message
# TODO: would be nice if this had a specific error message #941
expect(page).to have_content "has already been submitted"
}.not_to(change { Story.count })
end

scenario "resubmitting an old link" do
s = create(:story, created_at: (Story::RECENT_DAYS + 1).days.ago)
visit "/stories/new?url=#{s.url}"
context "resubmitting an old link" do
scenario "prompts user to start a conversation" do
s = create(:story, created_at: (Story::RECENT_DAYS + 1).days.ago)
visit "/stories/new?url=#{s.url}"

expect(page).to have_content "submit it again"
expect(page).to have_content "Previous discussions"
expect(page).to have_content "submitted before"
expect(page).to have_field :comment
end

scenario "without a comment doesn't work" do
s = create(:story, created_at: (Story::RECENT_DAYS + 1).days.ago)
visit "/stories/new?url=#{s.url}"
click_button "Submit"

expect(page).to have_content "submitted before"
expect(page).to have_content "is missing"
end

scenario "with a conversation starter works" do
s = create(:story, created_at: (Story::RECENT_DAYS + 1).days.ago)
visit "/stories/new?url=#{s.url}"
fill_in "comment", with: <<~COMMENT
Well, everyone knows Custer died at Little Bighorn.
What this book presupposes is... maybe he didn't.
COMMENT
click_button "Submit"

expect(page).to have_content s.title
expect(page).to have_content "maybe he didn't"
end
end

scenario "submitting a banned domain" do
Expand Down
8 changes: 8 additions & 0 deletions spec/requests/stories_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -292,4 +292,12 @@
expect(Vote.where(user: user).count).to eq(0)
end
end

describe "adding suggestions to a story"

describe "user editing an editable story"

describe "user editing a story too late"

describe "mod editing a story"
end

0 comments on commit 23c0c2d

Please sign in to comment.