Divide comments_controller_spec.rb into several smaller files
Description
The comments_controller_spec.rb file is about 1000 lines long and growing. It needs to be broken down so it’s more manageable.
Testing
Nothing to test.
Notes
The original approach proposed in this issue was:
Specs related to marking a comment as spam or ham in one file
Specs related to reviewing moderated comments in another file
Probably something else
It lead to the following conversation, which may be useful in determining an approach:
sarken 22:50 I'm not sure what other sort of logical division exists with these remaining sections: GET #add_comment_reply" POST #new" POST #create" GET #show_comments" GET #hide_comments" GET #add_comment" GET #cancel_comment GET #cancel_comment_reply GET #cancel_comment_delete GET #cancel_comment_edit GET #destroy GET #show GET #index shared_examples "no one can add or edit comments" context "on a work hidden by an admin" context "on an unrevealed work" new, create, destroy, show, and index can go in one called default_rails_actions_spec.rb, like we have for works, but the others...
22:54 misc_actions_spec.rb, aka "where every single thing will end up in the future, like a junk drawer" or "where no one will think to look for the thing they want"
redsummernight 23:00 we could break the comment specs apart by commentable types at the file level, with each file covering some/all actions
sarken 23:01 Why do you hate me, red? :P
redsummernight 23:02 I've been trying to convert the comment requests spec (covering restricted works) to controller specs, and struggling with organization as well, yeah
sarken 23:04 It's a good idea; it just happens that "being able to see what goes where in here" is the thing standing between me and a pull request 23:05 But if you don't mind shuffling my new stuff around too, I'm game with just... making a best guess 23:05 And letting you have some reorganizing fun at a later date 23:06 (Or, if you're almost done, waiting for you to finish)
redsummernight 23:09 I scrapped my previous attempts; but looking at the specs we do have, a tentative breakdown could be like: standard actions, logged in users' comments on basic/non-special works, super common stuff (show/add/delete) same commentable type as the first, but covering moderation actions commentable = hidden/unrevealed works (similar shared examples) commentable = restricted works (guest vs non-guest) commentable = other comments (here's where we test more complex threading, replying) commentable = other comments (but for freezing this time)
sarken 23:19 That might work, although other comments also needs to be broken down by ultimate parent 23:20 I've got tests for the freeze action on admin posts, tags, and works, because it behaves differently depending on the ultimate parent 23:22 Then I got lazy didn't test the complex scenarios, e.g. "freeze something in the middle of a thread," on each ultimate parent type 23:23 It occurs to me I also need to test deleting, editing, and marking them as spam (edited) 23:23 And probably freezing comments that are unreviewed 23:23 So, okay, way less done than I thought
redsummernight 23:24 you can start on a new file without refactoring the existing stuff - that's where we're headed anyway
redsummernight 23:27 does freezing work the same for all ultimate parent types / can you get away with some set of shared examples? 23:30 (one set, not some)
sarken 23:30 Different permissions, e.g. only PAC admins and work creators can freeze threads on works, only Tag Wrangling on tags, plus the complicating factor of regular users not being able to access tag comments, so you get a different behavior when you try to freeze a comment on a tag than when you try to freeze a comment on a work
sarken 23:35 For example, if you're a user: you get bounced to your dash with a "you don't have permission to access the page you were trying to reach" error for tags because the :check_tag_wrangler_access filter catches you before you get to the :check_permission_to_freeze filter ever has a chance to run you get bounced to the ultimate parent with a "sorry, you don't have permission to freeze that comment thread" error for works, because you do make it to the :check_permission_to_freeze filter
redsummernight 23:43 hmm yeah this is why it's so annoying to group/reuse these tests, there are 3 ways they can differ: which commentable, which current user, what expected behavior (like different error messages for guest vs non-guest) 23:43 and since we divide the tests by action instead of those ways, we end up having to repeat combinations of 3 everywhere...
sarken 23:50 Not just commentable, but parent 23:50 So 4 ways they can differ
The comments_controller_spec.rb file is about 1000 lines long and growing. It needs to be broken down so it’s more manageable.
Testing
Nothing to test.
Notes
The original approach proposed in this issue was:
Specs related to marking a comment as spam or ham in one file
Specs related to reviewing moderated comments in another file
Probably something else
It lead to the following conversation, which may be useful in determining an approach: