Skip to content

Conversation

@SomeMWDev
Copy link
Contributor

@SomeMWDev SomeMWDev commented Sep 27, 2025

Downstream task: https://issue-tracker.miraheze.org/T14342

ContentParseParams::getPage returns a PageReference.
This fixes:

Maps\MapsFactory::newSemanticGeoJsonStore(): Argument #2 ($subjectPage) must be of type MediaWiki\Title\Title, MediaWiki\Page\PageIdentityValue given, called in /srv/mediawiki/1.44/extensions/Maps/src/GeoJsonPages/GeoJsonContentHandler.php on line 51

from /srv/mediawiki/1.44/extensions/Maps/src/MapsFactory.php(238)
#0 /srv/mediawiki/1.44/extensions/Maps/src/GeoJsonPages/GeoJsonContentHandler.php(51): Maps\MapsFactory->newSemanticGeoJsonStore(MediaWiki\Parser\ParserOutput, MediaWiki\Page\PageIdentityValue)
#1 /srv/mediawiki/1.44/includes/content/ContentHandler.php(1692): Maps\GeoJsonPages\GeoJsonContentHandler->fillParserOutput(Maps\GeoJsonPages\GeoJsonContent, MediaWiki\Content\Renderer\ContentParseParams, MediaWiki\Parser\ParserOutput)
#2 /srv/mediawiki/1.44/includes/content/Renderer/ContentRenderer.php(75): MediaWiki\Content\ContentHandler->getParserOutput(Maps\GeoJsonPages\GeoJsonContent, MediaWiki\Content\Renderer\ContentParseParams)
#3 /srv/mediawiki/1.44/includes/Revision/RenderedRevision.php(260): MediaWiki\Content\Renderer\ContentRenderer->getParserOutput(Maps\GeoJsonPages\GeoJsonContent, MediaWiki\Page\PageIdentityValue, MediaWiki\Revision\RevisionStoreRecord, MediaWiki\Parser\ParserOptions, array)
#4 /srv/mediawiki/1.44/includes/Revision/RenderedRevision.php(233): MediaWiki\Revision\RenderedRevision->getSlotParserOutputUncached(Maps\GeoJsonPages\GeoJsonContent, array)
#5 /srv/mediawiki/1.44/includes/Revision/RevisionRenderer.php(236): MediaWiki\Revision\RenderedRevision->getSlotParserOutput(string, array)
#6 /srv/mediawiki/1.44/includes/Revision/RevisionRenderer.php(169): MediaWiki\Revision\RevisionRenderer->combineSlotOutput(MediaWiki\Revision\RenderedRevision, MediaWiki\Parser\ParserOptions, array)
#7 /srv/mediawiki/1.44/includes/Revision/RenderedRevision.php(196): MediaWiki\Revision\RevisionRenderer->MediaWiki\Revision\{closure}(MediaWiki\Revision\RenderedRevision, array)
#8 /srv/mediawiki/1.44/includes/poolcounter/PoolWorkArticleView.php(110): MediaWiki\Revision\RenderedRevision->getRevisionParserOutput()
#9 /srv/mediawiki/1.44/includes/poolcounter/PoolWorkArticleView.php(81): MediaWiki\PoolCounter\PoolWorkArticleView->renderRevision()
#10 /srv/mediawiki/1.44/includes/poolcounter/PoolCounterWork.php(173): MediaWiki\PoolCounter\PoolWorkArticleView->doWork()
#11 /srv/mediawiki/1.44/includes/page/ParserOutputAccess.php(364): MediaWiki\PoolCounter\PoolCounterWork->execute()
#12 /srv/mediawiki/1.44/includes/page/Article.php(834): MediaWiki\Page\ParserOutputAccess->getParserOutput(MediaWiki\Page\WikiPage, MediaWiki\Parser\ParserOptions, MediaWiki\Revision\RevisionStoreRecord, int)
#13 /srv/mediawiki/1.44/includes/page/Article.php(551): MediaWiki\Page\Article->generateContentOutput(MediaWiki\User\User, MediaWiki\Parser\ParserOptions, int, MediaWiki\Output\OutputPage, array)
#14 /srv/mediawiki/1.44/includes/actions/ViewAction.php(80): MediaWiki\Page\Article->view()
#15 /srv/mediawiki/1.44/includes/actions/ActionEntryPoint.php(728): MediaWiki\Actions\ViewAction->show()
#16 /srv/mediawiki/1.44/includes/actions/ActionEntryPoint.php(505): MediaWiki\Actions\ActionEntryPoint->performAction(MediaWiki\Page\Article, MediaWiki\Title\Title)
#17 /srv/mediawiki/1.44/includes/actions/ActionEntryPoint.php(143): MediaWiki\Actions\ActionEntryPoint->performRequest()
#18 /srv/mediawiki/1.44/includes/MediaWikiEntryPoint.php(202): MediaWiki\Actions\ActionEntryPoint->execute()
#19 /srv/mediawiki/config/initialise/entrypoints/index.php(71): MediaWiki\MediaWikiEntryPoint->run()
#20 {main}

