Skip to content

Commit 88aeed3

Browse files
romanetarsmarcet
romanetar
and
smarcet
committed
fix(speakers): honour payload bio on createMySpeaker instead of always overriding with member bio (#564)
* fix(speakers): honour payload bio on createMySpeaker instead of always overriding with member bio Signed-off-by: romanetar <roman_ag@hotmail.com> * fix(speakers): treat empty payload values as not sent on createMySpeaker Form clients commonly serialize untouched fields as empty strings. An empty or whitespace-only value for the defaultable keys (first_name, last_name, bio, twitter, irc) won the merge against the member defaults and created a speaker with an empty field instead of falling back to the member profile data. On create there is nothing to clear yet, so such values now count as not sent. The PUT path is untouched: empty strings there still clear the field. Adds a boundary regression test: a whitespace-only submitted bio falls back to the member bio (also covering the trim). --------- Signed-off-by: romanetar <roman_ag@hotmail.com> Co-authored-by: smarcet <smarcet@gmail.com>
1 parent b1a48c0 commit 88aeed3

2 files changed

Lines changed: 224 additions & 8 deletions

File tree

‎app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1354,17 +1354,26 @@ public function createMySpeaker()
13541354
'notes'
13551355
];
13561356

1357-
// set data from current member ...
1358-
$aux_payload = [
1359-
'member_id' => $current_member->getId(),
1357+
// Member fields are used as defaults only — user-submitted values take precedence.
1358+
// member_id is always forced from the authenticated user for security.
1359+
$member_defaults = [
13601360
'first_name' => $current_member->getFirstName(),
1361-
'last_name' => $current_member->getLastName(),
1362-
'bio' => $current_member->getBio(),
1363-
'twitter' => $current_member->getTwitterHandle(),
1364-
'irc' => $current_member->getIrcHandle(),
1361+
'last_name' => $current_member->getLastName(),
1362+
'bio' => $current_member->getBio(),
1363+
'twitter' => $current_member->getTwitterHandle(),
1364+
'irc' => $current_member->getIrcHandle(),
13651365
];
13661366

1367-
$payload = array_merge($payload, $aux_payload);
1367+
// On create there is nothing to clear yet, so an empty or whitespace-only
1368+
// submitted value counts as "not sent" and the member default applies.
1369+
foreach (array_keys($member_defaults) as $key) {
1370+
if (isset($payload[$key]) && is_string($payload[$key]) && trim($payload[$key]) === '') {
1371+
unset($payload[$key]);
1372+
}
1373+
}
1374+
1375+
$payload = array_merge($member_defaults, $payload);
1376+
$payload['member_id'] = $current_member->getId();
13681377

13691378
$speaker = $this->service->addSpeaker(HTMLCleaner::cleanData($payload, $fields), $current_member);
13701379

‎tests/oauth2/OAuth2SummitSpeakersApiTest.php‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
use models\summit\PresentationSpeaker;
2525
use utils\FilterParser;
2626
use models\summit\SpeakersSummitRegistrationPromoCode;
27+
use models\main\Member;
28+
use LaravelDoctrine\ORM\Facades\Registry;
29+
use models\utils\SilverstripeBaseModel;
2730

2831
final class OAuth2SummitSpeakersApiTest extends ProtectedApiTestCase
2932
{
@@ -2443,4 +2446,208 @@ public function testGetCurrentSummitSpeakersActivitiesCountWithAcceptedPresentat
24432446
$this->assertEquals($baseline + 1, $data->count);
24442447
}
24452448

2449+
private function resetEmIfNeeded(): void
2450+
{
2451+
if (!self::$em->isOpen()) {
2452+
self::$em = Registry::resetManager(SilverstripeBaseModel::EntityManager);
2453+
}
2454+
}
2455+
2456+
/**
2457+
* Regression test for: createMySpeaker must honour the bio submitted in the
2458+
* payload and must NOT silently overwrite it with the member's bio.
2459+
*
2460+
* Previously array_merge($payload, $aux_payload) put $aux_payload last so
2461+
* member->getBio() always won regardless of what the user submitted.
2462+
*/
2463+
public function testCreateMySpeakerPayloadBioTakesPrecedenceOverMemberBio()
2464+
{
2465+
// Create a fresh member (no speaker attached) with a known bio
2466+
$prefix = str_random(10);
2467+
$memberBio = "This is the OLD bio from the member/FNid profile.";
2468+
$payloadBio = "This is the NEW bio submitted by the user in the portal.";
2469+
2470+
$newMember = new Member();
2471+
$newMember->setEmail("test_bio_precedence_{$prefix}@example.com");
2472+
$newMember->setFirstName("Bio");
2473+
$newMember->setLastName("Precedence");
2474+
$newMember->setActive(true);
2475+
$newMember->setEmailVerified(true);
2476+
$newMember->setUserExternalId(mt_rand());
2477+
$newMember->setBio($memberBio);
2478+
self::$em->persist($newMember);
2479+
self::$em->flush();
2480+
2481+
// Authenticate as the new member
2482+
self::$service->setUserId($newMember->getUserExternalId());
2483+
self::$service->setUserExternalId($newMember->getUserExternalId());
2484+
self::$service->setUserEmail($newMember->getEmail());
2485+
self::$service->setUserFirstName($newMember->getFirstName());
2486+
self::$service->setUserLastName($newMember->getLastName());
2487+
2488+
$headers = [
2489+
"HTTP_Authorization" => " Bearer " . $this->access_token,
2490+
"CONTENT_TYPE" => "application/json",
2491+
];
2492+
2493+
$response = $this->action(
2494+
"POST",
2495+
"OAuth2SummitSpeakersApiController@createMySpeaker",
2496+
[],
2497+
[],
2498+
[],
2499+
[],
2500+
$headers,
2501+
json_encode(['bio' => $payloadBio])
2502+
);
2503+
2504+
// Restore authenticated member so tearDown works correctly
2505+
self::$service->setUserId(self::$member->getUserExternalId());
2506+
self::$service->setUserExternalId(self::$member->getUserExternalId());
2507+
self::$service->setUserEmail(self::$member->getEmail());
2508+
self::$service->setUserFirstName(self::$member->getFirstName());
2509+
self::$service->setUserLastName(self::$member->getLastName());
2510+
2511+
$this->assertResponseStatus(201);
2512+
$speaker = json_decode($response->getContent());
2513+
$this->assertNotNull($speaker);
2514+
$this->assertEquals($payloadBio, $speaker->bio,
2515+
"Speaker bio must match what the user submitted, not the member/FNid bio.");
2516+
2517+
// Cleanup — EM may have been closed by the BrowserKit HTTP simulation
2518+
$this->resetEmIfNeeded();
2519+
self::$em->remove(self::$em->find(Member::class, $newMember->getId()));
2520+
self::$em->flush();
2521+
}
2522+
2523+
/**
2524+
* When no bio is submitted in the payload, createMySpeaker should fall back
2525+
* to the member's bio so that a first-time speaker gets a sensible default.
2526+
*/
2527+
public function testCreateMySpeakerFallsBackToMemberBioWhenNoneSubmitted()
2528+
{
2529+
$prefix = str_random(10);
2530+
$memberBio = "Default bio coming from the FNid member profile.";
2531+
2532+
$newMember = new Member();
2533+
$newMember->setEmail("test_bio_fallback_{$prefix}@example.com");
2534+
$newMember->setFirstName("Bio");
2535+
$newMember->setLastName("Fallback");
2536+
$newMember->setActive(true);
2537+
$newMember->setEmailVerified(true);
2538+
$newMember->setUserExternalId(mt_rand());
2539+
$newMember->setBio($memberBio);
2540+
self::$em->persist($newMember);
2541+
self::$em->flush();
2542+
2543+
self::$service->setUserId($newMember->getUserExternalId());
2544+
self::$service->setUserExternalId($newMember->getUserExternalId());
2545+
self::$service->setUserEmail($newMember->getEmail());
2546+
self::$service->setUserFirstName($newMember->getFirstName());
2547+
self::$service->setUserLastName($newMember->getLastName());
2548+
2549+
$headers = [
2550+
"HTTP_Authorization" => " Bearer " . $this->access_token,
2551+
"CONTENT_TYPE" => "application/json",
2552+
];
2553+
2554+
// No 'bio' key in the payload — member bio should be used as default
2555+
$response = $this->action(
2556+
"POST",
2557+
"OAuth2SummitSpeakersApiController@createMySpeaker",
2558+
[],
2559+
[],
2560+
[],
2561+
[],
2562+
$headers,
2563+
json_encode(['title' => 'Engineer'])
2564+
);
2565+
2566+
self::$service->setUserId(self::$member->getUserExternalId());
2567+
self::$service->setUserExternalId(self::$member->getUserExternalId());
2568+
self::$service->setUserEmail(self::$member->getEmail());
2569+
self::$service->setUserFirstName(self::$member->getFirstName());
2570+
self::$service->setUserLastName(self::$member->getLastName());
2571+
2572+
$this->assertResponseStatus(201);
2573+
$speaker = json_decode($response->getContent());
2574+
$this->assertNotNull($speaker);
2575+
$this->assertEquals($memberBio, $speaker->bio,
2576+
"When no bio is submitted, speaker bio must default to the member/FNid bio.");
2577+
2578+
// Cleanup — EM may have been closed by the BrowserKit HTTP simulation
2579+
$this->resetEmIfNeeded();
2580+
self::$em->remove(self::$em->find(Member::class, $newMember->getId()));
2581+
self::$em->flush();
2582+
}
2583+
2584+
/**
2585+
* Boundary case: form clients commonly serialize untouched fields as empty
2586+
* strings. On create, an empty or whitespace-only bio must count as "not
2587+
* sent" and fall back to the member bio - not create a speaker with an
2588+
* empty bio.
2589+
*/
2590+
public function testCreateMySpeakerEmptyBioFallsBackToMemberBio()
2591+
{
2592+
$prefix = str_random(10);
2593+
$memberBio = "Bio coming from the FNid member profile.";
2594+
2595+
$newMember = new Member();
2596+
$newMember->setEmail("test_bio_empty_{$prefix}@example.com");
2597+
$newMember->setFirstName("Bio");
2598+
$newMember->setLastName("EmptyString");
2599+
$newMember->setActive(true);
2600+
$newMember->setEmailVerified(true);
2601+
$newMember->setUserExternalId(mt_rand());
2602+
$newMember->setBio($memberBio);
2603+
self::$em->persist($newMember);
2604+
self::$em->flush();
2605+
2606+
self::$service->setUserId($newMember->getUserExternalId());
2607+
self::$service->setUserExternalId($newMember->getUserExternalId());
2608+
self::$service->setUserEmail($newMember->getEmail());
2609+
self::$service->setUserFirstName($newMember->getFirstName());
2610+
self::$service->setUserLastName($newMember->getLastName());
2611+
2612+
$headers = [
2613+
"HTTP_Authorization" => " Bearer " . $this->access_token,
2614+
"CONTENT_TYPE" => "application/json",
2615+
];
2616+
2617+
try {
2618+
// Whitespace-only bio - must be treated as "not sent"
2619+
$response = $this->action(
2620+
"POST",
2621+
"OAuth2SummitSpeakersApiController@createMySpeaker",
2622+
[],
2623+
[],
2624+
[],
2625+
[],
2626+
$headers,
2627+
json_encode(['bio' => ' ', 'title' => 'Engineer'])
2628+
);
2629+
} finally {
2630+
// Restore authenticated member even if the request blows up,
2631+
// so the rest of the class does not run impersonated.
2632+
self::$service->setUserId(self::$member->getUserExternalId());
2633+
self::$service->setUserExternalId(self::$member->getUserExternalId());
2634+
self::$service->setUserEmail(self::$member->getEmail());
2635+
self::$service->setUserFirstName(self::$member->getFirstName());
2636+
self::$service->setUserLastName(self::$member->getLastName());
2637+
}
2638+
2639+
$this->assertResponseStatus(201);
2640+
$speaker = json_decode($response->getContent());
2641+
$this->assertNotNull($speaker);
2642+
$this->assertEquals($memberBio, $speaker->bio,
2643+
"An empty/whitespace-only submitted bio must fall back to the member/FNid bio.");
2644+
2645+
// Cleanup - EM may have been closed by the BrowserKit HTTP simulation
2646+
$this->resetEmIfNeeded();
2647+
$createdSpeaker = self::$em->find(PresentationSpeaker::class, intval($speaker->id));
2648+
if (!is_null($createdSpeaker)) self::$em->remove($createdSpeaker);
2649+
self::$em->remove(self::$em->find(Member::class, $newMember->getId()));
2650+
self::$em->flush();
2651+
}
2652+
24462653
}

0 commit comments

Comments
 (0)