🇬🇷Greece @vensires

I closed the MR so that anyone willing to tackle this issue may start the process from the very beginning.
To be honest though, I did try to find the comment described in the issue summary but couldn't find it at all in the code. Maybe something is off here or was it just me?

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

No, I didn't test it to be honest. It seemed like a small change. Did you review my solution yourself?

The original patch had set the return value in the catch{} block and it's actually working. My suggestion in the MR is to instead have it at the end of the function. It seems more clear to me.

@drupaldope thank you for the clarification! I set it as Needs Review then!

The my menu--navbar.html.twig file phrase in #3 confused me; I'm changing this to a support request.
Feel free to revert my change in case I was mistaken.

MR should fix the issue.

vensires → created an issue.

@wotnak, if you don't have any further issues with the MR, you might also consider mentioning either the vite plugin or just this issue in the module's project page description. I see you already have a link to the README.md file; it might just be a bit more fancy - maybe adding another "note-tip" CSS class. Whatever you decide though.

I have fixed the issue in the MR.
I also took the initiative to change the string used so that it says (minimal) and not (minimal-) if the version is missing. I think it's never actually translated so I won't introduce any further issues with this change.

I suspect it was to be set as "Reviewed & tested by the community" but the user misclicked. I set it as Needs Review for now and anyone is free to check the patch and either review it or create a MR based on it with any possible corrections.

@drupaldope, it's not clear to me whether the issue is resolved with your patch or whether the whole issue was a false report.
Could you please clarify?

So, @giannisMak do we need a new MR for this to fit what @saurav-drupal-dev described in #13?

the css should come inside toolbar file because we cannot access dropdown without the admin toolbar

@axioteo, feel free to update README.md with the correct details on how to fix this issue, either using your plugin either without it (if possible).

After this is resolved, we might open another issue to suggest the change of the description of the module if required by @wotnak.

@smustgrave, I think that the points already addressed by @theodorosploumis in #6 is the answer you are looking for:

1) Consistency, as mentioned on the issue title.
2) Performance. Mini pager is less heavy than the full pager.
3) Usability. Media are usually attached to Nodes. So navigating to the first or last item is not something really important.

Feel free to express your objection on the previous points though.

I leave this as Needs Work because I'm not 100% sure the patch and the MR are in accordance.

Added Greek Community digital presence info.

Since this issue is related directly to the navigation module of the Drupal core and not the menu links in general, I think it's well set as postponed, yes.

I add 📌 Plugin config: Delete entity along with GroupContent Needs review as a related issue due to the following comments in code:

1. GroupRelationship::preSave()(L.259-264)
   

   // We want to make sure that the entity we just added to the group behaves
   // as a grouped entity. This means we may need to update access records,
   // flush some caches containing the entity or perform other operations we
   // cannot possibly know about. Lucky for us, all of that behavior usually
   // happens when saving an entity so let's re-save the added entity.
   $this->getEntity()->save();
   

2. GroupRelationship::postDelete()(L.295-301)
   

   // For the same reasons we re-save entities that are added to a group,
   // we need to re-save entities that were removed from one. See
   // ::postSave(). We only save the entity if it still exists to avoid
   // trying to save an entity that just got deleted and triggered the
   // deletion of its relationship entities.
   // @todo Revisit when https://www.drupal.org/node/2754399 lands.
   $entity->save();
   

Created new MR based on 11.x. Uploading a patch for this too.

For tests, don't count on me unfortunately.

I have updated the issue summary.

It seems my previous code didn't work. I have changed the query to a formula in order to be able to check whether the comment_count has a non-zero value and act accordingly.

I have created a MR and also attach the diff as a patch for anyone needing this.

It actually seems like the "comment_last_timestamp" view field is a copy of the comment_count field with a different PHP attribute name.

So far, we have hook_requirements() - which can be bypassed if you use -y in drush, and composer's which may or may not be used for custom modules. My question remains: let's say we have a conflict in *.info.yml; is it there for informational reasons or should it 100% prohibit the coexistence of the module without any bypass?

@mschudders I fixed the MR.

The main question here is what's the action expected when a conflict is found. Depending on what we want the action to be, just by using hook_requirements() we might be ok.

We also faced the same error after updating the module to the latest version. In our case the issue was related to a "entity_type.manager" key because it has a dot (".") though we couldn't anywhere find this entry in the YML files. Decided to roll back to 1.9 for now.

In case it helps, the message returned is coming from ConfigBase::validateKeys.

