Commit graph

35 commits

Author SHA1 Message Date
Thiemo Kreuz 092e2a4959 Talk about "exclusions" instead of "blacklists"
AVoid the term if possible, in all internal code. The only remaining
use of the word is in the public $wgPopupsPageBlacklist config
variable.

Change-Id: Ib238731270f473ad44fcff13df617a70433f2452
2020-06-09 08:56:06 +02:00
DannyS712 68d52f7fa9 Use assertTrue instead of assertSame to compare with true
Bug: T244552
Change-Id: I42c50e37515e49a15658041147edc1d644812c46
2020-02-07 07:17:04 +00:00
Piotr Miazga 0ab39f88e7 Enable by default
Popups is enabled on the Wikipedias but disabled by default, which makes
it more difficult to enable on third-party wikis.

Things should be enabled by default, and them WMF configs should disable
features when they are not used/available.

Supporting changes:

- Rename PopupsContextTest#testAnonUserHasDisabledPagePreviews and
  remove the associated data provider, which provided only one datum

- Set $wgPopupsReferencePreviews to false in
  PopupsContextTest#testShouldSendModuleToUser in order to isolate the
  code under test

Change-Id: Ib8cc7041d792bed0a19d18e14506627099a69bef
2020-01-29 15:40:23 +00:00
Thiemo Kreuz 71ac670d9e Add visibility keywords to all constants in PHP code
Change-Id: I384cffed670c7e845465e3e18808e267327011ed
2019-10-22 07:10:31 +00:00
Thiemo Kreuz fe32c48268 Replace all mock builders with createMock() calls
This was motivated by two things:

1. Notice the bad class name in UserPreferencesChangeHandlerTest. This
was possible because the default mock builder does not check if a class
exists, *and* there was no type hint in place. createMock() *does* check
if a class exists.

2. We are free to use createMock() from PHPUnit 5+ now. It's not only
shorter but more strict and reliable.

Change-Id: I301a171d587026eab0a62575ab2fdbfd7733c661
2019-10-18 14:31:10 +00:00
Max Semenik b72a95b5c0 Fixes for PHPUnit 6
Bug: T192167
Change-Id: I354077e8e44cfea3e75219d6701a4f9e11d4c70a
2019-10-05 22:43:20 -07:00
Thiemo Kreuz 3eb7c9b976 Replace all loose assertEquals with strict PHPUnit assertions
A method that is expected to return, for example, a boolean true
should not return an other value that PHPs loose == comparison might
also consider true.

Same for all other loose equality checks.

Change-Id: If729c6e97d5337eee10b717da76dad428218ff69
2019-10-04 10:34:21 +02:00
Thiemo Kreuz e32fc4914e Add some missing newlines to separate PHP code better
I believe these additional newlines all make the code easier to read.
It's easier to see what belongs together, and what is a separate thing.
I found the Squiz.WhiteSpace.FunctionSpacing sniff very helpful to
enforce this code style. We enabled this already in almost all WMDE
codebases. It is not yet part of the upstream MediaWiki rule set, but
discussed.

Change-Id: Ibdf788529b28637bf98e7940c2516852c3afcef7
2019-03-11 11:33:13 +01:00
Derick Alangi 8fa98056f7 PopupsContextTest: Improve the PopupsContext test suit
-> Use single quotes for string literals.
-> Document missing parameter in test method.

This is just to improve on code readability, consistency
and maintainability.

Change-Id: I8de42df9f856ecb409637fe33b5f84b8bed1b547
2019-03-01 09:38:17 +01:00
Thiemo Kreuz 13015ad317 Fix a series of minor documentation issues in PHP code
E.g. type hints that have been missing, missing indention, and such.

Change-Id: I34610a03ad69d7988e9976a08a289c64121420ca
2019-02-01 09:42:14 +01:00
Thiemo Kreuz 7ca5d1fc9b Update PHPDocs and strict typing for array parameters
This does make generic `array` type hints more specific when possible.

I'm also applying my personal best practice to not have any @return
documentation on test @dataProviders. These don't provide any useful
information, and can't. The best type we could use is `@return array[]`,
but that would be the same for every single data provider. Copy pasting
these comments around is of no real value.

Also it was already inconsistent as some did not had this comment.

Change-Id: Id401c7e32493b6a9faaf6d47cddc01e2227102af
2019-01-24 15:44:26 +01:00
Derick Alangi 724d930c00 Remove irrelevant trailing forward slash from comment
Change-Id: Id0d148726bd198da2724393420b8f193950e0621
2019-01-24 15:20:53 +01:00
Thiemo Kreuz f67b7b0a66 Remove non-helpful auto-generated comments on constructors
The code literally explains itself. The comments don't add anything
to this. They are more distracting because one must read them first
to understand they don't contain anything.

