Implement Event Ratings #1204

shlok007 · 2016-09-22T15:19:55Z

Refers to issue #1158 .

Rfc: https://docs.google.com/document/d/1sWImuP14RFeTWrs3nqdRZt8xfD0-xr0hBj10B2_KCQQ/edit?usp=sharing

To-Do:

Introduce VotableFields and admin views to manage votable fields.
Enable votes for events ( proposals ) by users if the event has ended.
Add tests for votable fields introduced.
Introduce new voting on admin side as well.
Drop the whole current voting system on admin side.
Rake task to migrate current voting of past conferences to new 'overall' in VotableField.

shlok007 · 2016-09-23T03:46:52Z

The code generated is by ratyrate gem . Can somebody please help with the failing test here?

shlok007 · 2016-09-23T03:48:43Z

app/models/event.rb

@@ -84,6 +85,10 @@ def registration_possible?
    registrations.count < max_attendees
  end

+  def ended?
+    Time.now.to_i > event_schedules.find_by(schedule_id: program.selected_schedule_id).end_time.to_i


I am not sure if this is the right way to compute if an event has ended .

I think you should use the timezone of the conference this event belongs to. And you can compare times directly without converting them to string.

timezone = program.conference.timezone Time.find_zone(timezone).now > event_schedules.find_by(schedule: program.selected_schedule).end_time

Ana06 · 2017-01-02T11:00:36Z

app/models/event.rb

@@ -84,6 +85,10 @@ def registration_possible?
    registrations.count < max_attendees
  end

+  def ended?
+    Time.now.to_i > event_schedules.find_by(schedule_id: program.selected_schedule_id).end_time.to_i


I think you should use the timezone of the conference this event belongs to. And you can compare times directly without converting them to string.

timezone = program.conference.timezone Time.find_zone(timezone).now > event_schedules.find_by(schedule: program.selected_schedule).end_time

differentreality · 2017-02-02T10:13:13Z

@shlok007 did you review other gems for this as well?

differentreality · 2017-02-12T16:53:15Z

@shlok007 ?

shlok007 · 2017-02-13T15:21:51Z

Sorry for the late response. Yes, i stumbled upon letsrate gem as well. There is always a possibility to write our own rating system to rate proposals.

differentreality · 2017-02-13T15:33:40Z

Nah we don't have to do that, if there is something well maintained with updates that suits our needs. But I would like to know what other gems you evaluated and what the one you chose offers us. So can you provide some more details about your choice of gem? What advantages/disadvantages it has in comparison with other alternatives? Things like that which will help us make a conscious decision.

We will use voting extensively, so we should choose the gem wisely and then use it for all the places we have/will have voting.

shlok007 · 2017-02-16T21:00:21Z

I stumbled upon letsrate and its fork ratyrate. Seems like ratyrate is still maintained after letsrate. Also, ratyrate has a lot of options and well documented guide that we could use. There is also jquery-raty-rails which I avoided as people seem to be having issues with rails 4 https://github.com/bmc/jquery-raty-rails/issues/7 .

This poster lists the additional features that ratyrate has over letsrate.

At the end of the day, all the rating gems wraps the JQuery Raty library so if none works for us we have an option to manually add and use the library ourself.

shlok007 · 2017-02-18T16:25:57Z

Do let me know what should be appropriate here. I'll proceed with this PR and try fixing the Hakiri test failures as well if we decide to use ratyrate gem . 🙂

differentreality · 2017-02-26T16:48:40Z

Please rebase!

differentreality · 2017-03-07T12:22:33Z

app/assets/javascripts/jquery.raty.js

@@ -0,0 +1,760 @@
+/*!


Why is this file here?

differentreality · 2017-03-07T12:25:30Z

app/models/event.rb

@@ -1,5 +1,6 @@
 class Event < ActiveRecord::Base
  include ActiveRecord::Transitions
+  ratyrate_rateable 'speaker_quality', 'competence', 'slides'


Can we make this custom? Meaning an organizer chooses the qualities to rate on.

differentreality · 2017-03-07T12:29:16Z

What about making this available on the admin side as well? Let's first decide on how to implement it, in a way that does not disrupt too much conferences using the current way of voting (and only want 1 point of voting, overall voting - not voting on multiple qualities).

shlok007 · 2017-03-08T12:44:21Z

This is what I propose:

The votable fields can be decided by organizers in a separate view admin/votes#index and admin/votes#new .

Screenshot of admin/votes#index :

Rake task to migrate the current votes from the admin side into a new field ~~overall rating~~ overall .
~~The new fields added could have an option to be global ( like questions ) for all conferences and hence could be enabled or disabled for each conference .~~
VotableFields would have a ~~has_and_belongs_to_many~~ has_many belongs_to association with conference .

All we need to do is specify the dimensions on which the proposals have to be rated upon. Ratyrate should take care of everything else.

differentreality · 2017-03-19T11:24:52Z

app/assets/javascripts/ratyrate.js.erb

@@ -0,0 +1,62 @@
+$.fn.raty.defaults.half = false;


Why do we need this file if we have the gem?

The gem seems to have included it by itself. I am wondering the same. Although, I'll give it a check if the gem still works the same when I get rid of it.

It doesn't seem to work without it.

differentreality · 2017-03-19T11:25:50Z

app/models/rate.rb

+  belongs_to :rater, class_name: 'User'
+  belongs_to :rateable, polymorphic: true
+
+  # attr_accessible :rate, :dimension


Why do we need this comment here?

I don't think we do. This is automatically generated code. I'll look up more for comments/things like these that can be safely removed. Although, I shouldn't change much except comments as I won't know reason things were introduced.

differentreality · 2017-03-19T11:26:24Z

Do we truly need to introduce all those new models?

shlok007 · 2017-03-20T21:41:04Z

app/models/votable_field.rb

+  validates :title, :votable_type, presence: true
+
+  # ratyrate does not allow criterias to have spaces in them
+  validate :no_spaces_in_title


Another solution could be to replace spaces with another character like underscore ( _ ) and convert them back to spaces while rendering them in proposals#show . But this could be a problem if the user actually defines a field that has a underscore.

We ask for proper names.

By proper names you mean names without spaces, the way it is currently done, right? 🙂

You commented on the code that ratyrate does not allow votable_fields to have spaces, so that's what's proper. No spaces. This is very similar to the validation we do for conference.short_title

Would it be better if I use regexp instead of this to check for no spaces ?

differentreality · 2017-03-21T10:41:35Z

app/models/votable_field.rb

+  end
+
+  def correct_votable_type
+    errors.add(:votable_type, "should be one of the following: 'Event'") unless ['Event'].include? votable_type


available votable_types should be in a variable

agreed .. 😅

differentreality · 2017-03-21T10:42:11Z

config/routes.rb

@@ -1,5 +1,6 @@
 Osem::Application.routes.draw do

+  post '/rate' => 'rater#create', :as => 'rate'


differentreality · 2017-03-24T14:56:51Z

So I guess the remaining things to take into account here are:

Enable the gem for the admin voting on events
Show results of current implemented voting in admin

shlok007 · 2017-03-26T05:04:07Z

It seems like ratyrate does not allow using same names ( dimension as they call it ) to vote for the same model. Therefore, it won't be possible to have the same field name for admin and non admin side.

Ticket in ratyrate to address the same: wazery/ratyrate#143 .
Official Documentation for ratyrate: https://www.sitepoint.com/ratyrate-add-rating-rails-app/

shlok007 · 2017-03-26T05:22:59Z

The two drawbacks of using this gem are:

Title for the votable fields cannot have spaces so fields like speaker's familiarity with subject which cannot be represented by a single word might cause a problem

Possible Solution
Replace spaces with _ or some other character in https://github.com/openSUSE/osem/pull/1204/files#diff-6ecd61439b970b0e5ae49be738861446R4 ( with this approach speaker competance and speaker_competance will be treated the same, we might have to take care of that as well )
We cannot have same field name to rate a model in admin and non admin side. ( eg: if the attendees have an option to rate on 'overall' then the admins can't have an 'overall' field, if they do the ratings of both will be combined together )

Possible Solution
Prepend admin_ to the votable fields that are created for the admin side voting by the admin.

In both the cases we have an option to just go with the default behaviour although, it might reduce the usability.

shlok007 · 2017-03-27T05:43:08Z

Gemfile

@@ -23,6 +23,9 @@ gem 'mysql2'
 # for observing records
 gem 'rails-observers'

+# for rating
+gem 'ratyrate', github: 'wazery/ratyrate', branch: 'master'


Something I am really unsure about. The released version of ratyrate contains a bug in the rating_for_user method that shows a particular user's rating. This method is required for blind voting. However, the master is quite stable and does not contain the bug. This could be a temporary change until we get a response regarding wazery/ratyrate#125 ( issue asking for a new version release )

Does using the gem from the master branch instead of the released version something we can agree upon?

Using the gem from master:

Using the released version:

The function to show current_user's vote renders only a single star in the released version which has been fixed in master but a new version hasn't been released yet.

What's this master branch version you guys are talking about? I'm trying the master, but keep comes the alpha, is there another file to download?

I'm having a lot of issues with this gem for helper rating_for_user. I can't record at overrall_averages and average_cache tables, as long record the rater_id at rate table.
The method on this file is also comment:
https://github.com/wazery/ratyrate/blob/master/lib/ratyrate/model.rb

tks

Using the master branch is as simple as using this line above in the Gemfile. It would be great if we can discuss this more on the issue itself rather than here. 🙂

shlok007 · 2017-04-11T00:07:59Z

Except tests, the work on this PR should be complete. Can it be please temporarily deployed to heroku at this stage for everyone else to review ?

differentreality · 2017-04-13T11:12:31Z

@shlok007 it would be useful if you added the necessary records in the review app.

differentreality · 2017-04-18T18:55:41Z

Please rebase, and feel free to squash those commits to make rebasing easier (you can leave a couple of commits, if you think that's useful).

fix blind voting added more tests

shlok007 · 2017-04-28T14:33:11Z

Documenting the new rating feature: https://docs.google.com/document/d/1sWImuP14RFeTWrs3nqdRZt8xfD0-xr0hBj10B2_KCQQ/edit?usp=sharing

shlok007 · 2017-04-28T18:07:51Z

app/assets/stylesheets/osem-rating.css.scss

-  width: 24px;
-  height: 24px;
-  display: inline-block;
+img.raty-cancel {


The cancel button doesn't cancel the rating but instead clicking on cancel changes the rating to '0' stars. This property is to prevent the cancel button from being displayed.

shlok007 · 2017-04-28T18:12:14Z

app/controllers/admin/votable_fields_controller.rb

+
+    private
+
+    def remove_rates


To delete the existing rating for a votable field so when a new field of same title is created, it does not have any previous rating present already.

differentreality · 2017-05-02T11:24:26Z

I am thinking that if we manually have to introduce the custom votable fields we want, then we might as well not use the gem, and just extend the capabilities of the existing voting options we have, and refactor where needed.

differentreality · 2017-05-02T08:45:43Z

spec/models/votable_field_spec.rb

+    end
+
+    it 'is not valid without a votable field' do
+      should validate_presence_of(:votable_type)


Use this format
https://github.com/shlok007/osem/blob/360a764cb4e04619d6615aa952747ed141f43517/spec/models/user_spec.rb#L25

differentreality · 2017-05-02T09:14:05Z

app/assets/javascripts/osem-rating.js

+      $(".disabled-rate .star").each(function(){
+          $(this).raty('readOnly', true);
+      });
+});


differentreality · 2017-05-02T09:57:15Z

app/models/conference.rb

@@ -22,7 +22,7 @@ class Conference < ActiveRecord::Base
  has_many :supporters, through: :ticket_purchases, source: :user
  has_many :tickets, dependent: :destroy
  has_many :resources, dependent: :destroy
-
+  has_many :votable_fields, dependent: :destroy


Shouldn't that be in Program?

Then probably we should move VotableField to be a program option instead of conference in all places including views and routes of the app as well. I should do that also. :)

differentreality · 2017-05-02T09:58:27Z

app/models/event.rb

@@ -1,5 +1,8 @@
 class Event < ActiveRecord::Base
  include ActiveRecord::Transitions
+
+  scope :vote, ->(votable_fields) { votable_fields.each { |field| ratyrate_rateable field.title } }


Perhaps we can name this better, as vote does not really convey the meaning of what we are doing here. votable_on ?

differentreality · 2017-05-02T09:59:48Z

app/models/program.rb

  end

  ##
+


Remove this line

differentreality · 2017-05-02T10:00:42Z

app/models/vote.rb

@@ -1,14 +1,2 @@
 class Vote < ActiveRecord::Base


Why are we keeping this?

There is a rake task to migrate old votes into new. The Vote model class is required for that task. If this class is removed we might have to write raw SQL inside the task to access the old votes data table.

There are other ways to do that. Eg. https://github.com/openSUSE/osem/blob/master/db/migrate/20140610165551_migrate_data_person_to_user.rb#L2

differentreality · 2017-05-02T11:27:10Z

app/views/admin/votable_fields/_form.html.haml

+      = f.input :title
+      = f.input :votable_type, collection: options_for_select(['Event'])
+      = f.input :stars, label: 'Add the total number of stars for the field'
+      = f.input :for_admin, label: 'Add this field for voting in admin side only'


How about we have 2 boolean options, one for admin namespace, one for attendees? That way we can have votable fields available only for admin, or only for attendees, or for both.

differentreality · 2017-05-02T11:38:21Z

app/views/admin/programs/_form.html.haml

-      = f.input :blind_voting, hint: 'Enable this feature if you do not want to show voting results and voters prior to user submitting a vote. For the feature to work you need to set the voting dates below as well'
-      = f.input :voting_start_date, as: :string, input_html: { id: 'datetimepicker-voting_start_date', readonly: true, value: (f.object.voting_start_date.to_formatted_s(:db_without_seconds) unless f.object.voting_start_date.nil?) }
-      = f.input :voting_end_date, as: :string, input_html: { id: 'datetimepicker-voting_start_date', readonly: true, value: (f.object.voting_end_date.to_formatted_s(:db_without_seconds) unless f.object.voting_end_date.nil?) }
+      = f.input :rating_enabled, label: 'Enable voting', hint: 'To enable voting you need to set voting dates as well'


Let's add a new section to separate the voting options from the rest.

bear454 · 2017-12-15T20:30:09Z

I would like to close this PR:

The discussion looks like we could better solve the problem with enhancements to Surveys #1100 .
There have been no changes in over half a year, and it's now very conflicted with master.

differentreality · 2017-12-15T21:41:57Z

What I had in mind was to keep the stars rating, but maybe we should indeed look into integrating something specifically for this reason into surveys.

Indeed it's been a while since we worked on this. Closing.

shlok007 commented Sep 23, 2016

View reviewed changes

Ana06 requested changes Jan 2, 2017

View reviewed changes

shlok007 force-pushed the implement-rating-of-events branch from 2799ad2 to c511f19 Compare February 27, 2017 16:47

differentreality reviewed Mar 7, 2017

View reviewed changes

app/assets/javascripts/jquery.raty.js

@@ -0,0 +1,760 @@

/*!

Copy link

Contributor

differentreality Mar 7, 2017

Choose a reason for hiding this comment

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

Why is this file here?

differentreality reviewed Mar 7, 2017

View reviewed changes

differentreality reviewed Mar 19, 2017

View reviewed changes

shlok007 changed the title ~~Implement Event Ratings~~ [WIP] Implement Event Ratings Mar 20, 2017

shlok007 commented Mar 20, 2017

View reviewed changes

differentreality reviewed Mar 21, 2017

View reviewed changes

shlok007 force-pushed the implement-rating-of-events branch from 8f8fa24 to b348de8 Compare March 23, 2017 13:50

hennevogel temporarily deployed to osem-demo-pr-1204 March 24, 2017 13:27 Inactive

hennevogel temporarily deployed to osem-demo-pr-1204 March 27, 2017 05:09 Inactive

shlok007 commented Mar 27, 2017

View reviewed changes

shlok007 force-pushed the implement-rating-of-events branch from b374e0d to 9ab074c Compare April 5, 2017 16:53

shlok007 force-pushed the implement-rating-of-events branch from 9e4d2ef to 6110406 Compare April 7, 2017 05:54

hennevogel temporarily deployed to osem-demo-pr-1204 April 13, 2017 10:36 Inactive

shlok007 force-pushed the implement-rating-of-events branch from 202b072 to 53d89f8 Compare April 24, 2017 04:38

shlok007 changed the title ~~[WIP] Implement Event Ratings~~ Implement Event Ratings Apr 27, 2017

shlok007 added 7 commits April 28, 2017 10:16

Implement Event Ratings

20f4853

remove cancel rating button

3db27d2

test for validating presence of voting dates

ebe5fb7

multiple changes

e92114a

fix blind voting added more tests

fix rubocop issue

e3174fa

introduce disabled-rate class to disable rating

e32ff99

fix proposal#show view

c07fd30

shlok007 force-pushed the implement-rating-of-events branch from edeadc4 to c07fd30 Compare April 28, 2017 14:29

shlok007 commented Apr 28, 2017

View reviewed changes

fix hakiri issue and added authorization

360a764

shlok007 force-pushed the implement-rating-of-events branch from bad1373 to 360a764 Compare April 30, 2017 11:07

differentreality reviewed May 2, 2017

View reviewed changes

bear454 added the Grooming label Dec 15, 2017

differentreality closed this Dec 15, 2017

		@@ -1,5 +1,6 @@
		Osem::Application.routes.draw do

		post '/rate' => 'rater#create', :as => 'rate'

Implement Event Ratings #1204

Implement Event Ratings #1204

Conversation

shlok007 commented Sep 22, 2016 • edited Loading

shlok007 commented Sep 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

differentreality commented Feb 2, 2017

differentreality commented Feb 12, 2017

shlok007 commented Feb 13, 2017

differentreality commented Feb 13, 2017

shlok007 commented Feb 16, 2017

shlok007 commented Feb 18, 2017 • edited Loading

differentreality commented Feb 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

differentreality commented Mar 7, 2017

shlok007 commented Mar 8, 2017 • edited Loading

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

differentreality commented Mar 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

differentreality Mar 21, 2017 • edited Loading

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

differentreality commented Mar 24, 2017

shlok007 commented Mar 26, 2017

shlok007 commented Mar 26, 2017

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

shlok007 commented Apr 11, 2017

differentreality commented Apr 13, 2017

differentreality commented Apr 18, 2017

shlok007 commented Apr 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

differentreality commented May 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shlok007 May 3, 2017 • edited Loading

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

bear454 commented Dec 15, 2017

differentreality commented Dec 15, 2017

shlok007 commented Sep 22, 2016 •

edited

Loading

shlok007 commented Feb 18, 2017 •

edited

Loading

shlok007 commented Mar 8, 2017 •

edited

Loading

differentreality Mar 21, 2017 •

edited

Loading

shlok007 May 3, 2017 •

edited

Loading