Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multi-reply feature #2378

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
ed4b809
Move posts/editor and posts/editor_help to writable
redmvc Dec 26, 2024
4ad23aa
Reorder submit_tag properties in replies#_write for standardisation
redmvc Jan 2, 2025
d538f97
Fix local passed to writable/editor
redmvc Jan 2, 2025
bb7edd3
Create multi reply logic for new replies
redmvc Jan 2, 2025
8726ce9
Create get_multi_replies_json before_action
redmvc Jan 2, 2025
60dba60
Remove unused param
redmvc Jan 2, 2025
f2c8aa5
Destroy draft inside add_to_multi_reply
redmvc Jan 2, 2025
0d915c3
Don't show preview unnecessarily
redmvc Jan 2, 2025
58b9a9a
Use try destroy rather than destroy! to delete draft
redmvc Jan 2, 2025
4e54524
Make @multi_replies an instance variable
redmvc Jan 2, 2025
d3c8dbc
Save NPCs when adding replies
redmvc Jan 2, 2025
06b19e3
Allow adding multi replies when editing a reply
redmvc Jan 2, 2025
d8ac6f1
Set global params to char ID when saving NPC
redmvc Jan 3, 2025
98044f2
Set @reply correctly on preview
redmvc Jan 3, 2025
3f7daee
Fix global params being set
redmvc Jan 3, 2025
5f5587d
Add a success flash for replies discarded
redmvc Jan 3, 2025
ef048e3
Add system spec for creating multi replies
redmvc Jan 3, 2025
33cbcf3
Merge branch 'fix/preview-select-characters' into add/multi-reply
redmvc Jan 3, 2025
065ab1e
Add system spec for editing replies using the multi reply editor
redmvc Jan 3, 2025
27759de
Remove unnecessary "and return"
redmvc Jan 3, 2025
7d4741b
Add test for discarding edited multi replies
redmvc Jan 3, 2025
4c911ef
Merge branch 'main' into add/multi-reply
redmvc Jan 3, 2025
0341967
Lint and format
redmvc Jan 3, 2025
8e2b16a
Update app/controllers/replies_controller.rb
redmvc Jan 3, 2025
921ca6b
Revert "Update app/controllers/replies_controller.rb"
redmvc Jan 3, 2025
2462f5f
Update button labels
redmvc Jan 3, 2025
a68e988
Reorganise replies controller
redmvc Jan 3, 2025
e08b2b7
Remove injection from update_all
redmvc Jan 3, 2025
9388e86
Add spec for "Save Previewed" button
redmvc Jan 3, 2025
aac9700
Use more descriptive headers
redmvc Jan 3, 2025
af2a413
Merge branch 'main' into add/multi-reply
redmvc Jan 4, 2025
994bd8a
Sanitise multi reply params when fetching the JSON
redmvc Jan 4, 2025
ccd2de5
Pass original reply duplicate rather than params to edit function
redmvc Jan 4, 2025
236f18d
Update app/controllers/replies_controller.rb
redmvc Jan 4, 2025
c5256c4
Update app/controllers/replies_controller.rb
redmvc Jan 4, 2025
c1a4114
Fetch editor mode diretly from reply added to multi reply
redmvc Jan 4, 2025
479dfc1
Create function to check whether I'm editing a multi reply
redmvc Jan 5, 2025
6652047
Reorganise replies controller for more clarity
redmvc Jan 5, 2025
d1987b6
Extend new reply spec
redmvc Jan 5, 2025
80d59e6
Add TODO for testing errors with the multi reply editor
redmvc Jan 5, 2025
9947167
Fix audits when catching error
redmvc Jan 5, 2025
1562e1b
Add tests for multi reply errors
redmvc Jan 5, 2025
24388da
Fix fetching audits when handling errors
redmvc Jan 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 172 additions & 30 deletions app/controllers/replies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

