Skip to content

Commit

Permalink
#449 Switch venue / presentation type to a dedicated attribute
Browse files Browse the repository at this point in the history
  • Loading branch information
srogers committed Aug 3, 2020
1 parent 4d14fc9 commit 81ee073
Show file tree
Hide file tree
Showing 13 changed files with 44 additions and 24 deletions.
20 changes: 9 additions & 11 deletions app/assets/javascripts/conferences.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,20 @@

$ ->
# This is used in Conference and Preseentation to manage the location fields
set_location_fields = (location) ->
if location == 'Physical'
set_location_fields = (location_type) ->
if location_type == 'Physical'
$('.location-conditional-fields').slideDown()
venue = $('#conference_venue').val()
# When switching from Multiple or Virtual back to phsycial, these need to be blanked, so we're not counting on the
# user to do it - the old location type will stick if the user neglects to change it.
if venue == 'Multiple' || venue == 'Virtual'
if location_type == 'Multiple' || location_type == 'Virtual'
$('#conference_venue').val('')
$('#conference_city').val('')
if location == 'Virtual'
if location_type == 'Virtual'
$('.location-conditional-fields').slideUp()
$('#conference_venue').val('Virtual')
# the model handles setting conference city to Virtual
if location == 'Multiple'
if location_type == 'Multiple'
$('.location-conditional-fields').slideUp()
$('#conference_venue').val('Multiple')
# the model handles setting conference city to Multiple

# This is used by on-change events to set the conference name field to reflect the chosen organizer and start date,
Expand All @@ -37,15 +35,15 @@ $ ->
$("#conference_organizer_id").change ->
set_conference_name()

set_location_fields($('#conference_location').val())
set_location_fields($('#presentation_location').val())
set_location_fields($('#conference_location_type').val())
set_location_fields($('#presentation_location_type').val())

# Use the location selector to set the venue and show/hide appropriate fields
$('#conference_location').change ->
$('#conference_location_type').change ->
set_location_fields($(this).val())

# Use the location selector to set the venue and show/hide appropriate fields
$('#presentation_location').change ->
$('#presentation_location_type').change ->
set_location_fields($(this).val())

# Make Start/End date a little less tedious by setting the end dates to the start dates as selected.
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/shared_queries.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def base_query(query)

# Used to find the city names for multi-venue events, which live at the presentation level.
def multiples_query(query)
query.add :required, "conferences.venue = ?", Conference::MULTIPLE
query.add :required, "conferences.location_type = ?", Conference::MULTIPLE

return query
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/events_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def get_organizer_selections
def conference_params
params.require(:conference).permit(
:name, :event_type, :description, :organizer_id, :registration_url, :start_date, :end_date,
:venue, :venue_url, :city, :state, :country, :completed, :editors_notes
:location_type, :venue, :venue_url, :city, :state, :country, :completed, :editors_notes
)
end
end
2 changes: 1 addition & 1 deletion app/controllers/presentations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ def get_presentation

def presentation_params
params.require(:presentation).permit(:conference_id, :name, :description, :parts, :tag_list, :handout, :remove_handout,
:editors_notes, :date, :venue, :venue_url, :city, :state, :country)
:editors_notes, :date, :location_type, :venue, :venue_url, :city, :state, :country)
end

def presentation_speaker_params
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ def articlize(word)
# Must be TRUE for PDFs, which don't support entity codes. non-breaking looks better in HTML lists
def location(thing, options={})
return Presentation::UNSPECIFIED unless thing.present? && thing.respond_to?(:venue) && (thing.venue.present? || thing.location.present?)
return thing.venue if [Conference::VIRTUAL, Conference::MULTIPLE].include? thing.venue
return thing.location_type if [Conference::VIRTUAL, Conference::MULTIPLE].include? thing.location_type

elements = []
if thing.venue.blank?
Expand Down
12 changes: 8 additions & 4 deletions app/models/concerns/locations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,21 @@ module Locations
MULTIPLE = 'Multiple'.freeze
LOCATION_TYPES = [PHYSICAL, VIRTUAL, MULTIPLE].freeze

def physical?
location_type == PHYSICAL
end

def virtual?
venue == VIRTUAL
location_type == VIRTUAL
end

def multi_venue?
venue == MULTIPLE
location_type == MULTIPLE
end

def us_state_existence
return true unless country == 'US' # ignore foreign states because we can't validate them
return true if [VIRTUAL, MULTIPLE].include? venue # allow blank in this case
return true if [VIRTUAL, MULTIPLE].include? location_type # allow blank in this case
state.upcase!
return true if city.blank? # if city is skipped, state can be too - needed for events with sketchy info
errors.add(:state, 'Use the standard two-letter postal abbreviation for US states.') unless States::STATES.map{|s| s[0]}.include?(state)
Expand All @@ -45,7 +49,7 @@ def country_name
# Must be TRUE for PDFs, which don't support entity codes. non-breaking looks better in HTML lists
# :include_us - by default, country is omitted unless it is non-US
def location(options={})
return venue if [VIRTUAL, MULTIPLE].include? venue
return location_type if [VIRTUAL, MULTIPLE].include? location_type
include_country = options[:include_us] || country != 'US'
elements = [city.presence, state.presence]
elements << [country_name.presence] if options[:country_format].to_s == 'full' && include_country
Expand Down
1 change: 1 addition & 0 deletions app/models/presentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ def inherit_conference_defaults
self.state = conference.state if state.blank?
self.country = conference.country if country.blank?
self.venue = conference.venue if venue.blank?
self.location_type = conference.location_type if location_type.blank?
# the caller must save
end

Expand Down
2 changes: 1 addition & 1 deletion app/views/events/_form_fields.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
= f.trix_editor :description
.small.mb-3 Historic or organizational details specific to this event (e.g., attendance)
= f.input :registration_url, input_html: { autocomplete: "off" }, hint: "A link where attendees can sign up"
= render partial: 'shared/location', locals: {f: f, venue_type: @conference.venue}
= render partial: 'shared/location', locals: {f: f}
= f.input :completed, as: :boolean, hint: "Check this when all the event's presentations have been entered."
- if current_user&.editor? || current_user&.admin?
= f.label :editors_notes, "Editor's Notes"
Expand Down
2 changes: 1 addition & 1 deletion app/views/presentations/_base_form_fields.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
= f.input :date, as: :date, start_year: Setting.base_event_year, end_year: Date.today.year + 1, order: [:month, :day, :year]
-# Don't show Venue unless the presentation is attached to a multi-venue event
- if @presentation.try(:conference).try(:multi_venue?)
= render partial: 'shared/location', locals: {f: f, venue_type: @presentation.venue}
= render partial: 'shared/location', locals: {f: f}
- if current_user.editor? || current_user.admin?
= f.label :editors_notes, "Editor's Notes"
= f.trix_editor :editors_notes
Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/_location.html.haml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
= f.input :location, as: 'select', collection: Conference::LOCATION_TYPES, include_blank: false, selected: venue_type
= f.input :location_type, as: 'select', collection: Conference::LOCATION_TYPES, include_blank: false
-# This backs up the visible venue field so that when the selector changes it to Multiple/virtual and the field is
-# disabled, the changed value will still be submitted.
-# The initial state is set by JS in conferences.coffee
Expand Down
15 changes: 15 additions & 0 deletions db/migrate/20200802025802_add_location_type_to_conferences.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class AddLocationTypeToConferences < ActiveRecord::Migration[5.2]
def up
add_column "conferences", :location_type, :string, default: Locations::PHYSICAL, null: false
add_column "presentations", :location_type, :string, default: Locations::PHYSICAL, null: false

execute("UPDATE conferences SET location_type = venue WHERE venue in ('Virtual', 'Multiple')")
execute("UPDATE presentations SET location_type = venue WHERE venue in ('Virtual', 'Multiple')")
# TODO - unspecified for blank?
end

def down
remove_column "conferences", :location_type
remove_column "presentations", :location_type
end
end
4 changes: 3 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2020_02_20_135800) do
ActiveRecord::Schema.define(version: 2020_08_02_025802) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -45,6 +45,7 @@
t.string "registration_url"
t.text "description"
t.string "event_type"
t.string "location_type", default: "Physical", null: false
t.index ["creator_id"], name: "index_conferences_on_creator_id"
t.index ["organizer_id"], name: "index_conferences_on_organizer_id"
end
Expand Down Expand Up @@ -157,6 +158,7 @@
t.string "city"
t.string "state"
t.string "country"
t.string "location_type", default: "Physical", null: false
t.index ["conference_id"], name: "index_presentations_on_conference_id"
t.index ["creator_id"], name: "index_presentations_on_creator_id"
end
Expand Down
2 changes: 1 addition & 1 deletion spec/models/conference_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@
context "when changing the location type" do
[Conference::VIRTUAL, Conference::MULTIPLE].each do |location_type|
context "to #{ location_type }" do
before { conference.update venue: location_type }
before { conference.update location_type: location_type }

it "sets the city to #{location_type}" do
expect(conference.city).to eq(location_type)
Expand Down

0 comments on commit 81ee073

Please sign in to comment.