fix dismissing opportunities in list view

I added assignment_id to the PlannerOverride json so that we can easily
link a plannable to its associated assignment, if it has one. Before we
were just assuming that the override was on the assignment, but now
we've standardized to have the override on the assignment's associated
object, if it has one. Since opportunities loads assignments, we needed
a way to easily navigate from the override back to the assignment so the
opportunities menu could update the proper data.

fixes ADMIN-1214

test plan:
- Create a student with missing assignments of several types:
  - regular assignments
  - discussions
  - quizzes
- The student should be able to dismiss all of these types of
assignments from the opportunities menu
- Undismiss the opportunities in the rails console so they show up again
  for example:
  PlannerOverride.
    where(user: <student>, dismissed: true).
    update_all(dismissed: false)
- This will repopulate the opportunities drop down. The student should
  still be able to dismiss all the opportunities.
- Dismiss opportunities very quickly, as described in the Jira ticket.
  There should be no errors.

Change-Id: Ia96dd9d8c970e330d07b5a185a4f8b1195a7ee43
Reviewed-on: https://gerrit.instructure.com/161248
Reviewed-by: Mysti Sadler <mysti@instructure.com>
Tested-by: Jenkins
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Mysti Sadler <mysti@instructure.com>
This commit is contained in:
Jon Willesen 2018-08-17 16:05:56 -06:00 committed by Mysti Sadler
parent 48a488aec6
commit 0866f2d9dd
8 changed files with 77 additions and 7 deletions

View File

@ -46,6 +46,11 @@
# "example": 1578941,
# "type": "integer"
# },
# "assignment_id": {
# "description": "The id of the plannable's associated assignment, if it has one",
# "example": 1578941,
# "type": "integer"
# },
# "workflow_state": {
# "description": "The current published state of the item, synced with the associated object",
# "example": "published",

View File

@ -69,6 +69,11 @@ class PlannerOverride < ActiveRecord::Base
end
end
def associated_assignment_id
return self.plannable.id if self.plannable_type == 'Assignment'
self.plannable.assignment_id if self.plannable.respond_to? :assignment_id
end
def self.update_for(obj)
overrides = PlannerOverride.where(plannable_id: obj.id, plannable_type: obj.class.to_s)
overrides.update_all(workflow_state: plannable_workflow_state(obj)) if overrides.exists?

View File

@ -25,6 +25,7 @@ module Api::V1::PlannerOverride
json = api_json(override, user, session)
type = override.plannable.type if override.plannable_type == 'DiscussionTopic' && type.nil?
json['plannable_type'] = PLANNABLE_TYPES.key(type || json['plannable_type'])
json['assignment_id'] = override.associated_assignment_id
json
end
end

View File

@ -44,6 +44,11 @@ module Plannable
end
def planner_override_for(user)
if self.respond_to? :submittable_object
submittable_override = self.submittable_object&.planner_override_for(user)
return submittable_override if submittable_override
end
if self.association(:planner_overrides).loaded?
self.planner_overrides.find{|po| po.user_id == user.id && po.workflow_state != 'deleted'}
else

View File

