Skip to content

Conversation

@david-vega
Copy link

No description provided.

@@ -0,0 +1,6 @@
Fabricator :user do
email { sequence { |n| "user_#{n}@hackdmx.com" } }
Copy link
Contributor

Choose a reason for hiding this comment

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

Always use *@example.com dummy email addresses. That's the first step to avoid sending emails to actual email addresses when testing.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure because in that case it will send emails to *@example.com, and that domain actually exist and we don't control it, in my opinion it would be a better idea to use a domain that we do control.

Copy link
Contributor

Choose a reason for hiding this comment

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

grumpy cat

Seriously, example.com is a special and reserved domain, see this

Copy link
Owner

Choose a reason for hiding this comment

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


def update

find_event
Copy link
Contributor

Choose a reason for hiding this comment

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

Y U NO use a before_action ?

Ignacio Galindo and others added 3 commits October 3, 2013 01:50
redirect_to events_path
else
flash[:error] = 'Something went wrong'
render new_event_path
Copy link
Contributor

Choose a reason for hiding this comment

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

At line 37 you use render action: 'edit' and here you use a path helper. Isn't this somewhat inconsistent?

Copy link
Author

Choose a reason for hiding this comment

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

good catch

@vicmaster
Copy link
Owner

👍

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.

6 participants