From b717018b310dea45d2e02a1f69ae9f17912b538a Mon Sep 17 00:00:00 2001 From: AEtherC0r3 Date: Tue, 8 Aug 2017 12:41:37 +0300 Subject: [PATCH] Track related fixes Make the message in admin/Tracks form more visible by making it bold and adding links to venue and rooms Make papertrail track changes for all the track's attributes Add validation to require presence of description for self-organized tracks Add ID column to admin/Tracks#index Make the cfp inclusion column sortable Show success/error flash messages after toggling cfp inclusion --- app/assets/javascripts/osem-datatables.js | 24 +++--- app/controllers/admin/tracks_controller.rb | 11 ++- app/models/track.rb | 3 +- app/views/admin/tracks/_form.html.haml | 9 +- app/views/admin/tracks/index.html.haml | 6 +- app/views/admin/tracks/show.html.haml | 1 + .../admin/tracks/toggle_cfp_inclusion.js.erb | 8 ++ .../admin/tracks_controller_spec.rb | 82 ++++++++++++++++--- spec/models/track_spec.rb | 2 + 9 files changed, 114 insertions(+), 32 deletions(-) create mode 100644 app/views/admin/tracks/toggle_cfp_inclusion.js.erb diff --git a/app/assets/javascripts/osem-datatables.js b/app/assets/javascripts/osem-datatables.js index f950159e..a6f85c5d 100644 --- a/app/assets/javascripts/osem-datatables.js +++ b/app/assets/javascripts/osem-datatables.js @@ -1,18 +1,14 @@ $(function () { - $(document).ready(function() { - $('.datatable').DataTable({ - // ajax: ..., - stateSave: true, - autoWidth: false, - pagingType: 'full_numbers', - "lengthMenu": [[25, 50, 100, -1], [25, 50, 100, "All"]] - }); + $('.datatable').DataTable({ + // ajax: ..., + stateSave: true, + autoWidth: false, + pagingType: 'full_numbers', + "lengthMenu": [[25, 50, 100, -1], [25, 50, 100, "All"]], + }); - $('#versionstable').DataTable({ - pagingType: 'full_numbers', - order: [[ 0, 'desc' ]] - }); + $('#versionstable').DataTable({ + pagingType: 'full_numbers', + order: [[ 0, 'desc' ]] }); }); - - diff --git a/app/controllers/admin/tracks_controller.rb b/app/controllers/admin/tracks_controller.rb index 3d51a677..0f3940da 100644 --- a/app/controllers/admin/tracks_controller.rb +++ b/app/controllers/admin/tracks_controller.rb @@ -4,6 +4,9 @@ module Admin load_and_authorize_resource :program, through: :conference, singleton: true load_and_authorize_resource through: :program, find_by: :short_name + # Show flash message with ajax calls + after_action :prepare_unobtrusive_flash, only: :toggle_cfp_inclusion + def index; end def show @@ -55,9 +58,13 @@ module Admin def toggle_cfp_inclusion @track.cfp_active = !@track.cfp_active if @track.save - head :ok + flash[:notice] = "Successfully changed cfp inclusion of #{@track.name} to #{@track.cfp_active}" else - head :unprocessable_entity + flash[:error] = "Failed to toggle cfp inclusion of #{@track.name} to #{@track.cfp_active}" + end + + respond_to do |format| + format.js end end diff --git a/app/models/track.rb b/app/models/track.rb index 2b09b654..c624cab8 100644 --- a/app/models/track.rb +++ b/app/models/track.rb @@ -9,7 +9,7 @@ class Track < ActiveRecord::Base belongs_to :room has_many :events, dependent: :nullify - has_paper_trail only: [:name, :description, :color], meta: { conference_id: :conference_id } + has_paper_trail ignore: [:updated_at], meta: { conference_id: :conference_id } before_create :generate_guid validates :name, presence: true @@ -28,6 +28,7 @@ class Track < ActiveRecord::Base validates :end_date, presence: true, if: :self_organized_and_accepted_or_confirmed? validates :room, presence: true, if: :self_organized_and_accepted_or_confirmed? validates :relevance, presence: true, if: :self_organized? + validates :description, presence: true, if: :self_organized? validate :valid_dates validate :valid_room, if: :self_organized_and_accepted_or_confirmed? diff --git a/app/views/admin/tracks/_form.html.haml b/app/views/admin/tracks/_form.html.haml index eb12561b..9109e4f5 100644 --- a/app/views/admin/tracks/_form.html.haml +++ b/app/views/admin/tracks/_form.html.haml @@ -14,10 +14,15 @@ = f.input :color, input_html: {size: 6, type: 'color'}, required: true = f.input :start_date, as: :string, input_html: { id: 'registration-period-start-datepicker', start_date: @conference.start_date, end_date: @conference.end_date, readonly: 'readonly', required: @track.self_organized_and_accepted_or_confirmed? } = f.input :end_date, as: :string, input_html: { id: 'registration-period-end-datepicker', readonly: 'readonly', required: @track.self_organized_and_accepted_or_confirmed? } - - if @conference.venue + - if @conference.venue.try(:rooms) = f.input :room, as: :select, collection: (@conference.venue.rooms).map {|room| ["#{room.name}", room.id]}, include_blank: true, label: 'Room', input_html: { class: 'select-help-toggle', required: @track.self_organized_and_accepted_or_confirmed? } - else - Please add a venue with rooms, if you want to select a room for the track. + %b + Please add a + = link_to 'venue', admin_conference_venue_path(@conference.short_title) + with + = link_to 'rooms', admin_conference_venue_rooms_path(@conference.short_title) + , if you want to select a room for the track. = f.input :description, input_html: {rows: 2, data: { provide: 'markdown-editable' } }, hint: markdown_hint = f.input :cfp_active, label: 'Allow event submitters to select this track for their proposal' = f.action :submit, as: :button, button_html: { class: 'btn btn-primary' } diff --git a/app/views/admin/tracks/index.html.haml b/app/views/admin/tracks/index.html.haml index 5c47fe11..77a7cf02 100644 --- a/app/views/admin/tracks/index.html.haml +++ b/app/views/admin/tracks/index.html.haml @@ -1,3 +1,4 @@ +.unobtrusive-flash-container .row .col-md-12 .page-header @@ -8,6 +9,7 @@ .col-md-12 %table.table.table-hover.table-striped.table-bordered.datatable#tracks %thead + %th ID %th Name %th Description %th Room @@ -20,6 +22,8 @@ %tbody - @tracks.each do |track| %tr + %td + = track.id %td{style: "padding: 15px 0px 0px 10px;"} = link_to admin_conference_program_track_path(@conference.short_title, track), class: 'btn' do %span.label{style: "background-color: #{track.color}; color: #{ contrast_color(track.color) }"} @@ -35,7 +39,7 @@ = track.end_date.strftime('%A, %B %-d. %Y') if track.end_date %td = link_to track.submitter.name, admin_user_path(track.submitter) if track.self_organized? - %td.text-center + %td.text-center{ 'id' => "cfp_switch_#{track.id}", 'data-order' => track.cfp_active.to_s } = check_box_tag "#{@conference.short_title}_#{track.short_name}", track.id, track.cfp_active, class: 'switch-checkbox', method: :patch, url: toggle_cfp_inclusion_admin_conference_program_track_path(@conference.short_title, id: track.short_name)+"?included=", diff --git a/app/views/admin/tracks/show.html.haml b/app/views/admin/tracks/show.html.haml index a8c13d9d..78f8830e 100644 --- a/app/views/admin/tracks/show.html.haml +++ b/app/views/admin/tracks/show.html.haml @@ -1,3 +1,4 @@ +.unobtrusive-flash-container .row .col-md-12 .page-header diff --git a/app/views/admin/tracks/toggle_cfp_inclusion.js.erb b/app/views/admin/tracks/toggle_cfp_inclusion.js.erb new file mode 100644 index 00000000..6ee83003 --- /dev/null +++ b/app/views/admin/tracks/toggle_cfp_inclusion.js.erb @@ -0,0 +1,8 @@ +$('.alert').remove(); + +track_id = <%= @track.id %>; +track_cfp_td = $('#cfp_switch_' + track_id); +track_cfp_value = <%= @track.cfp_active %>; + +track_cfp_td.attr('data-order', track_cfp_value); +$('#tracks').DataTable().cell(track_cfp_td).invalidate(); diff --git a/spec/controllers/admin/tracks_controller_spec.rb b/spec/controllers/admin/tracks_controller_spec.rb index 70ca48a1..795906aa 100644 --- a/spec/controllers/admin/tracks_controller_spec.rb +++ b/spec/controllers/admin/tracks_controller_spec.rb @@ -228,16 +228,45 @@ describe Admin::TracksController do before :each do self_organized_track.cfp_active = false self_organized_track.save! - patch :toggle_cfp_inclusion, conference_id: conference.short_title, id: self_organized_track.short_name - self_organized_track.reload end - it 'assigns the correct track' do - expect(assigns(:track)).to eq self_organized_track + context 'toggles successfully' do + before :each do + patch :toggle_cfp_inclusion, conference_id: conference.short_title, id: self_organized_track.short_name, format: :js + self_organized_track.reload + end + + it 'assigns the correct track' do + expect(assigns(:track)).to eq self_organized_track + end + + it 'shows success message in flash notice' do + expect(flash[:notice]).to match('Successfully changed cfp inclusion of My awesome track to true') + end + + it 'becomes true' do + expect(self_organized_track.cfp_active).to eq true + end end - it 'becomes true' do - expect(self_organized_track.cfp_active).to eq true + context 'save fails' do + before :each do + allow_any_instance_of(Track).to receive(:save).and_return(false) + patch :toggle_cfp_inclusion, conference_id: conference.short_title, id: self_organized_track.short_name, format: :js + self_organized_track.reload + end + + it 'assigns the correct track' do + expect(assigns(:track)).to eq self_organized_track + end + + it 'shows error message in flash notice' do + expect(flash[:error]).to match('Failed to toggle cfp inclusion of My awesome track to true') + end + + it 'stays false' do + expect(self_organized_track.cfp_active).to eq false + end end end @@ -245,16 +274,45 @@ describe Admin::TracksController do before :each do self_organized_track.cfp_active = true self_organized_track.save! - patch :toggle_cfp_inclusion, conference_id: conference.short_title, id: self_organized_track.short_name - self_organized_track.reload end - it 'assigns the correct track' do - expect(assigns(:track)).to eq self_organized_track + context 'toggles successfully' do + before :each do + patch :toggle_cfp_inclusion, conference_id: conference.short_title, id: self_organized_track.short_name, format: :js + self_organized_track.reload + end + + it 'assigns the correct track' do + expect(assigns(:track)).to eq self_organized_track + end + + it 'shows success message in flash notice' do + expect(flash[:notice]).to match('Successfully changed cfp inclusion of My awesome track to false') + end + + it 'becomes false' do + expect(self_organized_track.cfp_active).to eq false + end end - it 'becomes false' do - expect(self_organized_track.cfp_active).to eq false + context 'save fails' do + before :each do + allow_any_instance_of(Track).to receive(:save).and_return(false) + patch :toggle_cfp_inclusion, conference_id: conference.short_title, id: self_organized_track.short_name, format: :js + self_organized_track.reload + end + + it 'assigns the correct track' do + expect(assigns(:track)).to eq self_organized_track + end + + it 'shows error message in flash notice' do + expect(flash[:error]).to match('Failed to toggle cfp inclusion of My awesome track to false') + end + + it 'stays true' do + expect(self_organized_track.cfp_active).to eq true + end end end end diff --git a/spec/models/track_spec.rb b/spec/models/track_spec.rb index ed058176..1ad54839 100644 --- a/spec/models/track_spec.rb +++ b/spec/models/track_spec.rb @@ -54,6 +54,7 @@ describe Track do end it { is_expected.to validate_presence_of(:relevance) } + it { is_expected.to validate_presence_of(:description) } end context 'when self_organized? returns false' do @@ -62,6 +63,7 @@ describe Track do end it { is_expected.to_not validate_presence_of(:relevance) } + it { is_expected.to_not validate_presence_of(:description) } end describe '#valid_dates' do