Skip to content
Open
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
3 changes: 3 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ CFP_APP_BASE_URL=
CFP_SUPPORT_EMAIL=
CFP_OAUTH2_SCOPES=
CFP_OAUTH2_CLIENT_ID=
# ceiling and default for an admin-granted per-presentation submission reopen window, in hours
CFP_MAX_REOPEN_HOURS=168
CFP_DEFAULT_REOPEN_HOURS=24

# RABBIT MQ
RABBITMQ_EXCHANGE_NAME=databus-exchange
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/push.yml
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: "ModelUnitTests", filter: "tests/Unit/Models/" }
- { name: "AuditUnitTests", filter: "tests/Unit/Audit/" }
- { name: "AuditOtlpStrategyTest", filter: "--filter AuditOtlpStrategyTest" }
- { name: "AuditEventTypesTest", filter: "--filter AuditEventTypesTest" }
Expand All @@ -68,7 +69,7 @@ jobs:
- { name: "CacheOptimizations", filter: "--filter '(PresentationSpeakerCacheTest|ResourceServerContextTest)'" }
# Named by path because no job in this matrix runs the tests/ root, only its
# subdirectories - a file added there runs nowhere unless it is listed here.
- { name: "PresentationMediaUploads", filter: "tests/PresentationMediaUploadsTest.php tests/PresentationMediaUploadsVisibilityTest.php tests/PresentationSerializerCacheKeyTest.php" }
- { name: "PresentationMediaUploads", filter: "tests/PresentationMediaUploadsTest.php tests/PresentationMediaUploadsVisibilityTest.php tests/PresentationSerializerCacheKeyTest.php tests/PresentationReopenModelTest.php tests/PresentationReopenApiTest.php tests/PresentationReopenAuthzTest.php" }
env:
OTEL_SERVICE_ENABLED: false
APP_ENV: testing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
use ModelSerializers\SerializerRegistry;
use OpenApi\Attributes as OA;
use services\model\IPresentationService;
use services\model\IPresentationSubmissionReopenService;
use utils\Filter;
use utils\FilterElement;
use utils\FilterParser;
Expand Down Expand Up @@ -84,6 +85,11 @@ final class OAuth2PresentationApiController extends OAuth2ProtectedController
*/
private $presentation_comments_repository;

/**
* @var IPresentationSubmissionReopenService
*/
private $presentation_submission_reopen_service;

/**
* OAuth2PresentationApiController constructor.
* @param IPresentationService $presentation_service
Expand All @@ -92,6 +98,7 @@ final class OAuth2PresentationApiController extends OAuth2ProtectedController
* @param IMemberRepository $member_repository
* @param ISummitPresentationCommentRepository $presentation_comments_repository
* @param IResourceServerContext $resource_server_context
* @param IPresentationSubmissionReopenService $presentation_submission_reopen_service
*/
public function __construct
(
Expand All @@ -100,7 +107,8 @@ public function __construct
ISummitEventRepository $presentation_repository,
IMemberRepository $member_repository,
ISummitPresentationCommentRepository $presentation_comments_repository,
IResourceServerContext $resource_server_context
IResourceServerContext $resource_server_context,
IPresentationSubmissionReopenService $presentation_submission_reopen_service
)
{
parent::__construct($resource_server_context);
Expand All @@ -109,6 +117,7 @@ public function __construct
$this->member_repository = $member_repository;
$this->summit_repository = $summit_repository;
$this->presentation_comments_repository = $presentation_comments_repository;
$this->presentation_submission_reopen_service = $presentation_submission_reopen_service;
}

//presentations
Expand Down Expand Up @@ -525,6 +534,113 @@ public function updatePresentationSubmission($summit_id, $presentation_id)
});
}

