From 33d3cd1a4190e9dfb6f81b7b074782528eccc3a6 Mon Sep 17 00:00:00 2001 From: Gunnar Velle Date: Fri, 13 Dec 2024 10:07:37 +0100 Subject: [PATCH] Remove unused contextFilters and add traits filter --- .../controller/SearchController.scala | 27 ++++++++++--------- .../parameters/DraftSearchParamsDTO.scala | 4 ++- .../parameters/SearchParamsDTO.scala | 3 +++ .../model/api/MultiSearchSummaryDTO.scala | 4 +-- .../searchapi/model/search/SearchTrait.scala | 12 ++++----- .../settings/MultiDraftSearchSettings.scala | 4 ++- .../search/settings/SearchSettings.scala | 3 +++ .../search/MultiDraftSearchService.scala | 5 ++++ .../service/search/MultiSearchService.scala | 6 +++++ .../search/SearchConverterService.scala | 3 +-- .../scala/no/ndla/searchapi/TestData.scala | 2 ++ .../search/MultiSearchServiceTest.scala | 13 +++++++-- .../search/SearchConverterServiceTest.scala | 2 +- 13 files changed, 60 insertions(+), 28 deletions(-) diff --git a/search-api/src/main/scala/no/ndla/searchapi/controller/SearchController.scala b/search-api/src/main/scala/no/ndla/searchapi/controller/SearchController.scala index 7ceece66e..71c5c77a0 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/controller/SearchController.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/controller/SearchController.scala @@ -32,7 +32,7 @@ import no.ndla.searchapi.integration.SearchApiClient import no.ndla.searchapi.model.api.grep.GrepSearchResultsDTO import no.ndla.searchapi.model.api.{ErrorHandling, GroupSearchResultDTO, MultiSearchResultDTO, SubjectAggregationsDTO} import no.ndla.searchapi.model.domain.{LearningResourceType, Sort} -import no.ndla.searchapi.model.search.SearchType +import no.ndla.searchapi.model.search.{SearchTrait, SearchType} import no.ndla.searchapi.model.search.settings.{MultiDraftSearchSettings, SearchSettings} import no.ndla.searchapi.service.search.{ GrepSearchService, @@ -125,12 +125,6 @@ trait SearchController { |If subjects are specified the learning resource must have specified relevances in relation to a specified subject. |If levels are specified the learning resource must have specified relevances in relation to a specified level.""".stripMargin ) - private val contextFilters = listQuery[String]("context-filters") - .description( - """A comma separated list of resource-types the learning resources should be filtered by. - |Used in conjunction with the parameter resource-types to filter additional resource-types. - """.stripMargin - ) private val includeMissingResourceTypeGroup = query[Boolean]("missing-group") .description( "Whether to include group without resource-types for group-search. Defaults to false." @@ -138,6 +132,8 @@ trait SearchController { .default(false) private val grepCodes = listQuery[String]("grep-codes") .description("A comma separated list of codes from GREP API the resources should be filtered by.") + private val traits = listQuery[String]("traits") + .description("A comma separated list of traits the resources should be filtered by.") private val scrollId = query[Option[String]]("search-context") .description( s"""A unique string obtained from a search you want to keep scrolling in. To obtain one from a search, provide one of the following values: ${InitialScrollContextKeywords @@ -199,10 +195,10 @@ trait SearchController { .in(contextTypes) .in(languageFilter) .in(relevanceFilter) - .in(contextFilters) .in(includeMissingResourceTypeGroup) .in(aggregatePaths) .in(grepCodes) + .in(traits) .in(embedResource) .in(embedId) .in(filterInactive) @@ -223,10 +219,10 @@ trait SearchController { contextTypes, languageFilter, relevanceFilter, - anotherResourceTypes, includeMissingResourceTypeGroup, aggregatePaths, grepCodes, + traits, embedResource, embedId, filterInactive, @@ -249,11 +245,12 @@ trait SearchController { sort = sort, withIdIn = learningResourceIds.values, subjects = subjects.values, - resourceTypes = groupTypes.values ++ anotherResourceTypes.values, + resourceTypes = groupTypes.values, learningResourceTypes = contextTypes.values.flatMap(LearningResourceType.valueOf), supportedLanguages = languageFilter.values, relevanceIds = relevanceFilter.values, grepCodes = grepCodes.values, + traits = traits.values.flatMap(SearchTrait.valueOf), shouldScroll = false, filterByNoResourceType = false, aggregatePaths = aggregatePaths.values, @@ -380,9 +377,9 @@ trait SearchController { .in(subjects) .in(languageFilter) .in(relevanceFilter) - .in(contextFilters) .in(scrollId) .in(grepCodes) + .in(traits) .in(aggregatePaths) .in(embedResource) .in(embedId) @@ -404,9 +401,9 @@ trait SearchController { subjects, languageFilter, relevanceFilter, - contextFilters, scrollId, grepCodes, + traits, aggregatePaths, embedResource, embedId, @@ -427,11 +424,12 @@ trait SearchController { sort = sort.getOrElse(Sort.ByRelevanceDesc), withIdIn = learningResourceIds.values, subjects = subjects.values, - resourceTypes = resourceTypes.values ++ contextFilters.values, + resourceTypes = resourceTypes.values, learningResourceTypes = contextTypes.values.flatMap(LearningResourceType.valueOf), supportedLanguages = languageFilter.values, relevanceIds = relevanceFilter.values, grepCodes = grepCodes.values, + traits = traits.values.flatMap(SearchTrait.valueOf), shouldScroll = shouldScroll, filterByNoResourceType = false, aggregatePaths = aggregatePaths.values, @@ -552,6 +550,7 @@ trait SearchController { draftStatus = stringListParam("draft-status").some, users = stringListParam("users").some, grepCodes = stringListParam("grep-codes").some, + traits = stringListParam("traits").flatMap(SearchTrait.withNameOption).some, aggregatePaths = stringListParam("aggregate-paths").some, embedResource = stringListParam("embed-resource").some, embedId = stringParamOrNone("embed-id"), @@ -652,6 +651,7 @@ trait SearchController { supportedLanguages = params.languageFilter.getOrElse(List.empty), relevanceIds = params.relevance.getOrElse(List.empty), grepCodes = params.grepCodes.getOrElse(List.empty), + traits = params.traits.getOrElse(List.empty), shouldScroll = shouldScroll, filterByNoResourceType = false, aggregatePaths = params.aggregatePaths.getOrElse(List.empty), @@ -690,6 +690,7 @@ trait SearchController { statusFilter = params.draftStatus.getOrElse(List.empty).flatMap(DraftStatus.valueOf), userFilter = params.users.getOrElse(List.empty), grepCodes = params.grepCodes.getOrElse(List.empty), + traits = params.traits.getOrElse(List.empty), shouldScroll = shouldScroll, searchDecompounded = false, aggregatePaths = params.aggregatePaths.getOrElse(List.empty), diff --git a/search-api/src/main/scala/no/ndla/searchapi/controller/parameters/DraftSearchParamsDTO.scala b/search-api/src/main/scala/no/ndla/searchapi/controller/parameters/DraftSearchParamsDTO.scala index ced0e87ab..edb4c459a 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/controller/parameters/DraftSearchParamsDTO.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/controller/parameters/DraftSearchParamsDTO.scala @@ -12,7 +12,7 @@ import io.circe.{Decoder, Encoder} import no.ndla.common.model.NDLADate import no.ndla.network.tapir.NonEmptyString import no.ndla.searchapi.model.domain.Sort -import no.ndla.searchapi.model.search.SearchType +import no.ndla.searchapi.model.search.{SearchTrait, SearchType} import sttp.tapir.Schema import sttp.tapir.Schema.annotations.description @@ -68,6 +68,8 @@ case class DraftSearchParamsDTO( users: Option[List[String]], @description("A list of codes from GREP API the resources should be filtered by.") grepCodes: Option[List[String]], + @description("A comma separated list of traits the resources should be filtered by.") + traits: Option[List[SearchTrait]], @description("List of index-paths that should be term-aggregated and returned in result.") aggregatePaths: Option[List[String]], @description( diff --git a/search-api/src/main/scala/no/ndla/searchapi/controller/parameters/SearchParamsDTO.scala b/search-api/src/main/scala/no/ndla/searchapi/controller/parameters/SearchParamsDTO.scala index e05506c8c..a327eb05f 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/controller/parameters/SearchParamsDTO.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/controller/parameters/SearchParamsDTO.scala @@ -13,6 +13,7 @@ import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder} import io.circe.{Decoder, Encoder} import no.ndla.network.tapir.NonEmptyString import no.ndla.searchapi.model.domain.Sort +import no.ndla.searchapi.model.search.SearchTrait import sttp.tapir.Schema import sttp.tapir.Schema.annotations.description @@ -58,6 +59,8 @@ case class SearchParamsDTO( languageFilter: Option[List[String]], @description("A list of codes from GREP API the resources should be filtered by.") grepCodes: Option[List[String]], + @description("A comma separated list of traits the resources should be filtered by.") + traits: Option[List[SearchTrait]], @description("List of index-paths that should be term-aggregated and returned in result.") aggregatePaths: Option[List[String]], @description( diff --git a/search-api/src/main/scala/no/ndla/searchapi/model/api/MultiSearchSummaryDTO.scala b/search-api/src/main/scala/no/ndla/searchapi/model/api/MultiSearchSummaryDTO.scala index 82e702d84..ba6aebe0d 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/model/api/MultiSearchSummaryDTO.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/model/api/MultiSearchSummaryDTO.scala @@ -12,7 +12,7 @@ import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder} import no.ndla.common.model.NDLADate import no.ndla.common.model.api.draft.CommentDTO import no.ndla.searchapi.model.domain.LearningResourceType -import no.ndla.searchapi.model.search.SearchType +import no.ndla.searchapi.model.search.{SearchTrait, SearchType} import sttp.tapir.Schema.annotations.description @description("Object describing matched field with matching words emphasized") @@ -38,7 +38,7 @@ case class MultiSearchSummaryDTO( @description("Languages the resource exists in") supportedLanguages: Seq[String], @description("Learning resource type") learningResourceType: LearningResourceType, @description("Status information of the resource") status: Option[StatusDTO], - @description("Traits for the resource") traits: List[String], + @description("Traits for the resource") traits: List[SearchTrait], @description("Relevance score. The higher the score, the better the document matches your search criteria.") score: Float, @description("List of objects describing matched field with matching words emphasized") highlights: List[HighlightedFieldDTO], @description("The taxonomy paths for the resource") paths: List[String], diff --git a/search-api/src/main/scala/no/ndla/searchapi/model/search/SearchTrait.scala b/search-api/src/main/scala/no/ndla/searchapi/model/search/SearchTrait.scala index cad147b93..c85840eba 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/model/search/SearchTrait.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/model/search/SearchTrait.scala @@ -18,15 +18,15 @@ sealed abstract class SearchTrait(override val entryName: String) extends EnumEn override def toString: String = entryName } object SearchTrait extends Enum[SearchTrait] with CirceEnumWithErrors[SearchTrait] { - case object Video extends SearchTrait("VIDEO") - case object H5p extends SearchTrait("H5P") - case object Audio extends SearchTrait("AUDIO") - case object File extends SearchTrait("FILE") + case object Video extends SearchTrait("VIDEO") + case object H5p extends SearchTrait("H5P") + case object Audio extends SearchTrait("AUDIO") + case object Podcast extends SearchTrait("PODCAST") def all: List[String] = SearchTrait.values.map(_.toString).toList override def values: IndexedSeq[SearchTrait] = findValues - - implicit def schema: Schema[SearchTrait] = schemaForEnumEntry[SearchTrait] + def valueOf(s: String): Option[SearchTrait] = SearchTrait.values.find(_.entryName == s) + implicit def schema: Schema[SearchTrait] = schemaForEnumEntry[SearchTrait] implicit val enumTsType: TSNamedType[SearchTrait] = TSType.alias[SearchTrait]("SearchTrait", TSUnion(values.map(e => TSLiteralString(e.entryName)))) diff --git a/search-api/src/main/scala/no/ndla/searchapi/model/search/settings/MultiDraftSearchSettings.scala b/search-api/src/main/scala/no/ndla/searchapi/model/search/settings/MultiDraftSearchSettings.scala index 1f8f25ccf..9298fac55 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/model/search/settings/MultiDraftSearchSettings.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/model/search/settings/MultiDraftSearchSettings.scala @@ -12,7 +12,7 @@ import no.ndla.common.model.domain.draft.DraftStatus import no.ndla.language.Language import no.ndla.network.tapir.NonEmptyString import no.ndla.searchapi.model.domain.{LearningResourceType, Sort} -import no.ndla.searchapi.model.search.SearchType +import no.ndla.searchapi.model.search.{SearchTrait, SearchType} case class MultiDraftSearchSettings( query: Option[NonEmptyString], @@ -33,6 +33,7 @@ case class MultiDraftSearchSettings( statusFilter: List[DraftStatus], userFilter: List[String], grepCodes: List[String], + traits: List[SearchTrait], shouldScroll: Boolean, searchDecompounded: Boolean, aggregatePaths: List[String], @@ -72,6 +73,7 @@ object MultiDraftSearchSettings { statusFilter = List.empty, userFilter = List.empty, grepCodes = List.empty, + traits = List.empty, shouldScroll = false, searchDecompounded = false, aggregatePaths = List.empty, diff --git a/search-api/src/main/scala/no/ndla/searchapi/model/search/settings/SearchSettings.scala b/search-api/src/main/scala/no/ndla/searchapi/model/search/settings/SearchSettings.scala index f076ecb59..05a931a5c 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/model/search/settings/SearchSettings.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/model/search/settings/SearchSettings.scala @@ -11,6 +11,7 @@ import no.ndla.common.model.domain.Availability import no.ndla.language.Language import no.ndla.network.tapir.NonEmptyString import no.ndla.searchapi.model.domain.{LearningResourceType, Sort} +import no.ndla.searchapi.model.search.SearchTrait case class SearchSettings( query: Option[NonEmptyString], @@ -27,6 +28,7 @@ case class SearchSettings( supportedLanguages: List[String], relevanceIds: List[String], grepCodes: List[String], + traits: List[SearchTrait], shouldScroll: Boolean, filterByNoResourceType: Boolean, aggregatePaths: List[String], @@ -53,6 +55,7 @@ object SearchSettings { supportedLanguages = List.empty, relevanceIds = List.empty, grepCodes = List.empty, + traits = List.empty, shouldScroll = false, filterByNoResourceType = false, aggregatePaths = List.empty, diff --git a/search-api/src/main/scala/no/ndla/searchapi/service/search/MultiDraftSearchService.scala b/search-api/src/main/scala/no/ndla/searchapi/service/search/MultiDraftSearchService.scala index a500ee18e..118cfda80 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/service/search/MultiDraftSearchService.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/service/search/MultiDraftSearchService.scala @@ -274,6 +274,10 @@ trait MultiDraftSearchService { if (settings.grepCodes.nonEmpty) Some(termsQuery("grepContexts.code", settings.grepCodes)) else None + val traitsFilter = + if (settings.traits.nonEmpty) Some(termsQuery("traits", settings.traits.map(_.entryName))) + else None + val embedResourceAndIdFilter = buildNestedEmbedField(settings.embedResource, settings.embedId, settings.language, settings.fallback) @@ -321,6 +325,7 @@ trait MultiDraftSearchService { statusFilter, usersFilter, grepCodesFilter, + traitsFilter, embedResourceAndIdFilter, revisionDateFilter, publishedDateFilter, diff --git a/search-api/src/main/scala/no/ndla/searchapi/service/search/MultiSearchService.scala b/search-api/src/main/scala/no/ndla/searchapi/service/search/MultiSearchService.scala index 14fc10ed1..8735f4593 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/service/search/MultiSearchService.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/service/search/MultiSearchService.scala @@ -146,6 +146,11 @@ trait MultiSearchService { Some(termsQuery("grepContexts.code", settings.grepCodes)) else None + val traitsFilter = + if(settings.traits.nonEmpty) + Some(termsQuery("traits", settings.traits.map(_.entryName))) + else None + val embedResourceAndIdFilter = buildNestedEmbedField(settings.embedResource, settings.embedId, settings.language, settings.fallback) @@ -181,6 +186,7 @@ trait MultiSearchService { taxonomyRelevanceFilter, taxonomyContextActiveFilter, grepCodesFilter, + traitsFilter, embedResourceAndIdFilter, availabilityFilter ).flatten diff --git a/search-api/src/main/scala/no/ndla/searchapi/service/search/SearchConverterService.scala b/search-api/src/main/scala/no/ndla/searchapi/service/search/SearchConverterService.scala index d128a5621..74b980a14 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/service/search/SearchConverterService.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/service/search/SearchConverterService.scala @@ -87,8 +87,7 @@ trait SearchConverterService { traits += SearchTrait.Video } case "audio" => traits += SearchTrait.Audio - case "podcast" => traits += SearchTrait.Audio - case "file" => traits += SearchTrait.File + case "podcast" => traits += SearchTrait.Podcast case _ => // Do nothing } }) diff --git a/search-api/src/test/scala/no/ndla/searchapi/TestData.scala b/search-api/src/test/scala/no/ndla/searchapi/TestData.scala index 9667dff1b..a2b27d818 100644 --- a/search-api/src/test/scala/no/ndla/searchapi/TestData.scala +++ b/search-api/src/test/scala/no/ndla/searchapi/TestData.scala @@ -1599,6 +1599,7 @@ object TestData { supportedLanguages = List.empty, relevanceIds = List.empty, grepCodes = List.empty, + traits = List.empty, shouldScroll = false, filterByNoResourceType = false, aggregatePaths = List.empty, @@ -1628,6 +1629,7 @@ object TestData { statusFilter = List.empty, userFilter = List.empty, grepCodes = List.empty, + traits = List.empty, shouldScroll = false, searchDecompounded = false, aggregatePaths = List.empty, diff --git a/search-api/src/test/scala/no/ndla/searchapi/service/search/MultiSearchServiceTest.scala b/search-api/src/test/scala/no/ndla/searchapi/service/search/MultiSearchServiceTest.scala index 29695bff3..a605b568c 100644 --- a/search-api/src/test/scala/no/ndla/searchapi/service/search/MultiSearchServiceTest.scala +++ b/search-api/src/test/scala/no/ndla/searchapi/service/search/MultiSearchServiceTest.scala @@ -17,7 +17,7 @@ import no.ndla.scalatestsuite.IntegrationSuite import no.ndla.searchapi.TestData.* import no.ndla.searchapi.model.api.MetaImageDTO import no.ndla.searchapi.model.domain.{IndexingBundle, LearningResourceType, Sort} -import no.ndla.searchapi.model.search.SearchPagination +import no.ndla.searchapi.model.search.{SearchPagination, SearchTrait} import no.ndla.searchapi.{TestData, TestEnvironment, UnitSuite} import scala.util.Success @@ -705,9 +705,18 @@ class MultiSearchServiceTest multiSearchService.matchingQuery(searchSettings.copy(query = Some(NonEmptyString.fromString("Ekstrastoff").get))) search.totalCount should be(1) search.results.head.id should be(12) - search.results.head.traits should be(List("H5P")) + search.results.head.traits should be(List(SearchTrait.H5p)) } + test("That search can be filtered by traits") { + val Success(search) = + multiSearchService.matchingQuery(searchSettings.copy(traits = List(SearchTrait.H5p))) + search.totalCount should be(1) + search.results.head.id should be(12) + search.results.head.traits should be(List(SearchTrait.H5p)) + } + + test("That searches for embed attributes matches") { val Success(search) = multiSearchService.matchingQuery( diff --git a/search-api/src/test/scala/no/ndla/searchapi/service/search/SearchConverterServiceTest.scala b/search-api/src/test/scala/no/ndla/searchapi/service/search/SearchConverterServiceTest.scala index a8d8d0a9a..2af05318d 100644 --- a/search-api/src/test/scala/no/ndla/searchapi/service/search/SearchConverterServiceTest.scala +++ b/search-api/src/test/scala/no/ndla/searchapi/service/search/SearchConverterServiceTest.scala @@ -538,7 +538,7 @@ class SearchConverterServiceTest extends UnitSuite with TestEnvironment { article, IndexingBundle(Some(TestData.emptyGrepBundle), Some(emptyBundle), None) ) - searchableArticle.traits should equal(List(SearchTrait.H5p, SearchTrait.File)) + searchableArticle.traits should equal(List(SearchTrait.H5p)) val article2 = TestData.emptyDomainArticle.copy(