From ed35c4010c8d26d81d0a0b783d2314927e6d2f17 Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Wed, 29 Dec 2021 11:01:38 -0600 Subject: [PATCH 1/5] #529 Remove the restriction on duplicate presentation names within the same event --- app/models/presentation.rb | 13 ------------- spec/models/presentation_spec.rb | 17 ++++++++++++----- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/app/models/presentation.rb b/app/models/presentation.rb index 75f5c4f1..2a9bae1f 100644 --- a/app/models/presentation.rb +++ b/app/models/presentation.rb @@ -19,7 +19,6 @@ class Presentation < ApplicationRecord has_many :users, through: :user_presentations # answers: who's watching this presentation validates :name, presence: true - validate :unique_per_conference # requiring a speaker at create is handled by PresentationsController before_save :update_sortable_name, :adjust_series_bounds @@ -69,18 +68,6 @@ def adjust_series_bounds end end - # presentations can exist with duplicate names, but presentation names must be unique within a conference - def unique_per_conference - if conference_id.present? - if id.present? - duplicate_count = Presentation.where("name = ? AND id != ? AND conference_id = ?", name, id, conference_id).length - else - duplicate_count = Presentation.where("name = ? AND conference_id = ?", name, conference_id).length - end - errors.add(:conference, "already has a presentation with the same name.") if duplicate_count > 0 - end - end - def speaker_names speakers.map{|s| s.name}.join(", ") end diff --git a/spec/models/presentation_spec.rb b/spec/models/presentation_spec.rb index e5426feb..1e60f633 100644 --- a/spec/models/presentation_spec.rb +++ b/spec/models/presentation_spec.rb @@ -79,14 +79,19 @@ let!(:duplicate_presentation) { create :presentation, name: valid_attributes[:name], conference_id: conference.id } - it "can't be associated with that conference" do - expect { Presentation.create!(valid_attributes.merge(conference_id: conference.id)) }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Conference already has a presentation with the same name.") + # This reverses an earlier validation that prevented duplicate names. That seemed like a sensible + # restriction, but it turns out the be just a pain, because series and tour events sometimes legitimately + # have presentations with the same name. + it "can be associated with that conference" do + presentation = Presentation.create!(valid_attributes.merge(conference_id: conference.id)) + expect(presentation).to be_valid end end context "series" do let!(:dummy) { conference.update(event_type: Conference::SERIES) } - # only series does this, because for conferences, a presentation date outside the window is probably a user error + # Only series does this, because for conferences, a presentation date outside the window is probably a user error. + # A series is always getting new presentations, so it's convenient for the date to just extend automatically. it "extends the series start date if the presentation date falls before" do presentation = Presentation.new(valid_attributes.merge(conference_id: conference.id, date: conference.start_date - 1.day )) expect(presentation).to be_valid @@ -125,8 +130,10 @@ let!(:duplicate_presentation) { create :presentation, name: presentation.name, conference_id: conference.id } - it "can't be associated with that conference" do - expect { presentation.update_attributes!(conference_id: conference.id) }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Conference already has a presentation with the same name.") + # This reverses an earlier validation preventing duplicate names, because that restriction is really just unhelpful + it "can be associated with that conference" do + presentation.update_attributes!(conference_id: conference.id) + expect(presentation).to be_valid end end end From 44c061512b6918eb28b2ae70ecc11648a70e9d44 Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Wed, 29 Dec 2021 11:26:22 -0600 Subject: [PATCH 2/5] #513 Don't allow publication delete directly on the manage_publications page - too easy to slip up --- app/views/presentations/manage_publications.html.haml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/presentations/manage_publications.html.haml b/app/views/presentations/manage_publications.html.haml index d16cdc5e..4ef5c9fa 100644 --- a/app/views/presentations/manage_publications.html.haml +++ b/app/views/presentations/manage_publications.html.haml @@ -25,7 +25,8 @@ = link_to icon('far', 'edit', :class => 'fa-fw'), edit_publication_path(publication, presentation_id: @presentation.id) = link_to icon('fas', 'unlink', :class => 'fa-fw'), presentation_publication_path(presentation_publication), :method => :delete, :post => true, class: 'text-danger' - = link_to icon('far', 'trash-alt', :class => 'fa-fw'), publication_path(publication, presentation_id: presentation_publication.presentation_id), :method => :delete, :data => { :confirm => 'remove this publication permanently?' }, :post => true, class: 'text-danger' + -# Don't allow delete here - too easy to slip up - follow the "eye" link to show, and delete there + =# link_to icon('far', 'trash-alt', :class => 'fa-fw'), publication_path(publication, presentation_id: presentation_publication.presentation_id), :method => :delete, :data => { :confirm => 'remove this publication permanently?' }, :post => true, class: 'text-danger' .row .col-md-12 From b1b012667f038007b393572818ed010f8d21ee06 Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Thu, 30 Dec 2021 08:01:26 -0600 Subject: [PATCH 3/5] #508 Auto-detect likely podcast URLs --- app/assets/javascripts/publications.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/javascripts/publications.coffee b/app/assets/javascripts/publications.coffee index b3166f03..5168a1d4 100644 --- a/app/assets/javascripts/publications.coffee +++ b/app/assets/javascripts/publications.coffee @@ -47,3 +47,5 @@ $ -> $format_selector.val("Campus") else if url.includes("estore.aynrand") $format_selector.val("e-Store") + else if url.includes("podcast") + $format_selector.val("Podcast") From 61cd9bb41536587ff971489c3fa3f1e027bde62b Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Thu, 30 Dec 2021 08:58:06 -0600 Subject: [PATCH 4/5] Fix some URL typos in text pages --- app/views/pages/guidelines.html.haml | 2 +- app/views/pages/tips.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/pages/guidelines.html.haml b/app/views/pages/guidelines.html.haml index 5eef816d..6cc740e7 100644 --- a/app/views/pages/guidelines.html.haml +++ b/app/views/pages/guidelines.html.haml @@ -55,7 +55,7 @@ can be some subtle consequences for these choices in terms of how things show up in searches and charts. %p The webinar series - = link_to 'Philosophy for Living On Earth', events_path('philosophy-for-living-on-earth') + = link_to 'Philosophy for Living On Earth', event_path('philosophy-for-living-on-earth') is a good example of a virtual event with no location. There are no physical attendees, just webinar participants. The = link_to "ARI Road to a Free Society", event_path('ari-road-to-a-free-society') diff --git a/app/views/pages/tips.html.haml b/app/views/pages/tips.html.haml index eaafd147..213bd60f 100644 --- a/app/views/pages/tips.html.haml +++ b/app/views/pages/tips.html.haml @@ -71,7 +71,7 @@ %p Series events contain thematically related presentations over an extended period, but without the structure of a conference. A series with virtual presentations (such as the - = link_to 'Philosophy for Living On Earth', events_path('philosophy-for-living-on-earth') + = link_to 'Philosophy for Living On Earth', event_path('philosophy-for-living-on-earth') webinars) has no location. A series may have multiple locations (such as the = link_to "ARI Road to a Free Society", event_path('ari-road-to-a-free-society') From 68eb43629c9eb21d30cbda8df64ef9f851b6e01c Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Thu, 30 Dec 2021 11:10:56 -0600 Subject: [PATCH 5/5] #532 Eliminate compact presentations option - use the handle in all listings except conference presentations list --- app/controllers/accounts_controller.rb | 2 +- app/views/accounts/_form_fields.html.haml | 1 - app/views/accounts/show.html.haml | 7 ------ .../events/_presentations_header.html.haml | 2 +- app/views/presentations/_header.html.haml | 2 +- .../presentations/_presentation.html.haml | 23 +++++-------------- .../speakers/_presentations_header.html.haml | 2 +- ...1230162139_remove_compact_presentations.rb | 5 ++++ db/schema.rb | 3 +-- 9 files changed, 16 insertions(+), 31 deletions(-) create mode 100644 db/migrate/20211230162139_remove_compact_presentations.rb diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index ed8d553a..9f20b5ca 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -53,6 +53,6 @@ def get_current_user def user_params params.require(:user).permit(:password, :password_confirmation, :email, :name, :city, :state, :country, :photo, :remove_photo, - :time_zone, :show_attendance, :show_contributor, :compact_presentations, :time_format) + :time_zone, :show_attendance, :show_contributor, :time_format) end end diff --git a/app/views/accounts/_form_fields.html.haml b/app/views/accounts/_form_fields.html.haml index 229addc0..d2859b50 100644 --- a/app/views/accounts/_form_fields.html.haml +++ b/app/views/accounts/_form_fields.html.haml @@ -18,5 +18,4 @@ = f.input :show_attendance, as: :boolean, label: "Allow other users to see my event attendance" - if @user.editor? || @user.admin? = f.input :show_contributor, as: :boolean, label: "List me on the Supporters page as a contributing editor" - = f.input :compact_presentations, as: :boolean, label: "Use compact format for presentation listing" = f.input :time_format, as: :select, collection: Publication::TIME_FORMATS, include_blank: false, label: 'Publication Time Format' diff --git a/app/views/accounts/show.html.haml b/app/views/accounts/show.html.haml index 50b5245b..86088d6c 100644 --- a/app/views/accounts/show.html.haml +++ b/app/views/accounts/show.html.haml @@ -63,13 +63,6 @@ %span.confirm= icon('fas', 'check') - else %span.deny= icon('fas', 'times') - .row - .col-sm-8.text-right Use compact format for presentation listing - .col-sm-4.font-weight-bold - - if @user.compact_presentations - %span.confirm= icon('fas', 'check') - - else - %span.deny= icon('fas', 'times') .row .col-sm-8.text-right Display format for times in publications diff --git a/app/views/events/_presentations_header.html.haml b/app/views/events/_presentations_header.html.haml index 39cf56dd..5b072847 100644 --- a/app/views/events/_presentations_header.html.haml +++ b/app/views/events/_presentations_header.html.haml @@ -9,5 +9,5 @@ = [ sorting_header('Date', :event_path, 'presentations.date', icon: 'sort-numeric'), @conference.multi_venue? ? 'Location' : nil].compact.join(' / ').html_safe - if params[:heart].present? .col Needs - .col.col-md-1.text-right Media + .col.col-md-1.text-center Media .col.d-none.d-md-block diff --git a/app/views/presentations/_header.html.haml b/app/views/presentations/_header.html.haml index 8d13ef5a..eeda9088 100644 --- a/app/views/presentations/_header.html.haml +++ b/app/views/presentations/_header.html.haml @@ -15,7 +15,7 @@ - if params[:heart].present? .col Needs - .col.col-sm-2.col-md-2.text-right + .col.col-sm-2.col-md-2.text-center Media - if current_user.present? diff --git a/app/views/presentations/_presentation.html.haml b/app/views/presentations/_presentation.html.haml index ee83f862..586f2cb1 100644 --- a/app/views/presentations/_presentation.html.haml +++ b/app/views/presentations/_presentation.html.haml @@ -4,30 +4,19 @@ .row.mb-2{ id: "presentation_#{presentation.id}" } %div{ class: current_user.present? ? 'col-md-5' : 'col-md-8'} -# Make the detailed view the default - see what indexing robots do with that - use compact for speakers - -if (controller_name != 'presentations' && current_user&.compact_presentations) || controller_name == 'speakers' - -# For compact use just the name for event presentation listing. Never include speaker in speaker presentation listing - redundant + -if @conference.present? # then it's the event listing, so show speakers but not tags - if @conference && @conference.use_episodes = presentation.episode = link_to(truncate(presentation.name, length: 80, separator: ' '), presentation_path(presentation)) - -elsif current_user&.compact_presentations + %br + by + = clickable_speaker_list(presentation) + -else .detail-selector = link_to icon('far', 'caret-square-right', class: 'fa-fw'), "#", class: 'detail-show', style: 'display: none;', title: ht_details = link_to icon('far', 'caret-square-down', class: 'fa-fw'), "#", class: 'detail-hide', style: 'display: none;', title: ht_hide_details = link_to icon('far', 'caret-square-right', class: 'fa-fw'), presentation_path(presentation), class: 'detail-initiator', rel: 'nofollow', data: { method: :get, remote: true, params: { details: true, skip_event: (controller_name == 'events' && action_name == 'show') }.to_param }, title: ht_details = link_to(truncate(presentation.name, length: 80, separator: ' '), presentation_path(presentation)) - -else - - if @conference && @conference.use_episodes - = presentation.episode - = link_to(truncate(presentation.name, length: 80, separator: ' '), presentation_path(presentation)) - %br - by - = clickable_speaker_list(presentation) - - unless controller_name == 'events' && params[:id].present? # in that case, we're looking at the event page - reiterating event is redundant - - if presentation.conference.present? && params[:skip_event] != 'true' - at - = link_to truncate(presentation.conference_name, length: 45, separator: ' '), event_path(presentation.conference) - %br - = linked_tag_names(presentation, class: 'slim') -if @conference.present? # then it's the event listing .col.col-sm-8.col-md-2 @@ -59,7 +48,7 @@ = icon('fas', 'puzzle-piece', class: 'fa-fw', title: 'Needs number of parts') if presentation.parts.blank? = icon('fas', 'chalkboard-teacher', class: 'fa-fw', title: 'Not yet associated with an event') if presentation.conference.blank? - .col.col-sm-2.col-md-2.text-right= linked_format_icons(presentation) + .col.col-sm-2.col-md-2.text-center= linked_format_icons(presentation) -# Jam the heart and edit buttons into one column - the heart takes up too much space in a column by itself - if current_user.present? diff --git a/app/views/speakers/_presentations_header.html.haml b/app/views/speakers/_presentations_header.html.haml index a428495d..204e8a9b 100644 --- a/app/views/speakers/_presentations_header.html.haml +++ b/app/views/speakers/_presentations_header.html.haml @@ -16,7 +16,7 @@ - if params[:heart].present? .col Needs - .col.col-sm-2.col-md-2.text-right + .col.col-sm-2.col-md-2.text-center Media - if current_user.present? diff --git a/db/migrate/20211230162139_remove_compact_presentations.rb b/db/migrate/20211230162139_remove_compact_presentations.rb new file mode 100644 index 00000000..0f9f34ab --- /dev/null +++ b/db/migrate/20211230162139_remove_compact_presentations.rb @@ -0,0 +1,5 @@ +class RemoveCompactPresentations < ActiveRecord::Migration[5.2] + def change + remove_column "users", :compact_presentations, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index d0f7fd0f..2d1eb7ed 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_12_25_153946) do +ActiveRecord::Schema.define(version: 2021_12_30_162139) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -329,7 +329,6 @@ t.integer "speaker_id" t.string "sortable_name" t.string "time_format", default: "hh:mm" - t.boolean "compact_presentations", default: false t.string "privacy_policy_version", default: "0.0" t.index ["sortable_name"], name: "index_users_on_sortable_name" end