#[OA\Put(
path: "/api/v1/summits/{id}/presentations/{presentation_id}/submission-period/reopen",
summary: "Admin-only: reopen the submission period for a presentation",
operationId: "reopenSubmissionPeriod",
security: [['summit_presentations_auth' => [SummitScopes::WriteSummitData, SummitScopes::WriteEventData, SummitScopes::WritePresentationData]]],
tags: ['Presentations'],
parameters: [
new OA\Parameter(name: 'id', in: 'path', required: true, schema: new OA\Schema(type: 'integer')),
new OA\Parameter(name: 'presentation_id', in: 'path', required: true, schema: new OA\Schema(type: 'integer')),
],
requestBody: new OA\RequestBody(
required: false,
content: new OA\JsonContent(
properties: [
new OA\Property(property: 'hours', type: 'integer'),
]
)
),
responses: [
new OA\Response(
response: Response::HTTP_CREATED,
description: "Created",
content: new OA\JsonContent(ref: "#/components/schemas/Presentation")
),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
new OA\Response(response: Response::HTTP_UNAUTHORIZED, description: "Unauthorized"),
new OA\Response(response: Response::HTTP_FORBIDDEN, description: "Forbidden"),
new OA\Response(response: Response::HTTP_NOT_FOUND, description: "Not Found"),
new OA\Response(response: Response::HTTP_PRECONDITION_FAILED, description: "Validation Error"),
new OA\Response(response: Response::HTTP_INTERNAL_SERVER_ERROR, description: "Server Error"),
]
)]
public function reopenSubmissionPeriod($summit_id, $presentation_id)
{
return $this->processRequest(function () use ($summit_id, $presentation_id) {

$summit = SummitFinderStrategyFactory::build($this->summit_repository, $this->resource_server_context)->find($summit_id);
if (is_null($summit)) return $this->error404();

$current_member = $this->resource_server_context->getCurrentUser();
if (is_null($current_member)) return $this->error403();

$isAdmin = $current_member->isAdmin()
|| $current_member->hasPermissionForOnGroup($summit, IGroup::SummitAdministrators);
if (!$isAdmin) return $this->error403();

$payload = $this->getJsonPayload(['hours' => 'sometimes|integer|min:1']);

// null, not the default: the hours rule (default AND ceiling) lives in the service.
$presentation = $this->presentation_submission_reopen_service->reopen(
$summit,
intval($presentation_id),
isset($payload['hours']) ? intval($payload['hours']) : null,
$current_member
);

// Private, NOT Admin: SerializerRegistry has no Admin key for Presentation and an
// unknown type silently falls back to Public, stripping the reopen fields.
return $this->updated(SerializerRegistry::getInstance()->getSerializer(
$presentation, SerializerRegistry::SerializerType_Private
)->serialize(
SerializerUtils::getExpand(),
SerializerUtils::getFields(),
SerializerUtils::getRelations()
));
});
}

#[OA\Delete(
path: "/api/v1/summits/{id}/presentations/{presentation_id}/submission-period/reopen",
summary: "Admin-only: close the reopened submission period for a presentation",
operationId: "closeSubmissionPeriod",
security: [['summit_presentations_auth' => [SummitScopes::WriteSummitData, SummitScopes::WriteEventData, SummitScopes::WritePresentationData]]],
tags: ['Presentations'],
parameters: [
new OA\Parameter(name: 'id', in: 'path', required: true, schema: new OA\Schema(type: 'integer')),
new OA\Parameter(name: 'presentation_id', in: 'path', required: true, schema: new OA\Schema(type: 'integer')),
],
responses: [
new OA\Response(response: Response::HTTP_NO_CONTENT, description: "No Content"),
new OA\Response(response: Response::HTTP_UNAUTHORIZED, description: "Unauthorized"),
new OA\Response(response: Response::HTTP_FORBIDDEN, description: "Forbidden"),
new OA\Response(response: Response::HTTP_NOT_FOUND, description: "Not Found"),
new OA\Response(response: Response::HTTP_INTERNAL_SERVER_ERROR, description: "Server Error"),
]
)]
public function closeSubmissionPeriod($summit_id, $presentation_id)
{
return $this->processRequest(function () use ($summit_id, $presentation_id) {

$summit = SummitFinderStrategyFactory::build($this->summit_repository, $this->resource_server_context)->find($summit_id);
if (is_null($summit)) return $this->error404();

$current_member = $this->resource_server_context->getCurrentUser();
if (is_null($current_member)) return $this->error403();

$isAdmin = $current_member->isAdmin()
|| $current_member->hasPermissionForOnGroup($summit, IGroup::SummitAdministrators);
if (!$isAdmin) return $this->error403();

$this->presentation_submission_reopen_service->closeNow(
$summit, intval($presentation_id), $current_member
);

return $this->deleted();
});
}