Change-Id: I6f152962ec634ae15d2bff4472e332453cb9b0bf
2019-01-16 15:34:19 +01:00
Thiemo Kreuz 6ed00b924a Add some more missing @covers tags
I figured a single "@covers ::__construct" in PopupsContextTest should
be enough, especially since this constructor is super trivial.

There is nothing to test in NullLogger. I also removed the duplicated
documentation as it does not really apply, and is still available in the
base class.

Change-Id: I9a2e4b06e5bfd015efa4a92ba802c942290ec49d
2018-12-20 15:31:14 +00:00
Thiemo Kreuz 8efd3b20a5 Remove misplaced comments from PHPUnit tests
Some comments are copy-pasted from other, unrelated code. Documenting
a PHPUnit test file as being a "module test" is, I would argue, somewhat
pointless.

Change-Id: Iac28dc9c7b3e321b682e94c6a48efb2db41ca5f7
2018-12-18 10:00:00 +01:00
James D. Forrester 505ddc61fa PHP tests: Use the . not + operator to join strings
This unbreaks the test suite in PHP 7.1+.

Bug: T206297
Change-Id: I29677e84f41da447180beb58ffbc60655640890c
2018-10-05 15:47:05 -07:00
Piotr Miazga df9e724d89 Fix failing isTranslatedTitleBlacklistedTest
Somhow the testIsTranslatedTitleBlacklisted start to fail, the test
looks like it's broken but because of some reason it used to work.
For now let me fix the test because it blocks merged, and later
I'll try to investigate whats wrong.

Changes:
 - testIsTranslatedTitleBlacklisted() has to define blacklisted
 pages in canonical way (eg: Special:Preferences, not in a language
 variant)
 - use MediaWikiServices::getSpecialPageFactory() as
 SpecialPageFactory static calls are deprectated

Bug: T203522
Change-Id: I0db1481c96c9c0e27364d028a57c1178865741ba
2018-09-05 21:04:52 +02:00
Piotr Miazga a492d5f609 Change the PopupsVisibility state to visible to match anon experience
PagePreviews are visible to anonymous users, we would like to match
the same experience, when user creates a new account. On account
creation we mark PagePreviews settings to ON or OFF (configurable
via PopupsOptInStateForNewAccounts settings). With that approach
we can provide the best experience for new users, and keep existing
users happy (not enabling feature by default for everyone).

Bug: T191888
Change-Id: I39f42aa7268ce59c51f038048025ccf1bdf16481
2018-05-10 00:57:44 +02:00
Stephen Niedzielski ae44042cbf Hygiene: add assertion messages
Change-Id: Ic0a47bd468532824e8648c3f6371cc403896603c
2018-05-08 15:55:23 -05:00
jdlrobson 4e3282e5ff Remove BetaFeature code
Popups is out of beta feature and this code is no longer needed.
Removing code is the happiest activity a developer can do.

Other changes:
* Remove redundant type field on extension.json
(If not set, the extension will default to the "other" section.)
* Repurpose `name` with `namemsg` and make use of existing i18n
messages

Bug: T193053
Change-Id: Iea832cd1f37b0e7df6ff95efd66e4a1ff2a9004e
2018-04-26 15:51:48 -07:00
Piotr Miazga 915a9708fb PopupsContext::areDependenciesMet should respect PopupsGateway config
On all wikis Popups use Restbase as default gateway. In that case we
do not require the TextExtracts nor PageImages extensions. Code should
reflect that.

Bug: T190818
Change-Id: If4ce8f709b2ca1bb3cc381afa5e80e978adf2498
2018-03-28 16:31:15 -07:00
Piotr Miazga 0de054cd79 Use canonical name for NS_SPECIAL titles when checking the blacklist
Changes
 - when verifying title use canonical names for pages in
 special namespace
 - improve unit tests to verify canonical names and translated titles

Bug: T170169
Change-Id: I49592133eb8286eacf04fd3034df091f7ef2aa50
2017-08-25 15:56:02 +02:00
Baha 29770a3ff7 Disable Previews on blacklisted pages
Introduce a config variable `PopupsPageBlacklist`. Previews code won't
be shipped to pages listed in the config variable.

Bug: T170169
Change-Id: Ia8342b55c682f444ba79e959dcc1180527a31374
2017-07-31 09:10:30 -04:00
Piotr Miazga 3d0c5b1cc3 Hygiene: Dependency Injection for Popups
Changes:
 - removed ugly PopupsContext::getInstance
 - removed ugly PopupsContextTestWrapper helper
 - defined all services inside ServiceWirings
 - fixed unit tests to test classes directly or use MediawikiServices