class RepliesController < WritableController
before_action :login_required, except: [:search, :show, :history]
before_action :get_multi_replies, only: [:create, :update]
before_action :find_model, only: [:show, :history, :edit, :update, :destroy]
before_action :editor_setup, only: [:edit]
before_action :require_create_permission, only: [:create]
Expand Down Expand Up @@ -116,7 +117,24 @@ def create
redirect_to post_path(post_id, page: :unread, anchor: :unread) and return
elsif params[:button_preview]
draft = make_draft
preview(ReplyDraft.reply_from_draft(draft)) and return
preview_reply(ReplyDraft.reply_from_draft(draft)) and return
elsif params[:button_submit_previewed_multi_reply]
if editing_multi_reply?
edit_reply(true)
else
post_replies
end
return
elsif params[:button_discard_multi_reply]
flash[:success] = "Replies discarded."
if editing_multi_reply?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid deeply nested control flow statements.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My brother in Christ if you have a better suggestion I am all ears

# Editing multi reply, going to redirect back to the reply I'm editing
redirect_to reply_path(@reply, anchor: "reply-#{@reply.id}")
else
# Posting a new multi reply, go back to unread
redirect_to post_path(params[:reply][:post_id], page: :unread, anchor: :unread)
end
return
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid too many return statements within this method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried reducing them as much as I could but I don't think I can do better than what I currently have.

end

reply = Reply.new(permitted_params)
Expand All @@ -139,10 +157,10 @@ def create
flash.now[:error] = "This looks like a duplicate. Did you attempt to post this twice? Please resubmit if this was intentional."
@allow_dupe = true
if most_recent_unseen_reply.nil? || (most_recent_unseen_reply.id == last_by_user.id && @unseen_replies.count == 1)
preview(reply)
preview_reply(reply)
else
draft = make_draft(false)
preview(ReplyDraft.reply_from_draft(draft))
preview_reply(ReplyDraft.reply_from_draft(draft))
end
return
end
Expand All @@ -154,20 +172,17 @@ def create
pluraled = num > 1 ? "have been #{num} new replies" : "has been 1 new reply"
flash.now[:error] = "There #{pluraled} since you last viewed this post."
draft = make_draft
preview(ReplyDraft.reply_from_draft(draft)) and return
preview_reply(ReplyDraft.reply_from_draft(draft)) and return
redmvc marked this conversation as resolved.
Show resolved Hide resolved
end
end

begin
reply.save!
rescue ActiveRecord::RecordInvalid => e
render_errors(reply, action: 'created', now: true, err: e)

redirect_to posts_path and return unless reply.post
redirect_to post_path(reply.post)
if params[:button_add_more]
# If they click "Add More", fetch the existing array of multi replies if present and add the current permitted_params to that list
add_to_multi_reply(reply, permitted_params)
elsif editing_multi_reply?
edit_reply(true, new_multi_reply: reply)
else
flash[:success] = "Reply posted."
redirect_to reply_path(reply, anchor: "reply-#{reply.id}")
post_replies(new_reply: reply)
end
end

Expand All @@ -187,26 +202,20 @@ def edit
def update
@reply.assign_attributes(permitted_params)
process_npc(@reply, permitted_character_params)
preview(@reply) and return if params[:button_preview]
if params[:button_preview]
preview_reply(@reply) and return
elsif params[:button_add_more]
# This will take us to reply create instead, but this reply's ID will be saved
add_to_multi_reply(@reply, permitted_params) and return
end

if current_user.id != @reply.user_id && @reply.audit_comment.blank?
flash[:error] = "You must provide a reason for your moderator edit."
editor_setup
render :edit and return
end

begin
@reply.save!
rescue ActiveRecord::RecordInvalid => e
render_errors(@reply, action: 'updated', now: true, err: e)

@audits = { @reply.id => @post.audits.count }
editor_setup
render :edit
else
flash[:success] = "Reply updated."
redirect_to reply_path(@reply, anchor: "reply-#{@reply.id}")
end
edit_reply(false)
end

