Skip to content

Fix WPML integration for Cloudinary media inserted prior to WPML activation#1159

Open
gabrielcld2 wants to merge 2 commits intodevelopfrom
bugfix/wpml-lazy-loading
Open

Fix WPML integration for Cloudinary media inserted prior to WPML activation#1159
gabrielcld2 wants to merge 2 commits intodevelopfrom
bugfix/wpml-lazy-loading

Conversation

@gabrielcld2
Copy link
Copy Markdown
Collaborator

The issue

When Cloudinary is enabled without WPML, cloudinary_relationships entries are inserted with a media_context set to default.

Once WPML is activated though, new cloudinary_relationships entries have their media_context set to the current language (e.g. en for English).
Furthermore, when posts are looking for the relationships data for specific images, it currently attempts to fetch them using the current language for context.
This works well in the case where WPML was activated prior to Cloudinary, however creates some issues when activation is done the other way around:

  • Posts created with cloudinary assets prior to WPML activation are tagged with the media_context set to default
  • Once WPML is active, the queries for the posts would then look for media_context = en and therefore won't find the corresponding cloudinary_relationships entries, resulting in not found assets
  • This issue is particularly noticeable on lazy-loading. Eager-loading implementation in Cloudinary already had a fallback in place to use the public ID found from the local URL. Which makes it work, but isn't a bulletproof solution

Approach

  • On lazy-loading, apply the same fallback to use the local URL public ID if relationship entry cannot be found
  • Adapt the query to also look for media_context = default, so that we can still fetch these if needed.

QA notes

  • Make sure Cloudinary plugin is active and configured, but WPML is not activated yet
  • Create a post and insert a Cloudinary asset within it
  • Then, activate WPML plugin
    • Note: if you don't have it, this is a paid plugin. You'll need to:
      • Download the OTGS Installer plugin from their site
      • Activate it, and make it install and activate the WPML Multilingual CMS plugin
  • View the previously created post to make sure the asset is shown
    • Try with both lazy-loading enabled and disabled, via the Cloudinary -> Lazy Loading settings page

Copy link
Copy Markdown

@PatelUtkarsh PatelUtkarsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Tricky one!


// phpcs:ignore WordPress.DB
$sql = $wpdb->prepare(
"SELECT * FROM {$table_name} WHERE `post_id` = %d AND `media_context` IN ({$context_query})", // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPress.DB.PreparedSQL.NotPrepared
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 suggestion: The query now searches for multiple contexts (e.g. en and default), but get_row() returns only one row. Without an ORDER BY, which row you get is up to MySQL. If a post has rows for both contexts, you want the current one (en) first and default only as a fallback.

Current code:

$sql  = $wpdb->prepare(
    "SELECT * FROM {$table_name} WHERE `post_id` = %d AND `media_context` IN ({$context_query})",
    $this->post_id,
    ...$contexts
);
$data = $wpdb->get_row( $sql, ARRAY_A );

Suggested fix:

$sql  = $wpdb->prepare(
    "SELECT * FROM {$table_name} WHERE `post_id` = %d AND `media_context` IN ({$context_query}) ORDER BY FIELD(`media_context`, %s) DESC LIMIT 1",
    $this->post_id,
    ...$contexts,
    $this->context
);
$data = $wpdb->get_row( $sql, ARRAY_A );

FIELD(media_context, %s) scores 1 for an exact match on the current context and 0 for everything else. DESC puts the exact match first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants