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

(dsl): Add has parent query #188

Merged
merged 10 commits into from
Apr 27, 2023
Merged

Conversation

markaya
Copy link
Contributor

@markaya markaya commented Apr 25, 2023

Resolves hasParentQuery in #91

Summary:

  • Added has parent query
  • Added tests for has parent query

Copy link
Collaborator

@dbulaja98 dbulaja98 left a comment

Choose a reason for hiding this comment

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

Seems good, can you just add it-tests?

Comment on lines 125 to 129
def withScore(value: Boolean): HasParentQuery[S]

def withScoreTrue: HasParentQuery[S] = withScore(true)

def withScoreFalse: HasParentQuery[S] = withScore(false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe to extract this like rest of option params? Maybe it will occur in some other query?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would leave it for now as I haven't found any that have this field.

score: Option[Boolean] = None
) extends HasParentQuery[S] { self =>

override def ignoreUnmapped(value: Boolean): HasParentQuery[S] =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove override.

override def ignoreUnmapped(value: Boolean): HasParentQuery[S] =
self.copy(ignoreUnmapped = Some(value))

override def paramsToJson(fieldPath: Option[String]): Json =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same.

* Constructs an instance of [[zio.elasticsearch.query.HasParent]] using the specified parameters.
*
* @param parentType
* a name of the parent relationship mapped for the join field.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* a name of the parent relationship mapped for the join field.
* a name of the parent relationship mapped for the join field

* @param parentType
* a name of the parent relationship mapped for the join field.
* @param query
* query you wish to run on parent documents of the parent_type field.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* query you wish to run on parent documents of the parent_type field.
* query you wish to run on parent documents of the `parent_type` field

@dbulaja98?

* Constructs an instance of [[zio.elasticsearch.query.HasParent]] using the specified parameters.
*
* @param parentType
* a name of the parent relationship mapped for the join field.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* a name of the parent relationship mapped for the join field.
* a name of the parent relationship mapped for the join field

* @param parentType
* a name of the parent relationship mapped for the join field.
* @param query
* query you wish to run on parent documents of the parent_type field.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* query you wish to run on parent documents of the parent_type field.
* query you wish to run on parent documents of the `parent_type` field

sealed trait HasParentQuery[S] extends ElasticQuery[S] with HasIgnoreUnmapped[HasParentQuery[S]] {
def withScore(value: Boolean): HasParentQuery[S]

def withScoreTrue: HasParentQuery[S] = withScore(true)
Copy link
Member

Choose a reason for hiding this comment

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

  1. Put final def here.
  2. Put withScoreFalse then withScoreTrue.
  3. Should we put withScore(...) in the new line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. We did not do it so far.

@drmarjanovic drmarjanovic merged commit ef85468 into main Apr 27, 2023
@drmarjanovic drmarjanovic deleted the task-support-has-parent-query branch April 27, 2023 11:03
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