Skip to content
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

[BUG] Islandora Advanced Search javascript breaks core Facets javascript #2100

Closed
wgilling opened this issue Apr 28, 2022 · 8 comments
Closed
Labels
Type: bug identifies a problem in the software with clear steps to reproduce

Comments

@wgilling
Copy link
Contributor

wgilling commented Apr 28, 2022

When the islandora_advanced_search module is installed and a user views a basic search, the javascript that is able to limit results via the "Soft limit" is not able to run. Disabling the module or copying the core facets/js/soft-limit.js and facets-views-ajax.js over the modified instances of these that exist under islandora_advanced_search/js/facets will allow both search interfaces to have the working functionality for at least the Soft limit feature.

The underlying conflict is that both modules have the same behavior name. Even though I do not see how the islandora_advanced_search javascript is being included (because I thought that required a libraries declaration and an #attached method to bring that into the resultant HTML).

I believe that the modifications that were made in islandora_advanced_search to this javascript must have been made for some reason, but I do not clearly see the distinction. At least, the "Soft limits" javascript should still perform when this module is enabled and while on the basic search interface.

NOTE: when it exhibits the bug, there are no javascript errors, but the code simply does not do what the core facets javascript used to do.

What steps does it take to reproduce the issue?

  1. Install islandora_advanced_search module (if it is not enabled already)
  2. Perform a basic search via solr-search/content?search_api_fulltext=&sort_by=search_api_relevance&sort_order=DESC&items_per_page=10 to see all of the facts that result from the content -- and with at least one of the facets that has more than 3 items, configure it to have a "Soft limit" of 3 via admin/config/search/facets
  3. Perform the same basic search and see that the facet still displays all possible facets instead of limiting the results as configured

Which page does it occur on?
any solr-search results page such as solr-search/content?search_api_fulltext=&sort_by=search_api_relevance&sort_order=DESC&items_per_page=10

What happens?
all facet configurations that are set to have any Soft limit are not being limited

To whom does it occur (anonymous visitor, editor, administrator)?
any user who performs the search

What did you expect to happen?
the facets on the basic solr_search search page solr-search/content?search_api_fulltext=&sort_by=search_api_relevance&sort_order=DESC&items_per_page=10 would be limited as they are configured.

Which version of Islandora are you using?
2.x

Any related open or closed issues to this bug report?
not that I am aware

@wgilling wgilling added the Type: bug identifies a problem in the software with clear steps to reproduce label Apr 28, 2022
@simonhm
Copy link
Contributor

simonhm commented Apr 28, 2022

Tested. I confirm this bug is also happening on our testing I8 sites.
We're running 2.2.1.

@simonhm
Copy link
Contributor

simonhm commented Apr 28, 2022

https://github.com/Islandora/islandora/blob/2.x/modules/islandora_advanced_search/js/facets/soft-limit.js

The Islandora Advanced Search module overrides the soft-limit.js behavior from the 'facets' module.
as when having many facets the original version causes the page to slow down and snap to hidden when rendering.

It looks like this override version doesn't work any more. I tried to fix it and the fixed version becomes as same as the original soft-limit.js of "facets" module. ^_^
Maybe don't override the soft-limit.js of "facet" module, then everything should work as expected.
Simon.

@wgilling
Copy link
Contributor Author

wgilling commented Apr 29, 2022

Thank you @simonhm for taking such a thorough look at this. There seemed to be some changes in the soft-limits.js and are those all able to be abandoned (the only bit I remember is that it was applying some logic to the

  • element that the core facets javascript didn't seem to do. I want to be sure that it still works for both modules, so whatever that logic was could maybe be merged in with the logic from the core soft-limits.js in order to do what it needs to for both.

  • @wgilling
    Copy link
    Contributor Author

    wgilling commented May 4, 2022

    I cannot write a new version of this that keeps all of the logic that was added for the islandora_advanced_search code in the javascript while not breaking the facets' module javascript ability to run the soft limits code. Perhaps the best way is to refactor the bit in this javascript that is breaking the soft-limits method when not using the islandora_advanced_search form.

    @nigelgbanks -- how do you propose this soft-limits.js be fixed?

    @nigelgbanks
    Copy link
    Member

    I believe the original reason for the override was to deal with performance. At the time the module was written if there were a couple hundred facets the site would take upwards of like 10 seconds to render. If the the soft-limit.js provided by the facets modules no longer has any performance issues in that sense and doesn't cause any further bugs I think the override could be removed completely.

    @wgilling
    Copy link
    Contributor Author

    If that is true (performance is no longer an issue), would the plan be to remove that overridden javascript file or files (including the hook to alter where these libraries are stored - so it pulls the core facets one from now on) and merge that into the dev-main for this?

    @nigelgbanks
    Copy link
    Member

    Ya basically there wouldn't be any need to override the facets module so those bits could be removed.

    @wgilling
    Copy link
    Contributor Author

    This should address the issue - and allow the core facets javascript to run. Islandora/islandora#870

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Type: bug identifies a problem in the software with clear steps to reproduce
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants