Skip to content

AO3-5717 Fix expected chapter count lower than actual count#4108

Merged
sarken merged 5 commits into
otwcode:masterfrom
luis-pabon-tf:AO3-5717_wip_length
Feb 8, 2022
Merged

AO3-5717 Fix expected chapter count lower than actual count#4108
sarken merged 5 commits into
otwcode:masterfrom
luis-pabon-tf:AO3-5717_wip_length

Conversation

@luis-pabon-tf

Copy link
Copy Markdown
Contributor

Pull Request Checklist

Issue

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

Purpose

Uses an existing virtual attribute so that the chapter count safeguards happen even if the chapter count provided is wrong.

Testing Instructions

In the JIRA ticket.

Credit

What name and pronouns should we use to credit you in the Archive of Our Own's Release Notes?

bird - (they)

Use wip_length instead of expected_number_of_chapters as the hidden field on work editing.
@redsummernight redsummernight changed the title AO3-5717 AO3-5717 Fix expected chapter count lower than actual count Dec 3, 2021

@redsummernight redsummernight left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add a test for this behavior of wip_length? In spec/models/work_spec.rb, we can have a block for describe "#wip_length=" and check that setting the attribute to e.g. 2 for a work with 1 chapter will end up with both expected_number_of_chapters and wip_length at 1.

Added test case to replicate the bug and verify the solution
Add space for style requirements
Comment thread spec/models/work_spec.rb Outdated
Comment thread spec/models/work_spec.rb Outdated
@tickinginstant

Copy link
Copy Markdown
Contributor

Would you mind removing expected_number_of_chapters from the work_params function in WorksController, as well?

def work_params
params.require(:work).permit(
:rating_string, :fandom_string, :relationship_string, :character_string,
:archive_warning_string, :category_string, :expected_number_of_chapters,

Otherwise someone would still be able to manipulate the HTML to get an invalid chapter count.

Removed expected_number_of_chapters from allowed params on works controller
Removed test that wasn't actually needed and renamed test

@redsummernight redsummernight left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks!

@sarken sarken merged commit bb2be99 into otwcode:master Feb 8, 2022
@luis-pabon-tf luis-pabon-tf deleted the AO3-5717_wip_length branch May 14, 2026 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants