Skip to content

fix(audit): allow nullable sponsor in SummitSponsorExtraQuestionType::getSponsor #531

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.

Sign up for GitHub

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

Merged
smarcet merged 4 commits into from
Apr 16, 2026
Merged

fix(audit): allow nullable sponsor in SummitSponsorExtraQuestionType::getSponsor #531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b430061
fix(audit): allow nullable sponsor in SummitSponsorExtraQuestionType:…
romanetar Apr 16, 2026
a8e3824
fix: update getSponsor return allowing nulls
romanetar Apr 16, 2026
eb2f888
chore(unit-tests): fix location venue generator
smarcet Apr 16, 2026
5b4b960
chore(unit-test): add regresion test
smarcet Apr 16, 2026
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
1 change: 1 addition & 0 deletions .github/workflows/push.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ jobs:
- { name: "SummitRSVPServiceTest", filter: "--filter SummitRSVPServiceTest" }
- { name: "SummitRSVPInvitationServiceTest", filter: "--filter SummitRSVPInvitationServiceTest" }
- { name: "EntityModelUnitTests", filter: "tests/Unit/Entities/" }
- { name: "AuditUnitTests", filter: "tests/Unit/Audit/" }
- { name: "AuditOtlpStrategyTest", filter: "--filter AuditOtlpStrategyTest" }
- { name: "AuditEventTypesTest", filter: "--filter AuditEventTypesTest" }
- { name: "GuzzleTracingTest", filter: "--filter GuzzleTracingTest" }
Expand Down
5 changes: 3 additions & 2 deletions app/Events/SponsorServices/SummitSponsorshipAddOnCreatedEventDTO.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public static function fromSponsorshipAddOn(SummitSponsorshipAddOn $add_on): sel
$sponsorship = $add_on->getSponsorship();
$summit_sponsorship_type = $sponsorship->getType();
$sponsorship_type = $summit_sponsorship_type->getType();
$sponsor = $sponsorship->getSponsor();

return new self(
$add_on->getId(),
Expand All @@ -67,8 +68,8 @@ public static function fromSponsorshipAddOn(SummitSponsorshipAddOn $add_on): sel
$sponsorship_type->getName(),
$add_on->getType(),
$add_on->getName(),
$add_on->getSponsorship()->getSponsor()->getId(),
$add_on->getSponsorship()->getSponsor()->getSummitId(),
$sponsor?->getId() ?? 0,
$sponsor?->getSummitId() ?? 0,
);
}

Expand Down
4 changes: 2 additions & 2 deletions app/Events/SponsorServices/SummitSponsorshipCreatedEventDTO.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ public static function fromSponsorship(SummitSponsorship $sponsorship): self

return new self(
$sponsorship->getId(),
$sponsor->getId(),
$sponsor?->getId() ?? 0,
$summit_sponsorship_type->getId(),
$sponsorship_type->getId(),
$sponsorship_type->getName(),
$sponsor->getSummitId()
$sponsor?->getSummitId() ?? 0
);
}

Expand Down
6 changes: 3 additions & 3 deletions app/Jobs/Emails/Registration/PromoCodes/SponsorPromoCodeEmail.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ public function __construct
$summit = $promo_code->getSummit();
$payload = [];
$sponsor = $promo_code->getSponsor();
$payload[IMailTemplatesConstants::sponsor_tier_name] = implode(',', $sponsor->getSponsorshipTierNames());
$payload[IMailTemplatesConstants::sponsor_tier_name] = $sponsor ? implode(',', $sponsor->getSponsorshipTierNames()) : '';
$payload[IMailTemplatesConstants::promo_code] = $promo_code->getCode();
$payload[IMailTemplatesConstants::company_name] = '';
$company = $sponsor->getCompany();
$company = $sponsor?->getCompany();
if (!is_null($company))
$payload[IMailTemplatesConstants::company_name] = $company->getName();

