Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/src/pcapi/connectors/clickhouse/queries/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
from .yearly_revenue import YearlyAggregatedCollectiveRevenueQuery
from .yearly_revenue import YearlyAggregatedIndividualRevenueQuery
from .yearly_revenue import YearlyAggregatedRevenueModel
from .yearly_revenue import YearlyAggregatedRevenueQuery
74 changes: 65 additions & 9 deletions api/src/pcapi/connectors/clickhouse/queries/yearly_revenue.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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": {
Expand All @@ -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 """
Expand All @@ -65,7 +125,3 @@ def raw_query(self) -> str:
GROUP BY year
ORDER BY year
"""

@property
def model(self) -> type[YearlyAggregatedRevenueModel]:
return YearlyAggregatedRevenueModel
11 changes: 11 additions & 0 deletions api/src/pcapi/core/offers/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
35 changes: 13 additions & 22 deletions api/src/pcapi/core/users/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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, offerers_models.UserOfferer.isValidated)
.group_by(offerers_models.UserOfferer.userId)
).scalar()


def get_newly_eligible_age_18_users(since: date) -> list[models.User]:
Expand Down
9 changes: 8 additions & 1 deletion api/src/pcapi/routes/pro/statistics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -26,5 +27,11 @@ def get_statistics(query: StatisticsQueryModel) -> StatisticsModel:
status_code=422,
)
check_user_has_access_to_venues(current_user, venue_ids)

Choose a reason for hiding this comment

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

Je vois que tu as fait une seconde PR pour le même ticket, du coup je me permet de remettre ici le warning concernant cette fonction que j’ai soulevé dans la première

Choose a reason for hiding this comment

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

Ce qu’on cherche à faire c’est, est-ce que pour un user donné, la liste des venue_ids demandées sont toutes bien liées aux structures auquel appartient notre utilisateur ?
Dis autrement, est-ce que venue_ids est bien un subset des id de toutes les venues de toutes les structures de notre utilisateur, ce qui peut ce traduire comme ceci en SQL:

(données de sanbox)

pass_culture>
 SELECT '{13, 14, 15}' <@ ARRAY_AGG(venue.id) FROM venue
 JOIN offerer ON offerer.id = venue."managingOffererId"
 JOIN user_offerer ON user_offerer."offererId" = offerer.id
 WHERE user_offerer."userId"=1
 GROUP BY user_offerer."userId";
+----------+
| ?column? |
|----------|
| True     |
+----------+
SELECT 1
Time: 0.084s
pass_culture>
 SELECT '{1, 13, 14, 15}' <@ ARRAY_AGG(venue.id) FROM venue
 JOIN offerer ON offerer.id = venue."managingOffererId"
 JOIN user_offerer ON user_offerer."offererId" = offerer.id
 WHERE user_offerer."userId"=1
 GROUP BY user_offerer."userId";
+----------+
| ?column? |
|----------|
| False    |
+----------+
SELECT 1
Time: 0.008s

check_user_has_access_to_venues pourrait donc être modifiée comme ceci:

db.session.execute(
    sa.select(
        sa.cast(postgresql.array(venue_ids), postgresql.ARRAY(postgresql.BIGINT)).contained_by(
            sa.func.array_agg(Venue.id)
        )
    )
    .select_from(Venue)
    .join(Offerer)
    .join(UserOfferer)
    .where(UserOfferer.userId == current_user)
    .group_by(UserOfferer.userId)
).scalar()

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)
41 changes: 31 additions & 10 deletions api/tests/connectors/clickhouse/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()]
Expand All @@ -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
),
]
Loading
Loading