#[OA\Put(
path: "/api/v1/summits/{id}/presentations/{presentation_id}/completed",
summary: "Mark a presentation submission as completed",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use models\summit\ISummitEventRepository;
use models\summit\ISummitRepository;
use models\summit\PresentationSpeaker;
use ModelSerializers\IPresentationSerializerTypes;
use ModelSerializers\ISerializerTypeSelector;
use ModelSerializers\SerializerRegistry;
use services\model\ISpeakerService;
Expand Down Expand Up @@ -2342,7 +2343,9 @@ public function getMySpeakerPresentationsByRoleAndBySelectionPlan($role, $select
return $this->ok($response->toArray(
SerializerUtils::getExpand(),
SerializerUtils::getFields(),
SerializerUtils::getRelations()
SerializerUtils::getRelations(),
[],
IPresentationSerializerTypes::Submission
));
});
}
Expand Down Expand Up @@ -2449,7 +2452,9 @@ public function getMySpeakerPresentationsByRoleAndBySummit($role, $summit_id)
return $this->ok($response->toArray(
SerializerUtils::getExpand(),
SerializerUtils::getFields(),
SerializerUtils::getRelations()
SerializerUtils::getRelations(),
[],
IPresentationSerializerTypes::Submission
));
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
**/
use Libs\ModelSerializers\One2ManyExpandSerializer;
use models\summit\Presentation;

/**
Expand Down Expand Up @@ -41,6 +42,29 @@ class AdminPresentationSerializer extends PresentationSerializer
'OverflowStreamIsSecure' => 'overflow_stream_is_secure:json_boolean',
'OverflowStreamKey' => 'overflow_stream_key:json_string',
'TrackChairAvgScoresPerRakingType' => 'track_chair_scores_avg:json_string_array',
'SubmissionReopenedUntil' => 'submission_reopened_until:datetime_epoch',
'SubmissionReopenedById' => 'submission_reopened_by_id:json_int',
];

/**
* Declared HERE and not on the base PresentationSerializer on purpose. getExpandsMappings()
* merges parent into child only, and SubmissionPresentationSerializer is a SIBLING of this
* class (both extend PresentationSerializer), so this relation is unreachable from the
* Submission and Public variants. A case in the base expand switch would not be -- that is
* the leak the Admin-only design exists to prevent, and why the SDS rejected id+expand when
* a base-class switch was the only mechanism considered.
*
* serializer_type is explicit because One2ManyExpandSerializer defaults to Public, which
* blanks the actor's email.
*/
protected static $expand_mappings = [
'submission_reopened_by' => [
'type' => One2ManyExpandSerializer::class,
'original_attribute' => 'submission_reopened_by_id',
'getter' => 'getSubmissionReopenedBy',
'has' => 'hasSubmissionReopenedBy',
'serializer_type' => SerializerRegistry::SerializerType_Private,
],
];

protected static $allowed_fields = [
Expand All @@ -64,7 +88,13 @@ class AdminPresentationSerializer extends PresentationSerializer
'etherpad_link',
'overflow_streaming_url',
'overflow_stream_is_secure',
'overflow_stream_key'
'overflow_stream_key',
'submission_reopened_until',
'submission_reopened_by_id',
];

protected static $allowed_relations = [
'submission_reopened_by',

@smarcet smarcet Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@caseylocker submission_reopened_by is a relation, and it should be declared as an expand mapping rather than flattened into a preformatted "Full Name (email)" string.

Reviewer note 4 says the SDS rejected the id+expand idiom because the expand switch lives in the base PresentationSerializer, so a case added there would be reachable from the Public and Submission variants. That concern is real — but it only applies to the base-class switch, and the fix is not to abandon expand. Your own docblock on Presentation::getSubmissionReopenedByNice() names the right mechanism: "the safe mechanism is a subclass-local $expand_mappings, never a base-class switch case." That is precisely the pattern in the file I linked: SummitAttendeeSerializer.php:181 declares protected static $expand_mappings on the subclass, keyed per relation, with original_attribute, getter, has and an explicit serializer_type. Declared on AdminPresentationSerializer it is unreachable from the Public and Submission variants, so the leak the SDS was guarding against does not occur.

Why it matters beyond style: the flattened string is not consumable. A client that needs the actor's id, or wants to link to the member, has to parse a display string and guess at names containing parentheses. It also cannot be expanded, filtered, or reused, and it bakes a presentation decision into the model layer — getSubmissionReopenedByNice() exists only because the serializer could not express the relation.

Suggested fix: keep submission_reopened_by_id:json_int in $array_mappings, drop SubmissionReopenedByNice and the submission_reopened_by string field, and add to AdminPresentationSerializer:

protected static $expand_mappings = [
    'submission_reopened_by' => [
        'type' => One2ManyExpandSerializer::class,
        'original_attribute' => 'submission_reopened_by_id',
        'getter' => 'getSubmissionReopenedBy',
        'has' => 'hasSubmissionReopenedBy',
        'serializer_type' => SerializerRegistry::SerializerType_Private,
    ],
];

Show Admin then reads submission_reopened_by_id off the getEvent payload as it does today, and gets the full member only when it asks. Worth noting the plumbing is already in place: reopenSubmissionPeriod forwards SerializerUtils::getExpand(), getFields() and getRelations() into serialize(), so ?expand=submission_reopened_by would work the moment the mapping exists — the declaration is the only missing piece. If the SDS text needs an amendment to record this, that is worth doing — the constraint it encoded is satisfied by the subclass-local form.

@caseylocker caseylocker Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in c1cffca. submission_reopened_by is a relation now: the payload carries submission_reopened_by_id, and ?expand=submission_reopened_by returns the serialized member. The preformatted string is gone.

One deliberate detail. The mapping is declared on AdminPresentationSerializer, not on the base. AbstractSerializer::getExpandsMappings() merges the class lineage parent to child only, and SubmissionPresentationSerializer is a sibling of this class, so the relation stays unreachable from the Submission and Public variants. That was the SDS's objection to id plus expand, and it does not apply to a subclass local mapping. serializer_type is Private, matching how created_by is already serialized here; the default is Public, which blanks the actor's email.

Covered by testReopenActorIsExpandableOnTheAdminResponse, which asserts the id key is replaced by the relation and that the relation carries the email.

On your last point, the SDS amendment: written and open as fntechgit/ftn-docsnsklz#115. It records the shape change, why the subclass local form satisfies the constraint §4 encoded, and the two places the body is now wrong about the shipped contract (§6's "no getEvent expand entry is needed" and §8's "plain scalars, no expand"). The body itself is untouched, the entry is additive. Vault main needs one non-author approval, so it is waiting on a reviewer.

];

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@
*/
class SubmissionPresentationSerializer extends PresentationSerializer
{
protected static $array_mappings = [
'SubmissionReopenedUntil' => 'submission_reopened_until:datetime_epoch',
];

protected static $allowed_fields = [
'submission_reopened_until',
];

/**
* @param string|null $relation
* @return string
Expand Down
Loading
Loading