Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User schedule #2185

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions app/assets/javascripts/osem-schedule.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// ADMIN SCHEDULE

var url; // Should be initialize in Schedule.initialize
var schedule_id; // Should be initialize in Schedule.initialize

Expand Down Expand Up @@ -114,6 +116,31 @@ $(document).ready( function() {
});
});


// PUBLIC SCHEDULE

function starClicked(e){
// stops the click from propagating
if (!e) var e = window.event;
e.preventDefault();
e.cancelBubble = true;
if (e.stopPropagation) e.stopPropagation();

var callback = function(data) {
$(e.target).toggleClass('fa-star fa-star-o');
}

var params = { favourite_user_id: $(e.target).data('user') };

$.ajax({
url: $(e.target).data('url'),
type: 'PATCH',
data: params,
success: callback,
dataType : 'json'
});
}

function eventClicked(e, element){
var url = $(element).data('url');
if(e.ctrlKey)
Expand Down
25 changes: 10 additions & 15 deletions app/assets/stylesheets/osem-schedule.scss
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ a.unstyled-link {
.program-dropdown, .schedule-dropdown{
margin-left: 20%;
margin-right: 20%;
margin-top: 40px;
margin-top: 10px;
margin-bottom: 20px;
}

Expand Down Expand Up @@ -221,20 +221,6 @@ td.no-padding{
cursor: pointer;
}

.program-dropdown{
margin-left: 20%;
margin-right: 20%;
margin-top: 40px;
}

.program-dropdown > button{
width: 100%;
}

.program-dropdown > .dropdown-menu{
width: 100%;
}

.no-events-day{
color: #D8D8D8 !important;
}
Expand All @@ -254,6 +240,15 @@ td.no-padding{
opacity: 0.5;
}

#star{
margin-top: 8px;
}

#star-events{
margin-right: 5px;
vertical-align: text-top;
}

