Skip to content

Commit

Permalink
(PC-34454)[API] fix: eligibility of futureOffer for indexation and tests
Browse files Browse the repository at this point in the history
  • Loading branch information
bpeyrou-pass committed Feb 10, 2025
1 parent f2a9ac3 commit 9be39cc
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 18 deletions.
15 changes: 12 additions & 3 deletions api/src/pcapi/core/offers/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -833,13 +833,22 @@ def isBookable(self) -> bool:

@hybrid_property
def is_eligible_for_search(self) -> bool:
if self.futureOffer:
return self.isReleased and self.futureOffer.isWaitingForPublication
if self.futureOffer and not self.isActive:
offerer = self.venue.managingOfferer
return (
offerer.isActive
and offerer.isValidated
and self.validation == OfferValidationStatus.APPROVED
and self.futureOffer.isWaitingForPublication
)
return self.is_released_and_bookable

@is_eligible_for_search.expression # type: ignore[no-redef]
def is_eligible_for_search(cls) -> BooleanClauseList: # pylint: disable=no-self-argument
return sa.and_(cls._released, sa.or_(Stock._bookable, FutureOffer.isWaitingForPublication))
return sa.or_(
sa.and_(cls._released, Stock._bookable),
sa.and_(cls.validation == OfferValidationStatus.APPROVED, FutureOffer.isWaitingForPublication),
)

@hybrid_property
def is_released_and_bookable(self) -> bool:
Expand Down
2 changes: 1 addition & 1 deletion api/src/pcapi/core/search/backends/algolia.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ def serialize_offer(cls, offer: offers_models.Offer, last_30_days_bookings: int)
"nativeCategoryId": offer.subcategory.native_category_id,
"prices": sorted(prices),
"publicationDate": (
offer.publicationDate.isoformat() if offer.publicationDate and not offer.isReleased else None
offer.publicationDate.timestamp() if offer.publicationDate and not offer.isReleased else None
),
"rankingWeight": offer.rankingWeight,
"releaseDate": release_date,
Expand Down
20 changes: 11 additions & 9 deletions api/tests/core/offers/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -747,18 +747,19 @@ def test_unicity_headline_offer_by_venue(self):

class OfferIsSearchableTest:
def test_offer_is_future(self):
offer_1 = factories.OfferFactory(isActive=True)
offer_2 = factories.OfferFactory(isActive=True)
offer_3 = factories.OfferFactory(isActive=False)
offer_1 = factories.OfferFactory(isActive=False)
offer_2 = factories.OfferFactory(isActive=False)
offer_3 = factories.OfferFactory(isActive=True)
factories.StockFactory(offer=offer_3)
future_publication_date = datetime.datetime.utcnow() + datetime.timedelta(days=30)
past_publication_date = datetime.datetime.utcnow() - datetime.timedelta(days=30)
_ = factories.FutureOfferFactory(offerId=offer_1.id, publicationDate=future_publication_date)
_ = factories.FutureOfferFactory(offerId=offer_2.id, publicationDate=past_publication_date)
_ = factories.FutureOfferFactory(offerId=offer_3.id, publicationDate=future_publication_date)
factories.FutureOfferFactory(offerId=offer_1.id, publicationDate=future_publication_date)
factories.FutureOfferFactory(offerId=offer_2.id, publicationDate=past_publication_date)
factories.FutureOfferFactory(offerId=offer_3.id, publicationDate=future_publication_date)

assert offer_1.is_eligible_for_search
assert not offer_2.is_eligible_for_search
assert not offer_3.is_eligible_for_search
assert offer_3.is_eligible_for_search

# hybrid property: also check SQL expression
results = (
Expand All @@ -768,13 +769,14 @@ def test_offer_is_future(self):
.filter(models.Offer.is_eligible_for_search)
.all()
)
assert len(results) == 1
assert len(results) == 2
assert results[0].id == offer_1.id
assert results[1].id == offer_3.id

