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

Event search with list of artists as search term #3

Open
ReIaxo opened this issue Jun 6, 2023 · 6 comments
Open

Event search with list of artists as search term #3

ReIaxo opened this issue Jun 6, 2023 · 6 comments

Comments

@ReIaxo
Copy link

ReIaxo commented Jun 6, 2023

Hi, first thank you for this amazing library. It is very convenient and nearly fulfills all my needs.
But I have on question: Is it possible to search for an event while adding multiple artist search terms?
I tried this, where i have to chain the artist search terms together with or :

            musicBrainzService.findEvent(Limit(50), Offset(0)) {
                event(EventName.UNKNOWN) and
                        beginDate {
                            Year(LocalDate.now().year) inclusive
                                    Year(LocalDate.now().year + 1)
                        } and
                        type("Festival") and
                        (artist(ArtistName("Nightwish")) or artist(ArtistName("Blind Guartian"))
            }

But I have a generic list of artists which all have to be concatinated with or. Is there a search term where i can pass a list of artist() or artist names and the operator (or/and)?

As an alternative can one requst results with a raw serch query? Do you may have an example for this?

@pandasys
Copy link
Collaborator

pandasys commented Jun 6, 2023 via email

@ReIaxo
Copy link
Author

ReIaxo commented Jun 6, 2023

Okay. I do not fully understand all the Term, Field and EventSearch Code, but i think CompoundTerm is the right term for this usecase. Or maybe CompoundFieldExpression? But both are internal sealed. Do we need something to wire it up for usage? Like

  public fun artistIds(operator: String, mbIds: List<ArtistMbid>): Field =
    add(ArtistId, CompoundTerm(operator, mbIds.map { Term(it) }))

in EventSearch? Can we replace sealed with open on CompoundTerm so we can instantiate it?

Created PullRequest, can you review it and tell me if this may work?

@ReIaxo
Copy link
Author

ReIaxo commented Jun 7, 2023

Ok, I think it works. Tested it with Artist Area in you sample application. I am not able to test it with my app, there a lots of dependency errors when i include ealvabrainz directly as project.

ArtistSearch.kt

public fun areas(operator: String, areaNames: List<AreaName>): Field =
    add(Area, CompoundTerm(operator, areaNames.map { Term(it) }))

ArtistSearchViewModel:

        brainz.findArtist {
          artist(ArtistName(query)) and areas(
            " or ",
            listOf(AreaName("Finland"), AreaName("Germany"))
          )

So i think the clean way is to add such a method to each field in each search right? Or is there a more generic way?
And maybe a constant string should be added for " or " and " and ".

Can you estimate how long it takes for you to review and maybe improve it? When the above written is valid, i can do the changes and update the pullrequest.

@pandasys
Copy link
Collaborator

pandasys commented Jun 7, 2023 via email

@ReIaxo
Copy link
Author

ReIaxo commented Jun 23, 2023

Hi, anything new here? I am just asking because this will become a little blocker soon for me. I will do as much as I can by myself, but I need the qestions to be answered.

  1. public fun areas(operator: String, areaNames: List<AreaName>): Field for each search or is there a more generic way?
  2. Better way to pass the operator. Create const String and pass it or maybe just changing the method signature to operator: WhateverTypeYourOperatorIs (I'm very new to Kotlin DSL)

I don't want to put pressure, it is not that bad if it takes longer, but i would be happy if we can make progress in the next days.

@ReIaxo
Copy link
Author

ReIaxo commented Nov 7, 2023

Ok, I tried another approach which also works but avoids makeing CompoundTerm open and with no need for a new operator or const String.

ArtistSearch.kt

public fun artistIds(compoundTerm: CompoundTerm): Field = add(ArtistId, compoundTerm)

ArtistSearchViewModel:

brainz.findArtist {
     artist(ArtistName(query)) and areas(OrOp(listOf("Finland".toTerm(), "Germany".toTerm())))
}

But this question is still valid:

So i think the clean way is to add such a method to each field in each search right? Or is there a more generic way?

But I think it can be answered with Yes, we have to add it for each field in each search.

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

No branches or pull requests

2 participants