AO3-5603 Revise WorksController to eliminate set_instance_variables.

[tickinginstant]

Issue

https://otwarchive.atlassian.net/browse/AO3-5603

Purpose

1. I overhauled the WorksController and the ChaptersController to try to eliminate the function set_instance_variables.
   • I moved the code to build a blank item into #new, the code to build an item from params into #create, and the code to modify an existing item into #update.
   • I moved the instance variable assignment into load_pseuds (for the ChaptersController) and set_work_form_fields (for the WorksController).
   • I did a similar thing for WorksController#set_instance_variables_tags.
2. In the process, I cleaned up the before_action filters a little for both controllers.
3. I also refactored several of the functions that I was modifying, to make it easier to follow the flow of control.
   • I moved the check for params[:cancel_button] to the very beginning of the function, before assigning to attributes. Because some attribute assignment is written to the database immediately (e.g. tags), I figured that that was contrary to the purpose of the button.
   • To further simplify the WorksController code, I also fixed up the work function set_challenge_claim_info. This means that it can be used alongside set_challenge_info (which works only for the work's assignments).
   • I also changed Work#check_for_invalid_tags into a validation. (It used to be a before_save callback that aborted in case of error, which meant that it would only display an error to the user if Work#save was called. Hence the complicated code in the WorksController#preview_mode function to try to catch invalid tags on preview.)
4. I added a new function WorksController#work_tag_params to make sure that it only allows the tags (and the language) to be changed, and not any of the other fields that are available in WorksController#work_params.
5. I also rearranged a few functions in the WorksController to make sure that more of the utility functions were protected, instead of mixed in alongside the public actions.

Testing

See Slack. In addition, it would be good to check that the refactored code behaves as expected.

[sarken]

Just a couple of initial comments/questions, no rush -- I still need to look at the works_controller more thoroughly.

[sarken]

The double !! is throwing me off here -- could you translate this from code to English for me?

[tickinginstant]

It's basically a type-conversion to ensure that @work.preview_mode is true or false instead of "Preview", "Edit", "Cancel", or nil. A single ! acts as negation and forces the results to be either true or false, so a double-negation cancels out the negation and just converts to a boolean. I'm pretty sure it would work without the double negation -- I only added it because at one point I tried to print out @work.preview_mode and found the fact that it held a string confusing.

[redsummernight]

This method was never called before, so having it now would likely fix AO3-3556 as well, we'll see.

---

Notes for me: @sarken said we handle prompter notification at:

otwarchive/lib/creatable.rb

Lines 100 to 109 in bd4f323

	# notify prompters of response to their prompt
	def notify_prompters
	if !self.challenge_claims.empty? && !self.unrevealed?
	if self.collections.first.nil?
	UserMailer.prompter_notification(self.id,).deliver
	else
	UserMailer.prompter_notification(self.id, self.collections.first.id).deliver
	end
	end
	end

and also (for unrevealed works specifically):

otwarchive/app/models/collection_item.rb

Lines 310 to 313 in 5954492

	# also notify prompters of responses to their prompt
	if item_type == "Work" && !item.challenge_claims.blank?
	UserMailer.prompter_notification(self.item.id, self.collection.id).deliver
	end

[redsummernight]

Looks good to me, though I'll leave the green label to the next reviewer.

[sarken]

Neglected to put my green check on it! Like red said, reviewer number 3 can flip the label.