add explicit poll choice ordering and poll_session deletion

fixes CNVS-13521,CNVS-13489
This commit adds 3 things.  One is a fix for deleting a poll_session when
there are poll_submissions created already for the poll_session. The
second is the addition of a new attribute on poll_choices called
'position'.
The 'position' attribute specifies the order a poll choice should be
returned in. The third is the visibility of poll sessions to students:
as long as a student is enrolled in the course (and course section, if
it exists on the poll session) associated with the poll session, they
can access it, whether or not it's open or closed.

Test plans
1. Deleting a poll session
As a teacher
- Create a poll and related poll session
- Open the poll session for submissions
As a student
- Create a submission for the newly created poll session
As a teacher
- You should be able to delete the poll session

2. Position ordering attribute
As a teacher
- Create a poll
- Create poll choices, passing along the 'position' attribute to specify
  ordering (try to mix these up, e.g. create a poll choice with a
  position
  of 2, then a poll choice with an position of 4, then a poll choice with
  an position of 3, then a poll choice with an position of 1)
- Retrieve the poll choices for the poll with the poll choices index
  endpoint
- The poll should be returned in the order they were specified using the
  'position' attribute

3. Poll session visibility to students
As a teacher
- Create a poll
- Create an associated poll session with a valid course
As a student enrolled in the given course
- You should be able to view the poll session whether or not it's open
  or closed.
As a student not enrolled in the given course
- You should not be able to view the poll session, whether or not it's
  open or closed.

Change-Id: Ifabd64540d843c83f3a22fb6668c4fcac8f728b0
Reviewed-on: https://gerrit.instructure.com/36087
Reviewed-by: Derek DeVries <ddevries@instructure.com>
QA-Review: Trevor deHaan <tdehaan@instructure.com>
Product-Review: Bryan Madsen <bryan@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Josh Simpson <jsimpson@instructure.com>

app/controllers/polling/poll_choices_controller.rb

@@ -44,6 +44,11 @@ module Polling
   #         "description": "The text of the poll choice.",
   #         "type": "string",
   #         "example": "Choice A"
+  #       },
+  #       "position": {
+  #         "description": "The order of the poll choice in relation to it's sibling poll choices.",
+  #         "type": "integer",
+  #         "example": 1
   #       }
   #     }
   #   }
@@ -101,6 +106,9 @@ module Polling
     # @argument poll_choices[][is_correct] [Optional, Boolean]
     #   Whether this poll choice is considered correct or not. Defaults to false.
     #
+    # @argument poll_choices[][position] [Optional, Integer]
+    #   The order this poll choice should be returned in the context it's sibling poll choices.
+    #
     # @example_response
     #   {
     #     "poll_choices": [PollChoice]
@@ -131,6 +139,9 @@ module Polling
     # @argument poll_choices[][is_correct] [Optional, Boolean]
     #   Whether this poll choice is considered correct or not.  Defaults to false.
     #
+    # @argument poll_choices[][position] [Optional, Integer]
+    #   The order this poll choice should be returned in the context it's sibling poll choices.
+    #
     # @example_response
     #   {
     #     "poll_choices": [PollChoice]

app/controllers/polling/poll_sessions_controller.rb

@@ -23,7 +23,7 @@ module Polling
   # @model PollSession
   #   {
   #     "id": "PollSession",
-  #     "required": ["id", "poll_id", "course_id", "course_section_id"],
+  #     "required": ["id", "poll_id", "course_id"],
   #     "properties": {
   #       "id": {
   #         "description": "The unique identifier for the poll session.",

app/models/polling/poll.rb

@@ -21,7 +21,7 @@ module Polling
     attr_accessible :user, :question, :description
 
     belongs_to :user
-    has_many :poll_choices, class_name: 'Polling::PollChoice', dependent: :destroy
+    has_many :poll_choices, class_name: 'Polling::PollChoice', dependent: :destroy, order: :position
     has_many :poll_submissions, class_name: 'Polling::PollSubmission', dependent: :destroy
     has_many :poll_sessions, class_name: 'Polling::PollSession', dependent: :destroy
 

app/models/polling/poll_choice.rb

@@ -18,7 +18,9 @@
 
 module Polling
   class PollChoice < ActiveRecord::Base
-    attr_accessible :text, :poll, :is_correct
+    set_table_name 'polling_poll_choices'
+
+    attr_accessible :text, :poll, :is_correct, :position
 
     belongs_to :poll, class_name: 'Polling::Poll'
     has_many :poll_submissions, class_name: 'Polling::PollSubmission', dependent: :destroy

app/models/polling/poll_session.rb

@@ -23,7 +23,7 @@ module Polling
     belongs_to :course
     belongs_to :course_section
     belongs_to :poll, class_name: 'Polling::Poll'
-    has_many :poll_submissions, class_name: 'Polling::PollSubmission'
+    has_many :poll_submissions, class_name: 'Polling::PollSubmission', dependent: :destroy
     validates_presence_of :poll, :course
     validate :section_belongs_to_course
 
@@ -34,11 +34,14 @@ module Polling
       can :read and can :create and can :delete and can :publish
 
       given do |user, session|
-        self.cached_context_grants_right?(user, session, :read) &&
-          self.is_published? &&
-          (self.course_section ? self.course_section.grants_right?(user, session, :read) : true)
+        self.visible_to?(user, session)
       end
-      can :read and can :submit
+      can :read
+
+      given do |user, session|
+        self.visible_to?(user, session) && self.is_published?
+      end
+      can :submit
     end
 
     def self.available_for(user)
@@ -65,6 +68,11 @@ module Polling
       save!
     end
 
+    def visible_to?(user, session)
+      self.cached_context_grants_right?(user, session, :read) &&
+      (self.course_section ? self.course_section.grants_right?(user, session, :read) : true)
+    end
+
     private
     def section_belongs_to_course
       if self.course && self.course_section

app/serializers/polling/poll_choice_serializer.rb

@@ -2,7 +2,7 @@ module Polling
   class PollChoiceSerializer < Canvas::APISerializer
     root :poll_choice
 
-    attributes :id, :text, :is_correct
+    attributes :id, :text, :is_correct, :position
 
     has_one :poll, embed: :id
 
@@ -31,7 +31,7 @@ module Polling
     end
 
     def student_keys
-      keys = [:id, :text]
+      keys = [:id, :text, :position]
       keys << :is_correct if poll.closed_and_viewable_for?(current_user)
       keys
     end

db/migrate/20140606220920_add_position_column_to_poll_choices.rb

@@ -0,0 +1,11 @@
+class AddPositionColumnToPollChoices < ActiveRecord::Migration
+  tag :predeploy
+
+  def self.up
+    add_column :polling_poll_choices, :position, :integer
+  end
+
+  def self.down
+    remove_column :polling_poll_choices, :position
+  end
+end

spec/apis/v1/polling/poll_choices_api_spec.rb

@@ -26,9 +26,11 @@ describe Polling::PollChoicesController, type: :request do
   describe 'GET index' do
     before(:each) do
       @poll = @teacher.polls.create!(question: "Example Poll")
-      5.times do |n|
-        @poll.poll_choices.create!(text: "Poll Choice #{n+1}", is_correct: false)
-      end
+      @poll.poll_choices.create!(text: "Poll Choice 1", is_correct:false, position: 1)
+      @poll.poll_choices.create!(text: "Poll Choice 3", is_correct:false, position: 3)
+      @poll.poll_choices.create!(text: "Poll Choice 4", is_correct:false, position: 4)
+      @poll.poll_choices.create!(text: "Poll Choice 2", is_correct:false, position: 2)
+      @poll.poll_choices.create!(text: "Poll Choice 5", is_correct:false, position: 5)
     end
 
     def get_index(raw = false, data = {})
@@ -46,10 +48,20 @@ describe Polling::PollChoicesController, type: :request do
       poll_choices_json.size.should == 5
 
       poll_choices_json.each_with_index do |pc, i|
-        pc['text'].should == "Poll Choice #{5-i}"
+        pc['text'].should == "Poll Choice #{i+1}"
       end
     end
 
+    it "returns the poll choices in the correct order" do
+      json = get_index
+      poll_choices_json = json['poll_choices']
+
+      poll_choices_json.each_with_index do |pc, i|
+        pc['position'].should == i+1
+      end
+    end
+
+
     context "as a student" do
       before(:each) do
         student_in_course(:active_all => true, :course => @course)
@@ -159,12 +171,13 @@ describe Polling::PollChoicesController, type: :request do
 
     context "as a teacher" do
       it "creates a poll choice successfully" do
-        post_create(text: 'Poll Choice 1', is_correct: false)
+        post_create(text: 'Poll Choice 1', is_correct: false, position: 1)
         @poll.poll_choices.first.text.should == 'Poll Choice 1'
+        @poll.poll_choices.first.position.should == 1
       end
 
       it "sets is_correct to false if is_correct is provided but blank" do
-        post_create(text: 'is correct poll choice', is_correct: '')
+        post_create(text: 'is correct poll choice', is_correct: '', position: 1)
         @poll.poll_choices.first.text.should == 'is correct poll choice'
         @poll.poll_choices.first.is_correct.should be_false
       end

spec/apis/v1/polling/poll_sessions_api_spec.rb

@@ -136,6 +136,20 @@ describe Polling::PollSessionsController, type: :request do
     end
 
     context "as a student" do
+      it "doesn't display if the student isn't enrolled in the associated course or course section" do
+        section = @course.course_sections.create!(name: 'Some Course Section')
+        @poll_session.course_section = section
+        @poll_session.save
+
+        student_in_course(active_all: true, course: @course)
+
+        get_show(true)
+
+        response.code.should == '401'
+        @poll_session.reload
+        @poll_session.poll_submissions.size.should be_zero
+      end
+
       it "returns has_submitted as true if the student has made a submission" do
         choice = @poll.poll_choices.create!(text: 'Choice A', is_correct: true)
         submission = create_submission(choice)

spec/apis/v1/polling/polls_api_spec.rb

@@ -66,9 +66,6 @@ describe Polling::PollsController, type: :request do
   describe 'GET show' do
     before(:each) do
       @poll = @teacher.polls.create!(question: 'An Example Poll')
-      5.times do |n|
-        @poll.poll_choices.create!(text: "Poll Choice #{n+1}", is_correct: false)
-      end
     end
 
     def get_show(raw = false, data = {})
@@ -114,7 +111,8 @@ describe Polling::PollsController, type: :request do
 
       it "shouldn't return the id of the user that created the poll" do
         student_in_course(:active_all => true, :course => @course)
-        @poll.poll_sessions.create!(course: @course)
+        session = @poll.poll_sessions.create!(course: @course)
+        session.publish!
 
         json = get_show
         poll_json = json['polls'].first