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

(PC-33154)[API] feat: remove update of collective offer template when editing collective offer #15197

Merged
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
5 changes: 0 additions & 5 deletions api/src/pcapi/routes/pro/collective_offers.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,11 +292,6 @@ def edit_collective_offer(
raise ApiErrors(error.errors, status_code=400)

offer = educational_repository.get_collective_offer_by_id(offer_id)
if offer.template and (not offer.template.domains or not offer.template.interventionArea):
offers_api.update_collective_offer_template(
offer_id=offer.template.id,
new_values={"domains": [domain.id for domain in offer.domains], "interventionArea": offer.interventionArea},
)
return collective_offers_serialize.GetCollectiveOfferResponseModel.from_orm(offer)


Expand Down
43 changes: 17 additions & 26 deletions api/src/pcapi/routes/serialization/collective_offers_serialize.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ class PostCollectiveOfferBodyModel(BaseModel):
name: str
booking_emails: list[EmailStr]
description: str
domains: list[int] | None
domains: list[int]
duration_minutes: int | None
audio_disability_compliant: bool = False
mental_disability_compliant: bool = False
Expand Down Expand Up @@ -591,37 +591,28 @@ def validate_name(cls, name: str) -> str:
check_collective_offer_name_length_is_valid(name)
return name

@root_validator
def validate_domains(cls, values: dict) -> dict:
domains = values.get("domains")
is_from_template = bool(values.get("template_id", None))
if not domains and not is_from_template:
@validator("domains")
def validate_domains(cls, domains: list[str]) -> list[str]:
if not domains:
raise ValueError("domains must have at least one value")
return values
return domains

@root_validator
def validate_intervention_area(cls, values: dict) -> dict:
intervention_area = values.get("intervention_area", None)
is_from_template = bool(values.get("template_id", None))
@validator("intervention_area")
def validate_intervention_area(
cls,
intervention_area: list[str] | None,
values: dict,
) -> list[str] | None:
offer_venue = values.get("offer_venue", None)
if not is_intervention_area_valid(intervention_area, offer_venue) and not is_from_template:
if not is_intervention_area_valid(intervention_area, offer_venue):
raise ValueError("intervention_area must have at least one value")
return values
return intervention_area

@root_validator(skip_on_failure=True)
def validate_booking_emails(cls, values: dict) -> dict:
booking_emails = values.get("booking_emails", [])
is_from_template = bool(values.get("template_id", None))
@validator("booking_emails")
def validate_booking_emails(cls, booking_emails: list[str]) -> list[str]:
if not booking_emails:
if is_from_template:
contact_email = values.get("contact_email", None)
if contact_email:
values["booking_emails"] = [contact_email]
else:
values["booking_emails"] = []
else:
raise ValueError("Un email doit être renseigné")
return values
raise ValueError("Un email doit etre renseigné.")
return booking_emails

class Config:
alias_generator = to_camel
Expand Down
26 changes: 0 additions & 26 deletions api/tests/routes/pro/patch_collective_offer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,32 +203,6 @@ def test_patch_offer_with_empty_intervention_area_in_offerer_venue(self, client)
assert response.status_code == 200
assert offer.interventionArea == []

def test_patch_collective_offer_update_legacy_template(self, client):
# Given
template = educational_factories.CollectiveOfferTemplateFactory(domains=[], interventionArea=[])
offer = educational_factories.CollectiveOfferFactory(templateId=template.id)
offerers_factories.UserOffererFactory(
user__email="user@example.com",
offerer=offer.venue.managingOfferer,
)
domain = educational_factories.EducationalDomainFactory(name="Architecture")
data = {
"domains": [domain.id],
"interventionArea": ["01", "2A"],
}

# WHEN
client = client.with_session_auth("user@example.com")
with patch(
"pcapi.routes.pro.collective_offers.offerers_api.can_offerer_create_educational_offer",
):
response = client.patch(f"/collective/offers/{offer.id}", json=data)

# Then
assert response.status_code == 200
assert template.domains == [domain]
assert template.interventionArea == ["01", "2A"]

def test_patch_collective_offer_update_student_level_college_6(self, client):
# Given
offer = educational_factories.CollectiveOfferFactory()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ def test_empty_domains(self, pro_client, payload):
response = pro_client.post("/collective/offers-template", json=data)

assert response.status_code == 400
assert response.json == {"__root__": ["domains must have at least one value"]}
assert response.json == {"domains": ["domains must have at least one value"]}

assert CollectiveOfferTemplate.query.count() == 0

Expand Down
149 changes: 36 additions & 113 deletions api/tests/routes/pro/post_collective_offers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,103 +150,6 @@ def test_create_collective_offer_empty_intervention_area(self, client):
# Then
assert response.status_code == 201

def test_create_offer_from_template_no_domains_nor_intervention_area_nor_booking_emails(self, client):
"""Ensure that if a template id is set, domains, intervention
area and booking emails can be missing
Also ensures the contact mail/phone can be missing
"""
# Given
venue = offerers_factories.VenueFactory()
template = educational_factories.CollectiveOfferTemplateFactory(venue=venue)
offerer = venue.managingOfferer
user = offerers_factories.UserOffererFactory(offerer=offerer, user__email="user@example.com").user

# When
data = {
**base_offer_payload(venue=venue, domains=[], template_id=template.id),
"interventionArea": [],
"bookingEmails": [],
}

with patch("pcapi.core.offerers.api.can_offerer_create_educational_offer"):
response = client.with_session_auth("user@example.com").post("/collective/offers", json=data)

# Then
assert response.status_code == 201, response.json

offer_id = response.json["id"]
offer = CollectiveOffer.query.get(offer_id)

assert_offer_values(offer, data, user, offerer)

def test_create_offer_from_template_no_contact_email(self, client):
"""Ensure that if a template id is set, domains, intervention
area and booking emails can be missing
Also ensures the contact mail/phone can be missing
"""
# Given
venue = offerers_factories.VenueFactory()
template = educational_factories.CollectiveOfferTemplateFactory(
venue=venue, contactEmail=None, contactPhone=None
)
offerer = venue.managingOfferer
user = offerers_factories.UserOffererFactory(offerer=offerer, user__email="user@example.com").user

# When
data = {
**base_offer_payload(venue=venue, domains=[], template_id=template.id),
"contactEmail": "",
"contactPhone": "",
}

with patch("pcapi.core.offerers.api.can_offerer_create_educational_offer"):
response = client.with_session_auth("user@example.com").post("/collective/offers", json=data)

# Then
assert response.status_code == 201, response.json

offer_id = response.json["id"]
offer = CollectiveOffer.query.get(offer_id)

# Alter contactEmail because we expect None although we sent ""
data["contactEmail"] = None
assert_offer_values(offer, data, user, offerer)

def test_create_offer_from_template_no_contact_email_nor_booking_emails(self, client):
"""Ensure that if a template id is set, domains, intervention
area and booking emails can be missing
Also ensures the contact mail/phone can be missing
"""
# Given
venue = offerers_factories.VenueFactory()
template = educational_factories.CollectiveOfferTemplateFactory(
venue=venue, contactEmail=None, contactPhone=None
)
offerer = venue.managingOfferer
user = offerers_factories.UserOffererFactory(offerer=offerer, user__email="user@example.com").user

# When
data = {
**base_offer_payload(venue=venue, domains=[], template_id=template.id),
"interventionArea": [],
"bookingEmails": [],
"contactEmail": "",
"contactPhone": "",
}

with patch("pcapi.core.offerers.api.can_offerer_create_educational_offer"):
response = client.with_session_auth("user@example.com").post("/collective/offers", json=data)

# Then
assert response.status_code == 201, response.json

offer_id = response.json["id"]
offer = CollectiveOffer.query.get(offer_id)

# Alter contactEmail because we expect None although we sent ""
data["contactEmail"] = None
assert_offer_values(offer, data, user, offerer)

@override_features(WIP_ENABLE_MARSEILLE=True)
def test_create_collective_offer_primary_level(self, client):
venue = offerers_factories.VenueFactory()
Expand Down Expand Up @@ -396,6 +299,42 @@ def test_create_collective_offer_booking_emails_invalid(self, client):
}
assert CollectiveOffer.query.count() == 0

def test_create_collective_offer_no_booking_email(self, client):
venue = offerers_factories.VenueFactory()
offerers_factories.UserOffererFactory(offerer=venue.managingOfferer, user__email="user@example.com")

data = {**base_offer_payload(venue=venue), "bookingEmails": []}
with patch("pcapi.core.offerers.api.can_offerer_create_educational_offer"):
response = client.with_session_auth("user@example.com").post("/collective/offers", json=data)

assert response.status_code == 400
assert response.json == {"bookingEmails": ["Un email doit etre renseigné."]}
assert CollectiveOffer.query.count() == 0

def test_create_collective_offer_no_intervention_area(self, client):
venue = offerers_factories.VenueFactory()
offerers_factories.UserOffererFactory(offerer=venue.managingOfferer, user__email="user@example.com")

data = {**base_offer_payload(venue=venue), "interventionArea": []}
with patch("pcapi.core.offerers.api.can_offerer_create_educational_offer"):
response = client.with_session_auth("user@example.com").post("/collective/offers", json=data)

assert response.status_code == 400
assert response.json == {"interventionArea": ["intervention_area must have at least one value"]}
assert CollectiveOffer.query.count() == 0

def test_create_collective_offer_no_domains(self, client):
venue = offerers_factories.VenueFactory()
offerers_factories.UserOffererFactory(offerer=venue.managingOfferer, user__email="user@example.com")

data = {**base_offer_payload(venue=venue), "domains": []}
with patch("pcapi.core.offerers.api.can_offerer_create_educational_offer"):
response = client.with_session_auth("user@example.com").post("/collective/offers", json=data)

assert response.status_code == 400
assert response.json == {"domains": ["domains must have at least one value"]}
assert CollectiveOffer.query.count() == 0


@pytest.mark.usefixtures("db_session")
class Returns404Test:
Expand Down Expand Up @@ -447,19 +386,3 @@ def test_create_collective_offer_with_no_collective_offer_template(self, client)
# Then
assert response.status_code == 404
assert response.json == {"code": "COLLECTIVE_OFFER_TEMPLATE_NOT_FOUND"}

def test_create_collective_offer_no_booking_email(self, client):
# Given
venue = offerers_factories.VenueFactory()
educational_factories.CollectiveOfferTemplateFactory(venue=venue)
offerer = venue.managingOfferer
offerers_factories.UserOffererFactory(offerer=offerer, user__email="user@example.com")

# When
data = {**base_offer_payload(venue=venue), "bookingEmails": []}

with patch("pcapi.core.offerers.api.can_offerer_create_educational_offer"):
response = client.with_session_auth("user@example.com").post("/collective/offers", json=data)

# Then
assert response.status_code == 400
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export type PostCollectiveOfferBodyModel = {
contactEmail?: string | null;
contactPhone?: string | null;
description: string;
domains?: Array<number> | null;
domains: Array<number>;
durationMinutes?: number | null;
formats?: Array<EacFormat> | null;
interventionArea?: Array<string> | null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export type PostCollectiveOfferTemplateBodyModel = {
contactUrl?: string | null;
dates?: DateRangeOnCreateModel | null;
description: string;
domains?: Array<number> | null;
domains: Array<number>;
durationMinutes?: number | null;
formats?: Array<EacFormat> | null;
interventionArea?: Array<string> | null;
Expand Down
Loading