User schedule #2185

differentreality · 2018-09-19T21:48:13Z

User can select favorite events, and show only their individual schedule
Continuing #1142 by @Ana06

Introduce an association between User and Event
Show individual User schedule

Ana06 · 2018-09-19T22:15:29Z

@differentreality thanks for taking care of this! 💘

bear454 · 2018-09-20T18:42:37Z

Just a little lint to remove:

app/models/event.rb:35:3: C: Rails/HasAndBelongsToMany: Prefer has_many :through to has_and_belongs_to_many.
  has_and_belongs_to_many :favourite_users, class_name: 'User'
  ^^^^^^^^^^^^^^^^^^^^^^^
app/models/user.rb:79:3: C: Rails/HasAndBelongsToMany: Prefer has_many :through to has_and_belongs_to_many.
  has_and_belongs_to_many :favourite_events, class_name: 'Event'
  ^^^^^^^^^^^^^^^^^^^^^^^
app/controllers/proposals_controller.rb:11:1: C: Layout/EmptyLines: Extra blank line detected.

hennevogel · 2018-09-20T23:15:06Z

I know it feels weird but try out code-review-by-commit. It makes things so much faster 🤠

codecov · 2018-10-02T18:30:37Z

Codecov Report

Merging #2185 into master will increase coverage by 3.27%.
The diff coverage is 40.9%.

@@            Coverage Diff             @@
##           master    #2185      +/-   ##
==========================================
+ Coverage   78.74%   82.01%   +3.27%     
==========================================
  Files         146      146              
  Lines        4826     4798      -28     
==========================================
+ Hits         3800     3935     +135     
+ Misses       1026      863     -163

Impacted Files	Coverage Δ
app/models/user.rb	`95% <100%> (+0.67%)`	⬆️
app/models/event.rb	`92.35% <100%> (+0.13%)`	⬆️
app/controllers/proposals_controller.rb	`86.79% <25%> (-5.13%)`	⬇️
app/controllers/schedules_controller.rb	`43.39% <33.33%> (-1.05%)`	⬇️
app/models/ability.rb	`96.51% <50%> (-1.11%)`	⬇️
app/controllers/commercials_controller.rb	`96.42% <0%> (+3.57%)`	⬆️
app/serializers/conference_serializer.rb	`94.73% <0%> (+5.26%)`	⬆️
app/helpers/users_helper.rb	`92.85% <0%> (+7.14%)`	⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 109cb7b...1e53e9a. Read the comment docs.

hennevogel · 2018-10-10T19:58:30Z

@differentreality once more! You can make it! ;-)

bear454 · 2018-12-29T04:42:41Z

@differentreality: do you think you'll get this wrapped up soon? I'd love to have this in by mid-February, for LFNW2019!

differentreality · 2018-12-29T09:28:50Z

Yes, I will! I had completely forgotten about this 😞

hennevogel · 2019-02-15T23:57:22Z

go stella, go stella

differentreality · 2019-03-10T19:26:58Z

Rebased 😀

differentreality · 2019-04-29T16:30:53Z

What else would we need for this?

bennettscience · 2020-01-06T15:52:11Z

Any chance of having this merged in soon-ish? We're hosting an event on Feb 6th and this would really help out!

differentreality · 2020-01-06T17:25:39Z

Yeah I'd love to see this merged as well! Still pending review though..
I should probably take a look at it again too.

bennettscience · 2020-01-13T17:28:41Z

@differentreality If I manually pull this commit into my local copy, I should just be able to migrate the database with the new relationship, right? The only failure in the CI build was a linter, so I'm not too worried...but I wanted to see what you thought first.

cycomachead

I love this idea!

I left some comments in terms of web accessibility with the front-end piece. Feel free to ignore it and just merge -- this seems great to me to have. (I'm hoping to make some small accessibility-related PRs soon.)

cycomachead · 2020-01-07T09:30:33Z

app/views/schedules/_event.html.haml

+    - 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' }", |
+                           "aria-hidden" => "true", |


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", |

cycomachead · 2020-01-07T09:31:24Z

app/views/schedules/_event.html.haml

-
+    - 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' }", |


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

cycomachead · 2020-01-07T09:34:52Z

app/views/schedules/_schedule_item.html.haml

    %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' }", |


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

cycomachead · 2020-01-07T09:35:39Z

app/views/schedules/_schedule_item.html.haml

+      - 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' }", |
+                      "aria-hidden" =>"true", |


Same as above. :)

Suggested change

"aria-hidden" =>"true", |

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

cycomachead · 2020-01-07T09:43:01Z

app/views/schedules/_schedule_item.html.haml

@@ -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))}" }


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.

cycomachead · 2021-04-08T18:02:27Z

app/controllers/proposals_controller.rb

      render action: 'edit'
    end
  end

+  def toggle_favorite
+    user = User.find(params[:favourite_user_id])


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.

cycomachead · 2021-04-08T18:07:13Z

app/controllers/schedules_controller.rb

