🇧🇪Belgium @borisson_

After asking in slack it looks like we could document this over at https://www.drupal.org/docs/drupal-apis/configuration-api/configuration-... →

I think we can document it there.

The suggestion in #53 makes complete sense to me, it's now updated so moving it back to rtbc.

I pinged kristiaan in slack, and in the meanwhile I figured there may be something else that we could do.
Instead of doing a min check here, we just need to know that the value is > 0, because smaller is not an option.

So I think isPositive might be a simpler way to validate this instead of being this specific.
However, waiting to put this back to needs work based on feedback.

I have one tiny nitpick on the comment, but I think @bbrala is right, this looks ready.

I think this looks good, I don't think we need specific test coverage for these values.

I wonder if we should ask for a user module maintainer for a review? Will ping kristiaan.

This looks great, thanks for cleaning this up.

The patches in #30 and #29 seem to be identical to #26.

I am not sure if the solution in there is actually a good one. There are good steps to reproduce, but I would like to get input from one of the config or entity system maintainers on the direction before writing tests.

borisson_ → created an issue.

Merged the MR.

The latest version of the patch is to disable the option in the views UI, but the issue summary doesn't yet reflect that. So that would be the first thing to do here. As a next step we should transform the patch into a merge request.

Actually I think this can go back to rtbc

ExtentionAvailable makes a lot of sense to me.

Back to rtbc now that it is moved.

While I agree with #5 there would not be a big maintenance burden, in the 5 years since this comment was posted there did not seem to be a big interest by others. I would live to see popularity numbers from this based on a contrib module and we could use that to inform the need for this, because to me it still seems like a super small usecase.

I will close this as won't fix for now.

The change record that is linked to this issue is not correct it seems?

I too am constantly bouncing between the need to validate everything strictly and the looseness that is allowed currently.

Can we do this the same way we define plugins? Each one having their own schema?

I reviewed the CR. It has all the information needed.

I have some small documentation remarks, but this looks great.

Still needs tests, the remark in #11 seems to be fixed.

Olivero was specifically designed like this, see 📌 Olivero: Center align content (instead of left align) Needs work for people who want to have a centered layout. But the drops background + left alignment is a feature, not a bug.

There are more tests that are failing on the last test run. All in ChainedFastBackend.

The change however makes sense and it has good test coverage.

bramdriesen → credited borisson_ → .

The parent issue was committed.

Since it's about removing a role, I'm not sure if we need to have strict validation.
However it could lead to an action that doesn't do anything when it has a role configured that does not exist I guess.

This looks great and it makes this test a bit faster.

Removing the tag, title looks great now.

Thanks for that information. Moving to rtbc based on #10.

Add belgium

There's a test failure in AssetAggregationAcrossPagesTest, but I've seen that fail in a couple other merge requests today. I'm thinking it is not related. The only bytes we are saving are from the comment not being there anymore.
I agree keeping the class around though, otherwise this would be a breaking change I think.

It is green now, and we're decreasing the page weight, that's great.
I am not sure about moving the file though, people are not supposed to reference the file by path, but it seems like it increases the likelihood this will be a breaking change?

Removing tags that are no longer relevant.
The new solution is well documented, but I'm not sure about the tradeoffs here, tagged for product manager review because of that.

I don't see why that test would start failing, and I can't find an issue about it having random failures. Since this a security hardening I think we should move this through.

nod_ → credited borisson_ → .

So, at the very least this issue sounds premature until the Drupal CMS being stable with a good search_api recipe is implemented. At that point, it sounds like the only concern is around user confusion?

This is where we are now, Drupal CMS 1.0 was released and it includes Search API with a good setup.
I'm not sure how much continued adaption it requires from people building on top of Drupal CMS, but I hope it is minimal.

I'm not sure about this. Administer Search is from the core search module. Search API doesn't have that permission, but it does have a Search index view mode as well.

How would we, as search API maintainers, allow this to work without asking people to enable core search, just to get that permission.

These changes look good. It also has the change to phpcs.xml.dist to prevent regressions.

bramdriesen → credited borisson_ → .

All threads seem to be complete. Looks good to me.

quietone → credited borisson_ → .

I agree with catch in #19, moving this to Drupal\Core\Config seems like a better solution, let's do that.

I don't understand having to bypass NotNullConstraint? I think we have to reimplement it in here?

We should probably start with a reroll here.

This needs another rebase.

I have gotten in the habit of using the markdown-like backticks when having to say that something is `code`. I think a lot of people will recognize this. Is that an option instead of using quotes?

We could even update the api.do parser to wrap those in a code-block as a next step?

Since this is only for the ajax behavior of the regular blocks, and we are currently only supporting ajax-like behavior through facets 3.x, I'm going to close this as won't fix.

The current suggestion is to try using views' ajax handling with facets 3.x for this.

It could be that this is also a bug on core views, but that would need a core issue to fix.

Only 2 instances, those look good. Back to rtbc.

I am not sure why this was suggested, but I don't think we need to check this? Any module could provide a plugin?

Title fix

While I agree that adding more functional tests is not great for the total test time duration, having them well documented is important.
We can always change the documentation when we want to add more testing to this.

To me, this change seems like it answers the question by @nod_.

Test looks solid, I think that's good. I wonder if we need a change record for this, but I don't think we do. RTBC++

This should be a merge request, not a patch?

I agree, this now no longer decreases readability, I think it does read better now.

This looks great and it has the phpcs change need to ensure we don't regress.

Found some places where we could align better, I personally really dislike things that end up looking like this: ]));

I'm not sure how facets should support this? As long as Search API gives us this as a string we can work with it. So I guess this is supposed to be in the realname project to support Search API

See #11

