FIX: resolve late registered form messages https://github.com/Crocoblock/issues-tracker/issues/19945

[ObitoTM]

No description provided.

[github-actions]

🤖 AI PR Review

Risk level: low

Review

Summary

• The PR ensures message types filtered via the 'jet-form-builder/message-types' hook are applied at retrieval time instead of only at construction. This fixes the issue where plugins registering message types later would not be picked up.

Files / functions touched

• modules/post-type/meta/messages-meta.php
  • __construct(): replaced direct filter application with get_filtered_messages()
  • get_filtered_messages(): new helper wrapping apply_filters()
  • messages(): now refreshes $this->messages on each call
  • query(): uses $this->messages() to build defaults and merges properly
  • get_by_key(): iterates over dynamic messages() instead of cached property

What looks good

• The change addresses the reported bug: late-registered filters will now be considered when messages are requested.
• The refactor is localized and preserves the original merge order (defaults first, then stored messages), keeping backward compatibility for output shape.
• PHPCS ignore for the hook name has been preserved.

Issues / Suggestions

1. Performance / caching

   • messages() now calls apply_filters() on every invocation. In most sites this will be negligible (message types are small), but on high-traffic or codepaths where messages() is called repeatedly this could be unnecessary overhead.\n - Suggestion: keep a lazy cache that refreshes only when empty or when explicitly requested, e.g. set $this->messages = null and only call get_filtered_messages() if $this->messages is empty; or provide an optional $force_refresh param to messages(). If there are scenarios where message types may change at runtime, provide an explicit invalidate() method.

2. Potential bug in get_by_key() (unrelated to this PR but worth fixing)

   • Current loop uses: if ( ! isset( $message[ $key ] ) ) { break; }
   • break exits the entire loop on the first message missing the key, which will skip other message types. This looks like it should be continue; to skip this item and keep searching. Please review and fix if indeed unintended.

3. Tests

   • Add or update unit/integration tests that verify:
     • Late-registered filters (hooked after plugin instantiation) are picked up when messages() / query() / get_by_key() are called.
     • Ensure merging behavior remains correct when DB messages exist and when none exist.
     • (Optional) Measure that repeated calls don't regress performance.

4. Backward compatibility

   • Change is backwards-compatible and improves behavior for third-party integrations that register message types on later hooks. No breaking changes to public APIs were made.

5. Minor

   • Consider documenting this behavior in a comment near get_filtered_messages() to explain why filtering happens on retrieval rather than construction (to avoid future regressions).

Tests missing / recommended

• No tests were provided in the patch. Please add tests covering late hook registration and get_by_key behavior.

Overall recommendation

• Approve after addressing the get_by_key() loop (change break to continue if that was the intent), and consider adding a lazy-cache or comment explaining intentional refresh behavior. Add tests for late-registration of filters.

Suggested changelog entry

- FIX: ensure late-registered message types (jet-form-builder/message-types) are applied when loading form messages (messages meta)