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

feat: using setQuery util in tests that need it #1322

Conversation

albertjcuac
Copy link
Contributor

@albertjcuac albertjcuac commented Oct 13, 2023

Pull request template

Motivation and context

This task is to check in the different modules that all mutations of a query are taking advantage of the setQuery util functionality and import the setQuery & use it if necessary. I noticed that in some tests we weren't using the setQuery function

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

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:

@albertjcuac albertjcuac requested a review from a team as a code owner October 13, 2023 06:31
Copy link
Contributor

@CachedaCodes CachedaCodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gj!

Aside from replacing the mutations in the module.ts of the modules you also have to check the types. The mutations type of the modules should extend of QueryMutations instead of defining the setQuery type. Check the History Queries module to see an example.


/**
* SemanticQueries store state.
*
* @public
*/
export interface SemanticQueriesState {
export interface SemanticQueriesState extends QueryState {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from extending QueryState you have to remove the query type definition in the state. QueryState already has the query type definition in it, so you're basically duplicating the type definition.

There are several modules that do this wrong so remember to fix those too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @albertjcuac, @CachedaCodes : I think this should be also done for mutations if we are extending QueryMutations: is that right?
Example below:
Captura de pantalla 2023-10-18 a les 8 47 59

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's true

@annacv annacv requested a review from CachedaCodes October 18, 2023 06:50

/**
* SemanticQueries store state.
*
* @public
*/
export interface SemanticQueriesState {
export interface SemanticQueriesState extends QueryState {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's true

import { SemanticQueriesXStoreModule } from './types';
import { fetchSemanticQuery } from './actions/fetch-semantic-query.action';
import { fetchAndSaveSemanticQuery } from './actions/fetch-and-save-semantic-query.action';
import { request } from './getters/request.getter';
import { normalizedQuery } from './getters/normalized-query.getter';

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here should be a linebreak

Copy link
Contributor

@annacv annacv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gj!
I have been searching and I think it is all done!

@annacv annacv requested a review from CachedaCodes October 19, 2023 07:15
@CachedaCodes CachedaCodes merged commit 9749462 into main Oct 19, 2023
3 of 4 checks passed
@CachedaCodes CachedaCodes deleted the feature/EMP-2424-Implement-query-utils-to-the-modules-that-have-a-setQuery-mutation branch October 19, 2023 12:16
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.

3 participants