refactor(prospecting): migrate SVC-006 transaction control to endpoint level
Some checks failed
Some checks failed
Move db.commit() from services to API endpoints and Celery tasks. Services now use db.flush() only; endpoints own the transaction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -48,6 +48,7 @@ def create_template(
|
||||
):
|
||||
"""Create a new campaign template."""
|
||||
template = campaign_service.create_template(db, data.model_dump())
|
||||
db.commit()
|
||||
return CampaignTemplateResponse.model_validate(template)
|
||||
|
||||
|
||||
@@ -60,6 +61,7 @@ def update_template(
|
||||
):
|
||||
"""Update a campaign template."""
|
||||
template = campaign_service.update_template(db, template_id, data.model_dump(exclude_none=True))
|
||||
db.commit()
|
||||
return CampaignTemplateResponse.model_validate(template)
|
||||
|
||||
|
||||
@@ -71,6 +73,7 @@ def delete_template(
|
||||
):
|
||||
"""Delete a campaign template."""
|
||||
campaign_service.delete_template(db, template_id)
|
||||
db.commit()
|
||||
return CampaignTemplateDeleteResponse(message="Template deleted")
|
||||
|
||||
|
||||
@@ -98,6 +101,7 @@ def send_campaign(
|
||||
prospect_ids=data.prospect_ids,
|
||||
sent_by_user_id=current_admin.user_id,
|
||||
)
|
||||
db.commit()
|
||||
return [CampaignSendResponse.model_validate(s) for s in sends]
|
||||
|
||||
|
||||
|
||||
@@ -38,6 +38,7 @@ def http_check_single(
|
||||
"""Run HTTP connectivity check for a single prospect."""
|
||||
prospect = prospect_service.get_by_id(db, prospect_id)
|
||||
result = enrichment_service.check_http(db, prospect)
|
||||
db.commit()
|
||||
return HttpCheckResult(**result)
|
||||
|
||||
|
||||
@@ -53,6 +54,7 @@ def http_check_batch(
|
||||
for prospect in prospects:
|
||||
result = enrichment_service.check_http(db, prospect)
|
||||
results.append(HttpCheckBatchItem(domain=prospect.domain_name, **result))
|
||||
db.commit()
|
||||
return HttpCheckBatchResponse(processed=len(results), results=results)
|
||||
|
||||
|
||||
@@ -65,6 +67,7 @@ def tech_scan_single(
|
||||
"""Run technology scan for a single prospect."""
|
||||
prospect = prospect_service.get_by_id(db, prospect_id)
|
||||
profile = enrichment_service.scan_tech_stack(db, prospect)
|
||||
db.commit()
|
||||
return ScanSingleResponse(domain=prospect.domain_name, profile=profile is not None)
|
||||
|
||||
|
||||
@@ -81,6 +84,7 @@ def tech_scan_batch(
|
||||
result = enrichment_service.scan_tech_stack(db, prospect)
|
||||
if result:
|
||||
count += 1
|
||||
db.commit()
|
||||
return ScanBatchResponse(processed=len(prospects), successful=count)
|
||||
|
||||
|
||||
@@ -93,6 +97,7 @@ def performance_scan_single(
|
||||
"""Run PageSpeed audit for a single prospect."""
|
||||
prospect = prospect_service.get_by_id(db, prospect_id)
|
||||
profile = enrichment_service.scan_performance(db, prospect)
|
||||
db.commit()
|
||||
return ScanSingleResponse(domain=prospect.domain_name, profile=profile is not None)
|
||||
|
||||
|
||||
@@ -109,6 +114,7 @@ def performance_scan_batch(
|
||||
result = enrichment_service.scan_performance(db, prospect)
|
||||
if result:
|
||||
count += 1
|
||||
db.commit()
|
||||
return ScanBatchResponse(processed=len(prospects), successful=count)
|
||||
|
||||
|
||||
@@ -121,6 +127,7 @@ def scrape_contacts_single(
|
||||
"""Scrape contacts for a single prospect."""
|
||||
prospect = prospect_service.get_by_id(db, prospect_id)
|
||||
contacts = enrichment_service.scrape_contacts(db, prospect)
|
||||
db.commit()
|
||||
return ContactScrapeResponse(domain=prospect.domain_name, contacts_found=len(contacts))
|
||||
|
||||
|
||||
@@ -154,6 +161,7 @@ def full_enrichment(
|
||||
# Step 5: Compute score
|
||||
db.refresh(prospect)
|
||||
score = scoring_service.compute_score(db, prospect)
|
||||
db.commit()
|
||||
|
||||
return FullEnrichmentResponse(
|
||||
domain=prospect.domain_name,
|
||||
@@ -174,4 +182,5 @@ def compute_scores_batch(
|
||||
):
|
||||
"""Compute or recompute scores for all prospects."""
|
||||
count = scoring_service.compute_all(db, limit=limit)
|
||||
db.commit()
|
||||
return ScoreComputeBatchResponse(scored=count)
|
||||
|
||||
@@ -51,6 +51,7 @@ def create_interaction(
|
||||
user_id=current_admin.user_id,
|
||||
data=data.model_dump(exclude_none=True),
|
||||
)
|
||||
db.commit()
|
||||
return InteractionResponse.model_validate(interaction)
|
||||
|
||||
|
||||
|
||||
@@ -82,6 +82,7 @@ def create_prospect(
|
||||
data.model_dump(exclude_none=True),
|
||||
captured_by_user_id=current_admin.user_id,
|
||||
)
|
||||
db.commit()
|
||||
return _to_response(prospect)
|
||||
|
||||
|
||||
@@ -94,6 +95,7 @@ def update_prospect(
|
||||
):
|
||||
"""Update a prospect."""
|
||||
prospect = prospect_service.update(db, prospect_id, data.model_dump(exclude_none=True))
|
||||
db.commit()
|
||||
return _to_response(prospect)
|
||||
|
||||
|
||||
@@ -105,6 +107,7 @@ def delete_prospect(
|
||||
):
|
||||
"""Delete a prospect."""
|
||||
prospect_service.delete(db, prospect_id)
|
||||
db.commit()
|
||||
return ProspectDeleteResponse(message="Prospect deleted")
|
||||
|
||||
|
||||
@@ -125,6 +128,7 @@ async def import_domains(
|
||||
domains.append(domain)
|
||||
|
||||
created, skipped = prospect_service.create_bulk(db, domains, source="csv_import")
|
||||
db.commit()
|
||||
return ProspectImportResponse(created=created, skipped=skipped, total=len(domains))
|
||||
|
||||
|
||||
|
||||
@@ -63,8 +63,7 @@ class CampaignService:
|
||||
is_active=data.get("is_active", True),
|
||||
)
|
||||
db.add(template)
|
||||
db.commit()
|
||||
db.refresh(template)
|
||||
db.flush()
|
||||
return template
|
||||
|
||||
def update_template(self, db: Session, template_id: int, data: dict) -> CampaignTemplate:
|
||||
@@ -72,14 +71,13 @@ class CampaignService:
|
||||
for field in ["name", "lead_type", "channel", "language", "subject_template", "body_template", "is_active"]:
|
||||
if field in data and data[field] is not None:
|
||||
setattr(template, field, data[field])
|
||||
db.commit()
|
||||
db.refresh(template)
|
||||
db.flush()
|
||||
return template
|
||||
|
||||
def delete_template(self, db: Session, template_id: int) -> bool:
|
||||
template = self.get_template_by_id(db, template_id)
|
||||
db.delete(template)
|
||||
db.commit()
|
||||
db.flush()
|
||||
return True
|
||||
|
||||
# --- Rendering ---
|
||||
@@ -143,7 +141,7 @@ class CampaignService:
|
||||
db.add(send)
|
||||
sends.append(send)
|
||||
|
||||
db.commit()
|
||||
db.flush()
|
||||
logger.info("Sent campaign %d to %d prospects", template_id, len(prospect_ids))
|
||||
return sends
|
||||
|
||||
|
||||
@@ -110,7 +110,7 @@ class EnrichmentService:
|
||||
if result["has_website"]:
|
||||
prospect.status = "active"
|
||||
|
||||
db.commit()
|
||||
db.flush()
|
||||
return result
|
||||
|
||||
def scan_tech_stack(self, db: Session, prospect: Prospect) -> ProspectTechProfile | None:
|
||||
@@ -177,7 +177,7 @@ class EnrichmentService:
|
||||
profile.scan_source = "basic_http"
|
||||
|
||||
prospect.last_tech_scan_at = datetime.now(UTC)
|
||||
db.commit()
|
||||
db.flush()
|
||||
return profile
|
||||
|
||||
except Exception as e:
|
||||
@@ -185,7 +185,7 @@ class EnrichmentService:
|
||||
if prospect.tech_profile:
|
||||
prospect.tech_profile.scan_error = str(e)
|
||||
prospect.last_tech_scan_at = datetime.now(UTC)
|
||||
db.commit()
|
||||
db.flush()
|
||||
return None
|
||||
|
||||
def scan_performance(self, db: Session, prospect: Prospect) -> ProspectPerformanceProfile | None:
|
||||
@@ -251,13 +251,13 @@ class EnrichmentService:
|
||||
profile.scan_strategy = "mobile"
|
||||
|
||||
prospect.last_perf_scan_at = datetime.now(UTC)
|
||||
db.commit()
|
||||
db.flush()
|
||||
return profile
|
||||
|
||||
except Exception as e:
|
||||
logger.error("Performance scan failed for %s: %s", domain, e)
|
||||
prospect.last_perf_scan_at = datetime.now(UTC)
|
||||
db.commit()
|
||||
db.flush()
|
||||
return None
|
||||
|
||||
def scrape_contacts(self, db: Session, prospect: Prospect) -> list[ProspectContact]:
|
||||
@@ -339,7 +339,7 @@ class EnrichmentService:
|
||||
break
|
||||
|
||||
prospect.last_contact_scrape_at = datetime.now(UTC)
|
||||
db.commit()
|
||||
db.flush()
|
||||
return contacts
|
||||
|
||||
def _detect_cms(self, html: str) -> str | None:
|
||||
|
||||
@@ -38,8 +38,7 @@ class InteractionService:
|
||||
created_by_user_id=user_id,
|
||||
)
|
||||
db.add(interaction)
|
||||
db.commit()
|
||||
db.refresh(interaction)
|
||||
db.flush()
|
||||
logger.info("Interaction logged for prospect %d: %s", prospect_id, data["interaction_type"])
|
||||
return interaction
|
||||
|
||||
|
||||
@@ -137,8 +137,7 @@ class ProspectService:
|
||||
)
|
||||
db.add(contact)
|
||||
|
||||
db.commit()
|
||||
db.refresh(prospect)
|
||||
db.flush()
|
||||
logger.info("Created prospect: %s (channel=%s)", prospect.display_name, channel)
|
||||
return prospect
|
||||
|
||||
@@ -161,7 +160,7 @@ class ProspectService:
|
||||
db.add(prospect)
|
||||
created += 1
|
||||
|
||||
db.commit()
|
||||
db.flush()
|
||||
logger.info("Bulk import: %d created, %d skipped", created, skipped)
|
||||
return created, skipped
|
||||
|
||||
@@ -178,14 +177,13 @@ class ProspectService:
|
||||
tags = json.dumps(tags)
|
||||
prospect.tags = tags
|
||||
|
||||
db.commit()
|
||||
db.refresh(prospect)
|
||||
db.flush()
|
||||
return prospect
|
||||
|
||||
def delete(self, db: Session, prospect_id: int) -> bool:
|
||||
prospect = self.get_by_id(db, prospect_id)
|
||||
db.delete(prospect)
|
||||
db.commit()
|
||||
db.flush()
|
||||
logger.info("Deleted prospect: %d", prospect_id)
|
||||
return True
|
||||
|
||||
|
||||
@@ -79,7 +79,7 @@ class ScoringService:
|
||||
score.score_breakdown = json.dumps(breakdown)
|
||||
score.lead_tier = lead_tier
|
||||
|
||||
db.commit()
|
||||
db.flush()
|
||||
logger.info("Scored prospect %d: %d (%s)", prospect.id, total, lead_tier)
|
||||
return score
|
||||
|
||||
|
||||
@@ -56,12 +56,14 @@ def batch_http_check(self, job_id: int, limit: int = 100):
|
||||
|
||||
job.status = "completed"
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - background task owns transaction
|
||||
|
||||
except Exception as e:
|
||||
logger.error("batch_http_check job %d failed: %s", job_id, e, exc_info=True)
|
||||
job.status = "failed"
|
||||
job.error_message = str(e)[:500]
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - persist failure status
|
||||
raise
|
||||
|
||||
|
||||
@@ -111,12 +113,14 @@ def batch_tech_scan(self, job_id: int, limit: int = 100):
|
||||
|
||||
job.status = "completed"
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - background task owns transaction
|
||||
|
||||
except Exception as e:
|
||||
logger.error("batch_tech_scan job %d failed: %s", job_id, e, exc_info=True)
|
||||
job.status = "failed"
|
||||
job.error_message = str(e)[:500]
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - persist failure status
|
||||
raise
|
||||
|
||||
|
||||
@@ -166,12 +170,14 @@ def batch_performance_scan(self, job_id: int, limit: int = 50):
|
||||
|
||||
job.status = "completed"
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - background task owns transaction
|
||||
|
||||
except Exception as e:
|
||||
logger.error("batch_performance_scan job %d failed: %s", job_id, e, exc_info=True)
|
||||
job.status = "failed"
|
||||
job.error_message = str(e)[:500]
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - persist failure status
|
||||
raise
|
||||
|
||||
|
||||
@@ -220,12 +226,14 @@ def batch_contact_scrape(self, job_id: int, limit: int = 100):
|
||||
|
||||
job.status = "completed"
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - background task owns transaction
|
||||
|
||||
except Exception as e:
|
||||
logger.error("batch_contact_scrape job %d failed: %s", job_id, e, exc_info=True)
|
||||
job.status = "failed"
|
||||
job.error_message = str(e)[:500]
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - persist failure status
|
||||
raise
|
||||
|
||||
|
||||
@@ -257,12 +265,14 @@ def batch_score_compute(self, job_id: int, limit: int = 500):
|
||||
job.total_items = count
|
||||
job.status = "completed"
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - background task owns transaction
|
||||
|
||||
except Exception as e:
|
||||
logger.error("batch_score_compute job %d failed: %s", job_id, e, exc_info=True)
|
||||
job.status = "failed"
|
||||
job.error_message = str(e)[:500]
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - persist failure status
|
||||
raise
|
||||
|
||||
|
||||
@@ -316,10 +326,12 @@ def full_enrichment(self, job_id: int, prospect_id: int):
|
||||
job.processed_items = 1
|
||||
job.status = "completed"
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - background task owns transaction
|
||||
|
||||
except Exception as e:
|
||||
logger.error("full_enrichment job %d failed: %s", job_id, e, exc_info=True)
|
||||
job.status = "failed"
|
||||
job.error_message = str(e)[:500]
|
||||
job.completed_at = datetime.now(UTC)
|
||||
db.commit() # SVC-006 - persist failure status
|
||||
raise
|
||||
|
||||
@@ -33,8 +33,10 @@ class TestInteractionService:
|
||||
)
|
||||
|
||||
assert interaction.id is not None
|
||||
assert interaction.interaction_type.value == "call"
|
||||
assert interaction.outcome.value == "positive"
|
||||
itype = interaction.interaction_type
|
||||
assert (itype.value if hasattr(itype, "value") else itype) == "call"
|
||||
outcome = interaction.outcome
|
||||
assert (outcome.value if hasattr(outcome, "value") else outcome) == "positive"
|
||||
|
||||
def test_create_interaction_with_follow_up(self, db, digital_prospect):
|
||||
"""Test creating an interaction with follow-up action."""
|
||||
|
||||
@@ -110,7 +110,8 @@ class TestProspectService:
|
||||
db, digital_prospect.id, {"notes": "Updated notes", "status": "contacted"}
|
||||
)
|
||||
assert updated.notes == "Updated notes"
|
||||
assert updated.status.value == "contacted"
|
||||
status = updated.status.value if hasattr(updated.status, "value") else updated.status
|
||||
assert status == "contacted"
|
||||
|
||||
def test_delete_prospect(self, db, digital_prospect):
|
||||
"""Test deleting a prospect."""
|
||||
|
||||
Reference in New Issue
Block a user