def destroy
Expand Down Expand Up @@ -297,19 +306,152 @@ def require_edit_permission
redirect_to post_path(@reply.post)
end

def preview(written)
@written = written
@post = @written.post
def get_multi_replies
# Get the multi replies stored in the page JSON
@multi_replies_params = JSON.parse(params.fetch(:multi_replies_json, "[]")).map do |reply_json|
permitted_params(ActionController::Parameters.new({ reply: reply_json }), [:id])
end
@multi_replies = @multi_replies_params.map do |reply_params|
Reply.new(reply_params).tap { |r| r.user = current_user }
end
end

def preview_reply(reply)
# Previewing a specific reply
@post = reply.post
@written = reply
@written.user = current_user unless @written.user
@audits = @written.id.present? ? { @written.id => @written.audits.count } : {}
@reply = @written # So that editor_setup shows the correct characters
preview_replies
end

def add_to_multi_reply(reply, reply_params)
# Adding a new reply to a multi reply
post_id = params[:reply][:post_id]
ReplyDraft.draft_for(post_id, current_user.id)&.destroy!

# Save NPC
if reply.character&.new_record?
if reply.character.save
flash[:success] = "Your new NPC has been persisted!"
params[:reply][:character_id] = reply.character.id
reply_params[:character_id] = reply.character.id
reply.character_id = reply.character.id
else
flash[:error] = "There was a problem persisting your new NPC."
end
end

# Set up editor
@post = reply.post
empty_reply_hash = permitted_params.permit(:character_id, :icon_id, :character_alias_id)
@empty_written = @post.build_new_reply_for(current_user, empty_reply_hash)
@empty_written.editor_mode = reply.editor_mode
@reply = @empty_written # So that editor_setup shows the correct characters
@audits = {}

# Add reply to list of multi replies
reply.user = current_user unless reply.user
@multi_replies << reply
reply_params[:id] = reply.id if reply.id.present?
@multi_replies_params << reply_params

preview_replies
end

def preview_replies
@page_title = @post.subject

@reply = @written # So that editor_setup shows the correct characters
editor_setup
render :preview
end

def post_replies(new_reply: nil)
redmvc marked this conversation as resolved.
Show resolved Hide resolved
redmvc marked this conversation as resolved.
Show resolved Hide resolved
redmvc marked this conversation as resolved.
Show resolved Hide resolved
@multi_replies << new_reply if new_reply.present?

first_reply = @multi_replies.first
begin
Reply.transaction { @multi_replies.each(&:save!) }
rescue ActiveRecord::RecordInvalid => e
errored_reply = @multi_replies.detect { |r| r.errors.present? } || first_reply
render_errors(errored_reply, action: 'created', now: true, err: e)

redirect_to posts_path and return unless errored_reply.post
redirect_to post_path(errored_reply.post) and return
end

if @multi_replies.length == 1
flash[:success] = "Reply posted."
else
flash[:success] = "Replies posted."
end
redirect_to reply_path(first_reply, anchor: "reply-#{first_reply.id}")
end

def editing_multi_reply?
# If the list of params is present and the first item on the list has the ID stored, I am editing it
@multi_replies_params.present? && (@reply = Reply.find_by_id(@multi_replies_params.first["id"]))
end

def edit_reply(editing_multi_reply, new_multi_reply: nil)
redmvc marked this conversation as resolved.
Show resolved Hide resolved
begin
Reply.transaction do
edit_multi_replies(new_multi_reply) if editing_multi_reply

@reply.save!
end
rescue ActiveRecord::RecordInvalid => e
if @multi_replies.blank?
render_errors(@reply, action: 'updated', now: true, err: e)
else
errored_reply = @multi_replies.detect { |r| r.errors.present? } || @reply
render_errors(errored_reply, action: 'updated', now: true, err: e)
end

@audits = { @reply.id => @post.audits.count }
editor_setup
render :edit
else
flash[:success] = "Reply updated."
redirect_to reply_path(@reply, anchor: "reply-#{@reply.id}")
end
end

def edit_multi_replies(new_multi_reply)
# Add new replies after the one being edited and before the next
if new_multi_reply.present?
# Including the reply in the text editor, not just the ones in the JSON
@multi_replies << new_multi_reply
@multi_replies_params << permitted_params
end
original_reply = @reply.dup

# Modify the original reply with the parameters of the first reply in the JSON
@reply.assign_attributes(@multi_replies_params.shift)
@multi_replies.shift

# Check whether there are any further replies beyond the very first one
num_new_replies = @multi_replies_params.length
return if num_new_replies == 0

# Reorder the replies after this one
original_reply_order = @reply.order
following_replies = @reply.post.replies.where("reply_order > ?", original_reply_order)
following_replies.update_all(["reply_order = reply_order + ?", num_new_replies]) # rubocop:disable Rails/SkipsModelValidations

# Create the new replies
@multi_replies_params.each_with_index do |reply_params, idx|
# Create a fake temporary reply with the contents of the original one to be in history
@multi_replies[idx] = new_reply = original_reply.dup
new_reply.order = original_reply_order + idx + 1
new_reply.save!

# Update the new reply added with the actual params that should be there
new_reply.update!(reply_params)
end
end

def make_draft(show_message=true)
if (draft = ReplyDraft.draft_for(params[:reply][:post_id], current_user.id))
draft.assign_attributes(permitted_params)
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/writable_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def process_npc(writable, permitted_character_params)
)
end

def permitted_params(param_hash=nil)
def permitted_params(param_hash=nil, extra_params=[])
(param_hash || params).fetch(:reply, {}).permit(
:post_id,
:content,
Expand All @@ -218,6 +218,7 @@ def permitted_params(param_hash=nil)
:audit_comment,
:character_alias_id,
:editor_mode,
*extra_params,
)
end

Expand Down
4 changes: 2 additions & 2 deletions app/views/posts/_write.haml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
= fields_for :character, post.character do |cf|
= cf.hidden_field :name, id: 'character_name'
= cf.hidden_field :npc, id: 'character_npc'
= render 'posts/editor_help', editor_location: 'post'
= render 'writable/editor_help', editor_location: 'post'
- content_for :buttons do
- if post.id.present? && !post.editable_by?(current_user)
= submit_tag 'Save', class: 'button', id: 'submit_button', data: { disable_with: 'Saving...' }
Expand Down Expand Up @@ -90,4 +90,4 @@
#post-editor.padding-10= yield :form
.subber.centered= yield :buttons
- else
= render 'editor', item: post
= render 'writable/editor', writable: post
27 changes: 18 additions & 9 deletions app/views/replies/_write.haml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
-# locals: ( reply: )
-# locals: ( reply:, multi_replies_params: nil, multi_reply_new: false )

- content_for :form do
= form_for reply, html: { id: 'post_form' } do |f|
Expand All @@ -10,6 +10,8 @@
%br
= unread_warning.try(:html_safe)
%br
- unless multi_replies_params.nil?
= hidden_field_tag :multi_replies_json, multi_replies_params.to_json
= f.hidden_field :post_id
= f.hidden_field :character_id
= f.hidden_field :icon_id
Expand All @@ -28,12 +30,19 @@
%br
= f.text_area :audit_comment, placeholder: 'Explain reason for moderation here', class: 'mod'
%br
= submit_tag (reply.new_record? ? 'Post' : 'Save'), class: 'button', id: 'submit_button', data: { disable_with: 'Saving...' }
= submit_tag "Preview", class: 'button', id: 'preview_button', name: 'button_preview', data: { disable_with: 'Previewing...' }
- if reply.new_record?
= submit_tag "Save Draft", class: 'button', id: 'draft_button', data: { disable_with: 'Drafting...' }, name: 'button_draft'
- if @draft.present?
= submit_tag "Delete Draft", class: 'button', id: 'delete_draft_button', data: { disable_with: 'Deleting...', confirm: "Delete draft?" }, name: 'button_delete_draft'
- if multi_replies_params.blank?
= submit_tag (reply.new_record? ? 'Post' : 'Save'), class: 'button', id: 'submit_button', data: { disable_with: 'Saving...' }
= submit_tag "Preview", class: 'button', id: 'preview_button', name: 'button_preview', data: { disable_with: 'Previewing...' }
- if reply.new_record?
= submit_tag "Save Draft", class: 'button', id: 'draft_button', name: 'button_draft', data: { disable_with: 'Drafting...' }
- if @draft.present?
= submit_tag "Delete Draft", class: 'button', id: 'delete_draft_button', name: 'button_delete_draft', data: { disable_with: 'Deleting...', confirm: "Delete draft?" }
- else
= submit_tag (multi_reply_new ? 'Post All' : 'Save All'), class: 'button', id: 'submit_button', data: { disable_with: 'Saving...' }
= submit_tag "Preview Current", class: 'button', id: 'preview_button', name: 'button_preview', data: { disable_with: 'Previewing...' }
= submit_tag "Add More Replies", class: 'button', id: 'add_more_button', name: 'button_add_more', data: { disable_with: 'Adding...' }
- if multi_replies_params.present?
= submit_tag "Discard Replies", class: 'button', id: 'discard_multi_reply_button', name: 'button_discard_multi_reply', data: { disable_with: 'Discarding...', confirm: "Discard all replies added?" }
- if reply.post.author_for(current_user).present?
%hr.clear
.post-note-editor
Expand Down Expand Up @@ -61,6 +70,6 @@
%p What would you like to do with your unsaved changes?
- else
%br
= render 'posts/editor_help', editor_location: 'reply'
= render 'writable/editor_help', editor_location: 'reply'

= render 'posts/editor', item: reply
= render 'writable/editor', writable: reply
30 changes: 25 additions & 5 deletions app/views/replies/preview.haml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,28 @@
.post-ender ... and #{@unseen_replies.total_entries - @unseen_replies.per_page} more ...
%br

.content-header= @post.subject
- if @post.description.present?
.post-subheader= sanitize_simple_link_text(@post.description)
= render 'replies/single', reply: @written, hide_footer: true
= render 'write', reply: @written
- if @multi_replies.present?
- multi_reply_new = @multi_replies_params.first["id"].blank?
- if multi_reply_new
.content-header Adding multiple replies
- else
.content-header Editing reply and adding more
- @multi_replies.each do |reply|
= render 'replies/single', reply: reply, hide_footer: true
= form_for @written || @empty_written, html: { id: 'post_form' } do
= hidden_field_tag :multi_replies_json, @multi_replies_params.to_json
= submit_tag (multi_reply_new ? 'Post Previewed' : 'Save Previewed'), class: 'button', id: 'submit_previewed_multi_reply_button', name: 'button_submit_previewed_multi_reply', data: { disable_with: 'Saving...', confirm: 'The reply currently in the editor will not be posted. Continue?' }
%br
%br
- else
- multi_reply_new = false

- if @written.present?
- if @multi_replies.present?
.content-header Previewing
- else
.content-header= @post.subject
- if @post.description.present?
.post-subheader= sanitize_simple_link_text(@post.description)
= render 'replies/single', reply: @written, hide_footer: true
= render 'write', reply: @written || @empty_written, multi_replies_params: @multi_replies_params, multi_reply_new: multi_reply_new
2 changes: 1 addition & 1 deletion app/views/users/profile_edit.haml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
= f.hidden_field :profile_editor_mode, id: 'profile_editor_mode'
#post-form-wrapper.no-margin-left
= f.text_area :profile, class: 'tinymce'
= render 'posts/editor_help', editor_location: 'profile'
= render 'writable/editor_help', editor_location: 'profile'
%tr
%th.sub.vtop= f.label :moiety, "Moiety hex"
%td{class: cycle('even', 'odd')}
Expand Down
Loading
Loading