🇨🇦Canada @Shiraz Dindar

Sooke, BC
Account created on 22 September 2006, about 18 years ago
#

Recent comments

🇨🇦Canada Shiraz Dindar Sooke, BC

I'm just following up my last comment as it assumes the relationship field is a reverse relationship.

Here's the code I wrote to take care of views with translated relationship fields causing duplicates in the view results. In now works with forward (normal) in addition to reverse relationships:

/**
 * Implements hook_views_query_alter().
 */
function my_module_views_query_alter(ViewExecutable $view, QueryPluginBase $query) {
  // Views bug: translated relationships (entity reference from parent to child)
  // does not filter by language. So enforce that here instead.
  $views_with_translatable_relationships = [
    'view_name_a' => [
      'type' => 'reverse',
      'field_name' => 'field_example_1',
    ],
    'view_name_b' => [
      'type' => 'reverse',
      'field_name' => 'field_example_2',
    ],
    'view_name_c'  => [
      'type' => 'forward', // this is not a reverse relationship
      'field_name' => 'field_example_3',
    ],
  ];
  foreach ($views_with_translatable_relationships as $view_name => $ref_field) {
    if ($view->id() === $view_name) {
      if ($ref_field['type'] == 'reverse') {
        $relationship_key = 'reverse__node__';
        $relationship_alias_prefix = 'node__';
      }
      else {
        $relationship_key = '';
        $relationship_alias_prefix = 'node_field_data_node__';
      }
      if (isset($view->relationship[$relationship_key . $ref_field['field_name']])) {
        /** @var \Drupal\views\Plugin\views\query\Sql $query */
        $query->addWhereExpression(0, $relationship_alias_prefix . $ref_field['field_name'] . '.langcode = node_field_data.langcode');
      }
    }
  }
}
🇨🇦Canada Shiraz Dindar Sooke, BC

Heads up! This patch (#41) is no longer working.

Between Drupal 10.2 and 10.3 views entity relationships code was refactored, such that the change in patch #41 to views.views.inc no longer works. The patch still applies, but it attempts to add a join to $target_base_table at a point where that variable no longer exists. The most obvious symptom of this is in watchdog you will see "Warning: Undefined variable $target_base_table", aside from fact that the original problem this patch addressed will reoccur.

I tried to redo the patch to work on the refactored views relationship code by adding the joins elsewhere but was unsuccessful. I then tried doing the joins for my own views in my a views_query_alter hook and was unsuccessful there too. Eventually my solution was to simply add a where expression in a views_query_alter hook:

$query->addWhereExpression(0, 'node__' . $ref_field . '.langcode = node_field_data.langcode');

wherein $ref_field is the translatable field used in the relationship.

A join is more efficient but I'm happy with this for now.

Note, I haven't checked out the 11.x patch, but I suspect it would have the same problem. So I advise you check your watchdog for the aforementioned warnings if you are using that.

Shiraz

🇨🇦Canada Shiraz Dindar Sooke, BC

Patch #39 works great for me.

Thanks everyone!

As an aside: this seems like it would be a rather common need and I'm a bit surprised by the lack of activity on this thread. But this is only place I see this happening.

🇨🇦Canada Shiraz Dindar Sooke, BC

Hi I just wanted to chime in here to say that an alternative to this patch would be to define a display mode for your image which is styled to show as an inline-block.

🇨🇦Canada Shiraz Dindar Sooke, BC

Hi folks,

I'm just joining this issue now.

I successfully applied the latest MR 5758 and rebuilt the cache but I'm not seeing any options for inline images as I see in screenshots pasted here. Nor do I see any new buttons or new configuration in the text format for the embed media filter.

Is there something else I need to do?

Thanks,
Shiraz

🇨🇦Canada Shiraz Dindar Sooke, BC

This is a very real issue and the patch in #41 solved my problem.

🇨🇦Canada Shiraz Dindar Sooke, BC

None of the solutions here were working for me.

My situation was:

1. trying to sort by the delta of an entity reference field
2. that entity reference field is translatable and there are in fact translations of the same referenced nodes within it
3. the view also has that translatable reference field as a filter ({field name}: Translation language (= Interface text language selected for page)).

Distinct setting was enabled, that's a given. But it couldn't deal with the *combo* of #1 and #3 that was causing the duplicates.

I tried a whole lot of stuff but eventually nailed it by enabling views aggregation. Then under the Sort Criteria wher the entity reference field is set as the sort, check aggregation settings, and set the aggregation type to maximum.

Boom!

The view is sorted according to the delta of the entity refs. Only the correct language is displayed. No duplicates are shown.

The view in question contains client-specific field names, otherwise I would share the view object. But if anyone has a very close situation to this one and can't quite dial it up, PM me and maybe I can help.

Shiraz

🇨🇦Canada Shiraz Dindar Sooke, BC

The patch in #7 wouldn't apply to my 3.1.0 release because some of the changes are already rolled in.

Here it is again with *just* the change to the info file. It applies now and I can disable ckeditor4 along with fakeobjects.

🇨🇦Canada Shiraz Dindar Sooke, BC

It appears that this has been committed to 3.11:

name: 'Lazy-load'
type: module
description: 'This module enables lazy-loading images and iframes via text-filters and image-fields. Requires lazysizes library.'
core_version_requirement: ^9.3 || ^10

But the project page does not reflect this.

Confused, but noting this for others.

🇨🇦Canada Shiraz Dindar Sooke, BC

Whoops, patch in #8 was named incorrectly. Here is the same patch, correctly named.

🇨🇦Canada Shiraz Dindar Sooke, BC

There was a bug in my patch that would cause S3FS uploads to fail when using media library widget and when the filename path had two or more directory components.

I have updated the patch accordingly. It should *not* be committed, as my use case is quite unique. I will update the ticket title accordingly, as I do still feel this might benefit others.

🇨🇦Canada Shiraz Dindar Sooke, BC

Hi Webchick! We met briefly once in Vancouver when I working for Affinity Bridge. I've always admired you from afar, both in terms of your commitment to Drupal and your balanced, kind approach to working with others (witnessed mostly through issue queues on D.O.)

Thanks for being so awesome, and absolutely, do not overcommit yourself to anything.

Much respect,
Shiraz (working with Kanopi Studios, living on Vancouver Island)

🇨🇦Canada Shiraz Dindar Sooke, BC

It looks like I'll be able to do what I need with https://www.drupal.org/project/ds_chains

Leaving this here in case this helps anyone else.

This module has a really elegant interface so I definitely see its place.

🇨🇦Canada Shiraz Dindar Sooke, BC

Here's the same patch re-rolled for the latest dev release.

🇨🇦Canada Shiraz Dindar Sooke, BC

@SomebodySysop just want to thank you for outlining your process. As far as I can tell, your comment is the only place on the web actually documenting this in any form whatsoever.

🇨🇦Canada Shiraz Dindar Sooke, BC

Here's the patch from #120 with the gitignore changes (as mentioned in #122) removed. It now applies properly to the latest stable Drupal as of this is writing.

🇨🇦Canada Shiraz Dindar Sooke, BC

1. The 429 too many requests error I mentioned above is no longer occurring. I think it was just a temporary thing, not a real issue.

2. I found that some of the nodes I was submitting for chunked translations were failing because the previous patch would throw an exception if there was a single sentence over the character limit (because that would make for an untranslatable chunk). In fact the over-limit sentences in question were base64-embedded images in the text of the field I was translating. So I've updated the patch to not fail on these, but instead not submit over-limit "sentences" for translation, but still include them in place. This way base64 images are still included in the translated node and there are no fails.

3. Further to #2, the regex that was being used to split text into sentences was failing on text which have base64-encoded images (ie. was not splitting these correctly). I played around with several tweaks on the regex but couldn't get it to work satisfactorily. So instead I found a php library on github which is designed specifically for splitting text into sentences. However, there were just a couple extra things it was doing which caused their own issues, so I forked that repo with the changes needed to make it work. SO, for anyone that happens to be reading this (I suspect in the future this *will* be needed), to get this patch to work, you will also need to add these lines to your project's composer.json:

In the repositories section:

 {
            "type": "git",
            "url": "https://github.com/kanopi/php-sentence"
        }

In the require section:

        "vanderlee/php-sentence": "dev-do-not-clean-unicode-and-do-not-replace-floats",

Hoping this helps someone out!

🇨🇦Canada Shiraz Dindar Sooke, BC

Please note that, even with chunking in place, I have run into certain translations that fail with "429 - too many requests" from the API. This is described at https://towardsdatascience.com/advanced-guide-avoiding-max-character-lim....

I've tried sleeping between the chunk translation submissions but even at 2 second sleeps it wasn't enough. I presume because the limit is per minute. (it's not super well documented. The above link is the bet I could find)

To be sure, this is separate from the character limitation.

I'm not sure if the account tier makes a difference here. I'll update this task as I find out more.

🇨🇦Canada Shiraz Dindar Sooke, BC

Thanks for this patch Graber!

This works great for me for chunking translations that are over the character limit.

I did have to re-roll it to work on the latest dev release.

I also took out the "translate only text" part of this patch -- it was throwing errors, and in any case, I was just interested in the chunking.

So attached is the patch rerolled by myself, which only does chunking.

Anyone reading this should also check out my other patch 🐛 increase max character limit to 50000 Fixed that increases the character limit to 50,000 (from 5000), as that alone may take care of your needs. In my case, we have translations over 50,000 characters so chunking is needed.

🇨🇦Canada Shiraz Dindar Sooke, BC

Agreed. And, my day has just ended! Warm wishes from Canada!

🇨🇦Canada Shiraz Dindar Sooke, BC

My opinion is it's a very simple change code-wise so there's no concern there. The bigger question is does it work, and in my tests, it did. Ideally someone else would test with a translation greater than 5000 characters to confirm, but it did work for me, and as far I personally am concerned, I'd like to see it get committed, but there's no rush. I'm not sure how actively used this module is -- in fact I was surprised to have someone reply so quickly (thank you).

🇨🇦Canada Shiraz Dindar Sooke, BC

Thank you! I can confirm that patch applies correctly on the dev release.

Not sure if you want to wait for others or not to commit.

🇨🇦Canada Shiraz Dindar Sooke, BC

Oh thanks so much. Yes, I did this on the stable release not the dev release, as we were hoping we could stick on stable, but if it's changed to the point where it needs to be on dev release, we'll switch. Thanks for looking at this!

🇨🇦Canada Shiraz Dindar Sooke, BC

For me,

#7 sets the default facet value in the facet field, but the results are not actually filtered accordingly.

#2 works for me. However, since then a new argument ($urlGenerator) was added to the url_processor constructor (the one we are extending), so I had to add it here as well, as follows:

<?php

namespace Drupal\my_module\Plugin\facets\url_processor;

use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\facets\Plugin\facets\url_processor\QueryString;
use Drupal\facets\Utility\FacetsUrlGenerator;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\HttpFoundation\Request;

/**
 * Query string URL processor.
 *
 * @FacetsUrlProcessor(
 *   id = "my_query_string",
 *   label = @Translation("My Query string"),
 *   description = @Translation("Add description")
 * )
 */
class MyQueryString extends QueryString {

  /**
   * A string of how to represent the facet in the url.
   *
   * @var string
   */
  protected $urlAlias;

  /**
   * The event dispatcher.
   *
   * @var \Symfony\Component\EventDispatcher\EventDispatcherInterface
   */
  protected $eventDispatcher;

  /**
   * {@inheritdoc}
   */
  public function __construct(array $configuration, $plugin_id, $plugin_definition, Request $request, EntityTypeManagerInterface $entity_type_manager, EventDispatcherInterface $eventDispatcher, FacetsUrlGenerator $urlGenerator) {
    $this->alterRequest($request);
    parent::__construct($configuration, $plugin_id, $plugin_definition, $request, $entity_type_manager, $eventDispatcher, $urlGenerator);
  }

  /**
   * @param \Symfony\Component\HttpFoundation\Request $request
   */
  private function alterRequest(Request $request): void {
    $query = $request->query;
    $facet_new_parameters = [];

    if ($facet_parameters = $query->get('f')) {
      $article_type_filter_exists = FALSE;

      foreach ($facet_parameters as $parameter) {
        $sepator = ':';
        $explosion = explode($sepator, $parameter);
        $facet_type = array_shift($explosion);
        if ($facet_type === 'article_type') {
          $article_type_filter_exists = TRUE;
        }
      }

      if (!$article_type_filter_exists) {
        $facet_new_parameters = array_merge($facet_parameters, ['article_type:10372']);
      }
    }
    else {
      $facet_new_parameters = ['article_type:10372'];
    }

    if (!empty($facet_new_parameters)) {
      $query->set('f', $facet_new_parameters);
    }
  }
}

That said, I agree with #9 -- this should be something that we can set in the facet config.

Thanks all!

🇨🇦Canada Shiraz Dindar Sooke, BC

I think you may be right with your sense of why the issue was occurring, and I certainly don't want to waste your time too much with this. My patch (combined with the other) definitely did fix my issue, and this is after spending an inordinate amount of time xdebugging in there, so I know I did at least something right for my own use case, and again just thought it might be common enough to share this patch here. I appreciate your thoroughness.

🇨🇦Canada Shiraz Dindar Sooke, BC

Hi again, yes, so for some context please see https://www.drupal.org/project/s3fs/issues/3345094#comment-14947120 📌 issue with S3fsFileService move method during migrations Postponed: needs info .

The same applies here, meaning I wouldn't necessarily expect you to roll the patch, but thought I would share to help others.

That said, patch seems more straight ahead than the last. I looked at the logic there -- if (!empty($this->config['ignore_cache']) && !($metadata && !$metadata['dir'])) -- and indeed how it ANDs with the NOTs in there actually results in the in, if the ignore cache setting is enabled, getS3fsObject still tries to grab directories from S3. ie. not the stated purpose as in the comment.

I"m reasonably confident that I fixed this in the right way and that you'll want to get this, or a variation of this, committed, but again, not pressure as far as I'm concerned.

Thanks!
Shiraz

🇨🇦Canada Shiraz Dindar Sooke, BC

Hello maintainer!

So the circumstances leading to my situation were exceptional enough that I didn't want to write them out as replication steps in this task. Specifically, this is for a migration from a D7 to D9 site, where the D7 site has hundreds of thousands of S3 files, using D7's S3FS. For the migration, I left the S3 files in their bucket and am just migrating the references to them. Of these S3 files, many of them are images. After the migration, on D9, the images render in full size but thumbnails were not generating. When I went tracing the issue (including looking closely at a number of issues on this project page) in the end I had to xdebug my way through it, and I came across the issue mentioned in this task. Between this issue, and the other one I posted, I was then able to get the imported S3 files to render thumbnails (ie. only with the two patches I've posted).

This situation is exceptional enough that I do not necessarily expect you to merge this patch in. However, I wanted to share it here because it certainly seems possible that others might run into this, and maybe this patch would solve their issue.

To be clear, in my xdebugging, I *definitely* came across cases where S3fsFileService's move function is being sent *both* directories and file URIs for $destination from other places within the S3FS module. This would happen within the same request in fact while it was trying to generate a thumbnail. But the filenames were definitely breaking things, and my patch definitely solved my problem.

So ya, there's some context, and the reason why I shared the patch publicly

🇨🇦Canada Shiraz Dindar Sooke, BC

Here's a simple patch for the issue I described above.

(to the module maintainer, I started a fork, but ended up using an old-school patch, as it seems that's what's mostly happening on this project. If you'd prefer me to make a fork, just let me know!)

🇨🇦Canada Shiraz Dindar Sooke, BC

I also checked how I could enable this "force" option. I seem to remember it in D7 but I'm not seeing it implemented in D9. There are some references to it in the code, including what you saw, but nowhere where it actually sends the option.

🇨🇦Canada Shiraz Dindar Sooke, BC

I was having this issue and your patch fixed it for me. Thanks! The nodes in qusetion were originally imported from Salesforce so many that has something to do with it.

Production build 0.71.5 2024