Last but not least: per @lauriii, there won't be a "search" or "search index" view mode for XB — search must be configured to use the "default" view mode's representation of content entities both for 1) crafting a search index to power the search, 2) presenting search results. Related: 🌱 [META] Support alternative renderings of prop data added for the 'full' view mode such as for search indexing or newsletters Active and 📌 Create a configurable search api processor for XB data Active

It's not clear to me yet how that avoids the

but without elements that would 'pollute' the search results like field labels, related content blocks, CTAs, social widgets etc. so that if I search for "newsletter" or "Facebook" I don't get every article on the site back in the results, but only the ones with content actually mentioning newsletters or Facebook.

problem described in 🌱 [META] Support alternative renderings of prop data added for the 'full' view mode such as for search indexing or newsletters Active though.

Rendering Search results in the same way as the content's detail page doe not seem like the most commonly chosen way of configuring it. I did a quick poll at the contribution room at Drupal Mountain Camp, and everyone who answered me always configures some kind of teaser, card or search-result view mode when rendering a search result page.

For the indexation part, I also don't see a way that would remove those pollutions without having a seperate view mode, but I personally think that's not as bad. When needing complete control over what all is indexed, it should be indexed using fields, because then it's possible to boost per-field. It gives a lot more control.

This however would mean something like https://www.drupal.org/project/layoutbuilder_search_api → for all components/props, which is another approach that could be taken. This removes the double rendering that catch talked about in #13.

This is however not very precise, if you have a common footer component on every page those elements end up in the index for every page.

We are using the view builder for an entity to run this, so footers/header will not be included directly, this should only render the main content, so that's only the case of a call to view() also includes a header and footer.
https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/src/Plugin/...

The proposed solution should provide a list of enabled component config objects and a checkbox for each of their props.
For example, you might have a card component with fields for image, title and teaser text.

This means that for every new component I create, I need to configure this processor? Or will it include new ones by default? Why would we do this at the Search API level and not on a new view mode?

As one of the people who's written parts of Search API's backend interface, I wonder how well that UI will work for the target audience of drupal cms. Search API is very powerful, but that power also gives you quite some configuration options. I don't think the target audience of experience builder will find joy in using our UI.

I also wonder how this will work with react components that are created through the ui, will we have to do some kind of parsing to get their data out? What if it uses some kind of API or Javascript service to create content, will those we have to render the javascript from the backend?

I think instead of creating a new processor, we might have to create a new DataSource instead? That datasource could do the specific rendering/fetching of data without it having to go in a processor with it's own form.

I think supporting this through an alternative view mode, is probably the better way to go instead of using a processor like this?

Can someone explain to me how to run this gitlab job? I was looking at this with @vrancje and we couldn't see it right away. This should also be documented

CR that's added looks great and has all the correct info, RTBC+1

Answered some of the comments.

vrancje → credited borisson_ → .

Looks great, no notes!

Facets 3

Facets 3.x only supports Search API, and introduces "Facets exposed filters" next to "Facet blocks".
We recommend using Facets exposed filters for new projects, when using Facets + Views
Using "Facets exposed filters" in Facets 3 has full AJAX support.

Facets 3 will not support AJAX for "Facet blocks".

Facets 2

Facets 2 works with Drupal Core Search and Search API.
It is kept available for existing projects using Facets 2 + Ajax as there is no upgrade path for this setup. Updating from 2 to 3 without AJAX support is possible.

I think this is very clear, for facets 2 you need the additional core search module as mentioned here, if this doesn't work I think we should file a bug report there. Readme update in #3506177

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

I can't reproduce this.

It no longer does out of the box, you can find the old code in the facets_core_search module, but this is not actively maintained. We however suggest you use Search API instead.

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

I don't really understand the issue here. Can you try again on the latest version and if it is still an issue can you add a screenshot of the issue?

I can't reproduce this.

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Thanks for reporting this issue, I hope it no longer applies on 3.x, if it does we can reopen this issue and target 3.x

Tagging as novice

updated the CR to be a bit easier.

I have tried to figure out if we are still missing something, but to me it seems like this is complete.

I was wondering if this would break styling on existing websites, but it won't, because the ID would already change when using ajax.

I reviewed the issue together with @wannesdr at drupal mountain camp. I had some remarks, which I shared in person. Wannes is fixing those as we speak. I think this issue looks good and I will rtbc this when the last things are fixed.

Updated the issue summary.

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

PS: It seems you (like many others – it's really easy to misinterpret) are confused by the "Issue tags" field. As the guidelines → state, they aren't meant for free text tags related to the issue, but only for specific categorization purposes, usually by module maintainers.
So, if you aren't sure your current usage is correct, please just leave the field empty.

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

You're right, we should update this, only in the 3.x version of the readme.

I also can't reproduce this, closing this issue.

I agree this is complex, however the validation we would need to check all options is not an option.
I am closing this one out.

Since this is in the 2.x line, but about ajax, I'm not sure how to feel about this. I don't want to risk breaking anything in that part, but this should no longer work on 11.x.

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Closing this issue, just like @timotej-pl I can't reproduce this.

Yes, that seems logical to first do operations on the tree before transforming it into hierarchy.

To me, this looks like an issue in drupal's installer process? Does this still happen? If it does we need to look at the dependencies of our own schema.

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

You are seeing this because it is just rendering the content coming back from the index. This should be resolved with the EntityTranslationProcessor.

The other module is what should be used for the 1.x and 2.x lines of the module. For the 3.x version this should be done through an exposed views filter.

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Because this is a static cache, it's impossible in my eyes for this to happen. I would like to have more information on this issue, but closing it as outdated for now, feel free te reopen with extra information if it still happens.