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

added specs and validations for Role model #5792

Merged
merged 1 commit into from
Mar 20, 2015

Conversation

MothOnMars
Copy link
Contributor

  • added shoulda-matchers gem for one-line specs
  • added FactoryGirl syntax methods

Partial fix for #4020

I also added a few validations that seemed to make sense for roles, and tweaked the Role.is_admin? method to return a boolean. Hopefully CI will catch any issues that these changes may cause, but please let me know if I should back out any of those changes to the model itself.

class Role < ActiveRecord::Base
belongs_to :person

validates :person, presence: true
validates :name, uniqueness: {scope: :person_id}
validates :name, inclusion: { in: %w(admin spotlight) }

Choose a reason for hiding this comment

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

Space inside { detected.
Space inside } detected.

@jhass
Copy link
Member

jhass commented Mar 17, 2015

Mh, I'm not sure I like shoulda-matchers, it feels inconsistent with the expect syntax.

@MothOnMars
Copy link
Contributor Author

@jhass, the one-line syntax used by shoulda-matchers is supported by RSpec, even when using the expect syntax: https://github.com/rspec/rspec-expectations/blob/master/Should.md#one-liners

I feel that the inconsistency is a small trade-off for being able to write a one-liner instead of multiple lines. It's a solid Thoughtbot gem with great support & documentation. It's probably saved me hundreds of hours and thousands of lines of spec code over the years. Check it out...let me know what you think...and in the meantime, I'll start making Hound happier... 😄

@jhass
Copy link
Member

jhass commented Mar 17, 2015

Mh, okay deal, we can try it for a while.

@@ -10,6 +10,7 @@
require 'webmock/rspec'
require 'factory_girl'
require 'sidekiq/testing'
require 'shoulda/matchers'

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@@ -25,39 +26,39 @@ def set_up_friends
end

def alice
@alice ||= User.where(:username => 'alice').first
@alice ||= User.where(:username => "alice").first

Choose a reason for hiding this comment

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

Use the new Ruby 1.9 hash syntax.

- added shoulda-matchers gem for one-line specs
- added FactoryGirl syntax methods
@jhass
Copy link
Member

jhass commented Mar 20, 2015

Thank you!

@jhass jhass merged commit d4f1a5d into diaspora:develop Mar 20, 2015
jhass added a commit that referenced this pull request Mar 20, 2015
added specs and validations for Role model
@jhass jhass added this to the next-major milestone Mar 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants