Commit graph

22 commits

Author SHA1 Message Date
Bartosz Dziewoński c7723baf72 CommentParser: Replace uses of Title with TitleValue
Another small step towards removing the reliance on global state.

Change-Id: Ifb4a5bcbef6606d02f1c7aa7385d72822cb0bad0
2022-03-18 18:24:34 +00:00
Bartosz Dziewoński b68832ace0 Fix parsing of non-English titles in tests
We were calling Title::newFromText() before setupEnv(), which meant
that the title for each test case was parsed using the default rules
for English, rather than the rules for the specified wiki.

This only makes a practical difference for tests with self-links.
Changed the only such test to demonstrate the fix.

Change-Id: I45561f1c9f0d149e2b743f0000b742bf6fc014af
2022-03-18 18:24:07 +00:00
Bartosz Dziewoński 77614a2d02 tests: Fix root node / container handling
Since times immemorial, and for reasons lost to history, our test code
was adding an extra <div> wrapper before parsing the HTML used for
tests. This wasn't a problem, until now, because I want to add some
tests for T303396 that need to check that the *real* wrappers present
in some test cases are handled correctly.

Changes to test cases mostly remove a leading "0/" from serialized
ranges, corresponding to removing the extra wrapper.

Change-Id: Ia50e3590538c8cd274b02d2a937ba1a3fbb4ac89
2022-03-10 18:43:58 +01:00
Ed Sanders dc8b4e8d4f Highlight all comments since the oldest in a thread bundle
For topic subscriptions, further restrict this to comments
in the same thread.

Bug: T302014
Change-Id: Ifba218871122901031a891034e709b886fc406da
2022-02-28 21:58:10 +00:00
Bartosz Dziewoński 8e44b43df0 Split off ThreadItemSet from CommentParser
Goal:
-----
Finishing the work from Iadb7757debe000025e52770ca51ebcf24ca8ee66
by changing CommentParser::parse() to return a data object, instead of
the whole parser.

Changes:
--------
ThreadItemSet.php:
ThreadItemSet.js:
* New data class to access the results of parsing a discussion. Most
  methods and properties are moved from CommentParser with no changes.

CommentParser.php:
Parser.js:
* parse() returns a new ThreadItemSet.
* Remove methods moved to ThreadItemSet.
* Placeholder headings are generated slightly differently, as we process
  things in a different order.
* Grouping threads and computing IDs/names is no longer lazy. We always
  needed IDs/names anyway.
* computeId() explicitly uses a ThreadItemSet to check the existing IDs
  when de-duplicating.

controller.js:
* Move the code for turning some nodes annotated by CommentFormatter
  into a ThreadItemSet (previously a Parser) from controller#init to
  ThreadItemSet.static.newFromAnnotatedNodes, and rewrite it to handle
  assigning parents/replies and recalculating legacy IDs more nicely.
* mw.dt.pageThreads is now a ThreadItemSet.

Change-Id: I49bfe019aa460651447fd383f73eafa9d7180a92
2022-02-21 16:22:32 +00:00
Bartosz Dziewoński 4613ae78e7 Change CommentParser into a service
Goal:
-----
To have a method like CommentParser::parse(), which just takes a node
to parse and a title and returns plain data, so that we don't need to
keep track of the config to construct a CommentParser object (the
required config like content language is provided by services) and
we don't need to keep that object around after parsing.

Changes:
--------
CommentParser.php:
* …is now a service. Constructor only takes services as arguments.
  The node and title are passed to a new parse() method.
* parse() should return plain data, but I split this part to a separate
  patch for ease of review: I49bfe019aa460651447fd383f73eafa9d7180a92.
* CommentParser still cheats and accesses global state in a few places,
  e.g. calling Title::makeTitleSafe or CommentUtils::getTitleFromUrl,
  so we can't turn its tests into true unit tests. This work is left
  for future commits.

LanguageData.php:
* …is now a service, instead of a static class.

Parser.js:
* …is not a real service, but it's changed to behave in a similar way.
  Constructor takes only the required config as argument,
  and node and title are instead passed to a new parse() method.

CommentParserTest.php:
parser.test.js:
* Can be simplified, now that we don't need a useless node and title
  to test internal methods that don't use them.

testUtils.js:
* Can be simplified, now that we don't need to override internal
  ResourceLoader stuff just to change the parser config.

Change-Id: Iadb7757debe000025e52770ca51ebcf24ca8ee66
2022-02-19 19:51:57 +01:00
Ed Sanders 34011b7a07 Parser: Pass in title of page being parsed
Will be used to parse selflinks in the future.

Change-Id: I2bc29d1c5c69cb6309f582f162f9af7d96ce8913
2022-01-12 21:17:59 +00:00
Ed Sanders 1fed7115f4 Tests: Add original titles to test cases
These are not used for anything yet, but soon the parser will
want to know the title of the page it is parsing.

Change-Id: I02fa5d63fae78f3e92032d93bc27ac5c744faecb
2022-01-12 22:16:03 +01:00
Ed Sanders a86d308d66 CommentItem.php: Store timestamp object instead of string
We do something similar in CommentItem.js with a moment object.
The object can be converted to a string when required.

Change-Id: Id7221e9201db0d89c3b771574634c878c9515ca0
2021-11-09 16:37:45 +00:00
Bartosz Dziewoński a6a547f2b2 Add some tests covering ThreadItem::getHTML() and related methods
* ThreadItem::getText
* CommentItem::getBodyText (used when generating notifications)
* ThreadItem::getHTML (may soon be used in API)
* CommentItem::getBodyHTML (may soon be used in API)
* ImmutableRange::cloneContents (the common implementation for all
  of the above)

The outputs are only lightly reviewed. This is mostly meant to
document the current behavior rather than the expected behavior,
to avoid making unintentional changes while refactoring.

Change-Id: I14471ee4969aa3d0b5577d9de2a6d4462fab4d09
2021-08-24 07:54:09 +02:00
Bartosz Dziewoński 8de8d80cde Deal with document body consistently
Use `DOMCompat::getBody( ... )` as a nicer getter than
`->getElementsByTagName( 'body' )->item( 0 )`.

Remove overly defensive checks and redundant annotations on its
return value. Since we're dealing with HTML documents throughout,
the document body is guaranteed to exist.

We previously needed some of them to convince Phan when it thought
the body may be null, but this seems to no longer be needed.

Change-Id: If7aee7b6adbfa78269c7ba28b26a6eaa21fe935b
2021-08-03 15:12:55 +02:00
libraryupgrader b0884b177c build: Updating dependencies
composer:
* mediawiki/mediawiki-codesniffer: 36.0.0 → 37.0.0

npm:
* postcss: 7.0.35 → 7.0.36
  * https://npmjs.com/advisories/1693 (CVE-2021-23368)
* glob-parent: 5.1.1 → 5.1.2
  * https://npmjs.com/advisories/1751 (CVE-2020-28469)
* trim-newlines: 3.0.0 → 3.0.1
  * https://npmjs.com/advisories/1753 (CVE-2021-33623)

Change-Id: I7a71e23da561599da417db3b3077b78d91173bbc
2021-07-22 16:29:04 +00:00
libraryupgrader 12fb65b9f1 build: Updating composer dependencies
* mediawiki/mediawiki-codesniffer: 35.0.0 → 36.0.0
* php-parallel-lint/php-parallel-lint: 1.2.0 → 1.3.0

Change-Id: I5c152292e83e7f3441e2c08b7d0ad23ac90f194b
2021-05-05 11:14:52 +00:00
Bartosz Dziewoński 1c3fada1fb Make CommentUtilsTest a proper unit test
Documentation:
https://www.mediawiki.org/wiki/Manual:PHP_unit_testing/Writing_unit_tests_for_extensions#Two_types_of_tests

We can do this because the tested methods do not depend on any globals
or on MediaWiki being installed.

