Refactor and clean up code

based on maintianer feedback on PR

- clean up validation filter on EventInstance to avoid nested conditional logic
- clean up CSS comments for clarity, point to Normalize as source
- remove an extraneous application helper
- remove unnecessary feature step definition database transaction

app/assets/stylesheets/application.css

@@ -81,6 +81,11 @@ select#submission_event_instance_id {
   margin-bottom: 1.5rem;
 }
 
+/**
+ * .fieldset redeclares some Normalize styles in a .fieldset class
+ * so they can be used to override Skeleton styles
+ */
+
 fieldset.fieldset {
   border: 1px solid #c0c0c0;
   border-radius: 3px;
@@ -89,11 +94,6 @@ fieldset.fieldset {
   padding: 0.5em 0.8em 0.75em;
 }
 
-/**
- * 1. Correct `color` not being inherited in IE 8/9/10/11.
- * 2. Remove padding so people aren't caught out if they zero out fieldsets.
- */
-
 fieldset.fieldset legend {
   border: 0; /* 1 */
   padding: .2em;

app/helpers/application_helper.rb

@@ -11,9 +11,4 @@ def markdown(text)
     markdown = Redcarpet::Markdown.new(renderer, options)
     markdown.render(text).html_safe
   end
-
-  # Returns a list of all extisting event names
-  def existing_event_names
-    Event.all.map(&:name)
-  end
 end

app/models/event_instance.rb

@@ -20,14 +20,22 @@ def name_and_year
   private
 
   def set_parent_event
-    # If both an event was selected and a new event name provided, add an error
-    if event && new_parent_event_name.present?
-      errors.add(:base, 'choose either an existing event or to create a new one')
-    # If neither an event or new event name was provided, also add an error
-    elsif event.nil? && new_parent_event_name.blank?
+    if received_bad_event_params
       errors.add(:base, 'choose either an existing event or to create a new one')
     else
-      create_event!(name: new_parent_event_name) unless event
+      event || create_event!(name: new_parent_event_name)
     end
   end
+
+  def received_bad_event_params
+    received_both_new_and_existing_events || received_neither_new_nor_existing_events
+  end
+
+  def received_both_new_and_existing_events
+    event && new_parent_event_name.present?
+  end
+
+  def received_neither_new_nor_existing_events
+    event.nil? && new_parent_event_name.blank?
+  end
 end

features/adding_event_instances.feature

@@ -13,11 +13,11 @@ Feature: Adding an Event Instance
 
   Scenario: Adding a new event instance for an existing event to a proposal
     Given there is a proposal called 'Reading Code Good'
-    And there is an event called RailsConf that does not have an instance for the year 2014
+    And there is an event called RailsConf that has an instance for the year 2014
     And I am on the 'Add an Event' page
     When I add an event instance with the following information:
       | name | RailsConf |
-      | year | 2014      | 
+      | year | 2015      | 
     And I visit the proposal page for 'Reading Code Good'
     When I add that the proposal was accepted for RailsConf in 2014
     Then I should see a record of the RailsConf 2014 acceptance

features/step_definitions/adding_event_instances.rb

@@ -8,11 +8,9 @@
   end
 end
 
-Given(/^there is an event called (\w+) that does not have an instance for the year (\d+)$/) do |conf, year|
+Given(/^there is an event called (\w+) that has an instance for the year (\d+)$/) do |conf, year|
   event = create(:event, name: conf)
-  if inst = event.instances.find_by(year: year)
-    inst.destroy
-  end
+  create(:event_instance, event: event, year: year)
 end
 
 When(/^I add an event instance with the following information:$/) do |table|