Skip to content

Frontend API documentation, #PG-5031#36

Merged
lachiebol merged 25 commits into
5.x-devfrom
PG-5031-swaggerui
May 8, 2026
Merged

Frontend API documentation, #PG-5031#36
lachiebol merged 25 commits into
5.x-devfrom
PG-5031-swaggerui

Conversation

@lachiebol
Copy link
Copy Markdown
Contributor

@lachiebol lachiebol commented May 4, 2026

Description

New page added at
index.php?module=OpenApiDocs&action=swagger&idSite=1&period=day&date=yesterday

Issue No

Steps to Replicate the Issue

Checklist

  • [✔] Tested locally or on demo2/demo3?
  • [✔] New test case added/updated?
  • [✔] 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 changed the title Pg 5031 swaggerui Frontend API documentation, #PG-5031 May 4, 2026
Comment thread Controller.php Outdated
Comment thread public/swagger-ui/swagger-ui-bundle.js Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@AltamashShaikh Have just copied this in manually, might need some guidance on where to put swagger-ui. I wasn't sure if we needed to use npm or not

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@lachiebol We can use npm and ship the same with plugins, this will be easy to update next time. We can also add it under libs/swagger-ui/ and css via stylesheets/swagger-ui.

We should basically think, how will we upgrade the library next time if needed, won't npm be sufficient ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@AltamashShaikh Using npm now, there's an npm script as well to move the files we need into a directory /lib/

@lachiebol lachiebol requested a review from AltamashShaikh May 5, 2026 01:22
@lachiebol lachiebol added the Needs Review For pull requests that need a code review. label May 5, 2026
@lachiebol lachiebol marked this pull request as ready for review May 5, 2026 01:26
Comment thread lang/en.json Outdated
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.

@lachiebol

Image

The execute button and Request URL colours is too light, not readable

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.

Image

This should point to my instance right ?

@lachiebol
Copy link
Copy Markdown
Contributor Author

lachiebol commented May 6, 2026

Image This should point to my instance right ?

@AltamashShaikh That's handled on the spec generation side, if you have generated any specs with this PR they'll have your local instance (#34)

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.

Can view the authorize button and works well locally 👍

Image

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.

@lachiebol Codex review

Medium-risk issues

  - The new page is explicitly superuser-only in Controller.php:21 and Menu.php:19, but the backing API methods the page depends on still allow any user with view access in API.php:33 and
    API.php:55. If the intent is that Swagger docs are admin-only, this does not actually enforce it: a non-superuser can bypass the controller entirely and call OpenApiDocs.getPluginWhitelist /
    OpenApiDocs.getOpenApiSpec directly.
  - The plugin now registers plugins/OpenApiDocs/vue/dist/OpenApiDocs.css manually in OpenApiDocs.php:32, but Matomo already auto-adds plugins/$plugin/vue/dist/$plugin.css for Vue UMD plugins in /
    var/www/html/work/matomo/core/AssetManager/UIAssetFetcher/StylesheetUIAssetFetcher.php:92. That means the same CSS file is loaded twice. It is not catastrophic, but it adds avoidable asset
    weight and can complicate CSS debugging/order.

  Low-risk / polish

  - The README now contains machine-specific absolute links such as /home/lachlan/... in README.md:8. Those links will be broken for every other developer and on GitHub. They should be plain
    relative repo paths or inline code paths instead.

I think we can change the access to view/write user for menu, should keep it same as what we have for API.

@lachiebol lachiebol force-pushed the PG-5031-swaggerui branch from bf49c42 to 20758fc Compare May 6, 2026 21:36
@lachiebol
Copy link
Copy Markdown
Contributor Author

@lachiebol Codex review

Medium-risk issues

  - The new page is explicitly superuser-only in Controller.php:21 and Menu.php:19, but the backing API methods the page depends on still allow any user with view access in API.php:33 and
    API.php:55. If the intent is that Swagger docs are admin-only, this does not actually enforce it: a non-superuser can bypass the controller entirely and call OpenApiDocs.getPluginWhitelist /
    OpenApiDocs.getOpenApiSpec directly.
  - The plugin now registers plugins/OpenApiDocs/vue/dist/OpenApiDocs.css manually in OpenApiDocs.php:32, but Matomo already auto-adds plugins/$plugin/vue/dist/$plugin.css for Vue UMD plugins in /
    var/www/html/work/matomo/core/AssetManager/UIAssetFetcher/StylesheetUIAssetFetcher.php:92. That means the same CSS file is loaded twice. It is not catastrophic, but it adds avoidable asset
    weight and can complicate CSS debugging/order.

  Low-risk / polish

  - The README now contains machine-specific absolute links such as /home/lachlan/... in README.md:8. Those links will be broken for every other developer and on GitHub. They should be plain
    relative repo paths or inline code paths instead.

I think we can change the access to view/write user for menu, should keep it same as what we have for API.

@AltamashShaikh Fixed these 👍

Just rebased and pulled in the backend changes, it should be using the new API endpoint now.

Comment thread Menu.php Outdated
{
public function configureAdminMenu(MenuAdmin $menu): void
{
if (!Piwik::hasUserSuperUserAccess()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@lachiebol This should check view access right ?

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.

@lachiebol This does make sense, can we include it in vue or via twig ?

OpenApiDocs.php:23-41: Swagger assets are registered globally through AssetManager.getStylesheetFiles / getJavaScriptFiles, so every Matomo page pays for this feature. The bundled files are
    large: swagger-ui-bundle.js is about 1.53 MB, swagger-ui.css about 179 KB. Even before gzip, that is a substantial global tax for a single admin page, plus broader CSS/JS collision surface. This
    should be page-scoped if Matomo’s asset pipeline allows it

@lachiebol lachiebol requested a review from AltamashShaikh May 7, 2026 04:54
@lachiebol
Copy link
Copy Markdown
Contributor Author

@AltamashShaikh Fixed those, I also fixed the annotation generation issue.

You should be able to force the task to run with this:

core:run-scheduled-tasks "Piwik\Plugins\OpenApiDocs\Tasks.generateConfiguredPluginSpecs"

Comment thread lang/en.json
"SwaggerApi": "Swagger API",
"SwaggerPagePluginEmpty": "No plugins are configured in the OpenApiDocs whitelist.",
"SwaggerPageRequestFailed": "The plugin whitelist could not be loaded.",
"SwaggerPageSpecLoadFailed": "The OpenAPI spec could not be loaded for this plugin.",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@lachiebol We should also create an FAQ, explaining how you can build the missing spec or it will be auto generated if flag is set, so that user's don't reach out to support after activating the plugin.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@lachiebol We should also create an FAQ, explaining how you can build the missing spec or it will be auto generated if flag is set, so that user's don't reach out to support after activating the plugin.

Would this be a part of the checklist? Adding relevant documentation?

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.

@lachiebol LGTM 👍

@lachiebol lachiebol requested a review from AltamashShaikh May 7, 2026 22:57
@lachiebol lachiebol merged commit 41aa12f into 5.x-dev May 8, 2026
7 checks passed
@lachiebol lachiebol deleted the PG-5031-swaggerui branch May 8, 2026 03:10
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