Skip to content

Replaced demo with local instance url in OpenApiDocs#34

Merged
lachiebol merged 5 commits into
5.x-devfrom
PG-5136-on-prem-api-docs
May 6, 2026
Merged

Replaced demo with local instance url in OpenApiDocs#34
lachiebol merged 5 commits into
5.x-devfrom
PG-5136-on-prem-api-docs

Conversation

@lachiebol
Copy link
Copy Markdown
Contributor

@lachiebol lachiebol commented Apr 28, 2026

Description

Removed mentions of demo.matomo.cloud.

Updated path resolver to post an event that allows other plugins to modify the artifact path used, will do a cloud PR to hook into that event

Issue No

Steps to Replicate the Issue

Checklist

  • [✔] Tested locally or on demo2/demo3?
  • [✔] New test case added/updated?
  • [NA] Are all newly added texts included via translation?
  • [NA] Are text sanitized properly? (Eg use of v-text v/s v-html for vue)
  • [NA] Version bumped?
  • [NA] Documentation updated?

@lachiebol lachiebol marked this pull request as ready for review April 29, 2026 02:29
@lachiebol lachiebol added the Needs Review For pull requests that need a code review. label Apr 29, 2026
@lachiebol
Copy link
Copy Markdown
Contributor Author

@AltamashShaikh I'll also do a cloud PR that hooks into OpenApiDocs.getArtifactBasePath and changes the path

Copy link
Copy Markdown
Contributor

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

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

Looks good, few concerns by AI to check

 High-risk issues (blocking)

  - The new event dispatch breaks the unit test path that used to work without a container. testReturnsPluginLocalPathsWhenCloudIsDisabled() now constructs a real
    PathResolver in tests/Unit/Specs/PathResolverTest.php:26, and getSpecDirectory() immediately calls Piwik::postEvent() through Specs/PathResolver.php:82. Running
    vendor/bin/phpunit --bootstrap tests/PHPUnit/proxy/includes.php plugins/OpenApiDocs/tests/Unit/Specs/PathResolverTest.php from the Matomo root errors with
    ContainerDoesNotExistException. That is a concrete regression introduced by this refactor.


  Low-risk / polish

  - Several docblocks in Annotations/AnnotationGenerator.php:857 still describe the behavior as querying demo.matomo.cloud, which is now inaccurate.
  - The PathResolver test suite mocks dispatchArtifactBasePathEvent() for most cases but not the default-path test, which is why the regression slipped through. The
    tests should consistently isolate external event dispatch.

@lachiebol
Copy link
Copy Markdown
Contributor Author

High-risk issues (blocking)

  • The new event dispatch breaks the unit test path that used to work without a container. testReturnsPluginLocalPathsWhenCloudIsDisabled() now constructs a real
    PathResolver in tests/Unit/Specs/PathResolverTest.php:26, and getSpecDirectory() immediately calls Piwik::postEvent() through Specs/PathResolver.php:82. Running
    vendor/bin/phpunit --bootstrap tests/PHPUnit/proxy/includes.php plugins/OpenApiDocs/tests/Unit/Specs/PathResolverTest.php from the Matomo root errors with
    ContainerDoesNotExistException. That is a concrete regression introduced by this refactor.

@AltamashShaikh Is this an issue? We just require the full bootstrap.php rather than includes.php. Seems like it's more of an integration test now that we rely on Piwik::postEvent()




› matomo_5x-dev  5.x-dev !2 ?8  ddev matomo:console tests:run plugins/OpenApiDocs                                                                                                                                                                        ✔
  Executing command: cd /var/www/html/tests/PHPUnit &&  /var/www/html/vendor/bin/phpunit   ../../plugins/OpenApiDocs

  If these tests time out consistently, it can be helpful to temporarily set testdox=true in the phpunit.xml.dist in order to see which test is causing the issue.
  PHPUnit 8.5.52 by Sebastian Bergmann and contributors.

  Runtime:       PHP 8.2.30
  Configuration: /var/www/html/tests/PHPUnit/phpunit.xml.dist

  ...............................................................  63 / 536 ( 11%)
  ............................................................... 126 / 536 ( 23%)
  ............................................................... 189 / 536 ( 35%)
  ............................................................... 252 / 536 ( 47%)
  ............................................................... 315 / 536 ( 58%)
  ............................................................... 378 / 536 ( 70%)
  ............................................................... 441 / 536 ( 82%)
  ............................................................... 504 / 536 ( 94%)
  ................................                                536 / 536 (100%)

@lachiebol lachiebol requested a review from AltamashShaikh May 4, 2026 21:54
@lachiebol lachiebol merged commit 423b53c into 5.x-dev May 6, 2026
7 checks passed
@lachiebol lachiebol deleted the PG-5136-on-prem-api-docs branch May 6, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review For pull requests that need a code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants