Also improve deletion performance by deleting in parallel
Change-Id: I38c13e543f0f962b5361b3a612882bc44067f3eb
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/328121
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
Reviewed-by: Alex Slaughter <aslaughter@instructure.com>
Migration-Review: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Alex Slaughter <aslaughter@instructure.com>
Product-Review: Alex Slaughter <aslaughter@instructure.com>
refs AE-57
flag=none
Change-Id: I864fe0d0e714a4af2a856e99fc67f53a05c649dd
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326678
Reviewed-by: Aaron Ogata <aogata@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Isaac Moore <isaac.moore@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
refs AE-57
flag=none
test plan:
- uploading with an assumable vault role works
- uploading with static IAM creds works
Change-Id: I509f479a01db975c3ac233b7569a54af1aefd919
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326675
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Isaac Moore <isaac.moore@instructure.com>
Reviewed-by: Aaron Ogata <aogata@instructure.com>
Tested-by: Isaac Moore <isaac.moore@instructure.com>
refs AE-57
flag=none
test plan:
- local uploads work with Vault roles for S3
- local uploads work with static creds for S3
Change-Id: I0c367f21f214b2f7614385798524af7b202acce0
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326460
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Aaron Ogata <aogata@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Isaac Moore <isaac.moore@instructure.com>
[skip-stages=Flakey]
Change-Id: I6abefdfa9fed6dd4525c8786e93efa548b3710f2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/319603
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
Migration-Review: Jacob Burroughs <jburroughs@instructure.com>
The after save processes for media files
were reuploading the lowest quality version
of the file, because instfs wasn't marking
the file as uploaded. This resolves that.
fixes MAT-1350
flag=none
qa-risk: low
Test plan
- Have notorious running
- Set up instfs (ask me if you need help)
- Upload a media file and wait a few minutes
to make sure the media id is populated in
the file you just uploaded
- Verify that you can download the file at
its original size
Change-Id: Id0f29165000513284961c7c7fde08cf87e9892d2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/317827
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Juan Chavez <juan.chavez@instructure.com>
QA-Review: Eric Saupe <eric.saupe@instructure.com>
Product-Review: Mysti Lilla <mysti@instructure.com>
test plan:
- with a local or S3 (not InstFS) file, opened in Rails console
with .open(integrity_check: true)
- should work if the md5 is present and correct
- should raise a CorruptedDownload if the md5 is incorrect
- should work if the md5 is missing
flag=none
closes gh-2147
closes FOO-3360
Change-Id: Ibe1c65af8f618343cdc399217c30d15bb1f53ca9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/310294
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Mysti Lilla <mysti@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
the `md5` attribute should store a hex digest and not the Digest
object iself. the latter works after type cast but prevents the
Attachment from being marshaled before reloading, resulting in
"no _dump_data is defined for class Digest::SHA512"
test plan: specs
fixes FOO-3306
Change-Id: I34d540bb6f110cf43d3ab68436277caf0d024db4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/309972
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
refs MAT-1091
flag=none
[fsc-timeout=30]
Test plan
- Specs pass
Change-Id: Iac01fda45b271340fed69c57b9647a1240367171
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/309298
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ed Schiebel <eschiebel@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Ed Schiebel <eschiebel@instructure.com>
Product-Review: Mysti Lilla <mysti@instructure.com>
fixes FOO-3295
flag = none
test plan:
• request a file preview of a specific course file i.e.
/courses/43511/files/271828/file_preview?verifier[%24acunetix]=1
• it shouldn't blow up, but instead will fail to pass verification
Change-Id: If959c259e03e0d4931fbfa3ae2262cc964f86ef7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/308247
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Charley Kline <ckline@instructure.com>
Product-Review: Charley Kline <ckline@instructure.com>
QA-Review: August Thornton <august@instructure.com>
test plan:
- modify the md5 column of an existing Attachment
- download it with Attachment#open and
pass `integrity_check: true`
- a CorruptedDownload exception should be raised
and the hash mismatch should be logged in an ErrorReport
flag=none
refs FOO-3256
Change-Id: I035d46cd4e44bd179d0943497147d6cc63ef3b9d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/306879
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Sean Scally <sean.scally@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
it doesn't do anything. we always download a file locally unless
a block is given to #open, and places exist that call #open
without the option and still depend on the local file being there
replace opts with keyword arguments while we're at it.
test plan: specs
flag=none
refs FOO-3256
Change-Id: I52461022cef457b85ab2f270f7d22d358617c786
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/306971
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Mysti Lilla <mysti@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
adds a Course-level setting to control the visibility of uploaded files
also adds a per-file setting to override the Course-level setting
Test Plan:
1. Validate the the Course-level setting can be set and updated. It should
behave similar to the Syllabus visibility setting, but should allow
being looser or tighter than the Course itself.
2. Validate that the File-level setting can be modified.
3. Validate that Files can only be accessed based on the setting.
4. Validate that the existing "Only with link" and "Schedule availability
rules work as expected with the new options.
flag = none
refs PFS-20427
[fsc-max-nodes=20]
[fsc-timeout=45]
Change-Id: Ie688264fff59e81b13c97ef58bf0ae81cf8a5610
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/300000
Migration-Review: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Mysti Lilla <mysti@instructure.com>
QA-Review: Douglas Rist <douglas.rist@instructure.com>
Product-Review: Jesse Poulos <jpoulos@instructure.com>
fixes LS-2897
flag=none
test plan:
- Have the INST-FS plugin ON
- Have a canvas export package with at least one file
- Import the export package into a fresh course
- Import the same export package again.
- Check that the file wasn't duplicated
- (You could, after the first import, alter the
attachment md5 column and put it's sha512 there,
then theorically you'd be testing this, but Ideally
you want instfs ON)
Change-Id: I0de79032d2ccd64e51cc09dea61b410aad43756f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282779
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Robin Kuss <rkuss@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Luis Oliveira <luis.oliveira@instructure.com>
auto-corrected, with a bit of manual massaging afterwards if there were
multiple delegations to the same object to combine them into a single
call
Change-Id: Ic93ccad26b037e1f89874d36f9759e5c34435892
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278965
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>
[skip-stages=Flakey]
auto-corrected
had to rework some instfs to play nice with stubs in its specs
Change-Id: I894831c6503095c6b197154e39fe17e803702d9c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278785
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>
[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>
all manual
several instances I noticed that the entire method was unused, so removed it
Change-Id: I14ffe7d4b6966ee790e32410f1dbaf5b26ea7f21
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/276513
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>
This reverts commit eb44849fd6.
Reason for revert: apt repo is no longer broken
Change-Id: I9224768d117e1b1a030f881ec03e0295afcb73d9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274305
Reviewed-by: Simon Williams <simon@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>
This reverts commit 2c5c3584ff.
Reason for revert: I'm back in the office and can debug the problems
Change-Id: Ib469fff450a8d51d7ca59cb9d7fa29874d6b6e53
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/268386
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>
This reverts commit 230033611d.
Reason for revert: COPY strategy is broken
Change-Id: I2cb4e5430c180caa1ceae6570d27a4c86a12d704
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/267654
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
* do everything as in_batches, returning a relation. properly
super `load` param in each backend
* plumb strategy through all entry points so it can be explicit
* special case in_batches.delete_all with no explicit strategy to
do a loop on a limited delete_all (avoids a dumb ORDER BY, or
having to transfer ids back and forth)
* since in_batches can easily be used with pluck now that a relation
is returned, just make find_in_batches_with_temp_table a shim that
does it the "nice" way
Change-Id: I716f188cdf676a725588f94a1036981ae798b09c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/266882
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>
this is intended to mitigate an issue where the source
verifier remains in the URL after a course copy
test plan:
- have a course wiht a file
- copy the course
- as an unauthenticated user, construct a URL to the new
course's copy of the file with the old course's file's uuid
as a verifier, i.e.
/courses/2/files/2/download?verifier=`file1.uuid`
refs LS-2225
Change-Id: Id7e61e2baa817b25f83cc125843ace70a5bf0497
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/265196
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Nate Armstrong <narmstrong@instructure.com>
QA-Review: Nate Armstrong <narmstrong@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
refs FOO-1648
flag=none
[fsc-propagate=false]
move general module code to "gems"
along with specs.
Leave shim in canvas to avoid breaking
things while callsites get changed.
change some limited callsites
from Canvas::Security to CanvasSecurity
that were doing spec stubbing that required
at least one namespace change anyway
generate readme describing common use
cases.
TEST PLAN:
1) security operations continue unimpeded.
Change-Id: Ia2d102d5038e2f5d0bb24201e38894e12b73063e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/259540
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Ethan Vizitei <evizitei@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>
Change-Id: I70825be7ec7e24458afe0c63dc48c5a76158f520
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/251150
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>
The two occurrences I found were `prepend_around_action`,
which prepends a callback around actions. The two actions
associated with this callback are no longer being used. Therefore
there was no evidence pointing to a callsite for
AuthenticationMethods#load_pseudonym_from_policy.
The files controller appears to call a new action `api_create`
vs the old :create. This handles the policy via its new code-path.
The content imports controller referenced a "migrate_content_upload"
action that is no longer in commission.
It appears the original intent behind the usage was for flash file
uploads. Usage existed in local_storage.rb & s3_storage.rb
[skip-cache]
refs FOO-1085
refs https://gerrit.instructure.com/c/migration_tool/+/251170
flag = none
test plan:
- specs pass
- bulk course migrations still work
Change-Id: Iedd0ff846e13840ce1c33f4da27b29ddd30f87ae
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/250591
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: August Thornton <august@instructure.com>
to avoid sending unnecessary whitespace to the database
test plan: specs
closes LS-1422
Change-Id: Iaaa73059b46964d4c51e920fd469b876256ec632
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/246811
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Mysti Lilla <mysti@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
closes ADMIN-2808
flag=direct_share
Test plan
- n/a
Change-Id: I1481e273b76bf969ce8e3a510702bfa1f11fb0d7
Reviewed-on: https://gerrit.instructure.com/204282
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins
Product-Review: Mysti Lilla <mysti@instructure.com>
fixes CORE-2538
test plan
- configure statsd to use data dog
- it should work
Change-Id: Ie8428e4e99973b35506bd7a8e4d1a18f5a7875a1
Reviewed-on: https://gerrit.instructure.com/182083
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
fixes RECNVS-383
test plan
- as a student: upload a file as an assignment submission
- as a teacher: download the zip of student submissions
- verify that the correct download size is show in the download modal
Change-Id: I26a5cf40e2181fa8626f77bc73d2da441009c135
Reviewed-on: https://gerrit.instructure.com/144564
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Jacob Fugal <jacob@instructure.com>
Tested-by: Jenkins
Product-Review: Michael Jasper <mjasper@instructure.com>
closes RECNVS-362
closes RECNVS-363
Test plan:
- with instfs configured and enabled:
- log into canvas and navigate to the users eportfolio
- dowload the eportfolio
- verify that the eportfolio was
- created successfully
- download from the <instfs domain>/files
- verify that the instfs logs show
- a post of the eportfolio to instfs
- a get of the eportfolio from instfs
Change-Id: Ia27ca9811598cea180432e730ae8d5163a720036
Reviewed-on: https://gerrit.instructure.com/143050
Tested-by: Jenkins
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Michael Jasper <mjasper@instructure.com>
fixes CORE-58
test plan:
- successfully deletes old ContentMigrations, including those tied to
old ContentExports
Change-Id: Ib9ca545da8a6929f495317bba08d0103acbaf1cc
Reviewed-on: https://gerrit.instructure.com/143330
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Jacob Burroughs