Thank you for the test @danrod. I can't personally understand why the double submission or the clear cache is required; I really think it's RTBC but let's keep it under "Needs Review" for a while more.

I don't doubt you but is does seem strange to me... It's just a simple maxlength attribute change. I also checked the schema.yml just in case and there isn't anything related in there either. Something is escaping us...
Setting this to "Needs work" until we find this and fix this.

@danrod, based on the change of the MR or the patch in #13, I can't guess why a reinstallation of the module would be required. A cache clear wouldn't suffice?

I know what you're talking about. To clarify things though, the maintainer @davereid replied on #3437974-27: Drupal 10.3 ImageStyleDownloadController::deliver signature change → the following:

Aside from that, whatever the rest of the community decides.

Also uploading the latest diff from the MR as patch.

Thank you @berdir for the update. I wasn't aware of the deprecation change. In my latest commit I transferred the hook implementations to the main module file as a first step short-term fix.

Next thing to do is to follow the guidelines and add the [#LegacyHook] attribute. If anyone else tries to address it, this article by dev.to might be helpful.

I fixed the PHPCS issues. There are still some PHPSTAN issues but I'm not sure they are related to this issue's code.
I also upload the latest patch based on the MR.

@prudloff I'm not completely sure at this time whether the "Works with" in the "Releases" tab uses the composer.json or the info.yml.
I think in most cases the dependencies are described in both composer.json and .info.yml. Specific version requirements are usually set in composer.json only; though there are some cases where it's used in info.yml too but I would call them rather rare (see example).

Whatever the maintainers decide though...

Until this is resolved, you could add the following to your composer's repositories property, just above the https://packages.drupal.org/8 repository:

{
    "type": "package",
    "package": {
    "name": "drupal/cas_attributes",
        "description": "Provides tokens for CAS attributes and an interface for token replacement in user account fields as well as advanced role mapping capability.",
        "type": "drupal-module",
        "keywords": ["Drupal", "CAS", "single sign-on"],
        "homepage": "https://www.drupal.org/project/cas_attributes",
        "support": {
            "issues": "https://www.drupal.org/project/issues/cas_attributes",
            "source": "https://git.drupalcode.org/project/cas_attributes"
        },
        "license": "GPL-2.0+",
        "require": {
            "drupal/cas": "^2.1 || ^3.0"
        },
        "version": "2.0.0-beta5",
        "source": {
            "url": "https://git.drupalcode.org/project/cas_attributes.git",
            "type": "git",
            "reference": "2.0.0-beta5"
        }
    }
},

Depending on the version you currently use, you might need to delete your "vendor" folder.

@miedward your latest issue refers to 📌 cas 3 compatibility Active .

@drunken-monkey thank you for your really detailed response!

I wasn't aware of 📌 Convert all hooks to events Active ; you are correct though that it would be better not to introduce a hook and then deprecate it. We could have this as an event directly instead. This way it would be ready for merging.

I had already thought of overriding the original $datasource's class or the LiveResults class using hook_search_api_autocomplete_search_info_alter(&$info) {...} but it really was too much of a code to be duplicated just for this change. Your proposal from MR!40 is much simpler and future-proof. Let's go on with this one then.

PS: I also update the issue summary with your solution.

Add this as related issue just for clarity.

Setting this back to 8.x-1.x-dev.

Drupal 10.2+ versions don't need this module at all thanks to the change described in [ #2972665 → ].
Drupal 10 up to 10.2 may use this module's v2 without any patch.
Drupal 9.5 versions should use version 8.x-1.x with the patch provided in #2 🐛 PHP 8.1: Passing null to parameter #1 ($string) of type string is deprecatedPassing null to parameter #1 ($string) of type string is deprecated RTBC above.

vensires → changed the visibility of the branch bugfix/3332610 to hidden.

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

I understand your point. In case I come up with another idea in the future, a more generic approach let's say, I will come back here.

In the meantime, I take the opportunity to change this to "Won't fix" in order to help anyone coming here in the future, considering it fixed and looking for what was changed.

In MR!39, I went on with the second proposed solution and introduced hook_search_api_autocomplete_suggester_item_url_alter().

Feel free to change the hook's name if you desire it; I tried to avoid using the "live_results" name somehow in case it's used somewhere else in the future too.

vensires → created an issue. See original summary → .

I wonder whether this is in the scope of this issue...
I did update the issue summary with your remarks though.

I changed the MR to match what's already proposed in #25 🐛 SDC components CSS & JS generated wrong url in windows / XAMPP Active along with a small comment why line of code is needed. I also I set it as RTBC based on #27, since it's decided we won't need tests.

Module already supports D11.

It seems there was a lot of deprecated code, leftover from D7 which is now removed.
Setting this to Fixed and creating a new stable version.

Thank you @m.managoudis for your work on this!

I believe we could go on with this issue for the standard mode, yes. I set it as "Needs Review" but it could be "RTBC" too.

Just one question to answer:
Do we need anything extra (change README.md, inline comment, extra validation etc) to mention that the implementation targets the Standard export mode only? Or is it just... discovered by anyone trying?

I tested both patches with the batch implementation and they fail to work properly. Whether this works or not depends on the row values of each batch. If no values exist at all in a specific column, it does get hidden. But if the first batch has values and the second hasn't, then the second set's columns are shifted. This is especially visible if there are other columns with values next to the one being hidden.

To be honest, I personally can't guess any possible way to implement this in batch without possible performance issues.

@m.managoudis please look into it along with 🐛 Add support for IRIS payments Active so that we can deploy a new version immediately after both.

vensires → created an issue.

Yes, let me explain to you my exact use case... Intersection!

Let's say we have two nodes:

Node #1:
- Tags: A, B, C, D

Node #2:
- Tags: C, D, E

A simple user uses facets to filter the results of an indexed view displaying the nodes matching B or C or D.
As an admin I set the view to sort the results based on Solr's score. So, I expect Solr to return Node #1 first and then Node #2 since #1 matches more terms than #2 - based on what the user has filtered by.

Since both nodes match the B or C or D condition, both are displayed. But... since using fq instead of q, their relevancy score is "1.0" for both.

@saschaeggi it's a small change and the screenshots provided in the latest comment do prove it's working correctly. I set it as RTBC.

@chaitanyadessai, patch from #2 seems to work fine in latest views_data_export stable version. Is your patch maybe based on top of the -dev version? I believe we can set it as RTBC but just wanted to clarify this one for the maintainers to know what to merge.

Thanks for the answer. Yes, I did debug the query (attached) and the conditions from facets turn out to really be living in fq.

What did the job for me was the following:

namespace Drupal\custom\EventSubscriber;

use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\search_api_solr\Event\PostConvertedQueryEvent;
use Drupal\search_api_solr\Event\SearchApiSolrEvents;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
 * Event subscriber for solr queries.
 */
final class CustomSolrQuerySubscriber implements EventSubscriberInterface {

  /**
   * Reacts to the post convert query event.
   *
   * @param \Drupal\search_api_solr\Event\PostConvertedQueryEvent $event
   *   The post converted query event.
   */
  public function solrQueryAlter(PostConvertedQueryEvent $event) {
    $search_api_query = $event->getSearchApiQuery();
    // The "content_relevancy_sorting" tag is set from Views UI.
    if (in_array('content_relevancy_sorting', $search_api_query->getTags(), TRUE)) {
      $sorts = &$search_api_query->getSorts();
      if (isset($sorts['search_api_relevance'])) {
        $solarium_query = $event->getSolariumQuery();
        $filter_queries = $solarium_query->getFilterQueries();
        $q = [];
        foreach ($filter_queries as $filter_id => $filter_query) {
          if (!str_starts_with($filter_id, 'filters_')) {
            continue;
          }
          $q[] = $filter_query->getOption('query');
        }
        $string_query = $solarium_query->getQuery() . ' AND ' . implode(' AND ', $q);
        $solarium_query->setQuery($string_query);
      }
    }
  }

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents(): array {
    return [
      SearchApiSolrEvents::POST_CONVERT_QUERY => 'solrQueryAlter',
    ];
  }

}

What I basically do in the query above is I take the conditions from fq and also add them with "AND" to the "q" parameter. But I wonder whether this should really require extra code instead of UI.

@smustgrave the navigation_top_bar module is still there but marked with hidden: true.

As a sum up, I think that what we should keep from the comments above is that patch #270 works and fixes all the issues BUT the one described in #283 where a facets summary exists in the page. Might it be that the solution identified in #274 is what we need?

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

I can't turn https://git.drupalcode.org/project/openai/-/merge_requests/98 to "ready" state but it does what it promises, related to the specific issue description. I set it as RTBC.

I went on with the rebase of the commits. I think the tests do need a check from someone who knows what he's doing + also check what @smustgrave said about the use of the starterkit_theme.