Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

Fixed new events flow, added sponsored field, removed suggestions link from navbar #111

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

Conversation

william-morrissy
Copy link

Closes #28: made the New Event page mimic the New Organization page
Closes #109: Added a boolean 'sponsored' to the database and to the event admin file
Closes #91
Removed suggestions link from navbar

@alexwgraves alexwgraves self-requested a review March 29, 2018 00:13
Copy link
Member

@alexwgraves alexwgraves left a comment

Choose a reason for hiding this comment

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

The CSS looks good!

There are some bugs/issues that need to be resolved though:

  • Flow is now messed up in the other direction – if you click Add Event from the calendar view, it takes you to a separate page. I think you should be able to fix this by checking which page the user is currently on, and changing what it displays based on that. Basically, if the user is coming from /events, it should take them to an entirely separate page. Otherwise, it should display it in the calendar sidebar.
  • When creating an event from the /events page, successful creation of the event takes you back to /events – it should go to the newly-created event's page. (It looks like it used to do this and you changed it – please change it back to this format.html { redirect_to @event, notice: "#{@event.title} was successfully created." } in events_controller.rb. Sorry if I told you something different before!)
  • It also displays two success banners (the notice is rendering twice?)

screen shot 2018-03-28 at 8 07 31 pm

- Trying to edit an event in the admin view throws an error:

screen shot 2018-03-28 at 8 17 18 pm

@@ -55,7 +54,7 @@ def create

respond_to do |format|
if @event.save
format.html { redirect_to @event, notice: "#{@event.title} was successfully created." }
format.html { redirect_to events_url, notice: "#{@event.title} was successfully created." }
Copy link
Member

Choose a reason for hiding this comment

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

Change this back – they should be taken to the new event when they create it.

@@ -48,6 +49,7 @@
f.input :recurrence_amt
f.input :featured
f.input :featured_snippet
f.input :sponsored, :toggle
Copy link
Member

Choose a reason for hiding this comment

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

I think fixing the error for this might be as simple as removing , :toggle from this. If you look at featured, which is also a boolean, it doesn't have the toggle param.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants