/otwarchive

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-5133 Rails BOT: Use first_chapter when posting a work and its first chapter #3022

Merged
merged 11 commits into from Sep 1, 2017

Conversation

Reviewers

@houndci-bot houndci-bot

@ariana-paris ariana-paris

Assignees
No one assigned
Labels
Priority: Broken on Test (High) Reviewed: Ready to Merge
Projects
None yet
Milestone
No milestone
3 participants
@sarken @ariana-paris @houndci-bot
@sarken
Owner

sarken commented Aug 30, 2017

Issue

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

Purpose

If you imported a multi-chapter work as a draft and tried to use the Post Draft option on your drafts page, the first chapter would not be posted, making the work inaccessible to anyone other than you. This is because importing URLs as chapters creates the chapters in an unexpected order, and it was interacting badly with the post_first_chapter method.

For example, here is an unposted work:

irb(main):001:0> draft = Work.find(1677558)
  Work Load (0.3ms)  SELECT  `works`.* FROM `works` WHERE `works`.`id` = 1677558 LIMIT 1
  ↳ (irb):1:in `irb_binding'
=> #<Work id: 1677558, expected_number_of_chapters: 2, created_at: "2017-08-30 08:57:36", updated_at: "2017-08-30 08:57:36", major_version: 1, minor_version: 0, posted: false, language_id: 1, restricted: false, title: "Rails 5.1 Chaptered (DW)", summary: "", notes: "<p>My note</p>", word_count: 633, hidden_by_admin: false, delta: false, revised_at: "2017-08-29 04:00:00", authors_to_sort_on: "testy", title_to_sort_on: "rails 5.1 chaptered (dw)", backdate: true, endnotes: nil, imported_from_url: "https://ao3testing.dreamwidth.org/2460.html", hit_count_old: 0, last_visitor_old: nil, complete: false, summary_sanitizer_version: 1, notes_sanitizer_version: 1, endnotes_sanitizer_version: 0, work_skin_id: nil, in_anon_collection: false, in_unrevealed_collection: false, anon_commenting_disabled: false, ip_address: nil, spam: false, spam_checked_at: nil, moderated_commenting_enabled: false>

And here are its chapters:

irb(main):002:0> draft.chapters
  Chapter Load (0.4ms)  SELECT  `chapters`.* FROM `chapters` WHERE `chapters`.`work_id` = 1677558 LIMIT 11
=> #<ActiveRecord::Associations::CollectionProxy [
#<Chapter id: 3570820, content: "\n<p></p><div class=\"inner\">\n  <p></p>\n  <div class...", position: 2, work_id: 1677558, created_at: "2017-08-30 08:57:36", updated_at: "2017-08-30 08:57:37", posted: true, title: "Rails 5.1 Chaptered (DW)", notes: "<p>My note on chapter 2.</p>", summary: nil, word_count: 317, hidden_by_admin: false, published_at: "2017-08-29", endnotes: nil, content_sanitizer_version: 1, notes_sanitizer_version: 1, summary_sanitizer_version: 0, endnotes_sanitizer_version: 0>,
#<Chapter id: 3570821, content: "\n<p></p><div class=\"inner\">\n  <p></p>\n  <div class...", position: 1, work_id: 1677558, created_at: "2017-08-30 08:57:37", updated_at: "2017-08-30 08:57:37", posted: false, title: nil, notes: nil, summary: nil, word_count: 316, hidden_by_admin: false, published_at: "2017-08-29", endnotes: nil, content_sanitizer_version: 1, notes_sanitizer_version: 0, summary_sanitizer_version: 0, endnotes_sanitizer_version: 0>]>

Notice that the chapter with position: 1 is actually the second chapter if you order by id.

This led to problems with the post_first_chapter method in the work model, which used self.chapters.first to determine the first chapter. Switching to self.first_chapter means we'll now use self.chapters.first on new records, but that we'll check the chapter positions on existing drafts to make sure we're getting the chapter that will display first on the work.

Testing

Refer to JIRA

sarken added some commits Aug 30, 2017

@sarken
AO3-5133 Test importing multi-chapter works and posting from drafts page
a7dc510
@sarken
AO3-5133 Make 'Post Draft' button post all chapters of multi-chapter … 
…works
eb36187
@sarken
AO3-5133 Undo hack-y fix in the controller
6bdc102
@sarken
AO3-5133 Use first_chapter rather than chapters.first in post_first_c… 
…hapter -- and important distinction because first_chapter looks at the chapter position on works that are not new records (i.e. drafts)

Some checks were not successful

1 failing and 3 successful checks
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
Scrutinizer — No new issues
Details
codeclimate — All good!
Details
hound — 9 violations found.
5169809

@sarken sarken added Awaiting review Priority: Broken on Test (High) labels Aug 30, 2017

app/models/work.rb
- self.chapters.first.published_at = Date.today unless self.backdate
- self.chapters.first.posted = self.posted
- self.chapters.first.save
+ if self.saved_change_to_posted? || (self.first_chapter && self.first_chapter.posted != self.posted)
@houndci-bot

houndci-bot Aug 30, 2017

Redundant self detected.

app/models/work.rb
- self.chapters.first.posted = self.posted
- self.chapters.first.save
+ if self.saved_change_to_posted? || (self.first_chapter && self.first_chapter.posted != self.posted)
+ self.first_chapter.published_at = Date.today unless self.backdate
@houndci-bot

houndci-bot Aug 30, 2017

Redundant self detected.

app/models/work.rb
- self.chapters.first.save
+ if self.saved_change_to_posted? || (self.first_chapter && self.first_chapter.posted != self.posted)
+ self.first_chapter.published_at = Date.today unless self.backdate
+ self.first_chapter.posted = self.posted
@houndci-bot

houndci-bot Aug 30, 2017

Redundant self detected.

app/models/work.rb
+ if self.saved_change_to_posted? || (self.first_chapter && self.first_chapter.posted != self.posted)
+ self.first_chapter.published_at = Date.today unless self.backdate
+ self.first_chapter.posted = self.posted
+ self.first_chapter.save
@houndci-bot

houndci-bot Aug 30, 2017

Redundant self detected.

@sarken sarken closed this Aug 30, 2017

sarken added some commits Aug 30, 2017

@sarken
AO3-5133 Fix spec failures I believe are the result of the chapter no… 
…t being explicitly posted on a posted work
645c6f6
@sarken
AO3-5133 Make sure posted works have posted first chapters
895f7e0
@sarken
AO3-5133 Restore the posted_work factory to its old form
4edfc59
@sarken
AO3-5133 work_import.feature needs a newline at the end
caf1826
@sarken
AO3-5133 Add a feature test to make sure the Post Draft option on a u… 
…ser's Drafts page continues to post only the first chapter of non-imported works
2e9f58e
@sarken
AO3-5133 Add some chapter specs to make sure chapters are created cor… 
…rectly, and add work specs to make sure the posted_work factory is producing a posted chapter
1fe2469
@sarken
AO3-5133 Assign self.first_chapter to a variable

Some checks haven’t completed yet

1 pending and 2 successful checks
codeclimate — Code Climate is analyzing this code.
Details
Scrutinizer — 5 new issues
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
115ee6e

@sarken sarken reopened this Aug 31, 2017

spec/models/chapter_spec.rb
@@ -3,6 +3,19 @@
describe Chapter do
+ it "has a valid factory" do
+ expect(build(:chapter)).to be_valid
@houndci-bot

houndci-bot Aug 31, 2017

Extra empty line detected at block body beginning.

spec/models/chapter_spec.rb
+
+ it "is unposted by default" do
+ chapter = create(:chapter)
+ chapter.posted.should == false
@houndci-bot

houndci-bot Aug 31, 2017

Extra empty line detected at block body end.

spec/models/chapter_spec.rb
+ chapter = create(:chapter)
+ chapter.posted.should == false
+ end
+
@houndci-bot

houndci-bot Aug 31, 2017

Extra empty line detected at block body end.

spec/models/work_spec.rb
@@ -7,6 +7,13 @@
expect(create(:work)).to be_valid
end
+ context "when posted" do
+ it "posts the first chapter" do
+ work = create(:posted_work)
@houndci-bot

houndci-bot Aug 31, 2017

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

spec/models/work_spec.rb
+ work = create(:posted_work)
+ work.first_chapter.posted.should == true
+ end
+ end
@houndci-bot

houndci-bot Aug 31, 2017

Space missing to the left of {.

@ariana-paris

ariana-paris approved these changes Aug 31, 2017
View changes

Looks good

@sarken sarken added Reviewed: Ready to Merge and removed Awaiting review labels Sep 1, 2017

@sarken sarken merged commit bcf6e10 into otwcode:master Sep 1, 2017
2 of 3 checks passed

2 of 3 checks passed

codeclimate Code Climate is analyzing this code.
Details
Scrutinizer 5 new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sarken
Owner

sarken commented Sep 1, 2017

(Ariana reviewed, I just merged)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment