pluginkollektiv / statify

Statify – statistics plugin for WordPress
https://wordpress.org/plugins/statify/
GNU General Public License v3.0
77 stars 23 forks source link

migrate to WP API (#219) #226

Closed stklcode closed 1 year ago

stklcode commented 2 years ago

This PR addresses #219

Introduce API endpoints

We introduce 2 API routes to model current behaviors:

Tracking

POST /statify/v1/track Track a visit via JS. This is intended to replace the WP AJAX calls.

We override the default authentication mechanism (ref for the track route. This would unauthenticate every request, i.e. is_user_logged_in() will always return false, if no nonce is present and returns 403 if the given nonce is expires.

So we do not send any nonce at all, if verification is disabled, and verify the nonce from request parameters manually, if necessary.

Stats retrieval

GET /statify/v1/stats Retrieve statistics data. Is uses the same authentication requirements as before and returns dashboard data as JSON:

{
  "visits": {
    "2022-02-18": 17,
    "2022-02-19": 4
  },
  "referrer": [
    { "host": "example.com", "url": "https://example.com/test/", "count": 3 }
  ],
  "target": [
    { "url": "/testpage/", "count": 10 }
  ],
  "totals": {
    "today": 4,
    "alltime": 21,
    "since": "2022-02-18"
  }
}

Tracking changes

Javascript and AMP tracking issue POST requests to the new API endpoint instead of using admin-ajax.php

Dashboard widget changes

The dashboard widget is no longer privisioned from a hidden table. The backend just generated a skeleton with placeholders that will be replaced after the data is retrieved from the API.

statify_widget_placeholder

stklcode commented 2 years ago

The "since 1.9" is not necessarily correct. I'd say this is quite a major change, so we should probably name it 2.0 if it's getting merged.

We switched from some custom logic to WP AJAX in 1.7 though and one might say the "tracking mechanism itself is not designed as public API.

florianbrinkmann commented 1 year ago

I know we cannot use many features of modern WP without JS, but I wonder if we couldn’t load the initial data via PHP and not JS?

stklcode commented 1 year ago

Sure we could.

Top lists could be rendered inline instantly. But charts are non-blocking JS-generated anyway, so there is always at least a minimal delay in the UI, even if data is read from hidden markup like before. It would just introduce some (rather small) additional JS code to have two different ways to fetch data.

Main benefit of dynamic requests I see here is that we could skip the data completely, if the widget is initially collapsed and fetch it when it’s actually opened (i.e. add a simple listener to the expand icon). So there is a chance to reduce overhead when accessing the dashboard. Same trade-off as always.

florianbrinkmann commented 1 year ago

Oh, yes, didn’t think about the chart and that it requires JS 🙈 Will dig deeper into the PR and give feedback :)

florianbrinkmann commented 1 year ago

Had two small notes/questions, besides that LGTM

stklcode commented 1 year ago

Resolved both and rebased the branch. Think we should not squash tracking and dashboard changes together, so I squashed both changes into the corresponding commits.

As previously said, we should think about a target version.

We did a AJAX migration back in 1.7.0. IIRC we considered the routine being internal logic, even though it apparently was kind of breaking for some users who had AJAX access blocked or long caching times. I know there are some folks out there that trigger tracking differently, so my preference would be 2.0 here and not 1.x. (even though the inline docs state @since 1.9 now - but sanity checks on these annotation should be part of release work anyway, so might be postponed).

stklcode commented 1 year ago

Rebased onto latest develop state:

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 11 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication