Commit Graph

68 Commits

Author SHA1 Message Date
Cody Cutrer e50edd485f RuboCop: heredocs
[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>
2021-11-16 06:36:00 +00:00
Cody Cutrer cf213ee24b RuboCop: Style/RedundantFreeze
[skip-stages=Flakey]

auto-corrected

Change-Id: Id1b8bafdd744219a4797e6e1ba5891cd7ce4bccd
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277888
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>
2021-11-11 02:06:47 +00:00
Cody Cutrer c65d57737a RuboCop: Layout lib
Change-Id: I0655d9a9d750f2debd6378b03d8ddc1403ebc31b
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274158
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>
2021-09-22 20:01:52 +00:00
Keith T. Garner 70599a545c use postgres any in duedatecacher quiz lti lookup
Switch to using the postgres any statement instead of the AR generated
IN statement in the ContentTag query in the DueDateCacher. Per our DBAs
it is much faster in the postgres planner. 1.2-2.1 ms down to .7-.9 for
large datasets.

test plan:
 - specs pass

Change-Id: I9fe29806df72fa186490087d32b5d22008a06fc6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271236
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
2021-08-12 14:43:25 +00:00
Matt Petro a05dc0f8dd Use SHA256 for due_date_cacher job strand name
This is part of the FIPS compliance project

Test plan:
- spec spec/lib/due_date_cacher_spec.rb passes

Change-Id: I8c2a3cc4cfa8828f06d325c8007796bf3a8bb1df
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/266073
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
2021-06-01 16:44:45 +00:00
Rob Orton 686271a82e always use jobs to update cached_due_dates
there is a known condition in our submission upsert that cause a
deadlock to happen because of a bunch of history that I have mostly
forgotten. see 7fd0528c3a for more context

The logic will rescue a dead lock, and then the job knows to retry if it
hit the dead lock. If it is not run in a job, we don't get the retry,
and it it hits the deadlock and fails the import.

test plan
 - specs should pass
 - move to section should not hit visible dead lock errors

fixes FOO-1555
flag=none

Change-Id: If8c7e6fa1d24e92a8dab1d6ebf5b29a7802ba5d3
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/257699
Reviewed-by: Michael Ziwisky <mziwisky@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
2021-03-26 22:34:54 +00:00
Keith T. Garner 7fd0528c3a log expected exceptions and retry grade jobs
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>
2020-12-14 19:26:24 +00:00
Cody Cutrer 548a2a1732 update all invocations of send_later and friends to new syntax
Change-Id: I7f40ed058b50882121da69f0cb05966854b8e920
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/250924
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
2020-10-30 19:13:54 +00:00
Cody Cutrer 06763dd519 add # frozen_string_literal: true for lib
Change-Id: I59b751cac52367a89e03f572477f0cf1d607b405
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/251155
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>
2020-10-27 20:49:50 +00:00
Cody Cutrer b4629c8b09 ruby 2.7: fix several deprecation warnings
Change-Id: I1bbad3fb41939dcb792b00cd4d37b8e390d2fdbb
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/249915
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>
2020-10-13 02:09:54 +00:00
Cody Cutrer c5227d3f1b shackles was renamed to guardrail
closes FOO-989, FOO-990

Change-Id: I49dfa130cb74c34dd0eb25952790176ae4951058
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/249365
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>
2020-10-06 19:23:08 +00:00
Adrian Packel bf1be2fe0d Add root_account_id to submissions
closes PLAT-5552
closes TALLY-706
flag=none

Test plan:

Migration:
- Verify migrations run
- Verify a root_account_id can be set on a
  Submission record
- Verify Submissions always live on the same shard
  as their root account

DueDateCacher:
- Do something that triggers a DueDateCacher run (e.g., add some
  students to a course)
- The root_account_id of the resulting submission objects should match
  the owning course's root account ID

Change-Id: Ib4cd4396b736c4e5329c67caeb92cb9b74ddb1ca
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/239395
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Adrian Packel <apackel@instructure.com>
Product-Review: Oxana Jurosevic
2020-06-09 18:15:54 +00:00
James Williams 3f4a5d1084 drop submission.context_code in favor of submission.course_id
test plan:
* smoke test todo list
 (needs grading and peer review lists in particular)

closes #LA-952

Change-Id: I613ffa2499986700744482c56976bba207cfb971
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/234903
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
2020-05-28 23:31:13 +00:00
James Williams 2569f75570 add course_id to submissions
Change-Id: I69778114343e84d4c6751199fdc248de2cdc78f7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/232318
Reviewed-by: Rob Orton <rob@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2020-04-13 18:53:06 +00:00
Rob Orton 4a05e3bb7a only update 1k records at a time for ddc
test plan:
 - specs pass

Change-Id: Ieaeadb44acdffd0e17587d765e8ea817b25af938
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/232252
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
2020-03-31 15:02:47 +00:00
Keith T. Garner 0cb379454d modify update to remove in_batches
in_batches is terrible, it caused a huge select that we didn't need.
Let's slice on assignments instead.

test plan:
 - specs pass

Change-Id: I5e3b45f38d075fed1554bfe21606d36e8399db24
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/232249
Reviewed-by: James Williams <jamesw@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
2020-03-31 13:56:05 +00:00
Keith T. Garner 6050875e11 fix some N+1s in the due date cacher
closes TALLY-777

test plan:
 - specs pass

Change-Id: I4f7b4a250534ba74f01c0d8dd908fd049b33be93
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/231710
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
QA-Review: Adrian Packel <apackel@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
2020-03-27 22:10:53 +00:00
Keith T. Garner 7d290296f7 add transaction around 2 step upsert in due date cacher
test plan:
 - spec pass

Change-Id: I9ffc029d38f63d3bf632e6a3737d5cc6977ba62c
Reviewed-on: https://gerrit.instructure.com/213251
Tested-by: Jenkins
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
2019-10-17 14:37:02 +00:00
James Williams bc6b21ec51 spec: various flaky spec fixes
Change-Id: Ia02bf73a1a332b60177a97afae5dfe8a18152c23
Reviewed-on: https://gerrit.instructure.com/208718
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2019-09-10 14:20:41 +00:00
James Williams 2b82831efe drop unused columns from submissions
Change-Id: I5f11f902ff7c7a155956b2cf4d6d8dcb275d3f7a
Reviewed-on: https://gerrit.instructure.com/202806
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2019-07-26 15:23:59 +00:00
Keith T. Garner 11cf6e9caf revert to old style upsert for duedatecacher
Errors of 'ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR:
duplicate key value violates unique constraint
"index_submissions_on_assignment_id_and_anonymous_id"' were found in
Sentry. Unfortunately, the postgres ON CONFLICT upsert will only allow
a single constraint to check against. In this patchset we revert to
the old style that will handle both issues. While this is prone to
intermittent failures, DueDateCacher jobs have a max_attempts of 10 so
the on later runs the content will be updated.

fixes GRADE-2313

test plan:
 - specs pass

Change-Id: I76d3f0084e258007245378533ae0ecf69bd51994
Reviewed-on: https://gerrit.instructure.com/201693
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
Tested-by: Jenkins
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
2019-07-18 14:08:55 +00:00
Keith T. Garner d0de7c172d fix yaml serialization issue with a set in duedatecacher
When a Set is serialized via YAML and reconstituted, its internal Hash
loses its default value of false. This is ruby bug
https://bugs.ruby-lang.org/issues/11059 In this particular case, we
were always expecting a bool and breaking when we were writing an
empty string to the database.

fixes GRADE-2302

test plan:
 - Specs pass

Change-Id: I2d876688914dc361bb1b93939349e2bfc63a3098
Reviewed-on: https://gerrit.instructure.com/201408
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
Tested-by: Jenkins
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
2019-07-17 14:23:35 +00:00
Keith T. Garner 536a72f2f8 cache quiz_lti on submissions
This caches/denormalizes the quiz_lti? information from assignments
onto submissions. This makes queries for missing/late Quizzes.Next
quizzes to be easier without having to join three tables. (Future
ticket coming to make this happen.)

closes GRADE-2231

test plan:
 - Have a course with a student and a Quizzes.Next assignment and
   another non-Q.N assignment. (You can fake the quizzes next assignment
   via the rails console)
 - On the rails console check the submission object for each
   assignment. Confirm that the quizzes.next assignment has
   cached_quiz_lti set to true and that the other assignment has it
   set to false
 - Create another assignment with on paper. Save.
 - Switch it to a Quizzes.Next assignment.
 - On the rails console check the submission object for each
   assignment. Confirm that the new quizzes.next assignment has
   cached_quiz_lti set to true
 - For the new assignment, in the rails console, find the ContentTag
   and .destroy it
-  On the rails console check the submission object for each
   assignment. Confirm that the new quizzes.next assignment has
   cached_quiz_lti set to false

Change-Id: I4bfc1d3a282863dde9de9658b335b9a24b2341e5
Reviewed-on: https://gerrit.instructure.com/197811
QA-Review: James Butters <jbutters@instructure.com>
Tested-by: Jenkins
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: James Williams <jamesw@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
2019-06-27 14:28:42 +00:00
Keith T. Garner 0de3ad6974 change to proper postgres upsert in due date cacher
Changes from our two stage "update/insert" into a proper postgres upsert.

closes GRADE-2210

test plan:
 - unit tests pass

Change-Id: I76fc7f33e6fb10aaa763b13c0cb99cb5a4188f2e
Reviewed-on: https://gerrit.instructure.com/195951
Tested-by: Jenkins
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
2019-05-31 21:20:43 +00:00
James Williams 295191b298 add caching around unread_submission_count_for
also move to slave

opens up the possibility of using :submission
cache key for other submission-dependent data as well
(although should still have substantial impact just here)

refs #CORE-2851

Change-Id: Ic6f5a997c8fe6b9d509f421b26729a87d21bcb91
Reviewed-on: https://gerrit.instructure.com/194301
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2019-05-21 13:52:38 +00:00
Gary Mei 7688733392 update submission workflow_state and updated_at
When a submission's workflow_state is changed as a result of
DueDateCacher running, the updated_at wasn't being touched, resulting
in a stale cache key and permissions not reflecting the state of
the submission now.

fixes GRADE-1846

Test Plan
- Create an assignment, and manually assign it to multiple students.
- Edit the assignment, and remove one of the students from the
  assignment.
- Attempt to grade the student via the api:
  `api/v1/courses/{course_id}/assignments/{assignment_id}/submissions
     /{submission_id}?submission[posted_grade]=10`
- The response should be a 401, since the student's submission is not
  currently assigned to that assignment.
- Edit the assignment so that the student is now assigned to the
  assignment again.
- Attempt once more to grade the student via the api; verify that
  a successful response is given.

Change-Id: I6d291f0972a921ef9f5cd2722b491e9e52b51ed0
Reviewed-on: https://gerrit.instructure.com/175715
Tested-by: Jenkins
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
2018-12-18 19:40:01 +00:00
Keith T. Garner 337c3e7bc1 add multiple attempts to calculating grades jobs
Grade calculation is really important.  Let's attempt up to 10 times if
the job fails or is killed.

closes GRADE-1895

test plan:
 - Jenkins passes

Change-Id: I6003e971600f85de5d35b85bec42c3bb4f5bebad
Reviewed-on: https://gerrit.instructure.com/175563
Tested-by: Jenkins
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
QA-Review: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
2018-12-17 15:34:21 +00:00
Keith T. Garner 2d734d014e update updated_at when soft-deleting submissions in duedatecacher
In DueDateCacher when soft deleting via changing the workflow_state
via update_all also update the updated_at timestamp. This will help us
trace back events that may have caused a submission to be deleted.

closes GRADE-1886

test plan:
 - Have a course with an assignment with a due date and two students
 - Unassign one of the students from the assignment
 - Using a rails console, find the student's soft deleted submission
   record.
 - Note that the updated at matches the time you made a visibility
   change.

Change-Id: If0af4e420ac75f865d2954f1454ffae106e13a98
Reviewed-on: https://gerrit.instructure.com/174937
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: James Butters <jbutters@instructure.com>
Tested-by: Jenkins
Product-Review: Keith Garner <kgarner@instructure.com>
2018-12-11 15:35:00 +00:00
Adrian Packel 8a594a5b14 Audit submission due date changes
closes GRADE-1487

Test plan:
- Specs pass
- Smoke test the following for an anonymous (or otherwise auditable)
  assignment:
  - Changing the due date for the whole assignment, a particular
    section, or a particular student
  - Adding or removing a student
  - Updating student groups
  - Updating grading periods
- Smoke test the above for a non-auditable assignment and make sure the
  functionality works as expected

Change-Id: I31858a38059bc3ff73d91dd138ffa20cd8a76fac
Reviewed-on: https://gerrit.instructure.com/164776
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Tested-by: Jenkins
QA-Review: James Butters <jbutters@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2018-10-01 16:34:37 +00:00
Nick Pitrak d058cfec8e automatically add students to moderation (back end)
closes GRADE-1177

test plan:
- turn on anonymous moderated marking
- create a course with a student
- create a moderated assignment in the course
- add another student to the course
- go to the moderation page for the assignment
- ensure both students have a speedgrader link in the first column
- go back to the assignment page
- edit the assignment to only be assigned to the first student
- open the rails console
- find the assignment
- ensure assignment.moderated_grading_selections.count returns 1

Change-Id: I8ef9fb7715b103469ea35c303c4301520b7bf7cb
Reviewed-on: https://gerrit.instructure.com/150969
Tested-by: Jenkins
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
QA-Review: James Butters <jbutters@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2018-05-30 22:46:21 +00:00
Keith Garner a89d8b7b8b change assignment due date caching from a singleton to a strand
When invoking the DueDateCacher with update_grades on, switch to using
a strand based on the course instead of a singleton based on the
assignment. This should avoid score table contention when lots of
assignments in a single course are edited close in time to each other.

closes GRADE-1194

test plan:
 - Have a course with some students and a bunch of assignments
 - Edit the assignments quickly back to back
 - Ensure the jobs queued as a strand
 - (Testing this may be easier if delayed jobs isn't running and you
   edit inst-jobs to always background even if not in production.)

Change-Id: I4b5600f52f2f09b6faf994e29e6056e1c6eaeb60
Reviewed-on: https://gerrit.instructure.com/151661
Tested-by: Jenkins
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: Jeremy Neander <jneander@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2018-05-30 16:01:30 +00:00
Adrian Packel 25870e5117 Move anonymous ID routines to own module
refs GRADE-1057

Test plan:
  - Specs pass
  - In a course with at least one student, create a new assignment
    and make sure there are no errors

Change-Id: If429480ea516177bfed91509d149275190b44931
Reviewed-on: https://gerrit.instructure.com/150069
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
Tested-by: Jenkins
QA-Review: Spencer Olson <solson@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2018-05-15 15:03:06 +00:00
Keith Garner 5f5c159fb0 make DDC calls more efficient if we can scope to specific users
This patchset changes calls of DueDateCacher.recompute_course to
DueDateCacher.recompute_users_for_course in places where we only have
specific enrollments or users we can work on and avoid doing the work
of the entire class.

fixes GRADE-1129

test plan:
 - specs pass

Change-Id: Ia6814e4584993bd9d0f2ba330eb33c3a635ca0e1
Reviewed-on: https://gerrit.instructure.com/148854
Tested-by: Jenkins
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2018-05-03 22:01:37 +00:00
Shahbaz Javeed 4cf57f608f log the current caller in DDC#recompute_course
Ensure the current caller is identified and passed to the DDC
initializer as we're doing with the other class methods on DueDateCacher

test plan
* Ensure specs continue to pass

Change-Id: I352051c70b04b2bf144542a3aac833ad1ca45cd1
Reviewed-on: https://gerrit.instructure.com/147905
Tested-by: Jenkins
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Product-Review: Shahbaz Javeed <sjaveed@instructure.com>
QA-Review: Shahbaz Javeed <sjaveed@instructure.com>
2018-04-24 21:47:51 +00:00
James Williams df8de0f03c handle many user ids in due date cacher strand name
closes #CORE-1252

Change-Id: Ia381bd2cba10a40622818d4e01365c19cd52c3bd
Reviewed-on: https://gerrit.instructure.com/146008
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
Tested-by: Jenkins
Reviewed-by: Spencer Olson <solson@instructure.com>
QA-Review: Spencer Olson <solson@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>
2018-04-05 17:00:10 +00:00
Mysti Sadler 57577f343b Update canvas-planner to use plannable_date
fixes ADMIN-130

Test plan
- Specs pass

Change-Id: I588f2f8b5db00d97560da31124a2ea6014f61e06
Reviewed-on: https://gerrit.instructure.com/145083
Tested-by: Jenkins
Reviewed-by: Ed Schiebel <eschiebel@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Mysti Sadler <mysti@instructure.com>
2018-04-03 19:39:44 +00:00
Derek Bender faf4445bad add anonymous_id to due date cacher to upsert
Relies on base58, which is rad because you can't mistake 0 (zero) for
uppercase O (oh) and uppercase I (eye) for lowercase l (el). The other
added benefit is some systems will interpret a prefixed 0 has indicating
a different literal (e.g. hex is `0x` prefixed). This avoids the problem
by never using zero.

    log2(58)*5 = ~29.28 bits (round up to 30 bits of entropy)
    58**5 = 656,356,768 combinations

For two generations there's a 1 in 656,356,768 chance of collision.

In the case of a collision, we'll loop until an unused one is generated.
Performance for this probably gets kinda bad once the assignment is half
full. However if we have >300,000,000 submissions the app has probably
already fallen over.

closes: GRADE-893

Test Plan:
1. Given a course
2. Given a student enrolled in the course
3. When creating the first assignment (e.g. 'Alpha')
4. Then all submissions have anonymous_ids:

    anonymous_ids = Assignment.find_by!(title: 'Alpha').
      all_submissions.pluck(:anonymous_id)

5. When Alpha has it's due date changed via the assignment edit page
6. Then none of the anonymous_ids have changed:

    anonymous_ids.sort! == Assignment.find_by!(title: 'Alpha').
      all_submissions.pluck(:anonymous_id).sort!

7. Given a second assignment (e.g. 'Omega')
8. Given the manual deletion of anonymous_ids for the Omega assignment:

    omega = Assignment.find_by!(title: 'Omega')
    omega.all_submissions.update_all(anonymous_id: nil)

9. When DueDateCacher is triggered by changing the due date
10.Then all submissions in Omega now have anonymous_ids:

    omega.all_submissions.reload.pluck(:anonymous_id)

11.When validating a new submission in the console:

    submission = omega.submissions.build
    submission.validate # => false

12.Then anonymous_id is present:

    submission.anonymous_id # e.g. "aB123"

Change-Id: I874ba5b0d6025c95af2f832c3cc5d83c3cbd20e7
Reviewed-on: https://gerrit.instructure.com/143314
Tested-by: Jenkins
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
QA-Review: Indira Pai <ipai@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2018-03-20 18:40:57 +00:00
James Williams cf2488b2bc switch grade logging from info to debug
Change-Id: I93bfcc9407e169e575d96c6d23c8020de07f639b
Reviewed-on: https://gerrit.instructure.com/142581
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Tested-by: Jenkins
Product-Review: James Williams  <jamesw@instructure.com>
QA-Review: James Williams  <jamesw@instructure.com>
2018-03-05 21:21:12 +00:00
Shahbaz Javeed 11f764f833 fix stale grades when re-assigning students to assignment
closes GRADE-819

The specific case was to unassign a graded student from an assignment by
removing "Everyone" and adding all students except one to the
assignment.  Then remove all students and assign "Everyone" to the
assignment.  This should make the previously unassigned student's grades
stale.

test plan:
* Configure delayed jobs to use 6 simultaneously running workers
  by editing config/delayed_jobs.yml to look like this:

  default:
    workers:
    - queue: canvas_queue
      workers: 6

  Notice the line that says "workers: 6"
* Restart the jobs container so all six new workers are started
* Create a published course with one assignment worth 150 points
  and two students enrolled
* For the remainder of these steps, let's assume unique identifer
  for the course is "course_id" and the unique identifier for
  the student is "student_id"
* Go to the Gradebook and grade the first student at 125 points
  and wait for their score to be updated in the gradebook.
* Verify the score in the Gradebook is 83.33%
* In a separate tab, visit the following URL, making sure to
  replace course_id with the actual course id and student_id
  with the actual student id:

  /api/v1/courses/course_id/enrollments?user_id=student_id

* Verify that you see the student's current_score as 83.33
* Repeat the following steps multiple times to ensure the
  problem does not manifest itself:

  - Modify the assignment: unassign it from the first student
    and only assign it to the second student only
  - Go to the gradebook and verify the cells for the first student
    are not editable any more
  - Go back to the API URL above and verify the student's
    current_score now says null

  - Modify the assignment: re-assign it to "Everyone"
  - Go to the gradebook and verify the cells for the first student
    are now editable again
  - Go back to the API URL above and verify the student's
    current_score now says 83.33 again.  If it ever says null
    at this point, there is a problem.

Change-Id: Ifaaf0609dfe5081697c1939db1b4a4e0a3e05bad
Reviewed-on: https://gerrit.instructure.com/141049
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
Tested-by: Jenkins
Product-Review: Keith T. Garner <kgarner@instructure.com>
QA-Review: Keith T. Garner <kgarner@instructure.com>
2018-02-22 16:04:24 +00:00
Spencer Olson 6b0910ae6b change delayed job behavior when grading period is updated
If a grading period or grading period group is updated, we were
previously creating a delayed-job-per-course for grade recalculation
and a delayed-job-per-course for DueDateCacher recalculation, with no
limit on how many of those jobs could be run in parallel.

Now, we create a delayed-job-per-1000-courses for grade recalculation,
and a delayed-job-per-1000-courses for DueDateCacher recalculation, and
the number of jobs that can be run in parallel are limited with an
n_strand.

closes GRADE-805

Test Plan:
1. Verify cached due dates and scores (grading period and overall) are
   recalulated when:

   a) Creating, updating, and deleting a Grading Period Set.
   b) Creating, updating, and deleting a Grading Period.

2. When creating/updating a Grading Period or a Grading Period Group
   such that a score + due date recalculation occurs, verify:

   a) Enrollment term delayed jobs
      (EnrollmentTerm#recompute_scores_for_batch) are being stranded by
      GradingPeriodGroup, and a job is being created for each 1000
      courses that need to be operated on in that term.
   b) Score recalculation delayed jobs are not being triggered (because
      the score recalculation is happening in the delayed job triggered
      in step a above).
   c) Due date recalculation delayed jobs are not being triggered
      (because the due date recalculation is happening in the delayed
      job triggered in step a above).

Change-Id: I99610d0559a449ad08b9209646490f7fa1cdf33b
Reviewed-on: https://gerrit.instructure.com/138508
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
Tested-by: Jenkins
Reviewed-by: Derek Bender <djbender@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
QA-Review: Keith T. Garner <kgarner@instructure.com>
2018-02-12 23:25:59 +00:00
Keith Garner 05cd8ee0cf add ability for due_date_cacher to work on a single user
This patchset adds the ability to run the DueDateCacher on a single
user in a course. This also updates the callback in Enrollment to call due

closes GRADE-832

test plan:
 - Have a course with a student and an assignment
 - In the rails console, add a new student to the course.
 - Confirm via the logs that only a single user id was worked on for
   the DueDateCacher
 - Soft delete one of the students in the course
 - Confirm via the logs that only that students Submissions were
   deleted
 - Confirm via the rails console that the still active students
   Submissions are not deleted