/* Small devices (tablets, 768px and up) */
@media (min-width: 768px) {
.room, .event-title{
Expand Down
13 changes: 12 additions & 1 deletion app/controllers/proposals_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,22 @@ def update
redirect_to conference_program_proposals_path(conference_id: @conference.short_title),
notice: 'Proposal was successfully updated.'
else
flash.now[:error] = "Could not update proposal: #{@event.errors.full_messages.join(', ')}"
flash[:error] = "Could not update proposal: #{@event.errors.full_messages.join(', ')}"
render action: 'edit'
end
end

def toggle_favorite
user = User.find(params[:favourite_user_id])
Copy link
Contributor

Choose a reason for hiding this comment

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

I've finally started working on merging this into my own fork for an event this summer. It seems to me that this allows anyone to toggle anyone else's favorite events. Should it not just use current_user? Since you should be logged in and have a cookie?

I can't really see a use case as well for someone else (such as an admin) toggling another user's favorites so I think the favourite_user_id parameter could probably just be removed, but I might be missing something.

users = @event.favourite_users
if users.include? user
@event.favourite_users.delete(user)
else
@event.favourite_users << user
end
render json: {}
end

def withdraw
authorize! :update, @event
@url = conference_program_proposal_path(@conference.short_title, params[:id])
Expand Down
15 changes: 14 additions & 1 deletion app/controllers/schedules_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
class SchedulesController < ApplicationController
load_and_authorize_resource
before_action :respond_to_options
before_action :favourites
load_resource :conference, find_by: :short_title
load_resource :program, through: :conference, singleton: true, except: :index
before_action :load_withdrawn_event_schedules, only: [:show, :events]
Expand Down Expand Up @@ -58,9 +59,17 @@ def events
@events_schedules = @program.selected_event_schedules(
includes: [:room, { event: %i[track event_type speakers submitter] }]
)

@events_schedules = @events_schedules.select{ |e| e.event.favourite_users.exists?(current_user.id) } if @events_schedules && current_user && @favourites
Copy link
Contributor

Choose a reason for hiding this comment

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

This may be a slightly different use case, but I think related enough...
How would you feel about showing a schedule which is favorite events + events where you are speaking?

I am thinking about creating an event_schedule_for_user(user) method, which is essentially your personal conference agenda. My app has assigned volunteers which could also how up there.

@events_schedules = [] unless @events_schedules
@favourites = params[:favourites] == 'true'

@unscheduled_events = @program.events.confirmed - @events_schedules.map(&:event)
@unscheduled_events = if @program.selected_schedule
@program.events.confirmed - @events_schedules.map(&:event)
else
@program.events.confirmed
end
@unscheduled_events = @unscheduled_events.select{ |e| e.favourite_users.exists?(current_user.id) } if current_user && @favourites

day = @conference.current_conference_day
@tag = day.strftime('%Y-%m-%d') if day
Expand All @@ -72,6 +81,10 @@ def app

private

def favourites
@favourites = params[:favourites] == 'true'
end

def respond_to_options
respond_to do |format|
format.html { head :ok }
Expand Down
4 changes: 4 additions & 0 deletions app/models/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ def signed_in(user)
event.users.include?(user)
end

can :toggle_favorite, Event do |event|
event.scheduled?
end

# can manage the commercials of their own events
can :manage, Commercial, commercialable_type: 'Event', commercialable_id: user.events.pluck(:id)

Expand Down
3 changes: 3 additions & 0 deletions app/models/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ class Event < ApplicationRecord
belongs_to :difficulty_level
belongs_to :program

has_and_belongs_to_many :favourite_users, class_name: 'User'

accepts_nested_attributes_for :event_users, allow_destroy: true
accepts_nested_attributes_for :speakers, allow_destroy: true
accepts_nested_attributes_for :users
accepts_nested_attributes_for :favourite_users

before_create :generate_guid

Expand Down
2 changes: 2 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ def for_registration conference
has_many :booths, through: :booth_requests
has_many :survey_replies
has_many :survey_submissions
has_and_belongs_to_many :favourite_events, class_name: 'Event'

accepts_nested_attributes_for :roles

scope :admin, -> { where(is_admin: true) }
Expand Down
3 changes: 2 additions & 1 deletion app/views/schedules/_carousel.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@
%td.room{ style: "height: #{ td_height(@rooms) }px;" }
.room.elipsis.break-words{ style: "-webkit-line-clamp: #{ room_lines(@rooms) }; height: #{ room_height(@rooms) }px;" }
= room.name

- event_schedules = (@event_schedules_by_room_id[room.id] || []).select{ |e| (e.end_time > start_time) && (e.start_time <= (start_time + hrs_per_slide.hour)) }

- event_schedules = event_schedules.select{ |e| e.event.favourite_users.exists?(current_user.id) } if current_user && @favourites

- interval_count.times do |offset|
- if span > 1
- span -= 1
Expand Down
8 changes: 6 additions & 2 deletions app/views/schedules/_event.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
= image_tag speaker.gravatar_url, :class => "img-circle pull-right all-speaker-pic", |
:alt => speaker.name, |
:title => speaker.name |

%p
= canceled_replacement_event_label(event, event_schedule)
= replacement_event_notice(event_schedule)

- if current_user
= link_to('#', onClick: 'starClicked();') do
%span#star-events{ class: "fa fa-lg #{ event.favourite_users.exists?(current_user.id) ? 'fa-star' : 'fa-star-o' }", |
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a class, not an id? (Since it seems like this partial is reusable)

"aria-hidden" => "true", |
Copy link
Contributor

Choose a reason for hiding this comment

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

In terms of accessibility, something like this would be provide a better description to anyone with a screen reader.

Suggested change
"aria-hidden" => "true", |
"aria-label" => "#{event.favourite_users.exists?(current_user.id) ? "un-" : ""}favourite event", |

"data-url" => toggle_favorite_conference_program_proposal_path(@conference.short_title, event.id), |
"data-user" => current_user.id }
%span.h3
= event.title
%br
Expand Down
9 changes: 7 additions & 2 deletions app/views/schedules/_schedule_item.html.haml
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
%td.event{ width: "#{ width * span }%" , |
colspan: span, |
role: "button" }
%a.unstyled-link{href: url_for(conference_program_proposal_path(@conference.short_title, event.id))}
%div{ onClick: 'eventClicked(event, this);', "data-url" => "#{url_for(conference_program_proposal_path(@conference.short_title, event.id))}" }
Copy link
Contributor

Choose a reason for hiding this comment

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

In terms of accessibility (both screen readers and keyboards), this is better as an a element. Otherwise, it needs role: 'button', tabindex: 0, onKeyPress: 'function(event) { if (event.charCode === 10 || event.charCode === 32) { eventCicked(event, this); } }' -- well, that last function is better put elsewhere, but that makes the div accept keypresses like a normal button.

%div{ class: "elipsis break-words event-title", |
style: "-webkit-line-clamp: #{ event_lines(@rooms) }; height: #{ event_height(@rooms) }px;"} |

= canceled_replacement_event_label(event, event_schedule, 'schedule-label')

- if current_user
= link_to('#', onClick: 'starClicked();') do
%span#star{ class: "fa fa-lg #{ event.favourite_users.exists?(current_user.id) ? 'fa-star' : 'fa-star-o' }", |
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, this should be a class, since it appears multiple times on the page.

"aria-hidden" =>"true", |
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above. :)

