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

Feature create event #24

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open

Feature create event #24

wants to merge 32 commits into from

Conversation

david-vega
Copy link
Contributor

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
Contributor 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
Contributor 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.

5 participants