2020-01-16 21:11:54 +00:00
|
|
|
<?php
|
|
|
|
namespace MediaWiki\Skins\Vector\Tests\Integration;
|
|
|
|
|
Add opt-out link to Sidebar for Vector/Logged-in Users Without Abstractions
This commit is singularly focused on adding a link to the sidebar for
Vector, logged-in users. It does the bare minimum to fulfill the
requirements of T243281.
Additionally, it will help to answer the question "Do we need to use
abstractions (other than maybe different templates) to separate Legacy
Vector from Vector" by intentionally leaving out any abstractions in
order to make it easier to compare with a follow-up patch
(Ib2ef15180df73360cc1de25b893e49d415d23e1a) which does use abstractions.
It is a good thing to question whether or not we need addtional
abstractions in VectorTemplate and if they will help us as unnecessary
abstractions can have the opposite effect and just lead to further
frustrations down the road.
Therefore, I urge you, the reviewer, to let me know your thoughts! If
abstractions are viewed as not making our lives any easier, the
follow-up patches may be completely discarded and that's totally okay
with me. :) I think it's a good think to talk about now though.
Important changes:
* The VectorTemplate constructor was changed to allow injecting the
config, templateParser, and isLegacy boolean (only the config was
allowed before this commit). According to MediaWiki's Stable Interface
Policy, "Constructor signatures are generally considered unstable unless
explicitly declared stable for calling" [3]. Given that VecorTemplate's
constructor is not marked as stable, it is justified to do this without
warning according to the policy.
* Due to the above, the 'setTemplate' method is no longer needed and was
marked as deprecated.
* VectorTemplateTest was made to adapt to the new VectorTemplate
constructor. Additionally, it now extends from
MediaWikiIntegrationTestCase which my intelliphense server can pick up.
I *think* MediaWikiTestCase is just an alias to
MediaWikiIntegrationTestCase [1] and MediaWikiTestCase file was renamed
to MediaWikiIntegrationTestCase in [2], but I'm willing to change it
back if there is pushback to this.
Open questions:
* What are VectorTemplate's responsibilities? To me, it acts right now
as a controller (because it echos the full HTML string from the
template), a model (because SkinTemplate::prepareQuickTemplate sets data
on it which it later retrieves through `$this->get()`), a presenter
(because it adds data tailored for a web-centric view), and a view
(because it renders HTML strings instead of letting the view/template be
solely responsible for that). Arguably, some business logic might be
mixed in there as well (because it checks to see if a User is logged
in/has necessary permissions to show x which my changes here add to).
This might not be a problem if we keep VectorTemplate relatively small,
but will it remain this way as we progress further in Desktop
Improvements?
* How do we write tests for VectorTemplate without exposing unnecessary
public methods? For example, if I want to test the `getSkinData()`
method to see what state will be sent to the template, how should I do
this? One option might be to use `TestingAccessWrapper` to expose these
private methods which is what
`VectorTemplateTest::testbuildViewsProps()` does. Another option is to
accept this method as public. Is there a better way? Keep in mind that
even with access to this method, there might be many things to mock.
[1] https://github.com/wikimedia/mediawiki/blob/0030cb525be6cabc1d63de80586b2017d4bbe354/tests/common/TestsAutoLoader.php#L64
[2] Ie717b0ecf4fcfd089d46248f14853c80b7ef4a76
[3] https://www.mediawiki.org/wiki/Stable_interface_policy
Bug: T243281
Change-Id: I0571b041bcd7f19bec9f103fa7bccdd093f6394d
2020-03-17 20:21:33 +00:00
|
|
|
use GlobalVarConfig;
|
|
|
|
use MediaWikiIntegrationTestCase;
|
|
|
|
use TemplateParser;
|
|
|
|
use VectorTemplate;
|
2020-01-16 21:11:54 +00:00
|
|
|
use Wikimedia\TestingAccessWrapper;
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Class VectorTemplateTest
|
|
|
|
* @package MediaWiki\Skins\Vector\Tests\Unit
|
|
|
|
* @group Vector
|
|
|
|
* @group Skins
|
|
|
|
*
|
|
|
|
* @coversDefaultClass \VectorTemplate
|
|
|
|
*/
|
Add opt-out link to Sidebar for Vector/Logged-in Users Without Abstractions
This commit is singularly focused on adding a link to the sidebar for
Vector, logged-in users. It does the bare minimum to fulfill the
requirements of T243281.
Additionally, it will help to answer the question "Do we need to use
abstractions (other than maybe different templates) to separate Legacy
Vector from Vector" by intentionally leaving out any abstractions in
order to make it easier to compare with a follow-up patch
(Ib2ef15180df73360cc1de25b893e49d415d23e1a) which does use abstractions.
It is a good thing to question whether or not we need addtional
abstractions in VectorTemplate and if they will help us as unnecessary
abstractions can have the opposite effect and just lead to further
frustrations down the road.
Therefore, I urge you, the reviewer, to let me know your thoughts! If
abstractions are viewed as not making our lives any easier, the
follow-up patches may be completely discarded and that's totally okay
with me. :) I think it's a good think to talk about now though.
Important changes:
* The VectorTemplate constructor was changed to allow injecting the
config, templateParser, and isLegacy boolean (only the config was
allowed before this commit). According to MediaWiki's Stable Interface
Policy, "Constructor signatures are generally considered unstable unless
explicitly declared stable for calling" [3]. Given that VecorTemplate's
constructor is not marked as stable, it is justified to do this without
warning according to the policy.
* Due to the above, the 'setTemplate' method is no longer needed and was
marked as deprecated.
* VectorTemplateTest was made to adapt to the new VectorTemplate
constructor. Additionally, it now extends from
MediaWikiIntegrationTestCase which my intelliphense server can pick up.
I *think* MediaWikiTestCase is just an alias to
MediaWikiIntegrationTestCase [1] and MediaWikiTestCase file was renamed
to MediaWikiIntegrationTestCase in [2], but I'm willing to change it
back if there is pushback to this.
Open questions:
* What are VectorTemplate's responsibilities? To me, it acts right now
as a controller (because it echos the full HTML string from the
template), a model (because SkinTemplate::prepareQuickTemplate sets data
on it which it later retrieves through `$this->get()`), a presenter
(because it adds data tailored for a web-centric view), and a view
(because it renders HTML strings instead of letting the view/template be
solely responsible for that). Arguably, some business logic might be
mixed in there as well (because it checks to see if a User is logged
in/has necessary permissions to show x which my changes here add to).
This might not be a problem if we keep VectorTemplate relatively small,
but will it remain this way as we progress further in Desktop
Improvements?
* How do we write tests for VectorTemplate without exposing unnecessary
public methods? For example, if I want to test the `getSkinData()`
method to see what state will be sent to the template, how should I do
this? One option might be to use `TestingAccessWrapper` to expose these
private methods which is what
`VectorTemplateTest::testbuildViewsProps()` does. Another option is to
accept this method as public. Is there a better way? Keep in mind that
even with access to this method, there might be many things to mock.
[1] https://github.com/wikimedia/mediawiki/blob/0030cb525be6cabc1d63de80586b2017d4bbe354/tests/common/TestsAutoLoader.php#L64
[2] Ie717b0ecf4fcfd089d46248f14853c80b7ef4a76
[3] https://www.mediawiki.org/wiki/Stable_interface_policy
Bug: T243281
Change-Id: I0571b041bcd7f19bec9f103fa7bccdd093f6394d
2020-03-17 20:21:33 +00:00
|
|
|
class VectorTemplateTest extends MediaWikiIntegrationTestCase {
|
2020-01-16 21:11:54 +00:00
|
|
|
|
|
|
|
/**
|
|
|
|
* @return \VectorTemplate
|
|
|
|
*/
|
|
|
|
private function provideVectorTemplateObject() {
|
Add opt-out link to Sidebar for Vector/Logged-in Users Without Abstractions
This commit is singularly focused on adding a link to the sidebar for
Vector, logged-in users. It does the bare minimum to fulfill the
requirements of T243281.
Additionally, it will help to answer the question "Do we need to use
abstractions (other than maybe different templates) to separate Legacy
Vector from Vector" by intentionally leaving out any abstractions in
order to make it easier to compare with a follow-up patch
(Ib2ef15180df73360cc1de25b893e49d415d23e1a) which does use abstractions.
It is a good thing to question whether or not we need addtional
abstractions in VectorTemplate and if they will help us as unnecessary
abstractions can have the opposite effect and just lead to further
frustrations down the road.
Therefore, I urge you, the reviewer, to let me know your thoughts! If
abstractions are viewed as not making our lives any easier, the
follow-up patches may be completely discarded and that's totally okay
with me. :) I think it's a good think to talk about now though.
Important changes:
* The VectorTemplate constructor was changed to allow injecting the
config, templateParser, and isLegacy boolean (only the config was
allowed before this commit). According to MediaWiki's Stable Interface
Policy, "Constructor signatures are generally considered unstable unless
explicitly declared stable for calling" [3]. Given that VecorTemplate's
constructor is not marked as stable, it is justified to do this without
warning according to the policy.
* Due to the above, the 'setTemplate' method is no longer needed and was
marked as deprecated.
* VectorTemplateTest was made to adapt to the new VectorTemplate
constructor. Additionally, it now extends from
MediaWikiIntegrationTestCase which my intelliphense server can pick up.
I *think* MediaWikiTestCase is just an alias to
MediaWikiIntegrationTestCase [1] and MediaWikiTestCase file was renamed
to MediaWikiIntegrationTestCase in [2], but I'm willing to change it
back if there is pushback to this.
Open questions:
* What are VectorTemplate's responsibilities? To me, it acts right now
as a controller (because it echos the full HTML string from the
template), a model (because SkinTemplate::prepareQuickTemplate sets data
on it which it later retrieves through `$this->get()`), a presenter
(because it adds data tailored for a web-centric view), and a view
(because it renders HTML strings instead of letting the view/template be
solely responsible for that). Arguably, some business logic might be
mixed in there as well (because it checks to see if a User is logged
in/has necessary permissions to show x which my changes here add to).
This might not be a problem if we keep VectorTemplate relatively small,
but will it remain this way as we progress further in Desktop
Improvements?
* How do we write tests for VectorTemplate without exposing unnecessary
public methods? For example, if I want to test the `getSkinData()`
method to see what state will be sent to the template, how should I do
this? One option might be to use `TestingAccessWrapper` to expose these
private methods which is what
`VectorTemplateTest::testbuildViewsProps()` does. Another option is to
accept this method as public. Is there a better way? Keep in mind that
even with access to this method, there might be many things to mock.
[1] https://github.com/wikimedia/mediawiki/blob/0030cb525be6cabc1d63de80586b2017d4bbe354/tests/common/TestsAutoLoader.php#L64
[2] Ie717b0ecf4fcfd089d46248f14853c80b7ef4a76
[3] https://www.mediawiki.org/wiki/Stable_interface_policy
Bug: T243281
Change-Id: I0571b041bcd7f19bec9f103fa7bccdd093f6394d
2020-03-17 20:21:33 +00:00
|
|
|
$template = new VectorTemplate(
|
|
|
|
GlobalVarConfig::newInstance(),
|
|
|
|
new TemplateParser(),
|
|
|
|
true
|
|
|
|
);
|
2020-05-02 23:56:09 +00:00
|
|
|
$template->set( 'skin', new \SkinVector() );
|
2020-01-16 21:11:54 +00:00
|
|
|
return $template;
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @param string $nodeString an HTML of the node we want to verify
|
|
|
|
* @param string $tag Tag of the element we want to check
|
|
|
|
* @param string $attribute Attribute of the element we want to check
|
|
|
|
* @param string $search Value of the attribute we want to verify
|
|
|
|
* @return bool
|
|
|
|
*/
|
|
|
|
private function expectNodeAttribute( $nodeString, $tag, $attribute, $search ) {
|
|
|
|
$node = new \DOMDocument();
|
|
|
|
$node->loadHTML( $nodeString );
|
|
|
|
$element = $node->getElementsByTagName( $tag )->item( 0 );
|
|
|
|
if ( !$element ) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
|
|
|
$values = explode( ' ', $element->getAttribute( $attribute ) );
|
|
|
|
return in_array( $search, $values );
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @covers ::makeListItem
|
|
|
|
*/
|
|
|
|
public function testMakeListItemRespectsCollapsibleOption() {
|
|
|
|
$template = $this->provideVectorTemplateObject();
|
|
|
|
$listItemClass = 'my_test_class';
|
|
|
|
$options = [ 'vector-collapsible' => true ];
|
|
|
|
$item = [ 'class' => $listItemClass ];
|
|
|
|
$nonCollapsible = $template->makeListItem( 'key', $item, [] );
|
|
|
|
$collapsible = $template->makeListItem( 'key', [], $options );
|
|
|
|
|
|
|
|
$this->assertTrue(
|
|
|
|
$this->expectNodeAttribute( $collapsible, 'li', 'class', 'collapsible' ),
|
|
|
|
'The collapsible element has to have `collapsible` class'
|
|
|
|
);
|
|
|
|
$this->assertFalse(
|
|
|
|
$this->expectNodeAttribute( $nonCollapsible, 'li', 'class', 'collapsible' ),
|
|
|
|
'The non-collapsible element should not have `collapsible` class'
|
|
|
|
);
|
|
|
|
$this->assertTrue(
|
|
|
|
$this->expectNodeAttribute( $nonCollapsible, 'li', 'class', $listItemClass ),
|
|
|
|
'The non-collapsible element should preserve item class'
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @covers ::makeListItem
|
|
|
|
*/
|
|
|
|
public function testWatcAndUnwatchHasIconClass() {
|
|
|
|
$template = $this->provideVectorTemplateObject();
|
|
|
|
$this->setMwGlobals( [
|
|
|
|
'wgVectorUseIconWatch' => true
|
|
|
|
] );
|
|
|
|
$listItemClass = 'my_test_class';
|
|
|
|
$options = [];
|
|
|
|
$item = [ 'class' => $listItemClass ];
|
|
|
|
|
|
|
|
$watchListItem = $template->makeListItem( 'watch', $item, [] );
|
|
|
|
$unwatchListItem = $template->makeListItem( 'unwatch', [], $options );
|
|
|
|
$regularListItem = $template->makeListItem( 'whatever', $item, $options );
|
|
|
|
|
|
|
|
$this->assertTrue(
|
|
|
|
$this->expectNodeAttribute( $watchListItem, 'li', 'class', 'icon' ),
|
|
|
|
'Watch list items require an "icon" class'
|
|
|
|
);
|
|
|
|
$this->assertTrue(
|
|
|
|
$this->expectNodeAttribute( $unwatchListItem, 'li', 'class', 'icon' ),
|
|
|
|
'Unwatch list items require an "icon" class'
|
|
|
|
);
|
|
|
|
$this->assertFalse(
|
|
|
|
$this->expectNodeAttribute( $regularListItem, 'li', 'class', 'icon' ),
|
|
|
|
'List item other than watch or unwatch should not have an "icon" class'
|
|
|
|
);
|
|
|
|
$this->assertTrue(
|
|
|
|
$this->expectNodeAttribute( $watchListItem, 'li', 'class', $listItemClass ),
|
|
|
|
'Watch list items require an item class'
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @covers ::makeListItem
|
|
|
|
*/
|
|
|
|
public function testWatchAndUnwatchHasIconClassOnlyIfVectorUseIconWatchIsSet() {
|
|
|
|
$template = $this->provideVectorTemplateObject();
|
|
|
|
$this->setMwGlobals( [
|
|
|
|
'wgVectorUseIconWatch' => false
|
|
|
|
] );
|
|
|
|
$listItemClass = 'my_test_class';
|
|
|
|
$item = [ 'class' => $listItemClass ];
|
|
|
|
|
|
|
|
$watchListItem = $template->makeListItem( 'watch', $item, [] );
|
|
|
|
|
|
|
|
$this->assertFalse(
|
|
|
|
$this->expectNodeAttribute( $watchListItem, 'li', 'class', 'icon' ),
|
|
|
|
'Watch list should not have an "icon" class when VectorUserIconWatch is disabled'
|
|
|
|
);
|
|
|
|
$this->assertTrue(
|
|
|
|
$this->expectNodeAttribute( $watchListItem, 'li', 'class', $listItemClass ),
|
|
|
|
'Watch list items require an item class'
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
2020-04-21 21:35:11 +00:00
|
|
|
* @covers ::getMenuProps
|
2020-01-16 21:11:54 +00:00
|
|
|
*/
|
2020-04-21 21:35:11 +00:00
|
|
|
public function testGetMenuProps() {
|
2020-01-16 21:11:54 +00:00
|
|
|
$langAttrs = 'LANG_ATTRIBUTES';
|
Add opt-out link to Sidebar for Vector/Logged-in Users Without Abstractions
This commit is singularly focused on adding a link to the sidebar for
Vector, logged-in users. It does the bare minimum to fulfill the
requirements of T243281.
Additionally, it will help to answer the question "Do we need to use
abstractions (other than maybe different templates) to separate Legacy
Vector from Vector" by intentionally leaving out any abstractions in
order to make it easier to compare with a follow-up patch
(Ib2ef15180df73360cc1de25b893e49d415d23e1a) which does use abstractions.
It is a good thing to question whether or not we need addtional
abstractions in VectorTemplate and if they will help us as unnecessary
abstractions can have the opposite effect and just lead to further
frustrations down the road.
Therefore, I urge you, the reviewer, to let me know your thoughts! If
abstractions are viewed as not making our lives any easier, the
follow-up patches may be completely discarded and that's totally okay
with me. :) I think it's a good think to talk about now though.
Important changes:
* The VectorTemplate constructor was changed to allow injecting the
config, templateParser, and isLegacy boolean (only the config was
allowed before this commit). According to MediaWiki's Stable Interface
Policy, "Constructor signatures are generally considered unstable unless
explicitly declared stable for calling" [3]. Given that VecorTemplate's
constructor is not marked as stable, it is justified to do this without
warning according to the policy.
* Due to the above, the 'setTemplate' method is no longer needed and was
marked as deprecated.
* VectorTemplateTest was made to adapt to the new VectorTemplate
constructor. Additionally, it now extends from
MediaWikiIntegrationTestCase which my intelliphense server can pick up.
I *think* MediaWikiTestCase is just an alias to
MediaWikiIntegrationTestCase [1] and MediaWikiTestCase file was renamed
to MediaWikiIntegrationTestCase in [2], but I'm willing to change it
back if there is pushback to this.
Open questions:
* What are VectorTemplate's responsibilities? To me, it acts right now
as a controller (because it echos the full HTML string from the
template), a model (because SkinTemplate::prepareQuickTemplate sets data
on it which it later retrieves through `$this->get()`), a presenter
(because it adds data tailored for a web-centric view), and a view
(because it renders HTML strings instead of letting the view/template be
solely responsible for that). Arguably, some business logic might be
mixed in there as well (because it checks to see if a User is logged
in/has necessary permissions to show x which my changes here add to).
This might not be a problem if we keep VectorTemplate relatively small,
but will it remain this way as we progress further in Desktop
Improvements?
* How do we write tests for VectorTemplate without exposing unnecessary
public methods? For example, if I want to test the `getSkinData()`
method to see what state will be sent to the template, how should I do
this? One option might be to use `TestingAccessWrapper` to expose these
private methods which is what
`VectorTemplateTest::testbuildViewsProps()` does. Another option is to
accept this method as public. Is there a better way? Keep in mind that
even with access to this method, there might be many things to mock.
[1] https://github.com/wikimedia/mediawiki/blob/0030cb525be6cabc1d63de80586b2017d4bbe354/tests/common/TestsAutoLoader.php#L64
[2] Ie717b0ecf4fcfd089d46248f14853c80b7ef4a76
[3] https://www.mediawiki.org/wiki/Stable_interface_policy
Bug: T243281
Change-Id: I0571b041bcd7f19bec9f103fa7bccdd093f6394d
2020-03-17 20:21:33 +00:00
|
|
|
$vectorTemplate = $this->provideVectorTemplateObject();
|
2020-04-07 22:55:08 +00:00
|
|
|
// used internally by getPersonalTools
|
2020-04-21 21:35:11 +00:00
|
|
|
$vectorTemplate->set( 'personal_urls', [] );
|
2020-04-07 22:55:08 +00:00
|
|
|
$vectorTemplate->set( 'content_navigation', [
|
|
|
|
'actions' => [],
|
|
|
|
'namespaces' => [],
|
|
|
|
'variants' => [],
|
|
|
|
'views' => [],
|
|
|
|
] );
|
2020-01-16 21:11:54 +00:00
|
|
|
$vectorTemplate->set( 'userlangattributes', $langAttrs );
|
|
|
|
$openVectorTemplate = TestingAccessWrapper::newFromObject( $vectorTemplate );
|
|
|
|
|
2020-04-21 21:35:11 +00:00
|
|
|
$props = $openVectorTemplate->getMenuProps();
|
2020-04-07 22:55:08 +00:00
|
|
|
$views = $props['data-page-actions'];
|
|
|
|
$namespaces = $props['data-namespace-tabs'];
|
|
|
|
|
2020-04-21 21:35:11 +00:00
|
|
|
$this->assertSame( $views, [
|
2020-04-21 21:56:39 +00:00
|
|
|
'id' => 'p-views',
|
2020-05-06 17:15:50 +00:00
|
|
|
'class' => 'emptyPortlet vectorMenu-tabs vectorTabs',
|
2020-02-14 20:33:54 +00:00
|
|
|
'label-id' => 'p-views-label',
|
2020-04-21 21:56:39 +00:00
|
|
|
'label' => 'Views',
|
2020-02-14 20:33:54 +00:00
|
|
|
'html-userlangattributes' => $langAttrs,
|
|
|
|
'html-items' => '',
|
2020-05-06 17:15:50 +00:00
|
|
|
'class' => 'emptyPortlet vectorMenu-tabs vectorTabs',
|
2020-02-14 20:33:54 +00:00
|
|
|
] );
|
2020-04-21 21:35:11 +00:00
|
|
|
|
2020-04-07 22:55:08 +00:00
|
|
|
$variants = $props['data-variants'];
|
|
|
|
$actions = $props['data-page-actions-more'];
|
2020-04-21 21:56:39 +00:00
|
|
|
$this->assertSame( $namespaces['class'],
|
2020-05-06 17:15:50 +00:00
|
|
|
'emptyPortlet vectorMenu-tabs vectorTabs' );
|
2020-04-21 21:35:11 +00:00
|
|
|
$this->assertSame( $variants['class'],
|
|
|
|
'emptyPortlet vectorMenu' );
|
|
|
|
$this->assertSame( $actions['class'],
|
|
|
|
'emptyPortlet vectorMenu' );
|
|
|
|
$this->assertSame( $props['data-personal-menu']['class'],
|
2020-05-05 23:26:35 +00:00
|
|
|
'emptyPortlet vectorMenu-default' );
|
2020-01-16 21:11:54 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
}
|