fixes#8460
refs #8318
we already fixed course migration notifications to send immediately, but we
missed content export notifications. also, the previous fix would only work for
users who didn't already have a NotificationPolicy for the migration emails.
this fix also removes those NotificationPolicy objects for existing users.
test-plan:
- make sure you already have a non-immediate notification policy for the
'Other Administrative Alerts' category. you can do this simply by going
to the notifications page and hitting save.
- to to courses/:id/content_exports, and perform a course export
- you should get an email when it completes
- perform a course copy
- you should not get any migration/export related emails.
Change-Id: Ie0aaeee1d8d2be4ad30fecf552973fc49e25ee28
Reviewed-on: https://gerrit.instructure.com/10652
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
This lays the groundwork for the new collections feature. See the API
documentation for details. There is still some pending tasks before this
is totally ready, see the development board.
test plan: no UI yet, but you can make api calls to add/edit/delete
collections, add/edit/delete items, and upvote items.
Change-Id: I8fa019d428d1aae3ac62b1f34ebe2ac3c77310be
Reviewed-on: https://gerrit.instructure.com/10465
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
Use a db function with a SELECT .. FOR UPDATE clause to avoid lock
contention on the table. The makes popping off the queue a completely
serial process -- but that's what we want here.
note this also defines a maximum (lowest) priority on jobs of 1,000,000
which we didn't have before, any lower priority will now be ignored.
however, the lowest priority we use in the code is 20.
closes#8517
test plan: ensure delayed jobs still function as expected, including
when there are a very large number of jobs in the queue. test with at
least 100,000 jobs.
Change-Id: Ibf0b4ce1d54f9400d7e734f0736c702e67316b76
Reviewed-on: https://gerrit.instructure.com/10618
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
we think this will avoid a deadlock condition where an insert takes the
ShareRowExclusiveLock at the wrong moment and deadlocks us inside the
trigger.
test plan: not consistently reproducable without major postgresql
instrumentation. ensure that jobs with a strand function as normal
(running the specs is a good sanity check)
Change-Id: Ib8c8ba972fd3c51ff116776213c936b9d7a48785
Reviewed-on: https://gerrit.instructure.com/10663
Reviewed-by: Zach Wily <zach@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
fixes two issue with the ungraded count logic:
1. the submission triggers updated the count regardless of the enrollment
status (or existence). teachers or people not even in the course could
cause the count to change (e.g. by taking/previewing a quiz)
2. the enrollment triggers did not factor in the enrollment type. so
teachers/tas (un)enrolling in the course would cause the count to
change if they had any submissions (e.g. from a quiz)
test plan:
1. create a quiz with essay questions (so that submissions will need to be
graded)
2. enroll a student in multiple sections in the course
3. take the quiz as the student
4. take the quiz as the teacher
5. take the quiz as an admin not in the course
6. confirm the ungraded count is just 1
7. unenroll the teacher
8. confirm the ungraded count is still 1
9. unenroll the student
10. confirm the ungraded count is now 0
Change-Id: I11dbf3915d79f9820267469fad8949abfa008e14
Reviewed-on: https://gerrit.instructure.com/10451
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Joe Tanner <joe@instructure.com>
The "FOR UPDATE" clause we added triggers a bug in Postgresql 9.0.{0-3},
fixed in 9.0.4. We're backing out the change for now.
we deleted the original migration that added the "FOR UPDATE", this new
migration is just to revert on any environments that had already run
that migration.
Change-Id: I021f64a63d8c401945b47a18409667422b808b76
Reviewed-on: https://gerrit.instructure.com/10569
Reviewed-by: Ben Chobot <bench@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
content migration notifications used to be in a category that forced them to be
sent immediately, but they inadvertently got moved to a category without that
enforcement. now they have their own category that forces sending again.
test plan:
- set your 'For Administrative Alerts' to something other than 'immediately'
- do a content import or export (not course copy)
- you should get an email as soon as the migration succeeds or fails (not in
a summary at the end of the day/week).
Change-Id: Ifb0eddf586f3e0defc11f6159f48625e7493ccf8
Reviewed-on: https://gerrit.instructure.com/10385
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
Schools want to be able to specify a saml attribute used for the user's login
instead of NameId. Usually that will be the EPPN (or the portion before the
@). This adds a Login Attribute settings to the SAML settings page, with
options for NameID (the default and historic behavior), the
eduPersonPrincipalName, and the eduPersonPrincipalName with the domain part
stripped.
test plan:
* Set up Shibboleth... just kidding. Better to just look at the spec.
Standing up your own shib instance is a ton of work.
Change-Id: I04e32022b940da4cb82ea5f9a59012eb48de2ab6
Reviewed-on: https://gerrit.instructure.com/10120
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
Tested-by: Zach Wily <zach@instructure.com>
closes#7561
Test plan:
- create some appointment groups:
- make one with multiple courses
- make one with a course, and then add more courses after saving
- make one restricted to some (not all) sections in a course [1]
- make one that has students sign up in groups [1]
- make sure that only students that match the above criteria are able
to see/reserve those appointment groups
[1] you can only choose the group-signup option or restrict
appointment groups to certain sections on creation (these options
won't be availabe when editing later)
Change-Id: I1cff5fb4ed233882c2061f8fd8a7ba6f2d4959b0
Reviewed-on: https://gerrit.instructure.com/9407
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>
this is similar to #7231, though the root cause is a little different.
users with duplicate concluded enrollments (e.g. two sections in a course)
may have had improperly tagged private conversations. this was because we
weren't uniq'ing User#conversation_context_codes, so it naively assumed
that both users were in those concluded courses in Conversation#
current_context_strings
the data migration is just like the one we ran in #7231, but a little more
broad (in this case, the Conversation#tags are correct, but the
ConversationParticipant#tags are not). refactored migration code and specs
accordingly, and made it a little nicer on the db.
test plan:
1. enroll user A in two sections of course A (may only be possible via SIS
or console)
2. conclude course A
3. enroll user A and user B in course B
4. start a private conversation between the users (doesn't matter who
starts it), but be sure to find the recipient by searching, NOT by
browsing the course
5. ensure that both users only see course B as the tag on the conversation
Change-Id: I3065b827433b33dbb7ff6a4d0d2affa73cc06e37
Reviewed-on: https://gerrit.instructure.com/10340
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
closes#6832
Test plan:
- in a separate branch (master), create a new appointment group and
assign it a group category or section
- switch to this branch and migrate; the appointment group should
still be assigned to the group or section you chose earlier
- create a new appointment group and assign it some course sections
- verify that students in those sections can reserve appointments, but
not students that do not belong to those sections
Change-Id: I1662374c5e6d2e5e9f7d6b54b0bc91420f150b7a
Reviewed-on: https://gerrit.instructure.com/8765
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>
this creates a "plugin" that allows an account to choose
assignment properties that can be locked when a course is
copied
Test Plan:
* enable the plugin for an account
* create an assignment and set it to be locked on copy
* copy the course to another course
* as an admin of the account you should still be able to edit the assignment
* as a non-admin teacher of the course you should not be able to edit the frozen properties
refs #7931
Change-Id: I61d5dbfdf10f8f7519f8db06449b14ef5e7b8454
Reviewed-on: https://gerrit.instructure.com/10190
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
we think this will avoid a deadlock condition where an insert takes the
ShareRowExclusiveLock at the wrong moment and deadlocks us inside the
trigger.
test plan: not consistently reproducable without major postgresql
instrumentation. ensure that jobs with a strand function as normal
(running the specs is a good sanity check)
Change-Id: I32fcac82dc5264445897a517ed582e17e896a14b
Reviewed-on: https://gerrit.instructure.com/10300
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
avoid a chicken/egg scenario where the data migrations might be running
broken model code, depending on how far apart the old and new versions of
canvas are
test plan:
1. set up a canvas install on a revision prior to f8caf7d4
2. create some conversations
3. pull the latest code
4. run migrations and ensure that there are no errors
Change-Id: I7013d6ea73d5913e291c44242a6eb1415100ddc7
Reviewed-on: https://gerrit.instructure.com/10267
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
also move logic into a separate file, and run it in a delayed job when in
production mode
test plan:
1. confirm the migration runs in postgres/sqlite/mysql
2. confirm the migration spec passes in postgres/sqlite/mysql
Change-Id: I1169e1fdf6d3d9996e7fbad0d2a4ab41662b7ae8
Reviewed-on: https://gerrit.instructure.com/9939
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>
* wrap temp table population and extraction in a transaction
* rework first conversation_message update to not use the temp table
* move migration into delayed job
test plan:
* run migration on mysql/postgres/sqlite
* ensure there are no errors
* run migration spec on mysql/postgres/sqlite
* ensure there are no errors
Change-Id: Ia6e645ed08dfaf75f9f3438c0968c74222f8e017
Reviewed-on: https://gerrit.instructure.com/9912
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
we tried to get away with the student view student not having a pseudonym, and
it worked until we got to file upload assignments. you need a pseudonym to
upload files, so we have to give the fake student one.
test-plan:
- before downloading, make sure you have a student view student created who
does not have a pseudonym
- run migration and make sure that student now does have a pseudonym
- make sure a new student view student gets a pseudonym on creation
- submit a file upload assignment while in student view
Change-Id: I1a494c36cebf2b94a74cc9e531ac64b220fbfe36
Reviewed-on: https://gerrit.instructure.com/9785
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
start tracking root_account_ids more consistently for messages, and
aggregate them on the conversation. add a scope when masquerading such
that conversations tied to other root accounts are hidden. a masquerader
must have access to all root accounts a conversation belongs to in order
to see it (unless the masquerader is a site admin)
test plan:
1. enroll a user in courses under two different root accounts (let's call
them accounts A and B)
2. as the user, log in under the account domain for A and send some
messages
3. log in under the account domain for B and send some messages to
the existing conversations, as well as to some new recipients
4. set up an account admin with access only to B
5. log in as the admin, and masquerade as the user
6. ensure you can only see conversations created under B (none of the ones
that are solely A, or both A and B)
7. log in as a site admin, and masquerade as the user
8. ensure you can see conversations from both accounts
Change-Id: I4a26b8b8914fd531103bb92c3d4442de843d9986
Reviewed-on: https://gerrit.instructure.com/9767
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Jon Jensen <jon@instructure.com>
fix an issue where private conversations were improperly merged when
merging user accounts (only a problem when it was being merged into an
existing conversation on the target user). this would cause page errors
due to orphaned conversation_participants.
includes a data migration to fix affected conversations.
test plan:
1. on the old code, create three users (A, B, C)
2. start a private conversation between A and B
3. start a private conversation between A and C
4. merge user B into user C
5. confirm that users A and C get a page error when going to conversations
6. upgrade the code and run the migration
7. confirm that conversations load and the merged conversation appears
correctly
8. on the new code, create three more users, repeat steps 2-4, and confirm
that conversations load and the merged conversation appears correctly
Change-Id: I50785764ae4376a38824ffa099ff678686ec5023
Reviewed-on: https://gerrit.instructure.com/9847
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
since an external connection pool might be in use, and it would mess
up future migrations
test plan:
* create two new shards through pgbouncer in the same database in
quick succession
Change-Id: I2bf582a543a48f9df83888feda54818211931a34
Reviewed-on: https://gerrit.instructure.com/9842
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
closes#7769
Test plan:
- create an appointment group
- open the appointment groups and edit one of its appointments
- change the number of users allowed to attend the appointment
- make sure that no more users can reserve the appointment than
configured
Change-Id: I4365113d81d123bd8c3cdc24af8761cc6b9027a3
Reviewed-on: https://gerrit.instructure.com/9648
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
* changes temp table usage to be more efficient (also, mysql doesn't let
you ref the same temp table twice in a query)
* make it non-transactional to reduce locking
* break up the big update statements into batches of 1000
test plan:
* confirm the migration runs in mysql/postgres/sqlite, and the migration
spec passes for all three as well
Change-Id: Ib15028fa49103230e2cb4957754d2ac19853f90c
Reviewed-on: https://gerrit.instructure.com/9802
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
This adds the ability for ContentMigration to copy courses
by exporting to a content package and then importing that
package. For attachments it just creates a new Attachment
object instead of exporting/importing them.
The course copy API did not change.
The endpoints used by the course copy UI did change and this
commit doesn't allow selective copying, it only has the
option to copy everything. Selective copying will come in
another commit.
There are also various bug fixes for export/import
Test Plan:
* copy a course through the API
* copy a course through the Content Imports path
* copy a course through the Copy Cours button on the course settings page
* export a course
refs #4645
Change-Id: Ie577329ab7caaea8dfb9359542224a1a2657e167
Reviewed-on: https://gerrit.instructure.com/9742
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
rather than proxy attachments through the conversations controller and
cause a long-running db transaction, we now just send them to the right
place (files controller, s3, whatever), and then create the message. we
now store the attachment in a special folder on the user so that they
can more easily be tracked in the future for quota management.
because we now just store one instance of each attachment, sending a
bulk private message w/ attachments should be a bit less painful.
known regression:
if a user deletes a conversation attachment from their files area, it
deletes if for all recipients. this is essentially the same problem as
tickets #6732 and #7481 where we don't let a "deleted" attachment to
still be viewed via associations with other objects.
test plan:
1. send an attachment on a new conversation and confirm that was sent
correctly and can be viewed by recipients
2. send an attachment on an existing conversation and confirm that was
sent correctly and can be viewed by recipients
3. send an attachment on a bulk private conversation and
1. confirm that was sent correctly and can be viewed by recipients
2. confirm that only one attachment was actually created, but each
message in each conversation is linked to it
4. send multiple attachments and confirm that they were sent correctly
and can be viewed by recipients
5. perform steps 1-4 for both local and s3 uploads
Change-Id: I7cb21c635f98e47163ef81f0c4050346c64faa91
Reviewed-on: https://gerrit.instructure.com/9046
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
affects: course model, api
test plan:
* add a course with the api, included a public description;
* verify that public description field is returned and
exists on the course in the db.
Change-Id: If2766cf7ba946af5cb8a5cafebaa8209754ec2b3
Reviewed-on: https://gerrit.instructure.com/9723
Tested-by: Cody Cutrer <cody@instructure.com>
Tested-by: Zach Wily <zach@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>
consumers can use this to update the view client-side and ensure it's
always up-to-date even if the view is behind.
Change-Id: I1ff0cd80a758a113f8881ef74de78cc28e2fa16b
Reviewed-on: https://gerrit.instructure.com/9717
Reviewed-by: Ryan Florence <ryanf@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
Tested-by: Brian Palmer <brianp@instructure.com>
basically for detailed reports, have a row per account, instead of
stuffing everything into one massive hash with every account
test plan:
* ensure that the account statistics page still works
* ensure that the statistics update when the delayed job runs
Change-Id: I2d4c9ff5a86c8df6d82097aa4f0203a0f55ddb2d
Reviewed-on: https://gerrit.instructure.com/9565
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Cody Cutrer <cody@instructure.com>
We changed the threaded boolean to a discussion_type string, with
currently two types: 'side_comment' and 'threaded'
test plan: get the list of topics in the api, verify threaded is
returned appropriately.
Change-Id: Id5bfb867329d93fe8be6131e2e7c401f70ced290
Reviewed-on: https://gerrit.instructure.com/9647
Reviewed-by: Zach Wily <zach@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
mysql always defines a field size limit, and has a default that is too
low for our purposes here, so we need to set an explicit limit
Change-Id: I86509dae04ca4f403dd77ccf45765798b2125ae8
Reviewed-on: https://gerrit.instructure.com/9639
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>
This is a cached-in-the-db view of the topic view json response, and
serialized arrays of participant ids and entry ids (for determining
unread entries). This is an optimization to make loading topics fast and
avoid a lot of db traffic.
test plan:
The UI for this new API is still under development, so this needs to
be tested via the API. With jobs turned off, hit the discussion view
endpoint and verify the 503 response. Then run the job to generate the
view, and verify the response json. Verify that any change to the
discussion that requires updating the view, properly updates the view.
Change-Id: I1eacace438a75a41b8b85ce6c42bfd042f27d1ea
Reviewed-on: https://gerrit.instructure.com/9443
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>
If a discussion topic is threaded, it'll allow sub-entries to have other
sub-entries as parents. otherwise it'll force sub-entries to always have
a root entry as a parent (restoring the behavior pre-e9edb2a86f
test plan:
Make a side comment in reply to another side comment, reload the page,
and verify it shows your comment. Do the same via the API. There's no UI
yet for marking a discussion as threaded, but you can do it from the
console and then verify that replies to replies are now nested.
Change-Id: I6bddcfa318f36bcbc6ddb03c152ff1fa8fa550b8
Reviewed-on: https://gerrit.instructure.com/9506
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
we were including deleted discussion entries in our default unread counts, but
there was no way to mark these as read from the UI so it looked like the counts
were just off.
test-plan:
- run the migration and make sure existing unread counts look good
- create a discussion, make some entries, and delete some
- as a different user, view that discussion and make sure counts look right
Change-Id: I601fc7e0726a7982a2843b5a8a71cf782e5092eb
Reviewed-on: https://gerrit.instructure.com/9468
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
Removes unused methods and associations from models, and removes the
PageViewRange model completely, which is no longer used.
test plan:
* run the specs
* verify that no active page-view-related functionality was inadvertently removed
Change-Id: I8522d151555db64a0ad2548334ae642f9fefd54e
Reviewed-on: https://gerrit.instructure.com/9324
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
fixes#5196
test plan:
* perform an action that causes lots more than
User.max_messages_per_day notifications to be sent out
* in the db, there should only be one entry with notification_id
is null for that communication channel
Change-Id: Ibd1c5f5273b171bd4ebb6252e942e23dac4aa7e3
Reviewed-on: https://gerrit.instructure.com/9260
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
Adds support for threaded discussions. The existing API has been updated
to allow posting replies to replies, and returns the whole thread of
sub-replies rather than just first-level side comments. For backwards
compatbility, the sub-replies are returned in a flattened newest-first,
consumers can look at the parent id of each entry to re-build the
nested, threaded structure.
Another commit will add a new API endpoint for retriving the threaded
structure of the conversation in a more efficient and natural format.
test plan: there's no UI for threaded discussions yet, but you can use
the API to post replies to replies, and retrieve threads and verify the
parent_id information.
Change-Id: I7d541d093fbbdb6fc8a18d079b4337515c02a8a4
Reviewed-on: https://gerrit.instructure.com/9121
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
this table, it is large
test plan: run the migration, verify a job is just created. run the job,
verify it updates null last_access rows as expected.
Change-Id: Id134d6795c8eef76181c20f5e3c0920dd51dda21
Reviewed-on: https://gerrit.instructure.com/9192
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
Before only the Submission was being updated and not the
QuizSubmission. Now the fudge points on the current QS are
updated to reflect the score set in the gradebook.
This prevented modules from progressing if the value was
changed in the gradebook.
Because the score set there should be the absolute score,
if the quiz is set to take the highest a flag is set to
prevent a previously-higher score from being the kept_score
Test Plan:
* Create a quiz with a question with 5 points and publish it
* Create a module item to to that quiz that needs 10 points to complete
* Take the quiz as a student, the module should not unlock
* As the teacher set the grade to 11 in the gradebook
* The module item should be unlocked for the student
closes#7331#4174
Change-Id: I30d92bfe865584b5b526cda36ef336348a647de4
Reviewed-on: https://gerrit.instructure.com/9091
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
If an LDAP server times out, log the time of failure to the
AccountAuthorizationConfig, and don't try to connect again until some
time passes.
test plan:
* set up an account that uses LDAP, point it to localhost port 6767
* run `nc -l 6767` or some other command to listen on that port, but
not ever respond
* attempt to log in on that account, the first log in should hang for
5 seconds. subsequent logins should skip even trying to talk to
ldap, until 1 minute passes.
* change the account authorization config, verify that it tries the
LDAP server again immediately
Change-Id: Iea5d7c27f72341d16ae370a0f6ad4ec90fb96b74
Reviewed-on: https://gerrit.instructure.com/9116
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
quiz submissions that have essay questions are marked as pending_review, but
they have a grade, so the previous trigger was ignoring them when incrementing
the needs_grading counts. this changes increments the counter for
pending_review even if there is already a grade.
test-plan:
- create a quiz with an essay question
- as a student, submit the quiz
- as a teacher, your to do list should indicate that you have a quiz that needs
grading.
fixes#5326fixes#7466
Change-Id: I7cf2293bb43df882000902109124810e896e66ad
Reviewed-on: https://gerrit.instructure.com/9095
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
also fix/enable zip file uploads in group and user contexts -- they were
partially enabled, but broken. This required generalizing
UnzipAttachment to work for any context, not just courses, though that
was mostly just a matter of renaming things.
The zip file uploads in accounts are still not enabled, as the accounts
file section is not implemented yet, though it's referenced in the
routes file.
fixes#5913fixes#5728fixes#5463fixes#5012
test plan:
* upload a zip file to a course, to a group, and to a user's files. in
each case, try uploading the zip both through the button in the file
browser, and by dragging a zip file into the file browser (in a
capable web browser)
Change-Id: I6c648ef677d2bd61ae41a2b8fe0f89be43d63375
Reviewed-on: https://gerrit.instructure.com/7402
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
also move notifications in "new email messages" to "administrative alerts",
because that's a much better description of them
test plan:
* go to communication preferences
* the last one should be "administrative alerts"
* "Files", "Private message from student", "New email messages",
and "Context Message" should be nowhere to be found
Change-Id: I8b30d53b8137ab4b17cb436920d87331327e01f9
Reviewed-on: https://gerrit.instructure.com/8996
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
the backend for tracking read/unread state of discussion entries/topics and
unread counts for discussion topics. the strategy is to assume that an entry is
unread unless there is an an entry in the join table indicating otherwise.
interface:
- DiscussionEntry has #read_state, #read?, and #unread?. You can use these
either by passing in a user directly, or by setting the current_user
attribute on the object before calling the method (useful for json). This
should already be handled for existing controllers.
- DiscussionTopic read_state functionality as above, plus #unread_count. Same
general idea with current_user as above.
- routes are setup through the api, use PUT to mark read, DELETE to mark
unread, or rails helpers like course_discussion_topic_mark_read_path
test plan:
- click through discussion pages, and make sure they all still work (no visible
changes though, this is backend only)
- manually call some of the above methods from the console
- make sure sql for updates is reasonable
Change-Id: I655ab6a69a8cbdf1c7c99a5548b8ed0d7eadba02
Reviewed-on: https://gerrit.instructure.com/8671
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>
test plan:
* run migration, it should pass
* run migration spec, it should pass
Change-Id: I024c7b07671212eb42943273d5c361cce516e015
Reviewed-on: https://gerrit.instructure.com/8961
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
* use a single DJ instead of a DJ per applicable attachment
* don't hold a transaction for any length of time
* do direct updates, instead of through AR, to avoid 7 extra
queries per update from before_save callbacks
test plan:
* run the migration; it should run relatively quickly, and not
have any errors
Change-Id: I6af6f0ddbcedbb69ad6afb3688d94feb32d0f201
Reviewed-on: https://gerrit.instructure.com/8905
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
previously, merging user accounts would result in conversations with
duplicate participants if both users were involved. additionally,
moved private conversations did not get their private hashes updated,
and private conversations between the merged users would lead to page
errors.
this fixes the behavior going forward (merging conversations if
needed) and adds a migration to fix broken conversations.
test plan:
1. create user A, user B and user C
2. create a private conversation between user A and user B
3. create a private conversation between user A and user C
4. create a private conversation between user B and user C
5. create a monologue for user A
6. create a monologue for user B
7. create a group conversation between all three users
8. merge user A into user B
9. confirm that user B now has three conversations:
1. a monologue containing the messages from 2, 5, and 6
2. a private conversation with C containing the messages from 3 and 4
3. a group conversation with just user C
10. confirm that all messages formerly by A now have B as the author
11. confirm that unread counts and message counts are correct
Change-Id: I1afd824ff8ce74430f247d5b4c728937786fa37f
Reviewed-on: https://gerrit.instructure.com/8738
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>
FileInContext (used by the zip importer) was bypassing the filename= setter
for Attachment, so when using attachment_fu, sanitize_filename was not getting
called on the filename. This resulted in us generating S3 urls with characters
in them like filename[0].txt. That normally worked fine. However, Firefox
would escape those characters when redirected to a URL like that, which would
cause a signature mismatch with S3.
This commit stops bypassing the filename= setter so files uploaded as zip
files and in migrations have escaped filenames. Because of difficulties
testing S3 attachments, the included spec is weak.
This also includes a migration that will rename attachments with []" in their
filenames, and make a copy of the S3 object to match.
There is also an unrelated spec refactor around faking out a portion of the
code about S3.
test plan:
* Enable S3
* Create a ZIP file with a file in it with a name like test[0].bin
* Upload that ZIP file to your files are, choosing to unpack the ZIP contents
* Verify that you can download the extracted file using firefox
* Also verify that the attachment's filename is escaped in the db
Change-Id: I54fc0682b64a9e0021b4b41236f8cab168a0e56e
Reviewed-on: https://gerrit.instructure.com/8875
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
Simon Williams