Suggested change
"aria-hidden" =>"true", |
"aria-label" => "#{event.favourite_users.exists?(current_user.id) ? "un-" : ""}favourite event", |

"data-url" => toggle_favorite_conference_program_proposal_path(@conference.short_title, event.id), |
"data-user" => current_user.id }
= event.title

- event.speakers_ordered.each do |speaker|
= image_tag speaker.gravatar_url, :class => "img-circle pull-right speaker-pic", |
:alt => speaker.name, |
:title => speaker.name, |
:style => "height: #{ speaker_height(@rooms) }px; width: #{ speaker_width(@rooms) }px;"

4 changes: 2 additions & 2 deletions app/views/schedules/_schedule_tabs.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
/ Nav tabs
%ul.nav.nav-tabs{ role: "tablist" }
%li{ class: "schedule #{ 'active' if active == 'schedule' }", role: "presentation" }
= link_to('Schedule', conference_schedule_path(@conference.short_title))
= link_to('Schedule', conference_schedule_path(@conference.short_title, favourites: @favourites))
%li{ class: "program #{ 'active' if active == 'program' }", role: "presentation" }
= link_to('All events', events_conference_schedule_path(@conference.short_title))
= link_to('All events', events_conference_schedule_path(@conference.short_title, favourites: @favourites))
7 changes: 7 additions & 0 deletions app/views/schedules/events.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
%h1.text-center
Copy link
Contributor

Choose a reason for hiding this comment

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

The above line -if @events_schedules.any? probably should be removed if this is merged in. It leads to a weird looking case when you have no events in your favorites schedule and the tabs disappear.

Program for
= @conference.title
- if current_user
.row
.col-md-12.text-center
= button_to (@favourites ? 'All events' : 'Only favourites events'),
events_conference_schedule_path(@conference.short_title),
params: { favourites: !@favourites },
method: :get, class: 'btn btn-success'
.dropdown.program-dropdown
%button{ type: "button", class: "btn btn-default dropdown-toggle", 'data-toggle' => "dropdown" }
Dates
Expand Down
7 changes: 7 additions & 0 deletions app/views/schedules/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
%h1.text-center
Schedule for
= @conference.title
- if current_user
.row
.col-md-12.text-center
= button_to (@favourites ? 'All events' : 'Only favourites events'),
conference_schedule_path(@conference.short_title),
params: { favourites: !@favourites },
method: :get, class: 'btn btn-success'
.dropdown.schedule-dropdown
%button{ type: "button", class: "btn btn-default dropdown-toggle", 'data-toggle' => "dropdown" }
= @day
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@
patch '/withdraw' => 'proposals#withdraw'
patch '/confirm' => 'proposals#confirm'
patch '/restart' => 'proposals#restart'
patch :toggle_favorite
end
end
resources :tracks, except: :destroy do
Expand Down
8 changes: 8 additions & 0 deletions db/migrate/20160811143427_create_events_users.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class CreateEventsUsers < ActiveRecord::Migration
def change
create_table :events_users, id: false do |t|
t.references :event
t.references :user
end
end
end
5 changes: 5 additions & 0 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,11 @@
t.datetime "created_at"
end

create_table "events_users", id: false, force: :cascade do |t|
t.integer "event_id"
t.integer "user_id"
end

create_table "lodgings", force: :cascade do |t|
t.string "name"
t.text "description"
Expand Down