Change-Id: Ie27e28bb07aebe01014848c290369b1b1f098e9b
2017-07-24 22:41:28 +00:00
Piotr Miazga 18b6675b4a Re-enable MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment sniff
Bug: T168384
Change-Id: Id64269f5950d6da5fee3f18825ce2d713d0446b0
2017-07-12 22:44:12 +02:00
Piotr Miazga 8bba8c1417 Send disabled event when user disables Page Preview
Changes:
 - introduced new UserPreferencesChangeHandler class that listens to
 PreferencesFormPreSave hook
 - introduced wrapper for EventLogging extension plus NullLogger when
 EventLogging extension is not availalbe
 - when user changes PagePreview to disabled system will trigger
 disabled event

Bug: T167365
Change-Id: I63faecb0495eb30a9fc2763e7ddf3944baf7f55a
2017-07-12 15:59:42 +02:00
Kunal Mehta ba9e3c68f6 build: Updating mediawiki/mediawiki-codesniffer to 0.9.0
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.FunctionComment.MissingParamComment
* MediaWiki.Commenting.FunctionComment.MissingParamTag
* MediaWiki.Commenting.FunctionComment.MissingReturn
* MediaWiki.Commenting.FunctionComment.ParamNameNoMatch
* MediaWiki.FunctionComment.Missing.Public
* MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment

Change-Id: Id9e98b7a4e87d00c63e7b509506c23f12cf0d380
2017-06-20 00:19:34 -07:00
jdlrobson 64e7bfd620 Hygiene: Rename isEnabledByUser to shouldSendModuleToUser
Sometimes we make choices on the users behalf if we don't have
sufficient information, so this name is more applicable.

If beta features is disabled and the user is anon they have not
explicitly opted in.

Change-Id: I5d816f569fc54f8bf74d6e5a06246b7fa7036e06
2017-02-10 10:07:28 -08:00
jdlrobson 4e2ce1ce39 Only add Popups code to output page where needed
If the Popups code is enabled in beta feature mode only then
only add it to the page if the beta feature is enabled.

isEnabledByUser now returns false if the user is anonymous
and Popups is restricted to beta mode.

Bug: T156290
Change-Id: If152d2a67a079050173c6d642e0960b59730bc6d
2017-02-09 10:43:53 -08:00
Piotr Miazga c13ab30bfa Move conflicting NavPopups gadget name into config var
Changes:
 - removed hardcoded comflicting Navigation Popups gadget name
 - introduced a config variable to specify conflicting gadget name

Bug: T151058
Change-Id: Ief5c6f90ec6dd6c074931f7d9ff20297d4718fe5
2017-01-12 18:07:20 +01:00
Piotr Miazga 211f1d1658 Disable Page Previews preferences when NavPopups are enabled
Changes:
 - introduced PopupsGadgetsIntegration class handles all checks
 - show help message on Preferences page when NavPopups is enabled
 - unit test everything

Bug: T151058
Change-Id: Ia474b1b30378efe84dedf3ad47c1f833e88d69b5
2017-01-11 01:06:48 +01:00
Piotr Miazga 05eccb31c1 Hygiene: use UserGetDefaultOptions instead of ExtensionRegistration
Changes:
 - removed onExtensionRegistration and related TODOs
 - introduced onUserGetDefaultOptions hook

Change-Id: I6648802c19d90df36a211d6494033aca30c078ca
2017-01-10 18:34:43 +01:00
Piotr Miazga c587a9785b Hygiene: Improve code quality by reaching 100% coverage
Changes:
 - added missing unit tests
 - introduced PopupsContextTestWrapper, removed all unit-tests logic
   from Hooks/Context classes
 - removed getModule() from Popups.hooks.php, it's hooks responsibility
   to keep single context instance
 - removed class_exists calls, use ExtensionRegistry instead
 - pass ExtensionRegistry as a dependency so it's easy to mock
 - reach 100% code coverage
 - introduce PoupsContext::isBetaOn() as it was used in many places

Change-Id: Id014efe72edf24628539c76968e53eb3e06709f4
2016-12-21 16:03:33 +01:00
Piotr Miazga 147772bbd8 ext.popups module should be loaded for anon users
Bug: T146889
Change-Id: I11dc6208334daa556d376f04aeb099a2642fcec0
2016-12-21 10:38:36 +00:00
Piotr Miazga 959bf40d9b Implement necessary wiring for preferences
Changes:
 - introduce unit tests
 - don't send module for anonymous users
 - renamed Module to Context
 - remove Config dependency from Popups.hooks.php

Bug: T146889
Change-Id: I3cbcdc1303411b28b613afa6dd1a00b410891471
2016-12-20 16:54:33 +01:00