Change-Id: If4a9a7d7cfa78a455013fe23152e1479f38840e2
Reviewed-on: https://gerrit.instructure.com/139943
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Tested-by: Jenkins
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2018-02-09 21:32:26 +00:00
Spencer Olson 0952197e69 restore submissions and scores for restored enrollments
Fixes a bug where submissions and scores were not restored when an
enrollment was updated from a "deleted" to "completed" state.

closes GRADE-394

Test Plan:
1. Set up a course with at least one published graded assignment and an
   enrolled student. Both course and student must have a SIS ID.
2. Navigate to the Gradebook and grade the student for the assignment.
3. Run a SIS Import to change the enrollment state from "active" to
   "deleted" for the student.
4. Run a second SIS Import to change the enrollment state from
   "deleted" to "completed" for the student.
5. Navigate into the course's gradebook and enable the option to
   "Show Concluded Enrollments."
6. Verify the grade awarded in step 2 is visible in the Gradebook.
7. On the global grades page for the student, verify the final grade
   for the course is accurate.

Change-Id: I95acf98f829f62388bffb61a8a09a29c2dbc8c82
Reviewed-on: https://gerrit.instructure.com/132001
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
Tested-by: Jenkins
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2017-11-08 21:28:51 +00:00
Keith Garner dae8d22d68 optimize duedatecacher enrollment query and submission deleting
This patchset removes the multiple enrollment/student queries with a
single pass to gather the needed data. It also delays the data
gathering until it's needed rather in the constructor saving on
queries when queuing the delayed job. Lastly, it tries to batch the
updating to workflow_state deleted for students removed from the course.

fixes GRADE-344

test plan:
 - Existing unit tests pass

Change-Id: Ic1889cad90350c24c3219fc3e1e5f7fb77a0c560
Reviewed-on: https://gerrit.instructure.com/129049
Reviewed-by: Derek Bender <djbender@instructure.com>
Tested-by: Jenkins
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: Neil Gupta <ngupta@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2017-10-10 21:23:19 +00:00
Jeremy Neander de3b4f0f3d ignore concluded enrollments in due date cacher
closes GRADE-269

QA Notes:

To run the due date cacher, open a Rails console and run:

    assignment = Assignment.find(<id of assignment>)
    DueDateCacher.recompute(assignment)

Check cached due dates for submissions in the Rails console.

    submission = Submission.find_by(
      user_id: <id of student>,
      assignment_id: <id of assignment>
    )
    submission.cached_due_date

test plan:
 A. Setup
   1. ensure the delayed jobs are not running
   2. create a course with one assignment
   3. enroll multiple students
   4. assign the assignment to everyone

 B. Student without a Submission
    1. enroll a student in the course
    2. conclude the student's enrollment
    3. manually run the due date cacher
    4. confirm the student does not have any submissions

 C. Changing a Due Date
    1. enroll a student in the course
    2. manually run the due date cacher
    3. conclude the student's enrollment
    4. change the due date on the assignment
    5. manually run the due date cacher
    6. confirm the student submission exists
    7. confirm the submission has the previous due date cached

 D. Unassigning an Assignment
    1. enroll a student in the course
    2. manually run the due date cacher
    3. conclude the student's enrollment
    4. create overrides for only the active students
    5. make the assignment due only to overrides
    6. manually run the due date cacher
    7. confirm the student submission exists
    8. confirm the submission has the previous due date cached