Summary by CodeRabbit

  • Bug Fixes
    • More reliable GeoJSON saving by normalizing page identity before storage, reducing failures and errors when creating or updating map data.
  • Chores
    • Updated extension compatibility: minimum required platform version raised from 1.40.0 to 1.43.0.

@coderabbitai
Copy link

coderabbitai bot commented Sep 27, 2025

Warning

Rate limit exceeded

@SomeMWDev has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 13 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between cfc66ba and 71c63df.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (1 hunks)
📝 Walkthrough

Walkthrough

Normalize the page reference when storing GeoJSON by converting non-Title page refs to a Title via Title::newFromPageReference and pass that Title to newSemanticGeoJsonStore; update extension requirement to MediaWiki >= 1.43.0.

Changes

Cohort / File(s) Summary
GeoJSON page handling
src/GeoJsonPages/GeoJsonContentHandler.php
Import Title; when cpoParams page is not a Title, convert it using Title::newFromPageReference; pass the normalized Title ($subjectPage) to newSemanticGeoJsonStore before storing the GeoJSON text.
Extension metadata
extension.json
Bump requires.MediaWiki from >= 1.40.0 to >= 1.43.0.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately describes the primary change introduced by the pull request, which is ensuring the subject page is cast to a Title instance when it’s not already one. It is concise, clear, and directly related to the main modification in the GeoJsonContentHandler without unnecessary detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f3df101 and f05a117.

📒 Files selected for processing (1)
  • src/GeoJsonPages/GeoJsonContentHandler.php (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/GeoJsonPages/GeoJsonContentHandler.php (1)
src/MapsFactory.php (2)
  • MapsFactory (51-284)
  • newSemanticGeoJsonStore (238-245)

This fixes "TypeError: Maps\MapsFactory::newSemanticGeoJsonStore(): Argument ProfessionalWiki#2 ($subjectPage) must be of type MediaWiki\Title\Title, MediaWiki\Page\PageIdentityValue given"
Raise MW requirement to 1.41.
@SomeMWDev
Copy link
Contributor Author

Raised the MW requirement to 1.41 since Title::newFromPageReference was added in that version and 1.40 is EOL anyway (1.41 is too, for that matter, so in theory it should be raised to 1.43)

@JeroenDeDauw
Copy link
Member

Better to bump to MW 43 right away then, since every time we increase the needed MW version, we need a major release

https://github.com/ProfessionalWiki/Maps/blob/master/INSTALL.md

1.40-1.42 have been EOL for a while.
Also remove redundant PHP requirement, since MW 1.43 requires PHP 8.
@SomeMWDev
Copy link
Contributor Author

Better to bump to MW 43 right away then, since every time we increase the needed MW version, we need a major release

https://github.com/ProfessionalWiki/Maps/blob/master/INSTALL.md

Done, also removed the redundant PHP requirement and updated the CI config.

@JeroenDeDauw JeroenDeDauw merged commit be70f40 into ProfessionalWiki:master Sep 27, 2025
2 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants