Skip to content

Commit

Permalink
Fix Dante's review comments.
Browse files Browse the repository at this point in the history
- Clean up gemspec file.
- Remove the autogenerated .rspec file.
- Fix JS defaults issue.
- Make rspec consistently use expect convention.
- #1: Removed all but one puts statement.
- Make sure rspec tests actually pass.
  • Loading branch information
navilan committed Jul 3, 2014
1 parent 7cc1ed8 commit 6fcaf78
Show file tree
Hide file tree
Showing 16 changed files with 32 additions and 450 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ spec/dummy/log/*.log
spec/dummy/tmp/
spec/dummy/.sass-cache
**/.tern-port
Gemfile.lock
135 changes: 0 additions & 135 deletions Gemfile.lock

This file was deleted.

17 changes: 0 additions & 17 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,10 @@ begin
rescue LoadError
puts 'You must `gem install bundler` and `bundle install` to run rake tasks'
end
begin
require 'rdoc/task'
rescue LoadError
require 'rdoc/rdoc'
require 'rake/rdoctask'
RDoc::Task = Rake::RDocTask
end

RDoc::Task.new(:rdoc) do |rdoc|
rdoc.rdoc_dir = 'rdoc'
rdoc.title = 'Unconfirm'
rdoc.options << '--line-numbers'
rdoc.rdoc_files.include('README.rdoc')
rdoc.rdoc_files.include('lib/**/*.rb')
end

APP_RAKEFILE = File.expand_path("../spec/dummy/Rakefile", __FILE__)
load 'rails/tasks/engine.rake'



Bundler::GemHelper.install_tasks

require 'rspec/core'
Expand Down
5 changes: 3 additions & 2 deletions app/assets/javascripts/unconfirm/jquery.unconfirm.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
var Unconfirm = root.Unconfirm = function unconfirmRoot(options) {
var self = this;
if (typeof options === 'string') {
options = {};
options.actionSelector = options;
options = {
actionSelector: options
};
}
this.options = $.extend({}, Unconfirm.DefaultOptions, options);
this.$selector = $(this.options.actionSelector);
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/unconfirm/user_settings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ def update
settings = UserSettings.for(current_user)
respond_to do |format|
if not settings.nil? and settings.update_attributes(params[:settings])
puts "User settings updated. Updating session now"
update_unconfirm_user_settings! settings
format.json {
render json: {:success => true,
:message => 'Settings were successfully updated.'} }
else
puts "update failed"
puts settings.errors.messages.inspect
Rails.logger.info(settings.errors.messages.inspect)
format.json {
render json: {:success => false,
:message => 'Unable to update settings.'} }
:errors => settings.try(:errors),
:message => 'Unable to update settings.'},
status: :unprocessable_entity
}
end
end
end
Expand Down
4 changes: 0 additions & 4 deletions app/helpers/unconfirm/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,11 @@ def update_unconfirm_user_settings!(settings_model)

def update_from_model(settings_model=nil)
settings_model ||= UserSettings.for(current_user)
puts "Model initialized"
if not settings_model.nil?
settings = settings_model.truthy_settings
end
puts "Settings initialized"
settings ||= {}
puts settings
session[:unconfirm_user_settings] = settings
puts "Session updated"
end

end
Expand Down
4 changes: 3 additions & 1 deletion app/helpers/unconfirm/view_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ def data_with_unconfirm(setting, data=nil)
res["unconfirm_#{field.to_s}"] = details[field]
end
end
res["unconfirm_setting"] = setting
if details.length > 0
res["unconfirm_setting"] = setting
end
res.merge!(data || {})
end

Expand Down
1 change: 0 additions & 1 deletion app/models/unconfirm/user_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ def self.setting_details_for (setting)
begin
add_settings HashWithIndifferentAccess.new(YAML.load(File.open(f)))
rescue Exception => ex
puts ex
Rails.logger.warn "Failed to import settings #{f}"
Rails.logger.warn ex
end
Expand Down
28 changes: 14 additions & 14 deletions spec/app/controllers/unconfirm/user_settings_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,26 @@ module Unconfirm
end

def ensure_close_form_status(status)
get :index, :format => :json
get :show, :id => 0, :format => :json
body = JSON.parse(response.body)
assert body.include? "success"
assert body["success"]
assert body.include? "settings"
expect(body).to include "success"
expect(body["success"]).to be true
expect(body).to include "settings"
if status
assert body["settings"].include? "skip_user_confirmation_dialog_on_close_form"
assert body["settings"]["skip_user_confirmation_dialog_on_close_form"]
expect(body["settings"]).to include "skip_user_confirmation_dialog_on_close_form"
expect(body["settings"]["skip_user_confirmation_dialog_on_close_form"]).to be true
else
refute body["settings"].include? "skip_user_confirmation_dialog_on_close_form"
expect(body["settings"]).not_to include "skip_user_confirmation_dialog_on_close_form"
end
end

it 'should not fetch settings for unauthenticated user' do
get :index, :format => :json
get :show, :id => 0, :format => :json
body = JSON.parse(response.body)
assert body.include? "success"
assert body["success"]
assert body.include? "settings"
refute body["settings"].include? "skip_user_confirmation_dialog_on_close_form"
expect(body).to include "success"
expect(body["success"]).to be true
expect(body).to include "settings"
expect(body["settings"]).not_to include "skip_user_confirmation_dialog_on_close_form"
end

it 'should fetch settings for authenticated user' do
Expand All @@ -39,8 +39,8 @@ def ensure_close_form_status(status)
sign_in @user
put :update, :id => @user.id, :settings=> {:skip_user_confirmation_dialog_on_close_form => true}, :format => :json
body = JSON.parse(response.body)
assert body.include? "success"
assert body["success"]
expect(body).to include "success"
expect(body["success"]).to be true
ensure_close_form_status true
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/app/helpers/view_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Unconfirm

def expect_dict_subset_of(d1, d2)
d1.each do |k, v|
assert d2.include? k
expect(d2).to include k
expect(d2[k]).to eq(v)
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/app/models/unconfirm/user_settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ module Unconfirm

it 'retrieves default attribute values based on the setting providers' do
sets = UserSettings.for @user
refute sets.skip_user_confirmation_dialog_on_close_form
expect(sets.skip_user_confirmation_dialog_on_close_form).not_to be true
end

it 'sets attribute values' do
sets = UserSettings.for @user
sets.update_attributes({skip_user_confirmation_dialog_on_close_form: true})
newsets = UserSettings.for @user
assert newsets.skip_user_confirmation_dialog_on_close_form
expect(newsets.skip_user_confirmation_dialog_on_close_form).to be true
end

it 'gets descriptive values' do
Expand Down
3 changes: 0 additions & 3 deletions spec/dummy/.rspec

This file was deleted.

1 change: 1 addition & 0 deletions spec/dummy/README
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This is a dummy app used to test the unconfirm gem.
Loading

0 comments on commit 6fcaf78

Please sign in to comment.