Change-Id: I5e7165c0120e5c87635da1fbbe47501970874653
Reviewed-on: https://gerrit.instructure.com/126270
Tested-by: Jenkins
Reviewed-by: Matt Taylor <mtaylor@instructure.com>
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: Spencer Olson <solson@instructure.com>
2017-09-19 19:42:12 +00:00
Keith Garner 3d09b73023 fix due date cacher delete query that was making db sad
fixes GRADE-236

Test Plan 1:
1. Assign 'everyone' to an assignment and save it.
2. Edit the assignment and assign it to only one person and save it.
3. Open a rails console and verify the only active submission for the
   assignment belongs to the sole student it is assigned to.

   assignment.submissions.count # should be 1

Test Plan 2:
1. Create a section (Section 1) with 2 students in it
   (Student A and Student B).
2. Create an assignment and assign it to Section 1.
3. Delete Student A's enrollment.
4. Change the assignment from step 2 to be assigned to 'everyone'.
5. Verify the submission for Student A has been removed.

  assignment.submissions.find_by(user_id: <Student A's user_id>)
    // should return nil

Change-Id: Ia42895d536150381a4664f7496c31c0a8faf524e
Reviewed-on: https://gerrit.instructure.com/124191
Tested-by: Jenkins
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
Reviewed-by: Neil Gupta <ngupta@instructure.com>
2017-08-30 21:51:15 +00:00
Neil Gupta 33a0f62210 don't create placeholder submissions for unassigned students
Fixes CNVS-37395

Test plan:
* Create a course with 2 students
* Create an assignment with a due date for everyone
* Create a differentiated assignment with a due date for only 1 student
* In rails console:
  * Run `DueDateCacher.for_course(course_id)
  * Make sure `Submission.count` is 3
  * Make sure the first assignment has 2 placeholder submissions
  * Make sure the second assignment only has one placeholder submission

Change-Id: I5c8084d4fb0138f65e4c117dcc97638cf8d4f49d
Reviewed-on: https://gerrit.instructure.com/114573
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
Product-Review: Neil Gupta <ngupta@instructure.com>
QA-Review: Neil Gupta <ngupta@instructure.com>
2017-06-27 20:33:35 +00:00
Shahbaz Javeed a4e109ea0c re-apply late policies to any late submissions as needed
the "as needed" refers to two cases:
* when late policy changes
* when an assignment's points_possible changes

closes CNVS-36656

test plan:
* Create a course with two assignments, two grading periods and
  two students enrolled
* Ensure one assignment is due in a closed grading period and the
  other in an open grading period
* Submit homework from both students so for each assignment one
  student submits the homework on time and the other submits it
  late
* Go to gradebook and grade the students
* Add a late policy to the course using the Rails console:

  course = Course.find(my_course_id)
  late_policy = LatePolicy.create(
    course: course,
    late_submission_deduction_enabled: true,
    late_submission_deduction: 50.0
  )
* Reload the gradebook and you should see the score of the late
  submission for the assignment in the open grading period to
  have changed (lowered)
* Verify that none of the other submissions had their scores
  changed

* Now edit the assignment in the open grading period and change
  its points possible to a higher number and save this change
* Reload the gradebook and you should see the score of the late
  submission for the assignment in the open grading period to
  have changed again
* Verify that none of the other submissions had their scores
  changed

* Now try this using three quiz submissions (early and late and
  just 45 seconds past the deadline).
* Verify in the gradebook that late policy deductions are applied
  only to quiz submissions that are later than 60 seconds after
  the quiz due date

Change-Id: I58ed3e3d0665870cf46d1b1e3ddf00f5f2f7008c
Reviewed-on: https://gerrit.instructure.com/110598
Tested-by: Jenkins
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2017-05-26 18:53:26 +00:00
Keith Garner 01715487bf switch to bulk update in due date cacher
Optimizes the DueDateCacher by using a bulk update strategy in
postgresql and removing the transaction guard.

fixes CNVS-36912
ref CNVS-33651

test plan:
* Create a course
* Enroll 3 students
* Create 2 assignments with different due dates
* In rails console:
	* Submission.pluck(:assignment_id, :cached_due_date) should return
	  an array of 6 tuples of (assignment id, its due date)
* Create an override for one student on one assignment
* In rails console:
  * Submission.where(
  		assignment_id: <assignment picked above>,
  		user_id: <user picked above>
  	).cached_due_date should equal the override date
* Smoke test grading and viewing students in different places to make
  sure the new dummy submissions aren't breaking anything

Change-Id: I98dea83ae726220b77f49eae1fb711ce61d7e212
Reviewed-on: https://gerrit.instructure.com/111812
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2017-05-16 00:48:45 +00:00
Neil Gupta 1e052eae19 Update DueDateCacher to use EffectiveDueDates
fixes CNVS-33651

Test plan:
* Create a course
* Enroll 3 students
* Create 2 assignments with different due dates
* In rails console:
	* Submission.pluck(:assignment_id, :cached_due_date) should return
	  an array of 6 tuples of (assignment id, its due date)
* Create an override for one student on one assignment
* In rails console:
  * Submission.where(
  		assignment_id: <assignment picked above>,
  		user_id: <user picked above>
  	).cached_due_date should equal the override date
* Smoke test grading and viewing students in different places to make
  sure the new dummy submissions aren't breaking anything

Change-Id: Idc2721fd3f05214555db780b452ddf53e67ff404
Reviewed-on: https://gerrit.instructure.com/109027
Tested-by: Jenkins
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2017-05-11 20:54:44 +00:00
Spencer Olson 03ef5d7d77 fix staleness issues with cached_due_date
closes CNVS-36401

test plan:

1. Add an indivdual student override for a student on an assignment.
   Verify the cached_due_date on the submission is updated to reflect
   the override due date (assuming there aren't other overrides for
   that user on the assignment that have a more lenient due date).

2. Remove an individual student override for a student on an
   assignment. Verify the cached_due_date on the submission is updated
   and no longer matches the due date on the override that was removed
   for the student.

3. Enroll a student in 2 sections in a course (Section 1 and Section
   2). On an assignment, add a section override for Section 1 and an
   individual student override for the student. The section override
   should have a more lenient due date than the individual student
   override. Then remove the student from Section 1. The cached_due_date
   on the submission for the assignment should be updated. It should no
   longer reflect the due date from the Section 1 override and should
   now reflect the due date for the individual student override.

Change-Id: I046eb6a468f61af905d96eb1b2a6e41efebc6b8e
Reviewed-on: https://gerrit.instructure.com/109005
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Tested-by: Jenkins
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
2017-04-28 16:01:47 +00:00