jenkins-bot has submitted this change and it was merged.
Change subject: Fix history permission check in RevisionFormatter
......................................................................
Fix history permission check in RevisionFormatter
* Add suppress browser tests
* DataManager.rb to simplify creation and
access of unique data.
* Add root-permission to history to hide
comments on suppressed topic/post.
Bug: T96933
Bug: T69610
Change-Id: I3e7497d162e77ddc4c0c5aba1725a3290353fbcf
---
M FlowActions.php
M includes/Block/BoardHistory.php
M includes/Block/Topic.php
M includes/Formatter/RevisionFormatter.php
M tests/browser/features/moderation.feature
M tests/browser/features/step_definitions/flow_steps.rb
M tests/browser/features/step_definitions/moderation_steps.rb
A tests/browser/features/step_definitions/suppress_steps.rb
A tests/browser/features/support/data_manager.rb
M tests/browser/features/support/hooks.rb
A tests/browser/features/support/pages/board_history_page.rb
M tests/browser/features/support/pages/flow_page.rb
A tests/browser/features/support/pages/topic_history_page.rb
A tests/browser/features/support/pages/wiki_page.rb
A tests/browser/features/suppress.feature
15 files changed, 256 insertions(+), 45 deletions(-)
Approvals:
Mattflaschen: Looks good to me, approved
jenkins-bot: Verified
diff --git a/FlowActions.php b/FlowActions.php
index 3e5e5bb..1ccbb16 100644
--- a/FlowActions.php
+++ b/FlowActions.php
@@ -731,6 +731,12 @@
PostRevision::MODERATED_DELETED => '',
PostRevision::MODERATED_SUPPRESSED => 'flow-suppress',
),
+ 'root-permissions' => array(
+ PostRevision::MODERATED_NONE => '',
+ PostRevision::MODERATED_HIDDEN => '',
+ PostRevision::MODERATED_LOCKED => '',
+ PostRevision::MODERATED_DELETED => '',
+ ),
'history' => array(), // views don't generate history
'handler-class' => 'Flow\Actions\FlowAction',
),
diff --git a/includes/Block/BoardHistory.php b/includes/Block/BoardHistory.php
index 2ff81ca..333cd66 100644
--- a/includes/Block/BoardHistory.php
+++ b/includes/Block/BoardHistory.php
@@ -63,7 +63,7 @@
$revisions = array();
foreach ( $history as $row ) {
- $serialized = $formatter->formatApi( $row,
$this->context );
+ $serialized = $formatter->formatApi( $row,
$this->context, 'history' );
if ( $serialized ) {
$revisions[$serialized['revisionId']] =
$serialized;
}
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index 8e2b4c6..1b02d8c 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -468,7 +468,7 @@
public function renderApi( array $options ) {
$output = array( 'type' => $this->getName() );
- $topic = $this->loadTopicTitle( $this->action === 'history' ?
'history' : 'view' );
+ $topic = $this->loadTopicTitle();
if ( !$topic ) {
return $output + $this->finalizeApiOutput($options);
}
@@ -733,7 +733,7 @@
$revisions = array();
foreach ( $history as $row ) {
- $serialized = $serializer->formatApi( $row,
$this->context );
+ $serialized = $serializer->formatApi( $row,
$this->context, 'history' );
// if the user is not allowed to see this row it will
return empty
if ( $serialized ) {
$revisions[$serialized['revisionId']] =
$serialized;
diff --git a/includes/Formatter/RevisionFormatter.php
b/includes/Formatter/RevisionFormatter.php
index 8bf995c..f33871d 100644
--- a/includes/Formatter/RevisionFormatter.php
+++ b/includes/Formatter/RevisionFormatter.php
@@ -160,9 +160,10 @@
/**
* @param FormatterRow $row
* @param IContextSource $ctx
+ * @param string $action action from FlowActions
* @return array|false
*/
- public function formatApi( FormatterRow $row, IContextSource $ctx ) {
+ public function formatApi( FormatterRow $row, IContextSource $ctx,
$action = 'view' ) {
$user = $ctx->getUser();
// @todo the only permissions currently checked in this class
are prev-revision
// mostly permissions is used for the actions, figure out how
permissions should
@@ -172,11 +173,8 @@
return false;
}
- $isContentAllowed = $this->includeContent &&
$this->permissions->isAllowed( $row->revision, 'view' );
- $isHistoryAllowed = $this->permissions->isAllowed(
$row->revision, 'history' );
-
- if ( !$isHistoryAllowed ) {
- return array();
+ if ( !$this->permissions->isAllowed( $row->revision, $action )
) {
+ return false;
}
$moderatedRevision = $this->templating->getModeratedRevision(
$row->revision );
@@ -234,7 +232,7 @@
);
}
- if ( $isContentAllowed ) {
+ if ( $this->includeContent ) {
// topic titles are always forced to plain text
$contentFormat = $this->decideContentFormat(
$row->revision );
diff --git a/tests/browser/features/moderation.feature
b/tests/browser/features/moderation.feature
index 7718e1a..b3f0260 100644
--- a/tests/browser/features/moderation.feature
+++ b/tests/browser/features/moderation.feature
@@ -16,15 +16,6 @@
And I click Delete topic
Then the top post should be marked as deleted
- Scenario: Suppressing a topic
- Given I have created a Flow topic with title "Suppressmeifyoudare"
- When I hover on the Topic Actions link
- And I click the Suppress topic button
- And I see a dialog box
- And I give reason for suppression as being "Quelling the peasants"
- And I click Suppress topic
- Then the top post should be marked as suppressed
-
Scenario: Cancelling a dialog without text
Given I have created a Flow topic with title "Testing cancel deletion of
topic"
When I hover on the Topic Actions link
@@ -38,7 +29,7 @@
When I hover on the Topic Actions link
And I click the Delete topic button
And I see a dialog box
- And I give reason for suppression as being "About to change my mind"
+ And I give reason for deletion as being "About to change my mind"
And I cancel the dialog
And I confirm
Then I do not see the dialog box
diff --git a/tests/browser/features/step_definitions/flow_steps.rb
b/tests/browser/features/step_definitions/flow_steps.rb
index aca75dc..73b5832 100644
--- a/tests/browser/features/step_definitions/flow_steps.rb
+++ b/tests/browser/features/step_definitions/flow_steps.rb
@@ -65,10 +65,6 @@
on(FlowPage).post_actions_link_element.when_present.hover
end
-When(/^I click the Suppress topic button$/) do
- on(FlowPage).topic_suppress_button_element.when_present.click
-end
-
When(/^I hover on the Topic Actions link$/) do
on(FlowPage).topic_actions_link_element.when_present.hover
end
@@ -82,18 +78,39 @@
end
When(/^I type "(.+)" into the new topic content field$/) do |flow_body|
- body_string = flow_body + @random_string + @automated_test_marker
+ body_string = @data_manager.get flow_body
on(FlowPage).new_topic_body_element.when_present.send_keys(body_string)
end
When(/^I type "(.+)" into the new topic title field$/) do |flow_title|
- @automated_test_marker = " browsertest edit"
on(FlowPage) do |page|
- @topic_string = flow_title + @random_string + @automated_test_marker
+ @topic_string = @data_manager.get flow_title
page.new_topic_title_element.when_present.click
page.new_topic_title_element.when_present.focus
page.new_topic_title_element.when_present.send_keys(@topic_string)
end
+end
+
+When(/I log out/) do
+ on(FlowPage) do |page|
+ page.logout
+ page.logout_element.when_not_visible
+ end
+end
+
+When(/^I visit the board history page$/) do
+ visit BoardHistoryPage
+ on(BoardHistoryPage).flow_board_history_element.when_present
+end
+
+When(/^I visit the topic history page$/) do
+ step 'I hover on the Topic Actions link'
+ step 'I click History from the Actions menu'
+ on(TopicHistoryPage).flow_topic_history_element.when_present
+end
+
+When(/^I click History from the Actions menu$/) do
+ on(FlowPage).topic_history_button_element.when_present.click
end
Then(/^I am on my user page$/) do
@@ -165,3 +182,58 @@
Then(/^the top post should not have a heading which contains "(.+)"$/) do
|text|
expect(on(FlowPage).flow_first_topic_heading).not_to match(text)
end
+
+Then(/^I see the topic "(.*?)" on the board$/) do |title|
+ full_title = @data_manager.get title
+ on(FlowPage).topic_with_title(full_title).when_present
+end
+
+Then(/^everybody sees the topic "(.*?)" on the board$/) do |title|
+ step 'I log out'
+ step 'I am on Flow page'
+ step "I see the topic \"#{title}\" on the board"
+end
+
+Then(/^I see the following entries in board history$/) do |table|
+ on(BoardHistoryPage) do |page|
+ table.hashes.each do |row|
+ action = row['action']
+ topic = @data_manager.get row['topic']
+ entry = %(#{action} "#{topic}")
+ expect(page.flow_board_history).to match(entry)
+ end
+ end
+end
+
+Then(/^I see the following entries in topic history$/) do |table|
+ on(TopicHistoryPage) do |page|
+ table.hashes.each do |row|
+ action = row['action']
+ topic = @data_manager.get row['topic']
+ entry = %(#{action} "#{topic}")
+ expect(page.flow_topic_history).to match(entry)
+ end
+ end
+end
+
+Then(/^I do not see the following entries in board history$/) do |table|
+ on(BoardHistoryPage) do |page|
+ table.hashes.each do |row|
+ action = row['action']
+ topic = @data_manager.get row['topic']
+ entry = %(#{action} "#{topic}")
+ expect(page.flow_board_history).to_not match(entry)
+ end
+ end
+end
+
+Then(/^I do not see the following entries in topic history$/) do |table|
+ on(TopicHistoryPage) do |page|
+ table.hashes.each do |row|
+ action = row['action']
+ topic = @data_manager.get row['topic']
+ entry = %(#{action} "#{topic}")
+ expect(page.flow_topic_history).to_not match(entry)
+ end
+ end
+end
diff --git a/tests/browser/features/step_definitions/moderation_steps.rb
b/tests/browser/features/step_definitions/moderation_steps.rb
index c642116..ba59879 100644
--- a/tests/browser/features/step_definitions/moderation_steps.rb
+++ b/tests/browser/features/step_definitions/moderation_steps.rb
@@ -10,24 +10,20 @@
on(FlowPage).dialog_submit_hide_element.when_present.click
end
-When(/^I click Suppress topic$/) do
- on(FlowPage).dialog_submit_suppress_element.when_present.click
-end
-
When(/^I give reason for deletion as being "(.*?)"$/) do |delete_reason|
- on(FlowPage).dialog_input_element.when_present.send_keys(delete_reason)
+ step "I type \"#{delete_reason}\" in the dialog box"
end
When(/^I give reason for hiding as being "(.*?)"$/) do |hide_reason|
- on(FlowPage).dialog_input_element.when_present.send_keys(hide_reason)
-end
-
-When(/^I give reason for suppression as being "(.*?)"$/) do |suppress_reason|
- on(FlowPage).dialog_input_element.when_present.send_keys(suppress_reason)
+ step "I type \"#{hide_reason}\" in the dialog box"
end
When(/^I see a dialog box$/) do
on(FlowPage).dialog_element.when_present
+end
+
+When(/^I type "(.*?)" in the dialog box$/) do |text|
+ on(FlowPage).dialog_input_element.when_present.send_keys(text)
end
Then(/^I confirm$/) do
@@ -40,8 +36,4 @@
Then(/^the top post should be marked as deleted$/) do
expect(on(FlowPage).flow_first_topic_moderation_msg_element.when_present.text).to
match("This topic has been deleted")
-end
-
-Then(/^the top post should be marked as suppressed$/) do
-
expect(on(FlowPage).flow_first_topic_moderation_msg_element.when_present.text).to
match("This topic has been suppressed")
end
diff --git a/tests/browser/features/step_definitions/suppress_steps.rb
b/tests/browser/features/step_definitions/suppress_steps.rb
new file mode 100644
index 0000000..e9cb687
--- /dev/null
+++ b/tests/browser/features/step_definitions/suppress_steps.rb
@@ -0,0 +1,37 @@
+
+Given(/^I have suppressed and restored the first topic$/) do
+ step 'I suppress the first topic'
+ step 'I undo the suppression'
+end
+
+When(/^I click the Suppress topic button$/) do
+ on(FlowPage).topic_suppress_button_element.when_present.click
+end
+
+When(/^I click Suppress topic$/) do
+ on(FlowPage).dialog_submit_suppress_element.when_present.click
+end
+
+When(/^I undo the suppression$/) do
+ on(FlowPage) do |page|
+ page.undo_suppression_button_element.when_present.click
+ page.undo_suppression_button_element.when_not_visible
+ end
+end
+
+When(/^I suppress the first topic with reason "(.*?)"$/) do |reason|
+ step 'I hover on the Topic Actions link'
+ step 'I click the Suppress topic button'
+ step 'I see a dialog box'
+ step "I type \"#{reason}\" in the dialog box"
+ step 'I click Suppress topic'
+ step 'the top post should be marked as suppressed'
+end
+
+When(/^I suppress the first topic$/) do
+ step "I suppress the first topic with reason \"no reason given\""
+end
+
+Then(/^the top post should be marked as suppressed$/) do
+
expect(on(FlowPage).flow_first_topic_moderation_msg_element.when_present.text).to
match("This topic has been suppressed")
+end
diff --git a/tests/browser/features/support/data_manager.rb
b/tests/browser/features/support/data_manager.rb
new file mode 100644
index 0000000..6d8c36f
--- /dev/null
+++ b/tests/browser/features/support/data_manager.rb
@@ -0,0 +1,10 @@
+class DataManager
+ def initialize
+ @data = {}
+ end
+
+ def get(part)
+ @data[part] = "#{part}-#{rand}" unless @data.key? part
+ @data[part]
+ end
+end
diff --git a/tests/browser/features/support/hooks.rb
b/tests/browser/features/support/hooks.rb
index 5ab6259..2d9756d 100644
--- a/tests/browser/features/support/hooks.rb
+++ b/tests/browser/features/support/hooks.rb
@@ -1,3 +1,6 @@
# Allow running of bundle exec cucumber --dry-run -f stepdefs
require "mediawiki_selenium"
require 'page-object'
+require_relative 'data_manager'
+
+Before { @data_manager = DataManager.new }
diff --git a/tests/browser/features/support/pages/board_history_page.rb
b/tests/browser/features/support/pages/board_history_page.rb
new file mode 100644
index 0000000..7f79b01
--- /dev/null
+++ b/tests/browser/features/support/pages/board_history_page.rb
@@ -0,0 +1,9 @@
+class BoardHistoryPage
+ include PageObject
+
+ include URL
+
+ page_url URL.url("Talk:Flow_QA?action=history")
+
+ div(:flow_board_history, class: 'flow-board-history')
+end
diff --git a/tests/browser/features/support/pages/flow_page.rb
b/tests/browser/features/support/pages/flow_page.rb
index d7c5909..d449504 100644
--- a/tests/browser/features/support/pages/flow_page.rb
+++ b/tests/browser/features/support/pages/flow_page.rb
@@ -1,7 +1,4 @@
-class WikiPage
- include PageObject
- a(:logout, css: "#pt-logout a")
-end
+require_relative 'wiki_page'
class FlowPage < WikiPage
include URL
@@ -44,6 +41,10 @@
# Posts
## Highlighted post
div(:highlighted_post, css: ".flow-post-highlighted")
+
+ def topic_with_title(title)
+ div_element(text: title)
+ end
## First topic
div(:flow_first_topic, css: ".flow-topic", index: 0)
@@ -102,6 +103,9 @@
ul(:topic_actions_menu, css: ".flow-topic .flow-topic-titlebar .flow-menu
ul", index: 0)
a(:topic_hide_button) do |page|
page.topic_actions_menu_element.link_element(text: "Hide topic")
+ end
+ a(:topic_history_button) do |page|
+ page.topic_actions_menu_element.link_element(text: "History")
end
a(:topic_delete_button) do |page|
page.topic_actions_menu_element.link_element(text: "Delete topic")
@@ -252,4 +256,10 @@
a(:board_unwatch_link, href: /Flow_QA&action=unwatch/)
a(:board_watch_link, href: /Flow_QA&action=watch/)
+
+ # undo suppression
+ button(:undo_suppression_button, text: "Undo")
+
+ # history
+ a(:view_history, text: 'View history')
end
diff --git a/tests/browser/features/support/pages/topic_history_page.rb
b/tests/browser/features/support/pages/topic_history_page.rb
new file mode 100644
index 0000000..875465a
--- /dev/null
+++ b/tests/browser/features/support/pages/topic_history_page.rb
@@ -0,0 +1,5 @@
+class TopicHistoryPage
+ include PageObject
+
+ div(:flow_topic_history, class: 'flow-topic-histories')
+end
diff --git a/tests/browser/features/support/pages/wiki_page.rb
b/tests/browser/features/support/pages/wiki_page.rb
new file mode 100644
index 0000000..3f0c5c9
--- /dev/null
+++ b/tests/browser/features/support/pages/wiki_page.rb
@@ -0,0 +1,4 @@
+class WikiPage
+ include PageObject
+ a(:logout, css: "#pt-logout a")
+end
diff --git a/tests/browser/features/suppress.feature
b/tests/browser/features/suppress.feature
new file mode 100644
index 0000000..c411af9
--- /dev/null
+++ b/tests/browser/features/suppress.feature
@@ -0,0 +1,74 @@
+@chrome @clean @en.wikipedia.beta.wmflabs.org @firefox @internet_explorer_10
@login @test2.wikipedia.org
+Feature: Suppress
+
+ Assumes Flow is enabled on Talk:Flow_QA
+
+ Background:
+ Given I am logged in
+ And I am on Flow page
+
+ Scenario: Suppressing a topic
+ Given I have created a Flow topic with title "suppress-topic"
+ When I suppress the first topic with reason "I suppress you"
+ Then the top post should be marked as suppressed
+
+ Scenario: Restoring a topic
+ Given I have created a Flow topic with title "suppress-restore-topic"
+ When I suppress the first topic with reason "I suppress you temporarily"
+ And I undo the suppression
+ Then I see the topic "suppress-restore-topic" on the board
+ And everybody sees the topic "suppress-restore-topic" on the board
+
+ Scenario: A suppressed topic is not in board history
+ Given I have created a Flow topic with title "suppress-not-in-history"
+ And I suppress the first topic
+ When I visit the board history page
+ Then I see the following entries in board history
+ |action |topic |
+ |suppressed the topic|suppress-not-in-history|
+ |created the topic |suppress-not-in-history|
+ When I log out
+ And I visit the board history page
+ Then I do not see the following entries in board history
+ |action |topic |
+ |suppressed the topic|suppress-not-in-history|
+ |created the topic |suppress-not-in-history|
+
+ Scenario: A suppressed and restored topic is in board history
+ Given I have created a Flow topic with title "suppress-restore-in-history"
+ And I have suppressed and restored the first topic
+ When I visit the board history page
+ Then I see the following entries in board history
+ |action |topic |
+ |restored the topic |suppress-restore-in-history|
+ |suppressed the topic|suppress-restore-in-history|
+ |created the topic |suppress-restore-in-history|
+ When I log out
+ And I visit the board history page
+ Then I see the following entries in board history
+ |action |topic |
+ |created the topic |suppress-restore-in-history|
+ Then I do not see the following entries in board history
+ |action |topic |
+ |restored the topic |suppress-restore-in-history|
+ |suppressed the topic|suppress-restore-in-history|
+
+ Scenario: A suppressed and restored topic is in topic history
+ Given I have created a Flow topic with title "suppress-restore-in-history"
+ And I have suppressed and restored the first topic
+ When I visit the topic history page
+ Then I see the following entries in topic history
+ |action |topic |
+ |restored the topic |suppress-restore-in-history|
+ |suppressed the topic|suppress-restore-in-history|
+ |created the topic |suppress-restore-in-history|
+ When I log out
+ And I am on Flow page
+ And I visit the topic history page
+ Then I see the following entries in topic history
+ |action |topic |
+ |created the topic |suppress-restore-in-history|
+ Then I do not see the following entries in topic history
+ |action |topic |
+ |restored the topic |suppress-restore-in-history|
+ |suppressed the topic|suppress-restore-in-history|
--
To view, visit https://gerrit.wikimedia.org/r/207797
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3e7497d162e77ddc4c0c5aba1725a3290353fbcf
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Sbisson <sbis...@wikimedia.org>
Gerrit-Reviewer: Mattflaschen <mflasc...@wikimedia.org>
Gerrit-Reviewer: Sbisson <sbis...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
