Skip to content
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.

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

Fix/same time shift protection 2 #175

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
84de258
[feature] basic alert logic when signing up for concurrent shift
demogorgonzola May 31, 2019
9ac1f4e
[feature] warnings present when admin assinging overlapping shifts
demogorgonzola May 31, 2019
dcdf416
[refactor] split formatting of warning page into partial
demogorgonzola Jun 2, 2019
6407c59
[refactor] took warning logic out of assignments
demogorgonzola Jun 3, 2019
ee6f963
[scrap] removed experimental feature warning admins of overlapping slots
demogorgonzola Jun 3, 2019
14211ec
[build] introduced working test environment
demogorgonzola Jun 11, 2019
1bf2ceb
[build] more test testing
demogorgonzola Jun 13, 2019
35183a7
Revert "[build] more test testing"
demogorgonzola Jun 14, 2019
094749e
Revert "[build] introduced working test environment"
demogorgonzola Jun 14, 2019
446ece5
Merge branch 'master' into feature/same-time-shift-protection
demogorgonzola Jun 25, 2019
d433e39
[enhance] warning on viewing concurrent slot
demogorgonzola Jun 25, 2019
b7709ff
Merge branch 'master' into feature/same-time-shift-protection
demogorgonzola Jul 16, 2019
ceb3e0c
[test] writing shift protection tests
demogorgonzola Jul 16, 2019
87344ae
[wip] using reports controller to poll if slot is concurrent now
demogorgonzola Jul 16, 2019
0cbcad2
[wip] revert to previous solution
demogorgonzola Aug 3, 2019
f1653bc
Merge branch 'master' into fix/same-time-shift-protection-2
demogorgonzola Aug 3, 2019
a11bee4
[feature] admin is now warned on assigning user with concurrent slot
demogorgonzola Aug 4, 2019
3bf7c21
[cleanup] taking warning out of view load
demogorgonzola Aug 4, 2019
52dbd4d
[cleanup] corrected endpoint test
demogorgonzola Aug 4, 2019
36f4f63
[fix] warning partial html fix and factory change
demogorgonzola Aug 26, 2019
b049eff
Merge branch 'master' into fix/same-time-shift-protection-2
Meleeman01 Dec 4, 2020
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
2 changes: 1 addition & 1 deletion laravel/app/Http/Controllers/ReportController.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function searchUsers(Request $request)

return json_encode($users);
}

public function getDepartments(Request $request)
{
$id = $request->get('event');
Expand Down
62 changes: 58 additions & 4 deletions laravel/app/Http/Controllers/SlotController.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public function view(Request $request, Slot $slot)
$request->session()->flash('error', "You must enter your name before you can sign up for shifts.");
return redirect('/profile/data/edit');
}

return view('pages/slot/view', compact('slot'));
}

Expand Down Expand Up @@ -101,6 +101,12 @@ public function take(SlotRequest $request, Slot $slot)
// Has somebody else already taken this slot?
if(is_null($slot->user))
{
$concurrent_slot_warning = $this->warnIfConcurrentSlotForUserExists($request, $slot, Auth::user());
if($concurrent_slot_warning)
{
return back();
}

$slot->user_id = Auth::user()->id;
$slot->save();

Expand Down Expand Up @@ -167,7 +173,7 @@ public function adminRelease(Request $request, Slot $slot)
{
if(!is_null($slot->user))
{
$username = Helpers::displayName($slot->user);
$user_name = Helpers::displayName($slot->user);

$slot->user_id = null;
$slot->save();
Expand All @@ -185,15 +191,63 @@ public function adminAssign(Request $request, Slot $slot)
{
$user = User::findorFail($request->get('user'));

$user_name = Helpers::displayName($user, false);

if(is_null($slot->user))
{
$username = Helpers::displayName($user);
$concurrent_slot_warning = $this->warnIfConcurrentSlotForUserExists($request, $slot, $user, true);
if($concurrent_slot_warning)
{
return back();
}

$slot->user_id=$user->id;
$slot->save();
event(new SlotChanged($slot, ['status' => 'taken', 'admin_assigned' => true, 'name' => $user->name, 'email' => $user->email]));
$request->session()->flash('success', 'You added '.$username.' to this shift');
$request->session()->flash('success', 'You added '.$user_name.' to this shift');
}
return redirect('/event/'.$slot->event->id);
}

/**
* Give a warning, if one hasn't already been delivered, to the client if a
* the user is trying to sign up or an admin is attempting to sign a user
* up for a slot one that overlaps with another slot the user is assigned
* to.
*
* @param Request $request the client request
* @param Slot $slot a currently unoccupied slot
* @param User $user the user that will fill the slot
* @param boolean $admin whether or not this an admin request
* @return Response a warning response or null
*/
private function warnIfConcurrentSlotForUserExists(Request $request, Slot $slot, User $user, $admin=false)
{
//search for all user occupied slots that are concurrent with the given one
$concurrent_slot = Slot::where('user_id', $user->id)
->where('start_date', $slot->start_date)
->where('start_time', '<', $slot->end_time)
->where('end_time', '>', $slot->start_time)
->first();

//check if an overlapping slot for the user exists
if($concurrent_slot)
{
//check if the client has already been warned that the user has a concurrent slot
if(intval($request->input('concurrent-slot-warning-user-id')) !== $user->id)
{
$request->session()->flash('warning', [
'layout' => 'concurrent-slot',
'user' => $user,
'slot' => $slot,
'admin' => $admin,
'user_id' => $user->id,
'user_name' => Helpers::displayName($user, false),
'concurrent_slot_id' => $concurrent_slot->id
]);
return true;
}
}
return false;
}
}
24 changes: 24 additions & 0 deletions laravel/database/factories/ScheduleFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

use App\Models\Department;
use App\Models\Event;
use App\Models\EventRole;
use App\Models\Role;
use App\Models\Schedule;
use App\Models\Shift;
use Carbon\Carbon;
Expand Down Expand Up @@ -54,6 +56,10 @@
return $duration->format('H:i:s');
},
'volunteers' => $faker->numberBetween($volunteer_min, $volunteer_max),
'dates' => function($schedule)
{
return json_encode([$schedule['start_date'], $schedule['end_date']]);
},
'department_id' => function ($schedule)
{
$event_id = function () use ($schedule)
Expand All @@ -77,3 +83,21 @@
},
];
});

$factory->afterCreating(Schedule::class, function(Schedule $schedule, Faker $faker)
{
//find the admin role
$volunteer_role = Role::where('name', 'volunteer')->first();
//if there is no admin role, create it
if (!$volunteer_role)
{
$volunteer_role = factory(Role::class)->create([
'name' => 'volunteer',
]);
}

$schedule->roles()->save(factory(EventRole::class)->make([
'role_id' => $volunteer_role->id,
'foreign_id' => $schedule->id,
]));
});
20 changes: 19 additions & 1 deletion laravel/database/factories/UserFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,25 @@
'name' => $faker->unique()->userName,
'email' => $faker->unique()->safeEmail,
'password' => bcrypt($faker->password),
];
];
});

$factory->afterCreating(User::class, function (User $user, Faker $faker)
{
//find the volunteer role
$volunteer_role = Role::where('name', 'volunteer')->first();
//if there is no volunteer role, create it
if (!$volunteer_role)
{
$volunteer_role = factory(Role::class)->create([
'name' => 'volunteer',
]);
}

$user->roles()->save(factory(UserRole::class)->make([
'role_id' => $volunteer_role->id,
'user_id' => $user->id,
]));
});

$factory->afterCreatingState(User::class, 'admin', function (User $user, Faker $faker)
Expand Down
10 changes: 10 additions & 0 deletions laravel/resources/views/app.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@
</div>
@endif

@if(Session::has('warning'))
<div class="general-alert alert alert-warning" role="alert">
@if(isset(Session::get('warning')['layout']))
<b>Warning!</b> @include('partials.warning.'.Session::get('warning')['layout'], Session::get('warning'))
@else
<b>Warning!</b> {{ Session::get('warning') }}
@endif
</div>
@endif

@if(Session::has('error'))
<div class="general-alert alert alert-danger" role="alert">
<b>Error!</b> {{ Session::get('error') }}
Expand Down
1 change: 1 addition & 0 deletions laravel/resources/views/pages/slot/view.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@
Please arrive at least 15 minutes ahead of time to be briefed by the previous shift team and answer any questions you have.
</p>

<input type="hidden" name="concurrent-slot-warning-user-id" value="{{ Session::get('warning')['user_id'] }}">
<button type="submit" class="btn btn-success">Take Shift</button>
@endif

Expand Down
11 changes: 11 additions & 0 deletions laravel/resources/views/partials/warning/concurrent-slot.blade.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<span>
@if(isset($admin) && $admin === true)
{{ $user_name }} is currently signed up for another
<a href=" {{ env('SITE_URL') }} /slot/{{ $concurrent_slot_id }}/view">overlapping shift</a>.
Copy link
Contributor

Choose a reason for hiding this comment

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

this space in the href url is causing problems, eliminate the space so we can actually see the page. infact all of the hrefs in this portion of the code

Copy link
Member

Choose a reason for hiding this comment

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

@Meleeman01 Can you fix this issue?

Are you sure you want to sign them up for this shift?
@else
You are currently signed up for another
<a href="{{ env('SITE_URL') }}/slot/{{ $concurrent_slot_id }}/view">overlapping shift</a>.
Are you sure you want to sign up for this shift?
@endif
</span>
66 changes: 66 additions & 0 deletions laravel/tests/Feature/SlotControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,79 @@
namespace Tests\Feature;

use App\Models\Slot;
use App\Models\UserData;
use App\Models\User;
use Tests\TestCase;
use Illuminate\Foundation\Testing\WithFaker;
use Illuminate\Foundation\Testing\RefreshDatabase;

class SlotControllerTest extends TestCase
{
use RefreshDatabase;

/**
* @test
*
* @return void
*/
public function same_time_shift_warned_on_take()
{
// Given
$take_slot = factory(Slot::class)->create();
$view_slot = factory(Slot::class)->create([
'start_date' => $take_slot->start_date,
'start_time' => $take_slot->start_time,
'end_time' => $take_slot->end_time,
]);
$user = factory(User::class)->create();
$user->data()->save(factory(UserData::class)->make());

$take_slot->user_id = $user->id;
$take_slot->save();

$this->actingAs($user);

// When
$response = $this->followingRedirects()->post("/slot/{$view_slot->id}/take");

// Then
$response->assertSee("You are currently signed up for another")
->assertSee("overlapping shift");
}

/**
* @test
*
* @return void
*/
public function same_time_shift_warned_on_admin_assign()
{
// Given
$take_slot = factory(Slot::class)->create();
$view_slot = factory(Slot::class)->create([
'start_date' => $take_slot->start_date,
'start_time' => $take_slot->start_time,
'end_time' => $take_slot->end_time,
]);
$user = factory(UserData::class)->create()->user;
$admin = factory(UserData::class)->create([
'user_id' => factory(User::class)->states('admin')->create()->id,
])->user;

$this->actingAs($admin);

// When
$take_slot->user_id = $user->id;
$take_slot->save();

$response = $this->followingRedirects()->post("/slot/{$view_slot->id}/adminAssign", [
'user' => $user->id,
]);

// Then
$response->assertSee("Are you sure you want to sign them up for this shift?");
}

/**
* @test
* @return void
Expand Down
2 changes: 1 addition & 1 deletion laravel/tests/Unit/ExampleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
class ExampleTest extends TestCase
{
//Use this to clear the database at the beginning of the test class
use RefreshDatabase;
// use RefreshDatabase;

/**
* Make sure to use either "test" in the method name
Expand Down