diff --git a/api/src/pcapi/connectors/clickhouse/queries/__init__.py b/api/src/pcapi/connectors/clickhouse/queries/__init__.py index 5412049d0c5..e60e0c08286 100644 --- a/api/src/pcapi/connectors/clickhouse/queries/__init__.py +++ b/api/src/pcapi/connectors/clickhouse/queries/__init__.py @@ -1,2 +1,4 @@ +from .yearly_revenue import YearlyAggregatedCollectiveRevenueQuery +from .yearly_revenue import YearlyAggregatedIndividualRevenueQuery from .yearly_revenue import YearlyAggregatedRevenueModel from .yearly_revenue import YearlyAggregatedRevenueQuery diff --git a/api/src/pcapi/connectors/clickhouse/queries/yearly_revenue.py b/api/src/pcapi/connectors/clickhouse/queries/yearly_revenue.py index f50da6feaec..7b78e0ed02f 100644 --- a/api/src/pcapi/connectors/clickhouse/queries/yearly_revenue.py +++ b/api/src/pcapi/connectors/clickhouse/queries/yearly_revenue.py @@ -7,18 +7,30 @@ from pcapi.routes.serialization.offers_serialize import to_camel -class Revenue(pydantic_v1.BaseModel): - total: Decimal +class IndividualRevenue(pydantic_v1.BaseModel): individual: Decimal + + class Config: + extra = "forbid" + + +class CollectiveRevenue(pydantic_v1.BaseModel): collective: Decimal class Config: extra = "forbid" +class CollectiveAndIndividualRevenue(IndividualRevenue, CollectiveRevenue): + total: Decimal + + class Config: + extra = "forbid" + + class AggregatedRevenue(pydantic_v1.BaseModel): - revenue: Revenue - expected_revenue: Revenue + revenue: CollectiveAndIndividualRevenue | CollectiveRevenue | IndividualRevenue + expected_revenue: CollectiveAndIndividualRevenue | CollectiveRevenue | IndividualRevenue class Config: extra = "forbid" @@ -33,7 +45,7 @@ class Config: alias_generator = to_camel -class YearlyAggregatedRevenueQuery(BaseQuery[YearlyAggregatedRevenueModel]): +class YearlyAggregatedRevenueQueryMixin: def _format_result(self, results: list) -> dict: return { "incomeByYear": { @@ -45,6 +57,54 @@ def _format_result(self, results: list) -> dict: } } + @property + def model(self) -> type[YearlyAggregatedRevenueModel]: + return YearlyAggregatedRevenueModel + + +class YearlyAggregatedCollectiveRevenueQuery( + YearlyAggregatedRevenueQueryMixin, BaseQuery[YearlyAggregatedRevenueModel] +): + @property + def raw_query(self) -> str: + return """ + SELECT + EXTRACT(YEAR FROM creation_year) AS year, + toJSONString(map( + 'collective', ROUND(SUM(revenue),2), + ) as revenue, + toJSONString(map( + 'collective', ROUND(SUM(expected_revenue),2), + ) as expected_revenue + FROM analytics.yearly_aggregated_venue_collective_revenue + WHERE "venue_id" in %s + GROUP BY year + ORDER BY year + """ + + +class YearlyAggregatedIndividualRevenueQuery( + YearlyAggregatedRevenueQueryMixin, BaseQuery[YearlyAggregatedRevenueModel] +): + @property + def raw_query(self) -> str: + return """ + SELECT + EXTRACT(YEAR FROM creation_year) AS year, + toJSONString(map( + 'individual', ROUND(SUM(revenue),2), + ) as revenue, + toJSONString(map( + 'individual', ROUND(SUM(expected_revenue),2), + ) as expected_revenue + FROM analytics.yearly_aggregated_venue_individual_revenue + WHERE "venue_id" in %s + GROUP BY year + ORDER BY year + """ + + +class YearlyAggregatedRevenueQuery(YearlyAggregatedRevenueQueryMixin, BaseQuery[YearlyAggregatedRevenueModel]): @property def raw_query(self) -> str: return """ @@ -65,7 +125,3 @@ def raw_query(self) -> str: GROUP BY year ORDER BY year """ - - @property - def model(self) -> type[YearlyAggregatedRevenueModel]: - return YearlyAggregatedRevenueModel diff --git a/api/src/pcapi/core/offers/repository.py b/api/src/pcapi/core/offers/repository.py index db3101c1ee2..3ff465b1840 100644 --- a/api/src/pcapi/core/offers/repository.py +++ b/api/src/pcapi/core/offers/repository.py @@ -1326,3 +1326,14 @@ def merge_products(to_keep: models.Product, to_delete: models.Product) -> models db.session.delete(to_delete) return to_keep + + +def venues_have_individual_and_collective_offers(venue_ids: list[int]) -> tuple[bool, bool]: + return ( + db.session.query(offers_model.Offer.query.filter(offers_model.Offer.venueId.in_(venue_ids)).exists()).scalar(), + db.session.query( + educational_models.CollectiveOffer.query.filter( + educational_models.CollectiveOffer.venueId.in_(venue_ids) + ).exists() + ).scalar(), + ) diff --git a/api/src/pcapi/core/users/repository.py b/api/src/pcapi/core/users/repository.py index 4e8add83313..03a2fd76559 100644 --- a/api/src/pcapi/core/users/repository.py +++ b/api/src/pcapi/core/users/repository.py @@ -6,7 +6,7 @@ from dateutil.relativedelta import relativedelta from flask_sqlalchemy import BaseQuery import sqlalchemy as sa -from sqlalchemy.orm import joinedload +from sqlalchemy.dialects import postgresql from sqlalchemy.sql.functions import func import pcapi.core.offerers.models as offerers_models @@ -89,28 +89,19 @@ def has_access(user: models.User, offerer_id: int) -> bool: def has_access_to_venues(user: models.User, venue_ids: list[int]) -> bool: - """Return whether the user has access to the requested venues' data.""" - query = offerers_models.UserOfferer.query - query = query.options( - joinedload(offerers_models.UserOfferer).load_only( - offerers_models.UserOfferer.offererId, - offerers_models.UserOfferer.userId, - offerers_models.UserOfferer.isValidated, + """Return whether the user has access to all the requested venues' data.""" + return db.session.execute( + sa.select( + sa.cast(postgresql.array(venue_ids), postgresql.ARRAY(postgresql.BIGINT)).contained_by( + sa.func.array_agg(offerers_models.Venue.id) + ) ) - ) - query = query.options( - joinedload(offerers_models.Venue).load_only(offerers_models.Venue.id, offerers_models.Venue.managingOffererId) - ) - filters = [ - offerers_models.UserOfferer.offererId == offerers_models.Venue.managingOffererId, - offerers_models.UserOfferer.userId == user.id, - offerers_models.UserOfferer.isValidated, - offerers_models.Venue.id.in_(venue_ids), - ] - - query = db.session.query(query.filter(*filters).exists()).scalar() - - return query + .select_from(offerers_models.Venue) + .join(offerers_models.Offerer) + .join(offerers_models.UserOfferer) + .where(offerers_models.UserOfferer.userId == user.id) + .group_by(offerers_models.UserOfferer.userId) + ).scalar() def get_newly_eligible_age_18_users(since: date) -> list[models.User]: diff --git a/api/src/pcapi/routes/pro/statistics.py b/api/src/pcapi/routes/pro/statistics.py index 13b05baf35e..a7e72132d48 100644 --- a/api/src/pcapi/routes/pro/statistics.py +++ b/api/src/pcapi/routes/pro/statistics.py @@ -2,6 +2,7 @@ from flask_login import login_required from pcapi.connectors.clickhouse import queries as clickhouse_queries +from pcapi.core.offers.repository import venues_have_individual_and_collective_offers from pcapi.models.api_errors import ApiErrors from pcapi.routes.apis import private_api from pcapi.routes.serialization.statistics_serialize import StatisticsModel @@ -26,5 +27,11 @@ def get_statistics(query: StatisticsQueryModel) -> StatisticsModel: status_code=422, ) check_user_has_access_to_venues(current_user, venue_ids) - result = clickhouse_queries.YearlyAggregatedRevenueQuery().execute(tuple(venue_ids)) + venues_have_individual, venues_have_collective = venues_have_individual_and_collective_offers(venue_ids) + if not venues_have_individual: + result = clickhouse_queries.YearlyAggregatedCollectiveRevenueQuery().execute(tuple(venue_ids)) + elif not venues_have_collective: + result = clickhouse_queries.YearlyAggregatedIndividualRevenueQuery().execute(tuple(venue_ids)) + else: + result = clickhouse_queries.YearlyAggregatedRevenueQuery().execute(tuple(venue_ids)) return StatisticsModel.from_query(income_by_year=result.income_by_year) diff --git a/api/tests/connectors/clickhouse/fixtures.py b/api/tests/connectors/clickhouse/fixtures.py index 1c22363ba97..db0ec9061b7 100644 --- a/api/tests/connectors/clickhouse/fixtures.py +++ b/api/tests/connectors/clickhouse/fixtures.py @@ -14,18 +14,27 @@ def __init__( collective: Decimal = Decimal("12.12"), expected_individual: Decimal = Decimal("13.12"), expected_collective: Decimal = Decimal("13.12"), + only_collective: bool = False, + only_individual: bool = False, ) -> object: self.year = year - self.revenue = json.dumps( - {"individual": str(individual), "collective": str(collective), "total": str(individual + collective)} - ) - self.expected_revenue = json.dumps( - { - "individual": str(expected_individual), - "collective": str(expected_collective), - "total": str(expected_individual + expected_collective), - } - ) + if only_collective: + self.revenue = json.dumps({"collective": str(collective)}) + self.expected_revenue = json.dumps({"collective": str(expected_collective)}) + elif only_individual: + self.revenue = json.dumps({"individual": str(individual)}) + self.expected_revenue = json.dumps({"individual": str(expected_individual)}) + else: + self.revenue = json.dumps( + {"individual": str(individual), "collective": str(collective), "total": str(individual + collective)} + ) + self.expected_revenue = json.dumps( + { + "individual": str(expected_individual), + "collective": str(expected_collective), + "total": str(expected_individual + expected_collective), + } + ) YEARLY_AGGREGATED_VENUE_REVENUE = [MockYearlyAggregatedRevenueQueryResult()] @@ -35,3 +44,15 @@ def __init__( 2022, Decimal("22.12"), Decimal("22.12"), Decimal("22.12"), Decimal("22.12") ), ] +YEARLY_AGGREGATED_VENUE_REVENUE_MULTIPLE_YEARS_ONLY_COLLECTIVE = [ + MockYearlyAggregatedRevenueQueryResult(only_collective=True), + MockYearlyAggregatedRevenueQueryResult( + 2022, Decimal("22.12"), Decimal("22.12"), Decimal("22.12"), Decimal("22.12"), only_collective=True + ), +] +YEARLY_AGGREGATED_VENUE_REVENUE_MULTIPLE_YEARS_ONLY_INDIVIDUAL = [ + MockYearlyAggregatedRevenueQueryResult(only_individual=True), + MockYearlyAggregatedRevenueQueryResult( + 2022, Decimal("22.12"), Decimal("22.12"), Decimal("22.12"), Decimal("22.12"), only_individual=True + ), +] diff --git a/api/tests/routes/pro/get_statistics_test.py b/api/tests/routes/pro/get_statistics_test.py index 0680189e71e..ac91dc754ec 100644 --- a/api/tests/routes/pro/get_statistics_test.py +++ b/api/tests/routes/pro/get_statistics_test.py @@ -3,7 +3,9 @@ import pytest from pcapi.core import testing +import pcapi.core.educational.factories as educational_factories import pcapi.core.offerers.factories as offerers_factories +import pcapi.core.offers.factories as offers_factories import pcapi.core.users.factories as users_factories from tests.connectors.clickhouse import fixtures @@ -18,10 +20,14 @@ def test_get_statistics_from_one_venue(self, run_query, client): offerers_factories.UserOffererFactory(user=user, offerer=offerer) venue = offerers_factories.VenueFactory(managingOfferer=offerer) venue_id = venue.id + educational_factories.CollectiveOfferFactory(venue=venue) + offers_factories.OfferFactory(venue=venue) test_client = client.with_session_auth(email=user.email) num_queries = testing.AUTHENTICATION_QUERIES num_queries += 1 # select Offerer + num_queries += 1 # select Offer + num_queries += 1 # select CollectiveOffer with testing.assert_num_queries(num_queries): run_query.return_value = fixtures.YEARLY_AGGREGATED_VENUE_REVENUE response = test_client.get(f"/get-statistics/?venue_ids={venue_id}") @@ -44,10 +50,14 @@ def test_get_statistics_from_multiple_venues(self, run_query, client): venue2 = offerers_factories.VenueFactory(managingOfferer=offerer) venue_id = venue.id venue2_id = venue2.id + educational_factories.CollectiveOfferFactory(venue=venue) + offers_factories.OfferFactory(venue=venue) test_client = client.with_session_auth(email=user.email) num_queries = testing.AUTHENTICATION_QUERIES num_queries += 1 # select Offerer + num_queries += 1 # select Offer + num_queries += 1 # select CollectiveOffer with testing.assert_num_queries(num_queries): run_query.return_value = fixtures.YEARLY_AGGREGATED_VENUE_REVENUE response = test_client.get(f"/get-statistics/?venue_ids={venue_id}&venue_ids={venue2_id}") @@ -70,10 +80,14 @@ def test_get_statistics_multiple_years(self, run_query, client): venue2 = offerers_factories.VenueFactory(managingOfferer=offerer) venue_id = venue.id venue2_id = venue2.id + educational_factories.CollectiveOfferFactory(venue=venue) + offers_factories.OfferFactory(venue=venue) test_client = client.with_session_auth(email=user.email) num_queries = testing.AUTHENTICATION_QUERIES num_queries += 1 # select Offerer + num_queries += 1 # select Offer + num_queries += 1 # select CollectiveOffer with testing.assert_num_queries(num_queries): run_query.return_value = fixtures.YEARLY_AGGREGATED_VENUE_REVENUE_MULTIPLE_YEARS response = test_client.get(f"/get-statistics/?venue_ids={venue_id}&venue_ids={venue2_id}") @@ -92,16 +106,84 @@ def test_get_statistics_multiple_years(self, run_query, client): } } + @patch("pcapi.connectors.clickhouse.testing_backend.TestingBackend.run_query") + def test_get_statistics_only_collective(self, run_query, client): + user = users_factories.UserFactory() + offerer = offerers_factories.OffererFactory() + offerers_factories.UserOffererFactory(user=user, offerer=offerer) + venue = offerers_factories.VenueFactory(managingOfferer=offerer) + venue_id = venue.id + educational_factories.CollectiveOfferFactory(venue=venue) + + test_client = client.with_session_auth(email=user.email) + num_queries = testing.AUTHENTICATION_QUERIES + num_queries += 1 # select Offerer + num_queries += 1 # select Offer + num_queries += 1 # select CollectiveOffer + with testing.assert_num_queries(num_queries): + run_query.return_value = fixtures.YEARLY_AGGREGATED_VENUE_REVENUE_MULTIPLE_YEARS_ONLY_COLLECTIVE + response = test_client.get(f"/get-statistics/?venue_ids={venue_id}") + assert response.status_code == 200 + assert response.json == { + "incomeByYear": { + "2022": { + "expectedRevenue": {"collective": 22.12}, + "revenue": {"collective": 22.12}, + }, + "2023": {}, + "2024": { + "expectedRevenue": {"collective": 13.12}, + "revenue": {"collective": 12.12}, + }, + } + } + + @patch("pcapi.connectors.clickhouse.testing_backend.TestingBackend.run_query") + def test_get_statistics_only_individual(self, run_query, client): + user = users_factories.UserFactory() + offerer = offerers_factories.OffererFactory() + offerers_factories.UserOffererFactory(user=user, offerer=offerer) + venue = offerers_factories.VenueFactory(managingOfferer=offerer) + venue_id = venue.id + offers_factories.OfferFactory(venue=venue) + + test_client = client.with_session_auth(email=user.email) + num_queries = testing.AUTHENTICATION_QUERIES + num_queries += 1 # select Offerer + num_queries += 1 # select Offer + num_queries += 1 # select CollectiveOffer + with testing.assert_num_queries(num_queries): + run_query.return_value = fixtures.YEARLY_AGGREGATED_VENUE_REVENUE_MULTIPLE_YEARS_ONLY_INDIVIDUAL + response = test_client.get(f"/get-statistics/?venue_ids={venue_id}") + assert response.status_code == 200 + assert response.json == { + "incomeByYear": { + "2022": { + "expectedRevenue": {"individual": 22.12}, + "revenue": {"individual": 22.12}, + }, + "2023": {}, + "2024": { + "expectedRevenue": {"individual": 13.12}, + "revenue": {"individual": 12.12}, + }, + } + } + def test_get_statistics_empty_result(self, client): user = users_factories.UserFactory() offerer = offerers_factories.OffererFactory() offerers_factories.UserOffererFactory(user=user, offerer=offerer) venue = offerers_factories.VenueFactory(managingOfferer=offerer) venue_id = venue.id + educational_factories.CollectiveOfferFactory(venue=venue) + offers_factories.OfferFactory(venue=venue) test_client = client.with_session_auth(email=user.email) num_queries = testing.AUTHENTICATION_QUERIES num_queries += 1 # select Offerer + num_queries += 1 # select Offer + num_queries += 1 # select CollectiveOffer with testing.assert_num_queries(num_queries): response = test_client.get(f"/get-statistics/?venue_ids={venue_id}") assert response.status_code == 200 @@ -129,17 +211,18 @@ def test_get_statistics_with_no_venue_id_should_fail(self, client): class Returns403Test: def test_get_statistics_from_not_owned_venue_should_fail(self, client): user = users_factories.UserFactory() - user2 = users_factories.UserFactory() offerer = offerers_factories.OffererFactory() - offerers_factories.UserOffererFactory(user=user2, offerer=offerer) + offerers_factories.UserOffererFactory(user=user, offerer=offerer) venue = offerers_factories.VenueFactory(managingOfferer=offerer) + venue_not_belonging_to_user = offerers_factories.VenueFactory() venue_id = venue.id + foreign_venue = venue_not_belonging_to_user.id test_client = client.with_session_auth(email=user.email) num_queries = testing.AUTHENTICATION_QUERIES num_queries += 1 # select Offerer with testing.assert_num_queries(num_queries): - response = test_client.get(f"/get-statistics/?venue_ids={venue_id}") + response = test_client.get(f"/get-statistics/?venue_ids={venue_id}&venue_ids={foreign_venue}") assert response.status_code == 403 assert response.json["global"] == [ "Vous n'avez pas les droits d'accès suffisants pour accéder à cette information." diff --git a/pro/src/apiClient/v1/index.ts b/pro/src/apiClient/v1/index.ts index afac63010ea..3c817f3f04b 100644 --- a/pro/src/apiClient/v1/index.ts +++ b/pro/src/apiClient/v1/index.ts @@ -36,6 +36,7 @@ export type { CategoriesResponseModel } from './models/CategoriesResponseModel'; export type { CategoryResponseModel } from './models/CategoryResponseModel'; export type { ChangePasswordBodyModel } from './models/ChangePasswordBodyModel'; export type { ChangeProEmailBody } from './models/ChangeProEmailBody'; +export type { CollectiveAndIndividualRevenue } from './models/CollectiveAndIndividualRevenue'; export { CollectiveBookingBankAccountStatus } from './models/CollectiveBookingBankAccountStatus'; export type { CollectiveBookingByIdResponseModel } from './models/CollectiveBookingByIdResponseModel'; export { CollectiveBookingCancellationReasons } from './models/CollectiveBookingCancellationReasons'; @@ -59,6 +60,7 @@ export type { CollectiveOfferTemplateBodyModel } from './models/CollectiveOfferT export type { CollectiveOfferTemplateResponseIdModel } from './models/CollectiveOfferTemplateResponseIdModel'; export { CollectiveOfferType } from './models/CollectiveOfferType'; export type { CollectiveOfferVenueBodyModel } from './models/CollectiveOfferVenueBodyModel'; +export type { CollectiveRevenue } from './models/CollectiveRevenue'; export type { CollectiveStockCreationBodyModel } from './models/CollectiveStockCreationBodyModel'; export type { CollectiveStockEditionBodyModel } from './models/CollectiveStockEditionBodyModel'; export type { CollectiveStockResponseModel } from './models/CollectiveStockResponseModel'; @@ -141,6 +143,7 @@ export type { GetVenueResponseModel } from './models/GetVenueResponseModel'; export type { GetVenuesOfOffererFromSiretResponseModel } from './models/GetVenuesOfOffererFromSiretResponseModel'; export type { HasInvoiceQueryModel } from './models/HasInvoiceQueryModel'; export type { HasInvoiceResponseModel } from './models/HasInvoiceResponseModel'; +export type { IndividualRevenue } from './models/IndividualRevenue'; export type { InviteMemberQueryModel } from './models/InviteMemberQueryModel'; export type { InvoiceListV2QueryModel } from './models/InvoiceListV2QueryModel'; export type { InvoiceListV2ResponseModel } from './models/InvoiceListV2ResponseModel'; @@ -211,7 +214,6 @@ export type { ProviderResponse } from './models/ProviderResponse'; export type { ReimbursementCsvByInvoicesModel } from './models/ReimbursementCsvByInvoicesModel'; export type { ReimbursementCsvQueryModel } from './models/ReimbursementCsvQueryModel'; export type { ResetPasswordBodyModel } from './models/ResetPasswordBodyModel'; -export type { Revenue } from './models/Revenue'; export type { SaveNewOnboardingDataQueryModel } from './models/SaveNewOnboardingDataQueryModel'; export type { SharedCurrentUserResponseModel } from './models/SharedCurrentUserResponseModel'; export type { SharedLoginUserResponseModel } from './models/SharedLoginUserResponseModel'; diff --git a/pro/src/apiClient/v1/models/AggregatedRevenue.ts b/pro/src/apiClient/v1/models/AggregatedRevenue.ts index ae58aca2b06..48a42c5d1b1 100644 --- a/pro/src/apiClient/v1/models/AggregatedRevenue.ts +++ b/pro/src/apiClient/v1/models/AggregatedRevenue.ts @@ -2,9 +2,11 @@ /* istanbul ignore file */ /* tslint:disable */ /* eslint-disable */ -import type { Revenue } from './Revenue'; +import type { CollectiveAndIndividualRevenue } from './CollectiveAndIndividualRevenue'; +import type { CollectiveRevenue } from './CollectiveRevenue'; +import type { IndividualRevenue } from './IndividualRevenue'; export type AggregatedRevenue = { - expectedRevenue: Revenue; - revenue: Revenue; + expectedRevenue: (CollectiveAndIndividualRevenue | CollectiveRevenue | IndividualRevenue); + revenue: (CollectiveAndIndividualRevenue | CollectiveRevenue | IndividualRevenue); }; diff --git a/pro/src/apiClient/v1/models/Revenue.ts b/pro/src/apiClient/v1/models/CollectiveAndIndividualRevenue.ts similarity index 80% rename from pro/src/apiClient/v1/models/Revenue.ts rename to pro/src/apiClient/v1/models/CollectiveAndIndividualRevenue.ts index 809bab7fa2e..9397894feb4 100644 --- a/pro/src/apiClient/v1/models/Revenue.ts +++ b/pro/src/apiClient/v1/models/CollectiveAndIndividualRevenue.ts @@ -2,7 +2,7 @@ /* istanbul ignore file */ /* tslint:disable */ /* eslint-disable */ -export type Revenue = { +export type CollectiveAndIndividualRevenue = { collective: number; individual: number; total: number; diff --git a/pro/src/apiClient/v1/models/CollectiveRevenue.ts b/pro/src/apiClient/v1/models/CollectiveRevenue.ts new file mode 100644 index 00000000000..ea4cc7e1edf --- /dev/null +++ b/pro/src/apiClient/v1/models/CollectiveRevenue.ts @@ -0,0 +1,8 @@ +/* generated using openapi-typescript-codegen -- do not edit */ +/* istanbul ignore file */ +/* tslint:disable */ +/* eslint-disable */ +export type CollectiveRevenue = { + collective: number; +}; + diff --git a/pro/src/apiClient/v1/models/IndividualRevenue.ts b/pro/src/apiClient/v1/models/IndividualRevenue.ts new file mode 100644 index 00000000000..3eb747fc3e9 --- /dev/null +++ b/pro/src/apiClient/v1/models/IndividualRevenue.ts @@ -0,0 +1,8 @@ +/* generated using openapi-typescript-codegen -- do not edit */ +/* istanbul ignore file */ +/* tslint:disable */ +/* eslint-disable */ +export type IndividualRevenue = { + individual: number; +}; +