closes AE-30
flag=none
test plan:
- verify Canvas boots in CD
- verify no influx of new errors in CD
[fsc-timeout=30]
Change-Id: Ifa04bebe1b09f01c6d3b8b2d8f3bb424759730f5
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/308067
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Isaac Moore <isaac.moore@instructure.com>
Build-Review: James Butters <jbutters@instructure.com>
Change-Id: Iad400936d7e53a5f92644f260c95bfb5bf9e972f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/293144
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: August Thornton <august@instructure.com>
Reviewed-by: Alex Slaughter <aslaughter@instructure.com>
Migration-Review: Alex Slaughter <aslaughter@instructure.com>
refs DE-1084
Prior to this change, an unlimited number of ScoreStatisticsGenerator
jobs could be run at once, which had the potential to overload the DB.
After this change, the number of simultaneously running jobs is capped
at 1 per root account by default. In order to change this, run:
```
Setting.set("ScoreStatisticsGenerator_num_strands", 10)
```
Change-Id: Ib9a24a04134c3ec0d621b2a81b1366541afc8ece
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/287069
QA-Review: Aaron Ogata <aogata@instructure.com>
Product-Review: Aaron Ogata <aogata@instructure.com>
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
[skip-stages=Flakey]
Naming/HeredocDelimiterNaming and Rails/SquishedSQLHeredocs
the former was manual, the latter was automatic. I also changed
some <<- to <<~ to allow for better formatting
I also had to change comments inside squished SQL heredocs to
be block comments (since newlines are removed); searching for those
I found some multi-line strings that are better as heredocs
Change-Id: I6b138f8e32544b97df1e4c56f09ee5316cbdef9d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278184
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
the balance. mostly. Lint/UriEscapeUnescape is put in the pending
block because it's so touchy, and I didn't want to deal with it
right now
all manual
Change-Id: Ibeb81e013f56f160d51f7d237a9bcfe98daa1e53
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277569
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
closes EVAL-1333
flag=grade_calc_ignore_unposted_anonymous
Test Plan:
1. Enable the 'Ignore Unposted Anonymous Assignments in Grade
Calculation' release flag. Create an anonymous assignment and
grade some students.
2. Before posting the assignment, export the gradebook. Verify:
- The CSV includes a column for the anonymous assignment.
- The CSV totals (assignment group and final grade) exclude the
scores for the anonymous assignment in their calculations.
3. Disable the 'Ignore Unposted Anonymous Assignments in Grade
Calculation' release flag.
4. Re-export the gradebook. Verify:
- The CSV includes a column for the anonymous assignment.
- The CSV totals (assignment group and final grade) include the
scores for the anonymous assignment in their calculations.
5. Post the assignment to students. Then export the gradebook again and
verify the exported CSV includes a column for the anonymous
assigment the totals include the scores for the anonymous
assignment.
6. Re-enable the 'Ignore Unposted Anonymous Assignments in Grade
Calculation' release flag and re-export the Gradebook. Verify the
exported CSV includes a column for the anonymous assigment the
totals include the scores for the anonymous assignment.
Change-Id: If83237344ee16c9f6621c5d58cfb49e2da099f7b
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/254081
Product-Review: Syed Hussain <shussain@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
This catches expected exceptions from the GradeCalculator and
DueDateCacher, logs them, and then signals to delayed jobs to retry.
test plan:
- specs pass
Change-Id: I7851c1df59c43facd621c7aab7fbc29fd6ff6fd9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/254962
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Syed Hussain <shussain@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
closes EVAL-1272
flag=grade_calc_ignore_unposted_anonymous
Test Plan:
1. Enable the 'Ignore Unposted Anonymous Assignments in Grade
Calculation' release flag. Create an anonymous assignment and
grade some students.
2. Before posting the assignment, enter a rails console and check the
unposted_* scores for the students. Verify:
- The unposted_* scores do not include the submission score from the
anonymous unposted assignment.
3. Disable the 'Ignore Unposted Anonymous Assignments in Grade
Calculation' release flag.
4. Enter a rails console again and check the unposted_* scores for the
students. Verify:
- The unposted_* scores include the submission score from the
anonymous unposted assignment.
5. Post the assignment to students. Then enter a rails console again and
verify the unposted_* scores for the students include the submission
score from the anonymous (now posted) assignment.
6. Re-enable the 'Ignore Unposted Anonymous Assignments in Grade
Calculation' release flag. Verify the unposted_* scores for the
students include the submission score from the anonymous (now posted)
assignment.
Change-Id: Ibb42b9f3164c08e3bc2a21fc02736760f58af51a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/251786
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Syed Hussain <shussain@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
QA-Review: Kai Bjorkman <kbjorkman@instructure.com>
fixes FOO-1201
Change-Id: I96d43545cf165bec3603430acab0ef47fe9f20c4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/254170
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Switching to postgres 12 exposed some float addition that was resulting
in long precision. It looks like 9.5 was hiding that helpfully, but the
behavior isn't guaranteed and somewhere from 9.5 to 12 had changes in
that regard.
fixes EVAL-1104
flag=none
Test Plan
- Have postgres12.
- Have 2 assignments worth 1000 points each.
- Give a grade of 142.7 and a grade of 99.6.
- Verify in a console that there isn't floating point weirdness for the
user's scores.
Change-Id: Ic2c85a12909a3c99d7a58351ca9b4f0df4ff656e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/245252
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Syed Hussain <shussain@instructure.com>
Reviewed-by: Kai Bjorkman <kbjorkman@instructure.com>
QA-Review: Kai Bjorkman <kbjorkman@instructure.com>
Product-Review: Syed Hussain <shussain@instructure.com>
closes TALLY-704
flag=none
Test Plan:
1. Create a brand-new course and add some students.
2. Verify that all scores associated with the course have a
root_account_id set:
c = Course.find(<id>)
scores = Score.where(enrollment: c.all_enrollments)
scores.pluck(:root_account_id) # should all be set (not null)
Change-Id: I549d82c40a2057cc1fdcff604b5e5e7a9285f0ac
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/239417
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Syed Hussain <shussain@instructure.com>
Product-Review: Syed Hussain <shussain@instructure.com>
This caches the averages for the teacher's grade as part of the score
statistics generator. This rewrite removes the need for a course-level
cache busting as it was only used
closes TALLY-851
flag=none
test plan:
- Have a few courses with a few assignments and a few students
- As a teacher, grade the students
- As a teacher, go to /grades
- Ensure values for all the teacher's courses appear
- Ensure the values are correct (i.e. the average of each courses
current scores for all active and invited enrollments)
Change-Id: Ieeb5ddb734d8bc21f40196014d062b9ba5935ebd
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/234109
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
We invalidate the course cache by touching the course too often. This
can cause issues in the database at scale. This patchset only touches
the course once after all the calculator work is done.
closes TALLY-849
test plan:
- specs pass
Change-Id: Id006b9618ab89c003bd2d4b354247036dca9ce09
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/234082
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
closes TALLY-792
flag=grade_calculator_performance_improvements
Fixes N+1s around observers, observer alerts, and observer alert
thresholds.
Test Plan:
1. Enable the 'Grade Calculator Performance Improvements' release
flag at /accounts/site_admin/settings
2. Smoke test to make sure observers receive 'course_grade_high'
and 'course_grade_low' alerts when an observee gets graded and the
grade is above or below the set threshold, respectively.
3. Disable the 'Grade Calculator Performance Improvements' release
flag at /accounts/site_admin/settings. Repeat step 2.
Change-Id: Ie9c3139006fed9eeec9af0aababad87bb6adb8df
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/232210
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Keith Garner <kgarner@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Spencer Olson <solson@instructure.com>
Fires any time a student's course grade has changed.
Dig needs this event.
This also updates ObserverAlerts to only make one alert per
student+course in the case that the student is enrolled in multiple
sections of the same course. (Checked with mobile team to make sure
this is OK).
It also moves creating the alerts until after calculate_hidden_scores.
This shouldn't matter because the hidden (unposted) scores don't affect
the ObserverAlerts which only care about current_score. (Really we
could ignore the observer alert code if @ignore_muted is true
but I didn't want to change too much. Likewise, @ignore_muted
and @emit_live_event should be synonymous but I wanted to be explicit
and not rely on that always being the case.)
This also reloads scores all at once instead of individually, which
means less SQL queries and simplifies the code (especially since
reloading an object undoes the preloading of the enrollment).
refs PLAT-5189
flag=none
Test plan:
- Add `puts JSON.pretty_generate(event)` to
gems/live_events/lib/live_events/client.rb:103 (#post_event) to see
events locally
- watch for live events when going through various workflows which
change course grades and make sure only one event per course+student is
emitted
1. change an assignment grade for a student enrolled in multiple
sections -- only one event should be triggered
2. group assignments
3. a call to compute_and_save_scores where
@update_all_grading_period_scores is true (we start grading a
course/student and this kicks off scoring of all grading period
scores)
4. a call to compute_and_save_scores where @update_course_score is
true (we start by scoring a grading period and this starts scoring the
related student/course)
5. any thing else we can think of. grade_calculator experts or other
product experts may be able to suggest other scenarios.
- test these scenarios out with the "observer alerts" in the Canvas
Parent mobile app to make sure these events still work right.
Change-Id: I1d6d530149962a97890656566e26bc32dbb4c190
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/219208
Tested-by: Jenkins
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Oxana Jurosevic
Reviewed-by: Matt Sessions <msessions@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
QA-Review: Tucker Mcknight <tmcknight@instructure.com>
According to sentry removing the transaction made the deadlock problem worse. Reverting until a new approach can be had.
test plan:
- grade calculator specs pass
This reverts commit 8aacb717cc.
Change-Id: I7899641b6e6f8430fa343bf9f1bd4ef6fe9822ea
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/215436
Tested-by: Jenkins
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
Reviewed-by: Nick Pitrak <npitrak@instructure.com>
Removes a database transaction in the grade calculator that is scoped
around too much work. The transaction was originally added before
postgres's upsert capability for two statements. Over time, more and
more work was added leading to deadlocks in production.
test plan:
- spec pass
Change-Id: Ia95035be6934b148f8d836bf4419f978a0b45918
Reviewed-on: https://gerrit.instructure.com/212873
Reviewed-by: Spencer Olson <solson@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
Tested-by: Jenkins
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
When the grade calculator attempts to apply drop rules, don't
automatically exempt muted assignments (or unposted submissions in the
case of Post Policies) from being dropped. Continue treating them as 0
points (if computing final score) or ignoring them altogether (if
computing posted score), but generally allow them to be dropped. This
fixes a calculation issue that could occur for an assignment group with
drop rules: muted/unposted submissions that should have been dropped
completely were still included (but treated as 0 points), with the
result that the group's points_possible value was higher than could ever
have been attained given the drop rules in place.
fixes GRADE-2356
Test plan:
- Have a course with New Gradebook and Post Policies enabled
- Set up an assignment group
- Add 2 assignments worth 10 points each
- Set a drop rule to drop the lowest assignment
- Open Gradebook
- Grade a student on the first assignment but not the second
- Export to CSV
- The "final score" column should reflect just the one graded
assignment and should not treat the ungraded assignment as 0
- Repeat this setup in an Old Gradebook course
- Mute the second assignment (to make it "unposted")
- Export to CSV
- The "final score" column should be the same as above
Change-Id: I91903d481fcbedf60180e5dcabe96106308586e9
Reviewed-on: https://gerrit.instructure.com/205856
Product-Review: Keith Garner <kgarner@instructure.com>
Tested-by: Jenkins
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
QA-Review: Derek Bender <djbender@instructure.com>
- A variable named "submission" was actually a hash containing (among
other things) a submission and an assignment, so checking
submission.posted? was not okay
- Moreover, the submission that said submission-hash contained could
have been set to nil if the submission had not yet been posted (or, in
the pre-post-policies world, the assignment was muted). This wasn't an
issue before, but became one since with post policies we check
individual submissions.
fixes GRADE-2268
Test plan:
- Specs pass
- Enable post policies/new gradebook
- Have a course with assignment groups and a drop rule (at least one
"drop highest" or "drop lowest")
- Assign some grades
- Grade calculator should not cause an error
Change-Id: I3a94e7ee4808cc6f38f247b6ee9652ddf6e7f6d0
Reviewed-on: https://gerrit.instructure.com/199187
Tested-by: Jenkins
Reviewed-by: Keith Garner <kgarner@instructure.com>
QA-Review: Adrian Packel <apackel@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
Convert post policies from a garden-variety feature flag into a global
setting and remove the old feature flag. Add some helper methods to
handle enabling and disabling, and have everywhere in the code that
references the feature use the new helper methods instead. Also, have
the activation of post policies for a given course depend on whether new
gradebook is active for that course.
closes GRADE-1974
Test plan:
Note that the Post Policies feature flag no longer exists, and instead
there's a setting you'll need to turn on. You can do so in the console
using:
> PostPolicy.enable_feature!
To disable it:
> PostPolicy.disabled_feature!
Note that, even when enabled, it should only apply to courses that have
new gradebook turned on.
- With the new setting ENABLED:
- Courses with new gradebook behave as though post policies is enabled
- Do a bit of smoke testing to see if stuff like posting/hiding and
changing assignment posting policies hasn't broken
- Courses with old gradebook do not
- For example, calling a mutation like postAssignmentGrades on an OG
course should return an error indicating the feature isn't enabled
- With the new setting DISABLED:
- Courses with new gradebook behave as though post policies is
disabled (e.g., old-style muting is active)
- Calling a mutation, as described above, should return an error
Change-Id: I5e223d2c4ca4202cce0641f316ecaa505a66298c
Reviewed-on: https://gerrit.instructure.com/196062
Tested-by: Jenkins
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
QA-Review: Derek Bender <djbender@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
When Post Policies is enabled, the submission details page will check
that the submission is posted. Otherwise, it checks if the assignment
is muted.
closes GRADE-1995
Test Plan
- Enable Post Policies.
- Create an assignment graded with a rubric.
- Grade a submission via the rubric.
- Leave comments on the submission as the teacher.
- Leave comments on the submission as the student.
- Hide the submission.
- As a teacher, verify that you can see the rubric assessment and all
comments.
- As the student, verify that you can see a blank rubric assessment
and only the student's own comments.
- Post submissions. As both the teacher and student, verify that you
can see all comments, rubric assessment, and the grade.
- Verify that when Post Policies is off, the assignment muted state
is what determines the above, rather than the submission's posted
state.
Change-Id: Id1662f43c24250a0ed6750a1f83f81ff5952ad95
Reviewed-on: https://gerrit.instructure.com/196253
Tested-by: Jenkins
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Jonathan Fenton <jfenton@instructure.com>
This fixes a very specific corner case in the grade calculator. When
drop rules eliminated all pointed assignments in the drop highest
phase and ended up with only unpointed assignments being considered in
the dropped lowest phase the grade calculator would error out as it
attempted to subtract from nil. Since we can't do the drop math that
is expected in this phase, the grade calculator will act similar to
dropping for all unpointed assignments.
fixes GRADE-2209
test plan:
- (For this test plan you'll need to look at the canvas logs)
- Create an course with a student
- In the default assignment group, create three assignments, 2 of
which are 0 points possible and 1 with some number of points
possible.
- Adjust the assignment group drop rules to drop one highest
and one lowest
- Score the assignments
- Confirm in the logs that none of the runs of the grade calculator
crashed with the error "undefined method `+' for nil:NilClass"
- Confirm the course score for the student is nil
Change-Id: I10ac06d5b99b5c328b0b509902268af57bfc0c37
Reviewed-on: https://gerrit.instructure.com/193580
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
QA-Review: Adrian Packel <apackel@instructure.com>
Tested-by: Jenkins
Product-Review: Keith Garner <kgarner@instructure.com>
ff83302bb0 stopped populating the cache with assignment stat
data, however, the code to invalidate the cache remained just burning
time. This removes the function to invalidate the cache and the code
that calls it.
test plan:
- specs pass
- Confirm grade calculator and student grades page still function.
Change-Id: I5bcd29359addfd9d46f161c74d2e433417914a4f
Reviewed-on: https://gerrit.instructure.com/193869
Reviewed-by: Adrian Packel <apackel@instructure.com>
Tested-by: Jenkins
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
If post policies is enabled, have the grade calculator, rather than
ignoring all submissions for muted assignments, ignore submissions that
have not been posted. Also make sure we recalculate grades when
posting/hiding submissions.
closes GRADE-36
Test plan:
- Enable post policies
- Create some assignments, make at least one manually-posted
- Assign some scores
- When you post or hide submissions, the Score objects associated
with the students/enrollments should be updated
- The computed_ values should not reflect unposted submissions
- The unposted_ values should reflect them
- In a course with post policies disabled
- Make sure muting/unmuting assignments kicks off the grade
calculator (and it calculates as expected)
Change-Id: If08fbfa090cf0174f9f6ee306a3d5ecec81351fc
Reviewed-on: https://gerrit.instructure.com/192075
Tested-by: Jenkins
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Keith Garner <kgarner@instructure.com>
QA-Review: James Butters <jbutters@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
Have the grade calculator update the scores and score_metadata tables
using a single INSERT ... ON CONFLICT DO UPDATE statement rather than
consecutive UPDATE and INSERT statements. This will (we hope) prevent
separate invocations of the grade calculator from stepping on each
other's toes and interrupting calculations midway through due to
constraint errors.
fixes GRADE-2163
Test plan:
- Specs pass
- Set up a course with some assignment groups and grading periods
- Assign some grades
- Check that scores are being calculated properly and no errors
Change-Id: I894d5040f8a536c4b91d344dffd246fb0d9a6311
Reviewed-on: https://gerrit.instructure.com/192330
Tested-by: Jenkins
Reviewed-by: Keith Garner <kgarner@instructure.com>
QA-Review: Derek Bender <djbender@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
closes GRADE-1512
test plan:
* Smoke test some grade calculations?
Change-Id: Ia26fe7fcb137dd1f5bc07bb77babf3694a268b70
Reviewed-on: https://gerrit.instructure.com/172851
Tested-by: Jenkins
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Keith Garner <kgarner@instructure.com>
QA-Review: James Butters <jbutters@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
Jacob Burroughs