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.

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

[WIP] - Feature absences spec #82

Closed
wants to merge 9 commits into from
Closed

[WIP] - Feature absences spec #82

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d1a7b35
Change test database from sqlite3 to psql
Laszlo-JFLMTCO Feb 3, 2017
d8bdd38
Create volunteer_schedule_absence_spec file, comment out content of a…
Laszlo-JFLMTCO Feb 4, 2017
f583691
Add launchy gem to support save_and_open_page for debugging Capybara …
Laszlo-JFLMTCO Feb 4, 2017
6d9b776
Add assigned=true to volunteer_with_region factory otherwise login_he…
Laszlo-JFLMTCO Feb 4, 2017
68205c0
Clean up extra indentation within volunteer_schedule_absence_spec
Laszlo-JFLMTCO Feb 7, 2017
1c197b4
Bring back database.yml.dist
Laszlo-JFLMTCO Feb 7, 2017
51c2d31
Remove unnecessary coments from absences_controller_spec while focusi…
Laszlo-JFLMTCO Feb 7, 2017
408f7b0
Fix incorrect indentation of volunteer_schedule_absence_spec
Laszlo-JFLMTCO Feb 7, 2017
8f9d8a3
Add DS_Store to gitignore. Update volunteer absence spec with happy p…
Laszlo-JFLMTCO Apr 17, 2017
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 Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ group :test do
gem 'rspec-rails', '~> 3.5'
gem 'factory_girl_rails'
gem 'capybara'
gem 'launchy'
gem 'poltergeist', '~> 1.12'
gem 'rack-test'
gem 'test-unit', '~> 3.0'
Expand Down
8 changes: 7 additions & 1 deletion Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ GEM
json_pure (1.8.3)
jstz-rails (1.0.4.1)
railties (~> 3.1)
launchy (2.4.3)
addressable (~> 2.3)
less (2.6.0)
commonjs (~> 0.2.7)
less-rails (2.6.0)
Expand Down Expand Up @@ -359,6 +361,7 @@ DEPENDENCIES
jquery-datatables-rails!
jquery-rails (= 2.1.4)
json
launchy
newrelic_rpm
paperclip!
pg
Expand Down Expand Up @@ -386,5 +389,8 @@ DEPENDENCIES
uglifier (>= 1.0.3)
yaml_db

RUBY VERSION
ruby 2.1.5p273

BUNDLED WITH
1.13.6
1.13.7
2 changes: 1 addition & 1 deletion config/database.yml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ production:
host: localhost
pool: 5
timeout: 5000

Copy link
Member

Choose a reason for hiding this comment

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

Commit message says:

Change test database from sqlite3 to psql

I would have expected this commit to change the test section of this file, moving from sqlite3 to postgresql, but leave the rest of the file alone.

I think it would be good to keep this file around for now, since it's used as kind of a template for people getting the project running locally. The readme has a step: "Next, copy /config/database.yml.dist to /config/database.yml and make any necessary changes".

Copy link
Author

Choose a reason for hiding this comment

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

@ericfreese Thank you for the comment. You are right. I just realized that database.yml was listed in .gitignore. I will have this corrected.

3 changes: 2 additions & 1 deletion spec/controllers/absences_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
require 'rails_helper'

RSpec.describe AbsencesController do
let(:volunteer) { create :volunteer_with_assignment }
let(:volunteer_with_assignment) { create :volunteer_with_assignment }

it_behaves_like 'an authenticated indexable resource'

end
11 changes: 10 additions & 1 deletion spec/factories/volunteers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@
waiver_signed true
waiver_signed_at Time.zone.now

factory :volunteer_with_region do
after(:create) do |v|
region = create(:region)
v.regions << region
v.assigned = true
v.save
end
end
Copy link
Member

Choose a reason for hiding this comment

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

I believe this added factory actually ends up doing the same thing as the :volunteer_with_assignment factory. Volunteers are assigned to regions through the Assignment model. When the assignment is created on line 21, a new region is also created by this line in the Assignments factory.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @ericfreese
For this particular test case this is being used to evaluate functionaliry when a volunteer without any assignment is trying to schedule an absence. It means not having an assignment is a precondition.
Since other tests are relying on the already existing, previously created factories, I didn't want to rename any previously created volunteer factories.
First I tried to rely on volunteer factory as it does not have assignments either, but that no regions assigned and its login fails with the Your account has not been assigned to a region yet. A region administrator must do this before you can sign in. error message.
This new factory is being called under let(:volunteer_without_assignment) within volunteer_schedule_absence_spec.rb It is already being called as volunteer_without_assignment, hoping that would clear up any possible confusion around the purpose of this factory.
Is there another factory I might have missed that already has volunteer with regions assigned without assignments?

Copy link
Member

@ericfreese ericfreese Mar 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like the new factory (volunteer_with_region) actually does end up producing a Volunteer with an Assignment.

If we put a binding.pry in the test and poke around a bit, we can see there is in fact an associated Assignment:

> volunteer_without_assignment.assignments.count
=> 1

The region = create(:region); v.regions << region lines in the volunteer_with_region factory must create an Assignment to link the Region to the Volunteer. Because the regions association on Volunteer is a has_many :through, there is no way to have a Volunteer associated to a Region without an Assignment to join them, so those lines effectively create both a Region and an Assignment linked to that Volunteer.

This actually is the same outcome as the volunteer_with_assignment factory. The a = create(:assignment,volunteer:v); v.assignments << a lines will create both a Region and an Assignment and associate them with the Volunteer. This is because the assignment factory is set up to build an associated Region by default.

This explains why you're not seeing the login failure error for this user. If the user truly had no assignments, they would never be able to log in.

For this particular test case this is being used to evaluate functionaliry when a volunteer without any assignment is trying to schedule an absence.

Because it is not possible for a volunteer with no assignments to log into the system to get to the "Schedule an Absence" form, I think it would be best to test other cases. I think it would be best to start with the happy path: A volunteer with a shift successfully scheduling an absence for that shift.


factory :volunteer_with_assignment do
after(:create) do |v|
a = create(:assignment,volunteer:v)
Expand All @@ -15,7 +24,7 @@
v.save
end
end

trait :not_waived do
waiver_signed false
waiver_signed_at nil
Expand Down
29 changes: 29 additions & 0 deletions spec/features/volunteer_schedule_absence_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
require 'rails_helper'

RSpec.describe 'Scheduling an Absence' do

let(:volunteer_with_assignment) { create :volunteer_with_assignment }
let(:volunteer_without_assignment) { create :volunteer_with_region }
let(:admin) { create :volunteer_with_assignment, admin: true }

context 'non-admin user' do
context 'WITHOUT assignment' do
it 'cannot schedule' do
login volunteer_without_assignment

within '.navbar' do
click_on 'Schedule An Absence'
end

expect(current_path).to eq(new_absence_path)

click_on 'Save changes'

expect(current_path).to eq(absences_path)
within('.alert') do
expect(page).to have_content('No shifts of yours was found in that timeframe')
end
Copy link
Member

Choose a reason for hiding this comment

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

Indentation still looks a little off for lines 14-23 and line 25.

end
end
end
end
Copy link
Member

Choose a reason for hiding this comment

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

The indentation is a little funky in this file. It looks like lines 9-28 are indented by two extra spaces and lines 14-23 and line 25 are indented by an extra space.