Expand Down Expand Up @@ -97,4 +97,4 @@ public static function getEmailTemplateSchema(): array{
$payload[IMailTemplatesConstants::sponsor_tier_name]['type'] = 'string';
return $payload;
}
}
}
6 changes: 4 additions & 2 deletions ...lizers/Summit/Registration/PromoCodes/SponsorSummitRegistrationDiscountCodeSerializer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ public function serialize($expand = null, array $fields = [], array $relations =
}
break;
case 'sponsor_name':{
$values['sponsor_name'] = $code->getSponsor()->getCompany()->getName();
if($code->hasSponsor()) {
$values['sponsor_name'] = $code->getSponsor()->getCompany()->getName();
}
}
break;
}
Expand All @@ -71,4 +73,4 @@ public function serialize($expand = null, array $fields = [], array $relations =

return $values;
}
}
}
6 changes: 4 additions & 2 deletions ...rializers/Summit/Registration/PromoCodes/SponsorSummitRegistrationPromoCodeSerializer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ public function serialize($expand = null, array $fields = [], array $relations =
}
break;
case 'sponsor_name':{
$values['sponsor_name'] = $code->getSponsor()->getCompany()->getName();
if($code->hasSponsor()) {
$values['sponsor_name'] = $code->getSponsor()->getCompany()->getName();
}
}
break;
}
Expand All @@ -72,4 +74,4 @@ public function serialize($expand = null, array $fields = [], array $relations =

return $values;
}
}
}
6 changes: 5 additions & 1 deletion app/ModelSerializers/Summit/Registration/SponsorBadgeScanCSVSerializer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ public function serialize($expand = null, array $fields = array(), array $relati
if (!$scan instanceof SponsorBadgeScan) return [];
$values = parent::serialize($expand, $fields, $relations, $params);
$sponsor = $scan->getSponsor();

//There are no sponsor questions to process without a sponsor
if (is_null($sponsor)) return $values;

$sponsor_questions = $sponsor->getExtraQuestions();
$setting = $sponsor->getSummit()->getLeadReportSettingFor($sponsor);
$setting_columns = $setting->getColumns();
Expand Down Expand Up @@ -144,4 +148,4 @@ public function serialize($expand = null, array $fields = array(), array $relati

return $values;
}
}
}
7 changes: 5 additions & 2 deletions app/ModelSerializers/Summit/Registration/SponsorUserInfoGrantCSVSerializer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,14 @@ public function serialize($expand = null, array $fields = [], array $relations =
$values['notes'] = 'VIRTUAL';

$sponsor = $grant->getSponsor();

//There are no sponsor questions to process without a sponsor
if (is_null($sponsor)) return $values;

$sponsor_questions = $sponsor->getExtraQuestions();
$setting = $sponsor->getSummit()->getLeadReportSettingFor($sponsor);
$setting_columns = $setting->getColumns();


// remove not allowed string columns and sort them by setting columns order
$new_values = [];
foreach(array_values($setting_columns) as $column) {
Expand Down Expand Up @@ -130,4 +133,4 @@ public function serialize($expand = null, array $fields = [], array $relations =

return $values;
}
}
}
8 changes: 4 additions & 4 deletions app/Models/Foundation/Summit/ExtraQuestions/SummitSponsorExtraQuestionType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,16 @@ class SummitSponsorExtraQuestionType extends ExtraQuestionType
];

/**
* @var Sponsor
* @var Sponsor|null
*/
#[ORM\JoinColumn(name: 'SponsorID', referencedColumnName: 'ID', onDelete: 'CASCADE')]
#[ORM\ManyToOne(targetEntity: \models\summit\Sponsor::class, inversedBy: 'extra_questions')]
private $sponsor;

/**
* @return Sponsor
* @return Sponsor|null
*/
public function getSponsor(): Sponsor
public function getSponsor(): ?Sponsor
Comment thread
coderabbitai[bot] marked this conversation as resolved.
{
return $this->sponsor;
}
Expand All @@ -61,4 +61,4 @@ public function clearSponsor(): void
{
$this->sponsor = null;
}
}
}
6 changes: 3 additions & 3 deletions app/Models/Foundation/Summit/Registration/PromoCodes/Traits/SponsorPromoCodeTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ public function setContactEmail(string $contact_email): void
}

/**
* @return Sponsor
* @return Sponsor|null
*/
public function getSponsor():Sponsor
public function getSponsor(): ?Sponsor
{
return $this->sponsor;
}
Expand All @@ -95,4 +95,4 @@ public function setSponsor(Sponsor $sponsor)
public function checkSubject(string $email, ?string $company):bool{
return true;
}
}
}
8 changes: 4 additions & 4 deletions app/Models/Foundation/Summit/Registration/SponsorUserInfoGrant.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class SponsorUserInfoGrant extends SilverstripeBaseModel

const ClassName = 'SponsorUserInfoGrant';
/**
* @var Sponsor
* @var Sponsor|null
*/
#[ORM\JoinColumn(name: 'SponsorID', referencedColumnName: 'ID')]
#[ORM\ManyToOne(targetEntity: \models\summit\Sponsor::class, inversedBy: 'user_info_grants')]
Expand All @@ -53,9 +53,9 @@ class SponsorUserInfoGrant extends SilverstripeBaseModel
];

/**
* @return Sponsor
* @return Sponsor|null
*/
public function getSponsor(): Sponsor
public function getSponsor(): ?Sponsor
{
return $this->sponsor;
}
Expand Down Expand Up @@ -95,4 +95,4 @@ public function getAttendeeLastName():?string{
public function getAttendeeEmail():?string{
return $this->allowed_user->getEmail();
}
}
}
8 changes: 4 additions & 4 deletions app/Models/Foundation/Summit/SponsorMaterial.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class SponsorMaterial extends SilverstripeBaseModel
private $order;

/**
* @var Sponsor
* @var Sponsor|null
*/
#[ORM\JoinColumn(name: 'SponsorID', referencedColumnName: 'ID', onDelete: 'CASCADE')]
#[ORM\ManyToOne(targetEntity: \Sponsor::class, inversedBy: 'materials', fetch: 'EXTRA_LAZY')]
Expand Down Expand Up @@ -144,9 +144,9 @@ public function setLink(string $link): void
}