In addition to being the new hotness, MediaWikiUnitTestCase allows the
test classes that use it instead of MediaWikiTestCase to start up much
faster. In my testing, running this test case individually now takes
0.35s, compared to 1.1s before.

Try:
* With new code:
  time php tests/phpunit/phpunit.php extensions/DiscussionTools/tests/phpunit/unit/CommentUtilsTest.php
* With old code:
  time php tests/phpunit/phpunit.php extensions/DiscussionTools/tests/phpunit/CommentUtilsTest.php

Change-Id: I771b1f3d101a394ee869e42547d9ae7839397752
2021-02-02 15:37:17 +01:00
Ed Sanders 9ba6c3d159 CommentItem/HeadingItem: Make more constructor args required
This ensures the getters always return the promised types.

Change-Id: I1a3c909f5395463ef7a89d896ead1520b2a17509
2021-01-08 20:45:29 +00:00
Bartosz Dziewoński 3137d76f40 Connect sub-threads to their parent threads
Our threads now also contain all replies to their sub-threads.
This is similar to how sections work in MediaWiki, where the parent
section also contains the content of all the lower-level sections.
We're going to need this for notifications about replies in a thread.

Bug: T264478
Change-Id: I241fc58e2088a7555942824b0f184ed21e3a8b6f
2020-10-22 02:05:02 +02:00
Bartosz Dziewoński a29c49ae70 Better way to update expected test outputs
Use an environment variable "DISCUSSIONTOOLS_OVERWRITE_TESTS".

Change-Id: I017112b7d6b1df9497f01f3f97f34e0935ca16f8
2020-10-19 23:53:30 +02:00
Bartosz Dziewoński f0225243e0 tests: Fix some issues with overwriting outputs from PHP tests
Follow-up to ccd9e411d2.

* Fix variable name in CommentTestCase::overwriteHtmlFile()
* Overwrite before assertions, because they abort execution if they fail

Change-Id: I5bba016ba93f9dd1994325ae82c3105ba11cf033
2020-08-11 06:45:45 +02:00
Bartosz Dziewoński ccd9e411d2 Allow updating the expected results when running PHP tests
This is similar to the code we already have in JS tests, but instead
of printing to the console where you have to copy-paste from, it just
overwrites the files.

Also, update all of the expected results by this method.

Changes in the expected outputs:
* In JSON files, the "warnings" are now always in the same place
  regardless of the type of the warning.
* In all HTML files, self-closing tags now include the trailing slash,
  some characters are no longer encoded as entities when not necessary,
  and attributes may be single-quoted when that makes them shorter.
* In Parsoid HTML files, the header is no longer terribly mangled.

Other notes:
* CommentParserTest.php: Change the output of serializeComments()
  to be in similar order as in JS, to reduce the diffs in this commit
  and because it's a better order for humans.
* modifier.test.js: Remove some hacks that were working around small
  inconsistencies between the previous expected outputs and the actual
  outputs.

Change-Id: I9f764640dae823321c0ac35898fa4db03f1ca364
2020-08-04 03:05:28 +02:00
Ed Sanders a2431fe006 Refactor CommentParser
* Pass rootNode to the constructor
* Rename getters to match CommentItem/HeadingItem/ThreadItem
  value classes.
* Always build the thread tree so CommentItem's always have
  and ID and replies/parent.

Change-Id: I508be9534de59016ff806e3d84edcbb1c76cb0c6
2020-07-20 23:38:10 +01:00
Ed Sanders a4636d39fc Move #getTranscludedFrom from parser to ThreadItem
Also requires moving getTitleFromUrl to CommentUtils

Change-Id: I9cb83a3fdd456eba66899433b866ce7a7f00eeb5
2020-07-20 15:56:48 +01:00
Ed Sanders 7ae5bbf384 Move #getAuthors from parser to ThreadItem
Change-Id: I16e513000e5366b3044b17a99da07d8d0f47a61f
2020-07-20 15:13:59 +01:00