🇧🇪Belgium @borisson_

bramdriesen → credited borisson_ → .

I don't know anything about combing facets, is this something that is built in or with another module? I think combination of values should happen at the Search API level?

I may be missing something, but I'd like to get more info

pjonckiere → credited borisson_ → .

This works now, Thanks to @fjgarlin.

borisson_ → created an issue.

Thanks @knurg!

Looks like the latest merge request broke all tests, because of a missing ".ignored-deprecations.txt", I don't think this is because of this MR though. I wonder if this is something more general that's going on. I asked in #gitlab on Slack.

I guess we already have a test, so we're no longer waiting for that, we should make this into a merge request to see if the test actually passes.

This patch really is essential for nearly all my systems and I am really tired to apply it every time. Can we please add it? it works great!

@knurg, this issue is in the "needs work" status, for it to be committed, it should at least get to the "Reviewed and Tested by the Community" status.

A comment like the ones in #42, #43 and #44 could also put this into rtbc, if you feel safe enough that this will not break any other installations of the module.

So in this case the next steps are:

• Make the patch into a merge Request
• Get a code review on the code and tests
• Get this into RTBC state

Not a big deal but why is BEF here? Does it have anything specifically to do with Facets?

Since facets 3.x, the rendering of facets is done by views filters (instead of writing our own rendering/ajax handling), BEF makes those filters more capable.

borisson_ → created an issue.

quietone → credited borisson_ → .

I think this issue was supposed to be marked as fixed?

I think this is all the test coverage we need. Looks good to me.

It's 4 years ago, I'm assuming no one will add anything to this one, there is no log of the meeting here, but going back this far in slack will not add any value. So I'm going to go ahead and mark this as fixed.

I have to be honest, I don't really remember all of that conversation, but I hope @webflo does.

pjonckiere → credited borisson_ → .

This is a dependency already. I can't reproduce this.

I've now read the code, and tested it locally as well. I have one question, but I think it looks ok so leaving at rtbc.

I think it would be a good idea to add this to the next release as well, so that hopefully drupal cms will start with this new more better user experience.

So this change does not need a new config action, it only needs this change and the config action/recipe will set it to the new :all selection?

I think this is a good solution and I can't find anything to complain about really, apart from there not being any test coverage.

At the least I think we should add to the RenderedItemTest.

borisson_ → created an issue.

The most important IMHO is to prevent search_api_views_data to put a lock on cache_config that would prevent drush cr from correctly rebuilding caches.

I talked to @webflo today about this issue. We think that putting a lock, in addition to removing the return is probably a really good way to ensure that we always have a valid views defnition.

We never want to have a silently broken site.

I haven't looked at the code yet, but the screencast here looks super impressive, I think this is a huge upgrade.

Haven't tested it out locally, but the code changes look great.

If a field type is layout_section, the value always seems to be "Text" instead of the actual text I filled in in the layout builder content. That gets sent to the AI, but a new layout section does not get created either.

It seems like there are multiple things that can still be improved here.

array:1 [▼
  0 => array:5 [▼
    "delta" => "850c5e44-8e63-4435-a015-2556aa8c323d"
    "field_name" => "info"
    "field_type" => "string"
    "value" => "Text"
    "parents" => array:2 [▼
      0 => "info"
      1 => 0
    ]
  ]
]

I have attempted to test this on a local install, and it looks to me like this doesn't work yet.
The problem I encounter is that only the title is translated, and nothing that was placed trough layout builder.

To me it seems like something that needs to be changed in the TextExtractor. Will investigate further.

borisson_ → created an issue.

borisson_ → created an issue.

borisson_ → created an issue.

borisson_ → created an issue.

Looks super simple in code, I think we should just do this.

I have prepared the first MR for recipe integration https://www.drupal.org/project/search_api/issues/3484304 ✨ Expose Index and Server methods as config actions Active . It exposes only already existing methods as config actions to start this feature rolling.

This is a good and easy first step, I think that one is ok.

• addDatasource, but at the moment the argument is of type Datasource and it can't be used in recipe. Maybe alternatively there could be a method addDatasourceByIdAndConfig that will have datasource id and its config as arguments.
• setTracker has and object as argument so it can't be used as action method, but maybe having a method setTrackerByIdAndConfig with tracker plugin id and config as arguments would make sense
• The same for setServer it makes sense to create setServerById?
• addProcessor has object as argument, but maybe new method addProcessorByIdAndConfig would be ok to add as well
• addField has Field object as argument, but maybe new method addFieldByFieldConfig would be ok to add

For datasource, and server I think adding a new method is a good idea.
Processors and fields are more difficult, because their configuration can be more extensive, but I guess we can go through the same way here as well?

I think tracker is probably not needed, since for 99% the basic tracker is used. So I'm not sure how I feel about creating this new method, however if we already do this same trick for the other 4 I guess it's not too bad?

if it is already comitted, I can set this to fixed.

borisson_ → created an issue.

I think we can still work on this issue, or if we move to another issue we need to credit the people who helped already here. I think it's a safe move to reuse this issue.

breidert → credited borisson_ → .

greggles → credited borisson_ → .

Kickoff meeting for event in 2025

borisson_ → created an issue.

Was fixed in 📌 Add config validation for weights (blocks, filters, etc. all use weights) Fixed

borisson_ → created an issue.

I participated also

I took pictures

I took pictures

I took pictures

I took pictures

Updated the summary, thanks!

I'm not sure we want to add a config schema for views plugins at this time, core views are not yet validated either yet.

esmoves → credited borisson_ → .

larowlan → credited borisson_ → .

wouters_f → credited borisson_ → .

Not sure what this tag was.

borisson_ → made their first commit to this issue’s fork.

I feel that ExtensionExists should be whether the extension exists and we should have a separate constraints - the other constraint should be called ExtensionInstalled. However this is unfortunate because we already have ExtensionExists and it's current meaning is that it is installed.

Not sure what to do.

This is a difficult question. In my opinion ExtensionExists/ExtensionInstalled makes sense, but renaming this and after that reusing the name for the other meaning is difficult. Can we use ExtensionPresent to indicate that should be present on the file system?

Committed, thanks!

I created a new MR that includes all fixes, also I replaced str_contains with strpos,
because not all websites support this (str_contains only exists in php8)

Hard disagree. Please use str_contains instead. https://github.com/symfony/polyfill-php80 will ensure everyone can use this.

hestenet → credited borisson_ → .

Created a release for 2.x

So it was released less than 24 hours before this issue, that's certainly very quick.

I'm not 100% sure, I think the current 2.x branch already supports drupal 11? If that is the case I can tag a new release soon.
I don't think there will be a release on the 3.x branch in the coming weeks, since that is currently still undergoing architectural changes.

We'll only tag a new release once we get to a more stable situation, this is still undergoing big changes as it is.

The earliest I see this happening is at drupalcon barcelona.

Commited and pushed to both 2.x and 3.x

borisson_ → made their first commit to this issue’s fork.

This looks correct, the 3.x (and especially this submodule) is not yet stable, so we're going to add this once we've settled. Will keep this issue open to resolve that when the time is there.

I reviewed it again today as well, and the one remark I had was answered. RTBC++

larowlan → credited borisson_ → .

Committed and pushed to 2.x and 3.x, thanks everyone!

quietone → credited borisson_ → .

It will be hard to keep this in mind while reviewing config schema issues, but since it's a simple set of rules that are easily documented we should proceed like this, I think #6 tackles my concerns.

Most things in system.site that are being made nullable in 📌 Add validation constraints to system.site Needs work , are not actually nullable from the standpoint of a running drupal installation, this especially rings true for name and UUID.

However, I'm not so sure about others. What is the semantic meaning of page.403 / page.404 being an empty string. It means this is not configured, right? I think it makes more sense in that case for it being NULL, because we can say it has to be null or it has to be a valid path.

If it is not nullable, we can not be as strict on the shape of the value?

page:
  403: ''
  404: ''

page:
  403: null
  404: null

I think this will reduce the need for nullable and means that we can leave values as strings and not have type flip-flopping.

Type flip-flopping is not great. and it doesn't make our code more strict.
All it does, is allows us to assume if the value is a string, it's a valid value, but I would not like to see a lot of places introduce is_string as checks to see if it's the right value.

I don't yet have a fully formed opinion on what the best way forward is.

I've opened #3461428: Add NotBlankAfterInstall constraint and use it for system.date:timezone.default to explore the NotBlankAfterInstall idea.

I think this is a very valueable new constraint.

Commited and pushed to both 2.x and 3.x, thanks!

Merged, thanks so much.

We no longer have the array of ignore things in ChemaCheckTrait, as far as I can see, so I think you're right that this can be closed.

Looks great, thanks all.

Thanks, pushed and committed to 2.x, can we can get a 3.x variant of the change as well?

I fully agree with #11, I think that example makes sense.

Spellcheck fails, but not sure how to better name this.