def test_offer_is_bookable(self):
offer_1 = factories.OfferFactory(isActive=True)
offer_2 = factories.OfferFactory(isActive=False)
_ = factories.StockFactory(offer=offer_1)
factories.StockFactory(offer=offer_1)

assert offer_1.is_eligible_for_search
assert not offer_2.is_eligible_for_search
Expand Down
18 changes: 14 additions & 4 deletions api/tests/core/search/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ def make_bookable_offer() -> offers_models.Offer:
return offers_factories.StockFactory().offer


def make_future_offer() -> offers_models.Offer:
offer = offers_factories.OfferFactory(isActive=False)
publication_date = datetime.datetime.utcnow() + datetime.timedelta(days=30)
offers_factories.FutureOfferFactory(offerId=offer.id, publicationDate=publication_date)
return offer


def make_booked_offer() -> offers_models.Offer:
offer = make_bookable_offer()
offers_factories.StockFactory(offer=offer)
Expand Down Expand Up @@ -126,12 +133,14 @@ def test_index_venues_in_queue(app):
class ReindexOfferIdsTest:
def test_index_new_offer(self):
offer = make_bookable_offer()
future_offer = make_future_offer()
assert search_testing.search_store["offers"] == {}
search.reindex_offer_ids([offer.id])
assert offer.id in search_testing.search_store["offers"]
search.reindex_offer_ids([offer.id, future_offer.id])
assert set(search_testing.search_store["offers"]) == {offer.id, future_offer.id}

def test_no_unexpected_query_made(self):
offer_ids = [make_bookable_offer().id for _ in range(3)]
offer_ids.extend([make_future_offer().id for _ in range(3, 6)])

with assert_no_duplicated_queries():
search.reindex_offer_ids(offer_ids)
Expand All @@ -154,20 +163,21 @@ def test_that_base_query_is_correct(self, app):
# `reindex_offer_ids()`.
unbookable = make_unbookable_offer()
bookable = make_bookable_offer()
future_offer = make_future_offer()
past = datetime.datetime.utcnow() - datetime.timedelta(days=10)
future = datetime.datetime.utcnow() + datetime.timedelta(days=10)
multi_dates_unbookable = offers_factories.EventStockFactory(beginningDatetime=past).offer
multi_dates_bookable = offers_factories.EventStockFactory(beginningDatetime=past).offer
offers_factories.EventStockFactory(offer=multi_dates_bookable, beginningDatetime=future)
offer_ids = {unbookable.id, bookable.id, multi_dates_unbookable.id, multi_dates_bookable.id}
offer_ids = {unbookable.id, bookable.id, future_offer.id, multi_dates_unbookable.id, multi_dates_bookable.id}
for offer_id in offer_ids:
search_testing.search_store["offers"][offer_id] = "dummy"
app.redis_client.hset(algolia.REDIS_HASHMAP_INDEXED_OFFERS_NAME, offer_id, "")

with assert_no_duplicated_queries():
search.reindex_offer_ids(offer_ids)

assert set(search_testing.search_store["offers"]) == {bookable.id, multi_dates_bookable.id}
assert set(search_testing.search_store["offers"]) == {bookable.id, future_offer.id, multi_dates_bookable.id}

@mock.patch("pcapi.core.search.backends.testing.FakeClient.save_objects", fail)
@pytest.mark.settings(CATCH_INDEXATION_EXCEPTIONS=True) # as on prod: don't raise errors
Expand Down
2 changes: 1 addition & 1 deletion api/tests/core/search/test_serialize_algolia.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ def test_serialize_future_offer():
offers_factories.FutureOfferFactory(offerId=offer_2.id, publicationDate=publication_date)

serialized = algolia.AlgoliaBackend().serialize_offer(offer_1, 0)
assert serialized["offer"]["publicationDate"] == publication_date.isoformat()
assert serialized["offer"]["publicationDate"] == publication_date.timestamp()

serialized = algolia.AlgoliaBackend().serialize_offer(offer_2, 0)
assert "publicationDate" not in serialized["offer"]
Expand Down

0 comments on commit 9be39cc

Please sign in to comment.