@@ -52,9 +53,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


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.

Introduce an association between User and Event for the user schedule. As there was already a relation call users in Event the new relation is called public_users. And for the same reason the relation in Event is called public_events.

Make it possible for users to select events which they want to see in the public schedule and in the All events section of the public schedule.

Show the current user favourites events for both schedule and all events. Unsed css removed.

Introduce a separate action `toggle_favorite` to update user favorites events. Otherwise users with permissions to update the event can have it as favorite.

cycomachead · 2021-07-27T03:53:28Z

app/views/schedules/events.html.haml

@@ -5,6 +5,13 @@
  %h1.text-center


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.

differentreality force-pushed the user_schedule branch from 7068bbc to 4610e9d Compare October 2, 2018 18:30

differentreality force-pushed the user_schedule branch from 4610e9d to a287348 Compare November 4, 2018 09:02

differentreality force-pushed the user_schedule branch from a287348 to 6317a52 Compare December 29, 2018 09:06

differentreality force-pushed the user_schedule branch from 6317a52 to 455c98e Compare February 16, 2019 08:05

Ana06 added the feature 💡 For something new in the app label Mar 8, 2019

differentreality force-pushed the user_schedule branch from 455c98e to 1111f6b Compare March 10, 2019 19:26

differentreality force-pushed the user_schedule branch from 1111f6b to e378956 Compare March 30, 2019 21:49

Ana06 requested a review from hennevogel June 13, 2019 08:23

differentreality force-pushed the user_schedule branch from e378956 to 717e487 Compare October 24, 2019 16:08

differentreality force-pushed the user_schedule branch from 717e487 to 1e53e9a Compare January 6, 2020 21:37

cycomachead reviewed Jan 14, 2020

View reviewed changes

cycomachead reviewed Apr 8, 2021

View reviewed changes

differentreality force-pushed the user_schedule branch from 1e53e9a to 77048df Compare April 11, 2021 07:01

Ana06 added 3 commits April 14, 2021 11:22

Introduce an association between User and Event

055afb1

Introduce an association between User and Event for the user schedule. As there was already a relation call users in Event the new relation is called public_users. And for the same reason the relation in Event is called public_events.

Make it possible to select favourites events

7cd4cf8

Make it possible for users to select events which they want to see in the public schedule and in the All events section of the public schedule.

Show the individual User schedule

ec2f978

Show the current user favourites events for both schedule and all events. Unsed css removed.

Ana06 and others added 3 commits April 14, 2021 11:22

Move update favorites to a separate action

65f3db6

Introduce a separate action `toggle_favorite` to update user favorites events. Otherwise users with permissions to update the event can have it as favorite.

Add authorization for toggle_favorite and rename route

94b5628

Remove duplicate code and style fixes

eebcf6b

differentreality force-pushed the user_schedule branch from 77048df to eebcf6b Compare April 14, 2021 08:22

cycomachead reviewed Jul 27, 2021

View reviewed changes

Merge branch 'master' into user_schedule

52eb61c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User schedule #2185

User schedule #2185

differentreality commented Sep 19, 2018

Ana06 commented Sep 19, 2018

bear454 commented Sep 20, 2018

hennevogel commented Sep 20, 2018 •

edited

Loading

codecov bot commented Oct 2, 2018 •

edited

Loading

hennevogel commented Oct 10, 2018

bear454 commented Dec 29, 2018

differentreality commented Dec 29, 2018

hennevogel commented Feb 15, 2019

differentreality commented Mar 10, 2019

differentreality commented Apr 29, 2019

bennettscience commented Jan 6, 2020

differentreality commented Jan 6, 2020

bennettscience commented Jan 13, 2020 •

edited

Loading

cycomachead left a comment

cycomachead Jan 7, 2020

cycomachead Jan 7, 2020

cycomachead Jan 7, 2020

cycomachead Jan 7, 2020

cycomachead Jan 7, 2020

cycomachead Apr 8, 2021

cycomachead Apr 8, 2021

cycomachead Jul 27, 2021

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

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

User schedule #2185

Are you sure you want to change the base?

User schedule #2185

Conversation

differentreality commented Sep 19, 2018

Ana06 commented Sep 19, 2018

bear454 commented Sep 20, 2018

hennevogel commented Sep 20, 2018 • edited Loading

codecov bot commented Oct 2, 2018 • edited Loading

Codecov Report

hennevogel commented Oct 10, 2018

bear454 commented Dec 29, 2018

differentreality commented Dec 29, 2018

hennevogel commented Feb 15, 2019

differentreality commented Mar 10, 2019

differentreality commented Apr 29, 2019

bennettscience commented Jan 6, 2020

differentreality commented Jan 6, 2020

bennettscience commented Jan 13, 2020 • edited Loading

cycomachead left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hennevogel commented Sep 20, 2018 •

edited

Loading

codecov bot commented Oct 2, 2018 •

edited

Loading

bennettscience commented Jan 13, 2020 •

edited

Loading