@ -23,9 +23,10 @@ function basicOpportunity (option) {
course_id: "1",
name: "always something",
planner_override:{
id:"1",
plannable_type:"Assignment",
plannable_id:"6",
id:"7",
plannable_type:"discussion_topic",
plannable_id:"5",
assignment_id:"6",
user_id:"3",
workflow_state:"active",
marked_complete: false,
@ -68,7 +69,7 @@ it('discards duplicate items on ADD_OPPORTUNITIES', () => {
expect(newState.items.length).toBe(2);
});
it('updates state correctly on DISMISSED_OPPORTUNITY with opportunity that has overide', () => {
it('updates state correctly on DISMISSED_OPPORTUNITY with opportunity that has override', () => {
const initialState = {
items: [basicOpportunity()],
nextUrl: null
@ -76,7 +77,7 @@ it('updates state correctly on DISMISSED_OPPORTUNITY with opportunity that has o
const newState = opportunitiesReducer(initialState, {
type: 'DISMISSED_OPPORTUNITY',
payload: {id: "6", marked_complete: false, plannable_id: "6", dismissed: true}
payload: {id: "6", marked_complete: false, assignment_id: "6", dismissed: true}
});
expect(newState.items[0].planner_override.dismissed).toBe(true);
@ -92,7 +93,7 @@ it('adds to opportunity object if no planner override DISMISSED_OPPORTUNITY', ()
const newState = opportunitiesReducer(initialState, {
type: 'DISMISSED_OPPORTUNITY',
payload: {id: "6", marked_complete: false, plannable_id: "6", dismissed: true}
payload: {id: "6", marked_complete: false, assignment_id: "6", dismissed: true}
});
expect(newState.items[0].planner_override.dismissed).toBe(true);

View File

@ -34,7 +34,9 @@ export default handleActions({
ADD_OPPORTUNITIES: setOpportunityState,
DISMISSED_OPPORTUNITY: (state, action) => {
let stateCopy = cloneDeep(state);
let dismissedOpportunity = stateCopy.items.find((opportunity) => opportunity.id === action.payload.plannable_id + "");
let dismissedOpportunity = stateCopy.items.find(
opportunity => opportunity.id === action.payload.assignment_id
);
if (dismissedOpportunity.planner_override) {
dismissedOpportunity.planner_override.dismissed = action.payload.dismissed;
} else {

View File

@ -39,5 +39,26 @@ describe Api::V1::PlannerOverride do
json = api.planner_override_json(po, @student, session)
expect(json['plannable_type']).to eq 'assignment'
end
it 'identifies an assignment and populates the assignment_id' do
assignment_model
po = @assignment.planner_overrides.create!(user: @student)
json = api.planner_override_json(po, @student, session)
expect(json['assignment_id']).to eq @assignment.id
end
it 'identifies a submittable linked to an assignment and populate the assignment_id' do
assignment_model(submission_types: 'discussion_topic')
po = @assignment.discussion_topic.planner_overrides.create!(user: @student)
json = api.planner_override_json(po, @student, session)
expect(json['assignment_id']).to eq @assignment.id
end
it 'leaves assignment_id null if there is no associated assignment' do
topic = discussion_topic_model
po = topic.planner_overrides.create!(user: @student)
json = api.planner_override_json(po, @student, session)
expect(json['assignment_id']).to be_nil
end
end
end

View File

@ -24,6 +24,36 @@ describe Plannable do
course_with_student(active_all: true)
end
it "returns a regular assignment's override" do
assignment = assignment_model
override = assignment.planner_overrides.create!(user: @student)
expect(assignment.planner_override_for(@student)).to eq override
end
it "returns the assignment's associated override" do
assignment = assignment_model(submission_types: 'discussion_topic')
discussion = assignment.discussion_topic
discussion_override = discussion.planner_overrides.create!(user: @student)
expect(assignment.planner_override_for(@student)).to eq discussion_override
end
it "returns the assignment's override if the associated object does not have an override" do
assignment = assignment_model()
assignment_override = assignment.planner_overrides.create!(user: @student)
assignment.submission_types = "discussion_topic"
assignment.save!
expect(assignment.planner_override_for(@student)).to eq assignment_override
end
it "prefers the associated object's override if both have an override" do
assignment = assignment_model()
assignment_override = assignment.planner_overrides.create!(user: @student)
assignment.submission_types = "discussion_topic"
assignment.save!
discussion_override = assignment.discussion_topic.planner_overrides.create!(user: @student)
expect(assignment.planner_override_for(@student)).to eq discussion_override
end
it 'should not return deleted overrides' do
assignment = assignment_model
override = assignment.planner_overrides.create!(user: @student)