migrate to WP API (#219) #226
Conversation
stklcode
force-pushed
the
feature/219-wp-api
branch
3 times, most recently
from
4768355 to
1283b1c
Compare
|
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. |
Use a custom WP REST route for tracking requests. The tracking logic is moved up into Statify class. The API logic is separated into a new class, so additional endpoints can be added there. Override the default nonce behavior for the tracking endpoint to preserve the logged-in flag with nonce verification disabled.
stklcode
force-pushed
the
feature/219-wp-api
branch
from
1283b1c to
d34cb50
Compare
stklcode
force-pushed
the
feature/219-wp-api
branch
from
d34cb50 to
7f5756e
Compare
stklcode
force-pushed
the
feature/219-wp-api
branch
from
7f5756e to
31b5d48
Compare
|
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? |
|
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. |
|
Oh, yes, didn’t think about the chart and that it requires JS 🙈 Will dig deeper into the PR and give feedback :) |
|
Had two small notes/questions, besides that LGTM |
stklcode
force-pushed
the
feature/219-wp-api
branch
from
31b5d48 to
7fa384c
Compare
|
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. |
florianbrinkmann
left a comment
florianbrinkmann
left a comment
There was a problem hiding this comment.
LGTM. For me it looks like a breaking change, too, so would vote for a v2
stklcode
force-pushed
the
feature/219-wp-api
branch
2 times, most recently
from
0f8f781 to
92a2294
Compare
|
Rebased onto latest develop state:
|
|
Kudos, SonarCloud Quality Gate passed!
|
This PR addresses #219
Introduce API endpoints
We introduce 2 API routes to model current behaviors:
Tracking
POST
/statify/v1/trackTrack 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 returnfalse, 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/statsRetrieve 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.