Skip to content

Commit

Permalink
fix(application): post-reg phase could clear data under certain circu…
Browse files Browse the repository at this point in the history 
…mstances (#68)

* fix(application): prevent changes to application after reg end
* fix(invitation): allow accepting invites only without application
* fix(application): inconsistent UX for accepting invites
* fix(application): removed redundant check for invalid app state.
* fix(oidc): incorrect route on expired refresh token
* fix(application): comment field not needed for assistants
* fix(application): require TOS for assistants after regular reg
* fix(application): prevent cancelling after checkout
* fix(application): assistants may only cancel until end of assistant reg
* fix(applications): custom wording for different user types
* fix(invitation): improved post-reg ux

---------

Co-authored-by: JulSteele <[email protected]>
  • Loading branch information
@Fenrikur @julsteele
Fenrikur and julsteele authored Mar 4, 2024
1 parent 9f76b41 commit ec7c563
Show file tree
Hide file tree
Showing 8 changed files with 189 additions and 89 deletions.
132 changes: 90 additions & 42 deletions app/Http/Controllers/Applications/ApplicationController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use App\Notifications\WelcomeAssistantNotification;
use App\Notifications\WelcomeNotification;
use App\Notifications\WelcomeShareNotification;
use Carbon\Carbon;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Auth;
Expand Down Expand Up @@ -117,28 +118,62 @@ public function store(ApplicationRequest $request)
/** @var Application|null */
$parent = self::determineParentByCode($code);

$application = Application::updateOrCreate([
"user_id" => Auth::id(),
], [
"table_type_requested" => $request->input('space'),
"type" => $applicationType,
"display_name" => $request->input('displayName'),
"website" => $request->input('website'),
"merchandise" => $request->input('merchandise'),
"is_afterdark" => $request->input('denType') === "denTypeAfterDark",
"additional_space_request" => $request->has('additionalSpaceRequest') && !empty($request->input('additionalSpaceRequestText')) ? $request->input('additionalSpaceRequestText') : null,
"is_power" => $request->has('power'),
"is_wallseat" => $request->has('wallseat'),
"wanted_neighbors" => $request->input('wanted'),
"comment" => $request->input('comment'),
"parent_id" => $parent?->id,
"invite_code_shares" => null,
"invite_code_assistants" => null,
"waiting_at" => null,
"offer_sent_at" => null,
"canceled_at" => null,
"table_number" => null,
]);
$application = null;
if (Carbon::parse(config('con.reg_end_date'))->isFuture()) {
$application = Application::updateOrCreate([
"user_id" => Auth::id(),
], [
"table_type_requested" => $request->input('space'),
"type" => $applicationType,
"display_name" => $request->input('displayName'),
"website" => $request->input('website'),
"merchandise" => $request->input('merchandise'),
"is_afterdark" => $request->input('denType') === "denTypeAfterDark",
"additional_space_request" => $request->has('additionalSpaceRequest') && !empty($request->input('additionalSpaceRequestText')) ? $request->input('additionalSpaceRequestText') : null,
"is_power" => $request->has('power'),
"is_wallseat" => $request->has('wallseat'),
"wanted_neighbors" => $request->input('wanted'),
"comment" => $request->input('comment'),
"parent_id" => $parent?->id,
"invite_code_shares" => null,
"invite_code_assistants" => null,
"waiting_at" => null,
"offer_sent_at" => null,
"canceled_at" => null,
"table_number" => null,
]);
} else if (
Carbon::parse(config('con.assistant_end_date'))->isFuture()
&& $applicationType === ApplicationType::Assistant
) {
abort_if(is_null($parent), 404, 'Invalid invite code');

// Only create new assistant applications while assistant registration is still open.
$application = Application::updateOrCreate([
"user_id" => Auth::id(),
], [
"table_type_requested" => null,
"type" => ApplicationType::Assistant,
"display_name" => null,
"website" => null,
"merchandise" => null,
"is_afterdark" => false,
"additional_space_request" => null,
"is_power" => false,
"is_wallseat" => false,
"wanted_neighbors" => null,
"parent_id" => $parent->id,
"invite_code_shares" => null,
"invite_code_assistants" => null,
"waiting_at" => null,
"offer_sent_at" => null,
"canceled_at" => null,
"table_number" => null,
]);
} else {
// No user-driven updates to applications after reg phases have ended.
abort(400, 'Registration phase has ended');
}

if ($applicationType !== ApplicationType::Assistant) {
// TODO: Refactor
Expand All @@ -147,10 +182,6 @@ public function store(ApplicationRequest $request)

InvitationController::clearInvitationCodeConfirmation($request);

if (!$application || $application->getStatus() !== ApplicationStatus::Open) {
abort(400, 'Invalid application state');
}

/** @var User */
$user = Auth::user();

Expand Down Expand Up @@ -186,20 +217,35 @@ public function update(ApplicationRequest $request)

$newParent = self::determineParentByCode($code);

$application->update([
"table_type_requested" => $request->input('space'),
"type" => $newApplicationType ?? $application->type,
"display_name" => $request->input('displayName'),
"website" => $request->input('website'),
"merchandise" => $request->input('merchandise'),
"is_afterdark" => $request->input('denType') === "denTypeAfterDark",
"additional_space_request" => $request->has('additionalSpaceRequest') && !empty($request->input('additionalSpaceRequestText')) ? $request->input('additionalSpaceRequestText') : null,
"is_power" => $request->has('power'),
"is_wallseat" => $request->has('wallseat'),
"wanted_neighbors" => $request->input('wanted'),
"comment" => $request->input('comment'),
"parent_id" => $newParent?->id ?? $application->parent_id,
]);
if (Carbon::parse(config('con.reg_end_date'))->isFuture()) {
$application->update([
"table_type_requested" => $request->input('space'),
"type" => $newApplicationType ?? $application->type,
"display_name" => $request->input('displayName'),
"website" => $request->input('website'),
"merchandise" => $request->input('merchandise'),
"is_afterdark" => $request->input('denType') === "denTypeAfterDark",
"additional_space_request" => $request->has('additionalSpaceRequest') && !empty($request->input('additionalSpaceRequestText')) ? $request->input('additionalSpaceRequestText') : null,
"is_power" => $request->has('power'),
"is_wallseat" => $request->has('wallseat'),
"wanted_neighbors" => $request->input('wanted'),
"comment" => $request->input('comment'),
"parent_id" => $newParent?->id ?? $application->parent_id,
]);
} else if (
Carbon::parse(config('con.assistant_end_date'))->isFuture()
&& $application->type !== ApplicationType::Dealer
&& $application->type !== ApplicationType::Share
&& $newApplicationType === ApplicationType::Assistant
) {
// Only update assistant applications while assistant registration is still open.
$application->update([
"type" => $newApplicationType ?? $application->type,
"parent_id" => $newParent?->id ?? $application->parent_id,
]);
} else {
// No user-driven updates to applications after reg phases have ended.
}

if ($application->isActive() && $newApplicationType !== ApplicationType::Assistant) {
// TODO: Refactor
Expand All @@ -224,7 +270,8 @@ public function delete()
{
$application = Auth::user()->application;
abort_if(is_null($application), 404, 'Application not found');
abort_if($application->status === ApplicationStatus::TableAccepted || $application->status === ApplicationStatus::CheckedIn, 403, 'Applications which have accepted their table may no longer be canceled.');
abort_if($application->type !== ApplicationType::Assistant && ($application->status === ApplicationStatus::TableAccepted || $application->status === ApplicationStatus::CheckedIn || $application->status === ApplicationStatus::CheckedOut), 403, 'Applications which have accepted their table may no longer be canceled.');
abort_if($application->type === ApplicationType::Assistant && Carbon::parse(config('con.assistant_end_date'))->isPast(), 403, 'Assistants may no longer cancel once the assistant registration period is over.');

return view('application.delete', [
"application" => $application,
Expand All @@ -237,7 +284,8 @@ public function destroy()
$user = Auth::user();
$application = $user->application;
abort_if(is_null($application), 404, 'Application not found');
abort_if($application->status === ApplicationStatus::TableAccepted || $application->status === ApplicationStatus::CheckedIn, 403, 'Applications which have accepted their table may no longer be canceled.');
abort_if($application->type !== ApplicationType::Assistant && ($application->status === ApplicationStatus::TableAccepted || $application->status === ApplicationStatus::CheckedIn || $application->status === ApplicationStatus::CheckedOut), 403, 'Applications which have accepted their table may no longer be canceled.');
abort_if($application->type === ApplicationType::Assistant && Carbon::parse(config('con.assistant_end_date'))->isPast(), 403, 'Assistants may no longer cancel once the assistant registration period is over.');

foreach ($application->children()->get() as $child) {
$child->update([
Expand Down
11 changes: 9 additions & 2 deletions app/Http/Controllers/Applications/InvitationController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,19 @@ public function view(Request $request)
]);
}

/** @var Application */
$application = Auth::user()->application;

if ($application->isActive()) {
throw ValidationException::withMessages([
"code" => "You cannot join another dealership while you still have an active, uncanceled application.",
]);
}

// Prevent people from sending direct join URLs
$confirmation = Str::random();
$request->session()->put(self::SESSION_CONFIRMATION_KEY, $confirmation);

$application = Auth::user()->application;

return view('invitation.confirm', [
'invitingApplication' => $invitingApplication,
'application' => $application,
Expand Down
1 change: 1 addition & 0 deletions app/Http/Controllers/Applications/InviteesController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function destroy(InviteeRemovalRequest $request)
{
$invitee = Application::findOrFail($request->get('invitee_id'));
abort_if(!Carbon::parse(config('con.reg_end_date'))->isFuture() && $invitee->type !== ApplicationType::Assistant, 403, 'Only assistants may be modified once the registration period is over.');
abort_if(!Carbon::parse(config('con.assistant_end_date'))->isFuture() && $invitee->type === ApplicationType::Assistant, 403, 'Assistants may no longer be modified once the assistant registration period is over.');

$invitee->update([
"type" => ApplicationType::Dealer,
Expand Down
5 changes: 2 additions & 3 deletions app/Http/Middleware/AccessTokenValidationMiddleware.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
use Closure;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Route;
use Illuminate\Support\Facades\Redirect;
use Illuminate\Support\Facades\Session;
use League\OAuth2\Client\Provider\Exception\IdentityProviderException;
use Redirect;
use UnexpectedValueException;

class AccessTokenValidationMiddleware
Expand Down Expand Up @@ -46,7 +45,7 @@ public function handle(Request $request, Closure $next)
} catch (IdentityProviderException|UnexpectedValueException $exception) {
// If refresh fails, try to reauth user.
Auth::logout();
return Redirect::route('auth.oidc.login');
return Redirect::route('auth.login');
}

Session::put('access_token', $token);
Expand Down
9 changes: 8 additions & 1 deletion app/Http/Requests/ApplicationRequest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,14 @@ public function rules(): array

if (Carbon::parse(config('con.reg_end_date'))->isFuture()) {
return array_merge($appValidations, $profileValidations);
} else {
} elseif (Carbon::parse(config('con.assistant_end_date'))->isFuture()) {
return array_merge([
"tos" => [
"exclude_unless:applicationType,assistant",
new RequiredIf($this->routeIs('applications.store')),
],
], $profileValidations);
}else {
return $profileValidations;
}
}
Expand Down
16 changes: 9 additions & 7 deletions resources/views/application/invitees.blade.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class="btn btn-sm btn-primary dd-table-button @if ($i >= $seats['table']) border
@endfor
@if (!is_null($seats['additional']))
<button class="btn btn-sm btn-dark dd-table-button" type="button"
title="Free ({{ ucfirst($seats['additional']) }})">F{{ ucfirst(substr($seats['additional'],0,1)) }}</button>
title="Free ({{ ucfirst($seats['additional']) }})">F{{ ucfirst(substr($seats['additional'], 0, 1)) }}</button>
@endif
@for ($i = 0; $i < $seats['assistants']; $i++)
<button
Expand Down Expand Up @@ -81,9 +81,9 @@ class="btn btn-sm bg-info dd-table-button @if ($i >= max($seats['table'] - $seat
</div>
</div>
<ul class="list-group list-group-flush">
@if (
($seats['free'] > 0 || $seats['additional'] === 'dealer') &&
Carbon\Carbon::parse(config('con.reg_end_date'))->isFuture())
@if (Carbon\Carbon::parse(config('con.reg_end_date'))->isPast())
<li class="list-group-item disabled">The registration period for shares has ended.</li>
@elseif ($seats['free'] > 0 || $seats['additional'] === 'dealer')
<li class="list-group-item">
<form method="POST" action="{{ route('applications.invitees.codes') }}">
@csrf
Expand Down Expand Up @@ -130,9 +130,9 @@ class="btn btn-sm bg-info dd-table-button @if ($i >= max($seats['table'] - $seat
</div>
</div>
<ul class="list-group list-group-flush">
@if (
($seats['free'] > 0 || $seats['additional'] === 'assistant') &&
Carbon\Carbon::parse(config('con.assistant_end_date'))->isFuture())
@if (Carbon\Carbon::parse(config('con.assistant_end_date'))->isPast())
<li class="list-group-item disabled">The registration period for assistants has ended.</li>
@elseif ($seats['free'] > 0 || $seats['additional'] === 'assistant')
<li class="list-group-item">
<form method="POST" action="{{ route('applications.invitees.codes') }}">
@csrf
Expand All @@ -154,10 +154,12 @@ class="btn btn-sm bg-info dd-table-button @if ($i >= max($seats['table'] - $seat
@foreach ($assistants as $assistant)
<li class="list-group-item">
<form method="POST" action="{{ route('applications.invitees.destroy') }}">
@if (config('con.assistant_end_date')->isFuture())
@method('DELETE')
@csrf
<input type="hidden" name="invitee_id" value="{{ $assistant->id }}">
<button type="submit" class="btn btn-sm btn-danger d-inline">X</button>
@endif
{{ $assistant->getFullName() }}
</form>
</li>
Expand Down
Loading

0 comments on commit ec7c563

Please sign in to comment.