Account created on 1 August 2013, over 11 years ago
#

Merge Requests

More

Recent comments

🇫🇷France MacSim

Setting back the status to "needs review" cause my case is a specific case as the block is loaded in another block programmatically. I also added the 'menu_link_content_list' to the second block cache tags and the problem is gone.

🇫🇷France MacSim

Applied the patch in #7 (same as #10) didn't solved the problem.

🇫🇷France MacSim

I had the same issue and after upgrading Redirect from 1.09 to 1.10 (which include the related patch in #2) the error is gone.

🇫🇷France MacSim

I am finally going to revert the commit made on 2.0.x cause it still needs some work

  • line 12 in index_now.api.php should be removed
  • I didn't tested every use case before to merge (shame on me)
  • For each entity type, we need to add a test that will cover the FALSE return on index_now_is_indexable_entity() function call (we'll check that the next function won't be call in those use cases). Those tests functions should be called test[ENTITY_TYPE]ExcludedByHook()
🇫🇷France MacSim

macsim created an issue.

🇫🇷France MacSim

Hi @adelgado

Sorry for the delay and thank you for the great job you've done here.
I merged your work on 2.0.x
Still need to port it on 3.1.x

🇫🇷France MacSim

Hi coffeemakr,

Nice catch ; I made the changes you suggested but I am struggling on the tests changes.
I'll commit this as soon as tests will be fixed

🇫🇷France MacSim

Thanks @sarwan_verma, it has been merged in 3.1.x and will be in the next release :)

🇫🇷France MacSim

I can confirm the patch provided in MR !20 fixes the error that was displayed on the config form but $this->nodeTypes = []; shouldn't be removed from the constructor.

🇫🇷France MacSim

@klonos the doc you're refering to is about XML attributes - we're hereby talking about (X)HTML attributes.

https://html.spec.whatwg.org/multipage/infrastructure.html#xml

Attribute names are said to be XML-compatible if they match the Name production defined in XML and they contain no U+003A COLON characters

My interpretation is that (X)HTML attributes can be XML-compatible but it's not an obligation.

🇫🇷France MacSim

Hi @adelgado12

Interesting feature request.

I personally prefer the hook proposition to the field proposition, especially because the field proposition would store entities ids in the settings and those ids might not exist or be different when we work with multiples environments.

🇫🇷France MacSim

Small update of patch provided in #100 🐛 HTML5 Validation Prevents Submission in Tabs Postponed: needs info in order not to break the behavior of adding a paragraph to a tab.

I tried '.form-submit:not([formnovalidate,.field-add-more-submit])' but it was not a valid selector (at least on firefox) ; so I just chained another :not() => '.form-submit:not([formnovalidate]):not(.field-add-more-submit)' and I am wondering if it's a good idea to have an array declaration in the first "not" since it's not well supported by all browsers yet.

🇫🇷France MacSim

The patch provided in #100 and MR!40 does not fully work for me.
I use horizontal tabs with a Claro 10.2.5.
If there are required fields in another tab that haven't been filled out, the JS takes me to one of those other tabs when I add a paragraph to the current tab.

🇫🇷France MacSim

The linked issue has been fixed, does it fix the tests on Scheduler side?

🇫🇷France MacSim

Hmm sorry I didn't reloaded the page and didn't saw your comment @axroth
What do you mean by "both features?" "per entity type" / "per bundle" ?

What I did here based on what was provided by @alexrayu in #2 is a "per entity type hooks config"
I need to upgrade Index Now Commerce to do the same thing.

We could open a new feature request for a "per bundle hooks config" but it would be more complex especially on the test level.

The feature request has only been merged on 3.1.x branch.

🇫🇷France MacSim

Interesting feature. Thanks for your contribution.
However I think we should either set it per entity type or per bundle instead of setting it globally.
That would give a more granular way to choose which content really is important to index

🇫🇷France MacSim

Nice catch. Thanks for the patch.
I tested it and it works like a charm

🇫🇷France MacSim

Hi @alexrayu !
Thanks for the patch it has been applied to 2.0.x and will be in the next release.
I'll backport it on 1.0.x and adapt it for 3.0.x + index_now_commerce 1.0.x ( https://www.drupal.org/project/index_now_commerce/issues/3425048 🐛 Provide language of the indexed entity when url is generated Active )

🇫🇷France MacSim

@ressa Sorry I didn't realised that second id.
I don't know where he found that but you don't need it
The diff is available here https://git.drupalcode.org/project/facets/-/merge_requests/129.diff and it's the same than https://git.drupalcode.org/project/facets/-/merge_requests/129.diff?diff... ;)

🇫🇷France MacSim

@mortona2k 129 is the id of the merge request ;)
Basically you just need to clic on the merge request and add ".diff" to the url

🇫🇷France MacSim

To review this:
Try to install 3.0.x-dev on a drupal 10.1 with drush 11 => composer should throw a conflict error but if you try with a drush 12 it should be ok

🇫🇷France MacSim

Sorry I had an error 500 while I created this issue and I re-created it:
https://www.drupal.org/project/stage_file_proxy/issues/3422123 📌 Drop drush < 12 support Needs review

Closing this one as a duplicate

🇫🇷France MacSim

I am not used to write tests extending DrushCommands but, since it's a functional test, don't we need to add

  protected static $modules = [
    'scheduler',
    'dblog',
  ];
🇫🇷France MacSim

@AdamPS

Did you test with both Drush 11 and 12?

Nope I didn't. I thought reviewers would do it.

Please could you raise an issue?

I can but I would say that it should be done in a 1.5.x version which doesn't exist right now.
We would also need to add the following lines to the composer.json:

     "conflict": {
        "drush/drush": "<12"
     }
AFAICS it works without this change in Drush 12.

Did you tried to use the command on 1.4.1 with drush 12?
Cause it should throw a "mailer:override command is undefined" error.

The doc says that drush.services.yml is deprecated in drush 12 and will be removed in Drush 13.
But it also says that

  • Drush 12 expects commandfiles to use a create() method to inject Drupal and Drush dependencies.
  • Drush 12 expects all commandfiles in the /Drush/ directory. The Drush subdirectory is a new requirement.

Those changes are required to run a command with drush 12.

🇫🇷France MacSim

I am sorry I am reopening this issue cause it's a major problem in this module architecture.

In 14 years using drupal it's the first time that I have such a problem... so it clearly sounds like a PWA issue.
Storing a fid value based on an upload which is happening on a single environment (the file doesn't even exist on other environment) as a config value which can be deploy in many environments doesn't make sense at all.

You could store paths instead or ask people to respect some paths you're asking for with specific dimensions.
People would respect it cause if they don't their icons wouldn't work.

Please have a look on how favicons and logos config are managed in system.theme.global.yml
This is exactly how you should manage your PWA icons.

We could check file usage in the validationForm() method, but I doubt that is needed, as the files should only ever be used as the app icons.

The summary I wrote however explains the opposite...

🇫🇷France MacSim

I am sorry my first commit was incomplete

🇫🇷France MacSim

I guess we also have to rewrite some tests according to https://www.drush.org/12.x/commands

6. Write PHPUnit tests based on Drush Test Traits.

🇫🇷France MacSim

Note: When we'll need to drop Drush 11 support, we'll just have to remove the drush.services.yml file

Production build 0.71.5 2024