Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenAPI: first pass on Plugins API - Shared Links #3378

Merged
merged 29 commits into from
Oct 2, 2023
Merged

Conversation

aerosol
Copy link
Member

@aerosol aerosol commented Sep 27, 2023

Changes

This PR implements Shared Links resource exposed via the new Plugins API (work in progress, initially aiming at new Wordpress plugin integration).

The API implements OpenAPI Specification 3 (via https://hexdocs.pm/open_api_spex/readme.html) for the benefit of easier 3rd party client integration with stronger contract enforcement.

The Plugins API is served off a new router (PlausibleWeb.Plugins.API.Router) that requests coming to /api/plugins are forwarded to. This is only for the sake of readability, the TCP endpoint remains the same for now.

The schema is served from /api/plugins/spec/openapi, read by the swagger UI at /api/plugins/spec/swagger-ui. Based on the schema, clients should be able to generate their implementations and keep them verified against the contract definition. At later stage, we'll look into generating and publishing documentation from it, for now the swagger UI should be good enough.

At later stage, incorporating schema generation into our CI flow will help us detect any breaking changes (we'll be able to compare the base schema to the PR submitted for example).

API authentication is done via tokens, carried over in the authorization header, implemented via #3373 and #3370.

The UI for provisioning authentication tokens will be done in a separate PR. Currently the Plausible.Plugins.API.Tokens.create/2 function can be used to play with the API locally.

Tests

  • Automated tests have been added
  • This PR does not require tests

Changelog

  • Entry has been added to changelog
  • This PR does not make a user-facing change

Documentation

  • Docs have been updated
  • This change does not need a documentation update

Dark mode

  • The UI has been tested both in dark and light mode
  • This PR does not change the UI

@aerosol aerosol changed the title OpenAPI: first pass on Plugins API [wip] OpenAPI: first pass on Plugins API Sep 27, 2023
@bundlemon
Copy link

bundlemon bot commented Sep 27, 2023

BundleMon

Unchanged files (7)
Status Path Size Limits
static/js/dashboard.js
318.59KB -
static/js/app.js
40.33KB -
static/css/app.css
14.54KB -
static/js/embed.host.js
5.58KB -
static/js/embed.content.js
5.08KB -
tracker/js/plausible.js
742B -
static/js/applyTheme.js
314B -

No change in files bundle size

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

@aerosol aerosol changed the title [wip] OpenAPI: first pass on Plugins API OpenAPI: first pass on Plugins API - Shared Links Sep 28, 2023
@aerosol aerosol requested a review from a team September 28, 2023 10:01
@aerosol aerosol marked this pull request as ready for review September 28, 2023 10:01
@aerosol aerosol added the deploy-to-staging Special label that triggers a deploy to a staging environment label Sep 28, 2023
WHEN g2.page_path IS NOT NULL THEN g2.page_path
WHEN g2.event_name IS NOT NULL THEN g2.event_name
WHEN g2.page_path IS NOT NULL THEN 'Page: ' || g2.page_path
WHEN g2.event_name IS NOT NULL THEN 'Event: ' || g2.event_name
Copy link
Member Author

Choose a reason for hiding this comment

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

On staging we've had goals equally named but of different types, so that's another edge case to be addressed. Ref #3343

Copy link
Member Author

Choose a reason for hiding this comment

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

We ran this on prod in correct order, but merging was a bit careless and reversed it

Copy link
Contributor

@ukutaht ukutaht left a comment

Choose a reason for hiding this comment

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

As you said - OpenAPI specs are a bit verbose to define but looks good to me! Love the assert_spec helper in tests :)

test/support/plugins_api_case.ex Outdated Show resolved Hide resolved
Comment on lines 14 to 15
def create(%Site{} = site, description, generate \\ Token.generate()) do
with generated_token <- generate,
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm maybe just name the argument generated_token?

Copy link
Member Author

Choose a reason for hiding this comment

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

nice! b7f65fe

@aerosol aerosol merged commit 082ec91 into master Oct 2, 2023
5 checks passed
@aerosol aerosol deleted the openapi-plugins branch October 2, 2023 09:18
zoldar added a commit that referenced this pull request Oct 10, 2023
The migration in question was renamed in order to fix order of executing migrations when run
from the ground up (via #3378).

As a side effect, it's executed again on databases that had it applied earlier, with
a different timestamp prefix.

As this migration is safe to run multiple times, it was modified to make forward
migration work gracefully when constraint already exists.
zoldar added a commit that referenced this pull request Oct 10, 2023
* Make FixBrokenGoals migration idempotent

The migration in question was renamed in order to fix order of executing migrations when run
from the ground up (via #3378).

As a side effect, it's executed again on databases that had it applied earlier, with
a different timestamp prefix.

As this migration is safe to run multiple times, it was modified to make forward
migration work gracefully when constraint already exists.

* Add `pending-migrations.sh` release script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-to-staging Special label that triggers a deploy to a staging environment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants