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

add helper & fix logic for users show #957

Closed
wants to merge 1 commit into from

Conversation

chrislabarge
Copy link

User UI improvements (fixes?) #950

Added some user feature specs to show desired logic.


visit user_path(:en, user.id)

expect(page).to have_content('This user hasn\'t created any gardens yet.')

Choose a reason for hiding this comment

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

Line is too long. [82/80]

@@ -26,6 +26,12 @@ class Garden
accepts_nested_attributes_for :pictures
scope :is_public, -> { where(is_private: true) }

def is_user_default?
default_name = I18n::t('registrations.your_first_garden')

Choose a reason for hiding this comment

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

Do not use :: for method calls.

@@ -26,6 +26,12 @@ class Garden
accepts_nested_attributes_for :pictures
scope :is_public, -> { where(is_private: true) }

def is_user_default?

Choose a reason for hiding this comment

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

Rename is_user_default? to user_default?.

end

def authorized_for?(user)
user == current_user && current_user.confirmed?

Choose a reason for hiding this comment

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

Use 2 (not 4) spaces for indentation.

end

def has_non_default_garden?(user)
user.gardens.each { |garden| return true unless garden.is_user_default?}

Choose a reason for hiding this comment

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

Use 2 (not 4) spaces for indentation.
Space missing inside }.

@@ -0,0 +1,15 @@
module UsersHelper
def list_gardens_on_show?
has_non_default_garden?(@user) || authorized_for?(@user)

Choose a reason for hiding this comment

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

Use 2 (not 4) spaces for indentation.

@@ -0,0 +1,15 @@
module UsersHelper

Choose a reason for hiding this comment

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

Missing magic comment # frozen_string_literal: true.

@chrislabarge
Copy link
Author

I don't believe any of the errors on Travis are a result from my code, as they are all showing up on my local master branch as well. Please correct me if I am wrong. :)

@Br3nda
Copy link
Member

Br3nda commented Apr 1, 2018

@chrislabarge can you merge in the latest gem file fixes.?

end

def has_non_default_garden?(user)
user.gardens.each { |garden| return true unless garden.user_default?}
Copy link

Choose a reason for hiding this comment

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

that line can be this way? user.gardens.none?(&:user_default?)

@RickCarlino
Copy link
Contributor

Closing this PR due to inactivity, although it appears that RSpec is working again after upgrades. Sorry for the radio silence on this one- I'm hoping to take a more active role in getting things merged in 2019. Please let me know if there is anything I can do to help- I realize it has been very quiet these days.

@RickCarlino RickCarlino closed this Mar 1, 2019
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.

5 participants