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

Fix: Next Queries not being calculated when using Related Tags #1428

Merged
merged 5 commits into from
Mar 7, 2024

Conversation

annacv
Copy link
Contributor

@annacv annacv commented Mar 5, 2024

Pull request template

Describe the purpose of the change, the specific changes done in detail, and the issue you have fixed.
Make Next Queries requests take into account when a query is selected along with a related tag:

  • Make NQ storeModule have a RTs array in the state to save the selected RTs
  • Make NQ storeModule have a mutation to update the RTs in the state
  • Make NQ storeModule have a query getter to concat the query with the RTs (if selected)
  • Make NQ request use the query getter.
  • Make NQ wiring react to the event SelectedRelatedTagsChanged & call setRelatedTags mutation

Motivation and context

Currently, the Next Queries module doesn’t take into account the selected related tags. This creates some weird scenarios like having NQ when searching for novela negra but not having NQ when searching for novela and then selecting the RT negra

  • Dependencies. If any, specify:
  • Open issue. If applicable, link: EMP-3653

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that causes existing functionality to not work as expected)
  • Change requires a documentation update

What is the destination branch of this PR?

  • Main
  • Other. Specify:

How has this been tested?

Test in CDL:
Use a local build of x-components & env=live to test

  • Test: ?env=live&query=novela%20negra
  • Test: ?env=live&query=novela + select the “negra” RT
    Then:
  • Next queries state has an RT array with the RT saved
  • A NQ request has been done & given the same results searching for the full query “novela negra” as searching for “novela” and then, selecting the “negra” RT

Tests performed according to testing guidelines:

Checklist:

  • My code follows the style guidelines of this project.
  • I have performed a self-review on my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

@annacv annacv changed the title Fix: 3597 nq not calculated when using rt Fix: Next Queries not being calculated when using Related Tags Mar 5, 2024
@annacv
Copy link
Contributor Author

annacv commented Mar 5, 2024

👀 We should also make changes to CustomNextQueryPreview component in the archetype to handle the use case of a query + an RT when showing what users have searched for. A task created accordingly.

@annacv annacv marked this pull request as ready for review March 5, 2024 09:29
@annacv annacv requested a review from a team as a code owner March 5, 2024 09:29
@albertjcuac albertjcuac merged commit 50306b6 into main Mar 7, 2024
4 checks passed
@annacv annacv deleted the bugfix/EMP-3597-NQ-not-calculated-when-using-RT branch March 26, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants