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

Already on GitHub? Sign in to your account

AO3-4862 Extending work controller test coverage #3165

Open
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

hatal175 commented Nov 17, 2017

Issue

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

Purpose

This PR extends the test coverage for works controller.

I've also made a few changes for this purpose and I would really appreciate a review on them:

  1. I've added a route for show_multiple under pseuds so you can see and edit only the works under that pseud.
  2. I've deleted a code path in preview_mode that could not really be reached in current code base.
  3. Deleted unused function tag_list
  4. various bug fixes in previously uncovered code.

Testing

Run tests and see coverage improvement.

Credit

Tal Hayon

Please use he.

hatal175 added some commits Nov 13, 2017

AO3-4855 Started working on extending works controller tests
Also implemented show multiple for pseuds
AO3-4862 Extended work controller test coverage
- Added edit multiple works for pseud
- Removed several dead code paths
@@ -3,7 +3,7 @@
class WorksController < ApplicationController
# only registered users and NOT admin should be able to create new works
before_action :load_collection
- before_action :load_owner, only: [:index]
+ before_action :load_owner, only: [:index, :show_multiple]
@houndci-bot

houndci-bot Nov 17, 2017

Use %i or %I for an array of symbols.

@@ -298,7 +298,7 @@ def create
@work.ip_address = request.remote_ip
# If Edit or Cancel is pressed, bail out and display relevant form
if params[:edit_button]
- render :new && return
+ render :new and return
@houndci-bot

houndci-bot Nov 17, 2017

Use && instead of and.

@@ -693,8 +687,8 @@ def post_draft
def show_multiple
@user = current_user
- if params[:pseud_id]
- @works = Work.joins(:pseuds).where(pseud_id: params[:pseud_id])
+ if @pseud
@houndci-bot

houndci-bot Nov 17, 2017

Use the return of the conditional for variable assignment and comparison.

@@ -1011,21 +992,13 @@ def log_admin_activity
# what potential values `saved` has as used elsewhere (which is what is
# passed as `condition`) and thus the usual approach of condition=nil
# followed by a ||= cannot be reliably used. -@duckinator
- def preview_mode(page_name, condition = (@work.has_required_tags? && @work.invalid_tags.blank?))
+ def preview_mode(page_name, condition = (@work.invalid_tags.blank?))
@houndci-bot

houndci-bot Nov 17, 2017

Unused method argument - page_name. If it's necessary, use _ or _page_name as an argument name to indicate that it won't be used.
Don't use parentheses around a method call.

@@ -174,6 +174,42 @@ def call_with_params(params)
end
end
+ describe "edit" do
+ let(:user) { create(:user) }
+ let(:work) {
@houndci-bot

houndci-bot Nov 17, 2017

Avoid using {...} for multi-line blocks.

+
+ context "destroy" do
+ let(:user) { create(:user) }
+ let!(:work) {
@houndci-bot

houndci-bot Nov 17, 2017

Avoid using {...} for multi-line blocks.

- let(:update_work) {
+ let!(:update_user) { create(:user) }
+ let!(:update_chapter) { create(:chapter) }
+ let!(:update_work) {
@houndci-bot

houndci-bot Nov 17, 2017

Avoid using {...} for multi-line blocks.

+ end
+
+ it "should show results" do
+ get :collected, params: { user_id: collected_user.login, work_search: { query: "fandom_ids:#{collected_fandom.id}" }}
@houndci-bot

houndci-bot Nov 17, 2017

Space inside } missing.

+ it "should throw error when there are invalid tags and trying to preview" do
+ allow_any_instance_of(Work).to receive(:invalid_tags).and_return([create(:unsorted_tag)])
+
+ expect {patch :update_tags, params: { id: work, preview_button: true } }.to raise_error UncaughtThrowError
@houndci-bot

houndci-bot Nov 17, 2017

Space missing inside {.

@@ -134,6 +134,28 @@
end
end
+ context 'when work parameters are invalid' do
+ let(:work_params) {
@houndci-bot

houndci-bot Nov 17, 2017

Avoid using {...} for multi-line blocks.

+ context 'when work parameters are invalid' do
+ let(:work_params) {
+ {
+ work: {
@houndci-bot

houndci-bot Nov 17, 2017

Use 2 spaces for indentation in a hash, relative to the start of the line where the left curly brace is.

+ let(:work_params) {
+ {
+ work: {
+ summary: "a" * (ArchiveConfig.SUMMARY_MAX + 1),
@houndci-bot

houndci-bot Nov 17, 2017

Use 2 spaces for indentation in a hash, relative to the start of the line where the left curly brace is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment