Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid creating a new empty talk page when removing a template from a non-existent page #5518

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions lib/wiki_assignment_output.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@ def build_talk_page_update
# Do not post templates to disambugation pages
return nil if includes_disambiguation_template?(initial_page_content)

# We only want to add assignment tags to non-existant talk pages if the
# We only want to add assignment tags to non-existent talk pages if the
# article page actually exists, and is not a disambiguation page.
article_content = WikiApi.new(@wiki).get_page_content(@title)
return nil if article_content.blank?
return nil if includes_disambiguation_template?(article_content)
return nil unless normal_article?

# We only want to remove assignments if talk page already exists.
# This is to avoid creating a new empty talk page.
return nil if @assignments.empty? && initial_page_content.empty?

page_content = build_assignment_page_content(assignments_tag, initial_page_content)
page_content
Expand All @@ -52,6 +54,12 @@ def build_talk_page_update
###################
# Helper methods #
###################
# Normal article means the article page actually exists, and is not a disambiguation page.
def normal_article?
article_content = WikiApi.new(@wiki).get_page_content(@title)
article_content.present? && !includes_disambiguation_template?(article_content)
Copy link
Member Author

Choose a reason for hiding this comment

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

Another option would be to use the negation of or operator, but I feel like and operator is usually easier to understand:
!(article_content.blank? || includes_disambiguation_template?(article_content))

Copy link
Member

Choose a reason for hiding this comment

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

Your implementation looks good to me.

end

def assignments_tag
return '' if @assignments.empty?

Expand Down Expand Up @@ -84,7 +92,7 @@ def assignments_tag
def build_assignment_page_content(new_tag, page_content)
page_content = page_content.dup.force_encoding('utf-8')
# Return if tag already exists on page.
# However, if the tag is empty, that means to blank the prior tag (if any).z
# However, if the tag is empty, that means to blank the prior tag (if any).
if new_tag.present?
return nil if page_content.include? new_tag
end
Expand Down
23 changes: 18 additions & 5 deletions spec/lib/wiki_assignment_output_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -264,13 +264,26 @@
let(:title) { 'Selfie' }
let(:talk_title) { 'Talk:Selfie' }

context 'when the article exists but talk page does not' do
context 'when the article exists but talk page does not,' do
let(:talk_title) { 'Talk:THIS PAGE DOES NOT EXIST' }

it 'returns content' do
VCR.use_cassette 'wiki_edits/talk_page_update' do
page_content = wiki_assignment_output.build_talk_page_update
expect(page_content).to include('{{dashboard.wikiedu.org assignment | course = ')
context 'adding or updating assignments' do
it 'returns content' do
VCR.use_cassette 'wiki_edits/talk_page_update' do
page_content = wiki_assignment_output.build_talk_page_update
expect(page_content).to include('{{dashboard.wikiedu.org assignment | course = ')
end
end
end

context 'trying to remove assignments' do
let(:assignments) { [] }

it 'returns nil' do
VCR.use_cassette 'wiki_edits/talk_page_update' do
page_content = wiki_assignment_output.build_talk_page_update
expect(page_content).to be_nil
end
end
end
end
Expand Down
Loading