/**
* @return Sponsor
* @return Sponsor|null
*/
public function getSponsor(): Sponsor
public function getSponsor(): ?Sponsor
{
return $this->sponsor;
}
Expand All @@ -162,4 +162,4 @@ public function setSponsor(Sponsor $sponsor): void
public function clearSponsor():void{
$this->sponsor = null;
}
}
}
4 changes: 2 additions & 2 deletions app/Models/Foundation/Summit/SponsorStatistics.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class SponsorStatistics extends SilverstripeBaseModel
use One2ManyPropertyTrait;

/**
* @var Sponsor
* @var Sponsor|null
*/
#[ORM\JoinColumn(name: 'SponsorID', referencedColumnName: 'ID', onDelete: 'CASCADE')]
#[ORM\OneToOne(targetEntity: Sponsor::class, inversedBy: 'sponsorservices_statistics', fetch: 'EXTRA_LAZY')]
Expand Down Expand Up @@ -66,7 +66,7 @@ public function __construct()
$this->documentsQty = 0;
}

public function getSponsor(): Sponsor
public function getSponsor(): ?Sponsor
{
return $this->sponsor;
}
Comment on lines +69 to 72
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect known call sites from current context.
fd -i 'SponsorUserInfoGrantService.php' app --exec sed -n '220,260p' {}
fd -i 'SponsorBadgeScanSerializer.php' app --exec sed -n '50,95p' {}

# Find direct dereferences after getSponsor() (unsafe with nullable contracts).
rg -nP --type=php '\->getSponsor\(\)\s*->' app

# Find getSponsor() passed as arguments for manual nullable-acceptance checks.
rg -nP --type=php '\->getSponsor\(\)\s*[,\)]' app

Repository: OpenStackweb/summit-api

Length of output: 5085

Fix unguarded getSponsor() dereferences that assume non-null return.

The following locations directly dereference getSponsor() results without null checks and will fail if sponsor is null:

  • app/ModelSerializers/Summit/Registration/PromoCodes/SponsorSummitRegistrationDiscountCodeSerializer.php:66 — $code->getSponsor()->getCompany()->getName()
  • app/ModelSerializers/Summit/Registration/PromoCodes/SponsorSummitRegistrationPromoCodeSerializer.php:67 — $code->getSponsor()->getCompany()->getName()

Add null checks or use hasSponsor() guards (as done in SponsorBadgeScanSerializer.php:69) before dereferencing. Alternatively, adjust callers to handle null values explicitly.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/Models/Foundation/Summit/SponsorStatistics.php` around lines 69 - 72, The
serializers dereference $code->getSponsor()->getCompany()->getName() without
guarding for a null sponsor; update
SponsorSummitRegistrationDiscountCodeSerializer and
SponsorSummitRegistrationPromoCodeSerializer to check $code->hasSponsor() (or
$code->getSponsor() !== null) before calling getCompany()/getName(), mirroring
the pattern used in SponsorBadgeScanSerializer::hasSponsor(); if no sponsor,
return a safe fallback (e.g., null or empty string) or handle it explicitly in
the serialized output so the code never calls getCompany() on a null Sponsor.

Expand Down
8 changes: 4 additions & 4 deletions app/Models/Foundation/Summit/SummitLeadReportSetting.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class SummitLeadReportSetting extends SilverstripeBaseModel
const SponsorExtraQuestionsKey = 'extra_questions';

/**
* @var Sponsor
* @var Sponsor|null
*/
#[ORM\JoinColumn(name: 'SponsorID', referencedColumnName: 'ID', onDelete: 'SET NULL')]
#[ORM\OneToOne(targetEntity: \models\summit\Sponsor::class, inversedBy: 'lead_report_setting')]
Expand All @@ -51,9 +51,9 @@ public function __construct()
}

/**
* @return Sponsor
* @return Sponsor|null
*/
public function getSponsor(): Sponsor
public function getSponsor(): ?Sponsor
{
return $this->sponsor;
}
Expand Down Expand Up @@ -134,4 +134,4 @@ public function validateFor(Summit $summit, ?Sponsor $sponsor = null): void
}
}
}
}
}
6 changes: 3 additions & 3 deletions app/Models/Foundation/Summit/SummitSponsorship.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class SummitSponsorship extends SilverstripeBaseModel
];

/**
* @var Sponsor
* @var Sponsor|null
*/
#[ORM\JoinColumn(name: 'SponsorID', referencedColumnName: 'ID')]
#[ORM\ManyToOne(targetEntity: Sponsor::class)]
Expand All @@ -64,7 +64,7 @@ public function __construct()
$this->add_ons = new ArrayCollection();
}

public function getSponsor(): Sponsor
public function getSponsor(): ?Sponsor
{
return $this->sponsor;
}
Expand Down Expand Up @@ -133,4 +133,4 @@ public function setType(SummitSponsorshipType $type): void
{
$this->type = $type;
}
}
}
Loading
Loading