Skip to content

fix: sponsor statistics assignment tweak#516

Merged
smarcet merged 2 commits intomainfrom
fix/sponsor-statistics-crud
Mar 6, 2026
Merged

fix: sponsor statistics assignment tweak#516
smarcet merged 2 commits intomainfrom
fix/sponsor-statistics-crud

Conversation

@romanetar
Copy link
Collaborator

No description provided.

Signed-off-by: romanetar <roman_ag@hotmail.com>
@romanetar romanetar requested a review from smarcet March 6, 2026 16:38
@@ -1185,7 +1185,7 @@ public function updateSponsorServicesStatistics(Summit $summit, int $sponsor_id,
$statistics = $summit_sponsor->getSponsorServicesStatistics();
if (!$statistics) {
$statistics = new SponsorStatistics();
Copy link
Collaborator

Choose a reason for hiding this comment

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

@romanetar please use SponsorServicesStatisticsFactory::build
dont do here

 $statistics = new SponsorStatistics();

if (!$statistics) {
$statistics = new SponsorStatistics();
$statistics->setSponsor($summit_sponsor);
$summit_sponsor->setSponsorServicesStatistics($statistics);
Copy link
Collaborator

@smarcet smarcet Mar 6, 2026

Choose a reason for hiding this comment

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

@romanetar also please add an unit test for this case and lets be sure that are running at CI (.github/workflows/push.yml)
add proper entry on dep matrix here

- { name: "Repositories", filter: "--filter tests/Repositories/" }

Copy link
Collaborator

@smarcet smarcet left a comment

Choose a reason for hiding this comment

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

@romanetar please review comments

Signed-off-by: romanetar <roman_ag@hotmail.com>
Copy link
Collaborator

@smarcet smarcet left a comment

Choose a reason for hiding this comment

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

LGTM

@smarcet smarcet merged commit 6252044 into main Mar 6, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants