PDPS-2029: Index ACF custom fields into a new text-2 resource field - #10
Conversation
StrawHat-Dery
left a comment
There was a problem hiding this comment.
Overall, the core ACF indexing flow looks good and the existing WordPress content remains unaffected. I verified that standard post/page content continues to index into text-1, while supported ACF content is added to text-2. Updating a non-empty ACF value also updates the existing text-2 field as expected.
Automated checks also passed successfully:
PHP lint: PASS
PHPUnit: PASS — 120 tests / 684 assertions
During runtime testing, however, I confirmed one issue that I believe should be addressed before merge:
Should Fix/High Value — stale text-2 after ACF content is cleared
When an ACF value is removed completely, the previously indexed text-2 value remains in PAR and continues to be searchable. Re-adding a non-empty value updates text-2 again, so the issue appears specific to the empty/cleared state not removing the existing upstream field.
I also confirmed a second issue with ACF Page Link fields. A real Page Link pointing to another published WordPress page was not included in text-2, while a normal ACF Text field on the same resource continued to sync successfully. The current Page Link handling appears to expect a post ID/object even though the formatted ACF value is a URL/permalink. I’ve left an inline comment with more detail.
One additional static-review item is the Flexible Content handling, which appears likely to miss real ACF layout structures due to the layout-name/key lookup. I wasn’t able to runtime-test that path because it requires functionality outside the free ACF setup used for this review, so I would treat that as a high-value follow-up rather than a runtime-confirmed blocker.
Aside from those items, the core ACF-to-text-2 implementation behaved as expected in the scenarios tested.
- Always send text-2 so clearing ACF fields removes stale indexed content - Resolve Page Link fields via their permalink URL, not a post ID - Fix Flexible Content layout lookup (layouts is a sequential list, not keyed by name)
StrawHat-Dery
left a comment
There was a problem hiding this comment.
Follow-up review for 34aed39.
The peer-review fixes look good, and the previously reported runtime issues have now been re-tested successfully.
Runtime validation
- Stale
text-2— Fixed. Clearing all ACF values fromPAR Widget Testremoved the previously indexedtext-2content. - Page Link indexing — Fixed. Re-adding
Branch Nameand a real Page Link value resulted in both values appearing correctly undertext-2. - Previously non-ACF resource — Passed. Adding ACF values to
Sample Page, which previously had notext-2, successfully created and populatedtext-2.
text-1 remained unaffected, and supported ACF values continue to be flattened into a single text-2 field as intended.
Remaining follow-ups
- Flexible Content was not runtime-tested because this environment does not have ACF Pro. The updated layout-name lookup looks correct from static review, and no concrete regression was identified.
- Alternate return-format handling for single taxonomy values, image/file/user/select variants, and Page Link archive URLs remains unchanged and can be tracked separately.
Merge readiness
No blocking or high-value issues remain from this review.
Ready to merge from my side.
ACF field data (e.g. Branch pages' location, hours, contact details) was never read by the sync pipeline, which only sent post_content as text-1. Adds AcfAdapter to map every ACF field type into plain text, sent alongside post_content as a new text-2 PLAIN field on each indexed resource, so this content becomes searchable.