PDPS-2095: Unblock admin render from blocking sync checks - #11
Conversation
174665e to
773c3e4
Compare
StrawHat-Dery
left a comment
There was a problem hiding this comment.
Overall, this PR appears to address the primary admin-page hang successfully.
I verified that the main plugin admin page now renders promptly, and the slower sync/status work happens afterward through admin-ajax rather than blocking the initial document request. The Indexation page remained usable while that background request completed.
I also tested Taxonomy labeling with nuclia_labelsets_cache explicitly cleared. The page itself still rendered promptly (~417 ms DOMContentLoaded / ~433 ms full load), so I did not reproduce a document-level hang on that tab.
SHOULD FIX / HIGH VALUE — cold-cache Add Mapping shows no PAR labelsets
This was runtime-confirmed. On the first visit after clearing the labelset cache, the WordPress taxonomy and terms appeared correctly, but the PAR Labelset dropdown contained no labelsets. After refreshing the page once, the labelsets appeared and could be selected.
This matches the current flow where mapping.labelsets is localized from the cache before the later live labelset fetch populates it. New mapping rows therefore use the already-localized empty list until the page is reloaded.
NON-BLOCKING / FOLLOW-UP — synchronous labelset fetch remains in Taxonomy labeling
Static review shows that Taxonomy labeling still has a synchronous labelset-fetch path during render. I did not reproduce a hang in this environment, so I would treat this as a residual risk if PAR is slow or unreachable rather than a confirmed defect.
The Scheduler change to use query_actions( ..., 'count' ) also looks like a straightforward performance improvement and did not raise any issues in review.
Merge readiness: the main purpose of the PR is working as intended at runtime, and I do not see a blocker to merging for the admin-hang fix. The cold-cache Add Mapping behavior is a confirmed UX/functional issue that should be fixed or explicitly accepted as a follow-up.
|
Follow-up review for e336451. The original admin-render hang fix still holds, and the cold-start Add Mapping issue is now runtime-confirmed fixed. With no existing mapping and an empty nuclia_labelsets_cache, the Taxonomy Labeling page renders normally and PAR labelsets load on the first visit without requiring a refresh. One remaining edge case was reproduced with an existing saved mapping and an empty labelset cache. On the cold first render, the saved labelset selection is not preserved in the dropdown. If the user manually reselects the labelset, the saved term-label selections are rebuilt unchecked. The underlying saved mapping is not changed simply by viewing the page, but saving while the UI is in that transient state can overwrite/drop the persisted taxonomy mapping. Classification: SHOULD FIX / HIGH VALUE The main PR goals are otherwise working as intended: admin / Indexation render remains non-blocking Recommended fix: preserve the persisted labelset selection during the cold-cache AJAX backfill, and preserve saved term-label selections if the checkboxes are rebuilt. I’d recommend addressing this edge case before merge if timing allows. The original hang and cold-start issues themselves are resolved. |
Fixes a 504 timeout on every tab of the admin page, caused by synchronous blocking work during render. - AdminPage: read cached background-sync status and cached labelsets during render instead of running a full upstream reconcile/recover pass and a live labelsets fetch. Automatic sync and cache refresh still happen via the existing AJAX poll and explicit save/manual sync actions, just not during page load. - ApiClient: add get_labelsets_cached(), a cache-only reader with no upstream HTTP call, used by the render path above. - Scheduler: count_actions() now issues a real SQL COUNT via ActionScheduler::store()->query_actions(..., 'count') instead of loading every matching action row into memory just to count them.
e336451 to
8b26b36
Compare
StrawHat-Dery
left a comment
There was a problem hiding this comment.
Follow-up review for 6f74493.
The remaining taxonomy-labeling cold-cache issues are now runtime-confirmed fixed.
Validated successfully:
Main PAR admin / Indexation render remains non-blocking; slower work happens later through admin-ajax.
With an empty labelset cache, Add Mapping now loads PAR labelsets on the first visit without requiring a refresh.
Existing saved mappings remain intact on a cold-cache load: wp-taxonomy-test stayed selected and PAR Test Category stayed checked through the background fetch.
Saving from that cold-cache state preserved the existing mapping after reload.
Remaining follow-ups:
Re-selecting the same labelset can still rebuild checkboxes unchecked.
The “No labelsets available…” message can remain after a successful background fetch.
There is still no browser-level automated test for the saved-mapping + empty-cache flow.
No blocking or high-value issues remain from this review.
Ready to merge from my side.
Fixes a 504 timeout on every tab of the admin page, caused by synchronous blocking work during render.