Skip to content

Add facets 3.x to composer.json #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

aOelschlager
Copy link
Contributor

What does this Pull Request do?

Adds facets 3.x to the composer.json file. I haven't tested this yet so that's why it's a draft for now. Need to see if the change to AJAX will break anything.

Provide a brief description of what the intended result of the PR will be and/or what
problem it solves.
It shouldn't prevent people from using this module if they have facets 3.x installed.

  • Related GitHub Issue: (link)
    issue #62

  • Other Relevant Links: (Google Groups discussion, related pull requests,
    Release pull requests, etc.)

What's new?

A in-depth description of the changes made by this PR. Technical details and
possible side effects.

  • Changes x feature to such that y
  • Added x
  • Removed y
  • Does this change add any new dependencies?
  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)?
  • Could this change impact execution of existing code?

How should this be tested?

A description of what steps someone could take to:

  • Reproduce the problem you are fixing (if applicable)
  • Test that the Pull Request does what is intended.
  • Please be as detailed as possible.
  • Good testing instructions help get your PR completed faster.

Documentation Status

  • Does this change existing behaviour that's currently documented?
  • Does this change require new pages or sections of documentation?
  • Who does this need to be documented for?
  • Associated documentation pull request(s): ___ or documentation issue ___

Additional Notes:

Any additional information that you think would be helpful when reviewing this
PR.

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

@aOelschlager
Copy link
Contributor Author

I just tried testing this as a patch and I can not install facets 3.x. I don't think that is because this pull request is incorrect but because composer is checking the Drupal repository directly which does not have the patch. I'm not sure how to go about this after the namespace change.

@aOelschlager aOelschlager marked this pull request as ready for review May 2, 2025 17:47
@aOelschlager
Copy link
Contributor Author

aOelschlager commented May 2, 2025

I did manage to get this working with help from @joecorall . Now I'm getting the WSOD.

Update: drush cr fixed that.

@aOelschlager
Copy link
Contributor Author

Search button is not working on the advance search block and the sort drop down does absolutely nothing now.

@joecorall
Copy link
Member

joecorall commented May 14, 2025

@joshdentremont to test this, in your composer.json you'll need to update the repositories key, adding a reference to Annie's fork.

    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/aOelschlager/advanced_search"
        },
        {
            "type": "composer",
            "url": "https://packages.drupal.org/8"
        }
    ]

Then in the require key you can update the drupal/advanced_search version value to

"drupal/advanced_search": "dev-update_facets",

Then run compsoer update and this PR should be pulled in to your environment.

@joshdentremont
Copy link
Contributor

I think more work needs to be done. I did some testing and:

  • Advanced search "search" button changes the query string but the view is not updating.
    • From my advanced search page the search button works, so it seems to just be the search button on the results page
  • Reset button works
  • Toggling facets on and off works

My guess is the search button failure is because facets 3.x removed facets-views-ajax.js, which we are overriding in advanced search.

This is missing in facets 3.x:

https://git.drupalcode.org/project/facets/-/blob/2.0.x/src/Plugin/facets/facet_source/SearchApiDisplay.php?ref_type=heads#L440

https://git.drupalcode.org/project/facets/-/blob/2.0.x/facets.libraries.yml?ref_type=heads#L76

So when loading our search results it's not pulling in the facets-views-ajax.js from the advanced search module any more.

@ysuarez
Copy link

ysuarez commented May 21, 2025

We talked about this PR at the tech call today. We will turn this into a draft PR soon to give time to address issues Annie ran into. For example, there is some AJAX related issues in version 3.x

@adam-vessey adam-vessey marked this pull request as draft May 21, 2025 17:10
@aOelschlager
Copy link
Contributor Author

I'm just going to add these notes here that I meant to post last week.

The facet module says 3.x doesn't support ajax on facets as blocks and it recommends using the facets exposed filters submodule instead: https://git.drupalcode.org/project/facets/-/blob/3.0.x/docs/facet_blocks_support.md

However, upgrading is not required and there is no automatic upgrade path:
https://git.drupalcode.org/project/facets/-/blob/3.0.x/docs/exposed_filters.md#should-i-upgrade

Copy link

This PR is being marked as stale from inactivity and will be automatically closed in 90 days unless further action is taken. If this PR is still relevant please comment. Please also consider attending the weekly Tech Call to discuss the PR

@github-actions github-actions bot added the Stale label Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants