this is already true, just make sure it stays that way
test plan: n/a
Change-Id: Ia7340e8de57b18da509d51580b5827c7069af80a
Reviewed-on: https://gerrit.instructure.com/7166
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: JT Olds <jt@instructure.com>
Test plan:
- send a message to multiple people you have not previously had a
conversation with
- observe that the message was successfully sent (but nothing is added
to the conversation list)
Change-Id: I3b3b57a58faa10bfeb3067112810226434ffd532
Reviewed-on: https://gerrit.instructure.com/8001
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Ryan Florence <ryanf@instructure.com>
fixes#6530
Test plan:
- create a quiz with a question that takes a numerical answer
- enter a number and press enter
Change-Id: I961fb65262a505057448c91c2f5930ecac7005c8
Reviewed-on: https://gerrit.instructure.com/8016
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Ryan Florence <ryanf@instructure.com>
as a student, when you go to /courses/x/assignments
it was showing the course name under each assignment,
which was unneeded since it is inferred since you
are "in" that course.
fixes: #6792
test plan:
go to /courses/x/assignments as a student
verify that it looks like this:
http://cl.ly/2m0j450E2C1i3G0A271J
not this:
http://cl.ly/11011P2O1R1z433a2F3F
Change-Id: I5c5944b2d8abd948fbbf05bb38ff8428f26c195e
Reviewed-on: https://gerrit.instructure.com/7888
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Cameron Matheson <cameron@instructure.com>
I couldn't reproduce this bug, either locally or on canvas.instructure.com so
I just wrote some selenium tests that cover this behavior so that it doesn't
break again in the future.
test plan:
- create an assignment (leave type = assignment)
- edit the assignment, change type to discussion, save
- a discussion should be created for the assignment, which you can find in the
discussions tab or by clicking on the assignment.
closes#6611
Change-Id: I0a5340c5161c6f03c26e24c6db293e438389207a
Reviewed-on: https://gerrit.instructure.com/8019
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
It turns out that the defaults for daily/weekly policies actually do work -
the issue before was that they won't work for a user whose first communication
channel is retired. This fixes that problem by using the first active email
channel.
Another problem was that we would only build placeholder policies if the user
had NO notification policies at all. However, that rarely happens - when a
user receives a notification and they don't have a policy for it, that policy
gets built. In this case, when the user went to their preferences page, they
would see all the notification categories they didn't have policies for as
"never". This creates placeholder policies for all the notification categories
the user doesn't have one for.
It also makes sure the correct defaults are displayed on the preferences
screen.
test plan:
* Create a new user A, and enroll them in a course.
* As another user B, create a calendar event in that course. A calendar event
is by default a "never" item.
* As user A, go to your communication prefs and notice that all the default
preferences are still appearing. In the console you can verify that the
policy was created for you too:
> User.find(<A.id>).notification_policies
* Now, delete that policy from the database.
* As user A, add a new comm channel and delete the old one.
* As user B, create another calendar event
* Verify in the console that user A got a notification policy created for the
second communication channel.
* As user B, create a discussion topic.
* As user A, add an entry to that topic.
* As user B, comment on that topic as well.
Discussion entries are by default 'daily', so user A should have a
DelayedMessage. Verify in console:
> DelayedMessage.last
Change-Id: I3f7827d2aa83d5b72ab5f8f96aa92ef96652f115
Reviewed-on: https://gerrit.instructure.com/8062
Reviewed-by: JT Olds <jt@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
turnitin is unescaping the post values it receives before computing the md5, so
if we send a name that contains something like "%20" in it, then the md5's
won't match. so we need to escape our post values after computing our md5 so
that they match on turnitin's end.
closes#6572
Change-Id: I34624db50f63078d9a984456756cea1bd8aebc3f
Reviewed-on: https://gerrit.instructure.com/7979
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
affects: pseudonyms_controller, api
test plan:
* as an authorized user, make request and verify that
requested user's pseudonyms for given account are
returned;
* make same request for a user with multiple pseudonyms
in a single account and verify that all are returned;
* make request as an unauthorized user and verify that
401 is returned.
Change-Id: I0633e79108c0c6e626cf52f5d39d91a9e67edbb6
Reviewed-on: https://gerrit.instructure.com/8025
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
When a user had not configured any notification policies, on the communication
preferences page, they would see everything as "immediate". That would lead
people to believe that those are the defaults.
Note that for now, only "immediate" notifications actually will be sent by
default. All others are assumed to be never, so that should be reflected in
the UI.
test plan:
* Create a new user
* Go to the Profile -> Communication Preferences page
* Notice that the pre-selected preferences are immediate for immediate
notifications, and never for everything else.
Change-Id: Ibbd908a5b6f38d144fd4f752bc57258e96c4dc78
Reviewed-on: https://gerrit.instructure.com/8050
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
we are seeing an issue with some requests where the account association
info in a course loaded with these enrollments is getting a proc stored
on it, so that it can't be Marshal'd and the caching throws an error.
We're still investigating the cause, but this fixes up the errors.
fixes#6867
Change-Id: Ibc24e7583d7f3a4f1f4362680f6a2c080a53ba53
Reviewed-on: https://gerrit.instructure.com/8051
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
the functionality where, if you chose a different
section to show, it would show only users from that
section in the gradebook2 broke. this fixes it.
refs: #6844, fixes: #6849
test plan:
* go to /courses/x/gradebook2 for something with 2
sections
* choose a different section to show in the top
left
* verify that it filtered the students it is
showing to just those in that section
Change-Id: I8051c6778e6959c9f2767edaa9e89ae453727bdb
Reviewed-on: https://gerrit.instructure.com/7990
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Ryan Florence <ryanf@instructure.com>
test plan:
* in gradebook1, have the total grade for a row
have a decimal (like 33.3%)
* load gradebook2 and verify that it is a decimal
rounded to 1 place too (so 33.3% and not 33% or
33.33%)
Change-Id: I5f5f2de5b3fe953690b678aa956279c471c84d9d
Reviewed-on: https://gerrit.instructure.com/8036
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jake Sorce <jake@instructure.com>
Tested-by: Jake Sorce <jake@instructure.com>
affects: course creation on /accounts/:id
test plan:
* navigate to courses page for account;
* click 'Add a New Course' button;
* fill out new course form and click 'Add Course';
* verify that (1) modal closes, and (2) flash message
appears.
Change-Id: Idce434d288dd8b81a29edfd1b5144c0dcedfc7ea
Reviewed-on: https://gerrit.instructure.com/8032
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Ryan Florence <ryanf@instructure.com>
affects: pseudonyms_controller/api
test plan:
* as authorized user, post request and verify
that new pseudonym is created;
* as authorized user, post duplicate pseudonym
and verify that 400 is returned.
* as unauthorized user, post request and verify
that 401 is returned.
Change-Id: I4b2ad73dd68f43a935e37d3fe9f02f4d0269b72a
Reviewed-on: https://gerrit.instructure.com/8024
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>
In the SpeedGrader, if you went past a user with > 1 submission, grading
subsequent students with only one submission would result in a page error in
the submission content iframe after the grade is saved. We were only
populating the <select> element for > 1 submissions people, and the change()
binding on it would call for everyone else, triggering badness.
test plan:
* Create 2 users in a course with an assignment.
* Submit the assignment twice as student A, and once as student B.
* In SpeedGrader for that assignment, browser to student A, then student B.
* Grade student B's submission.
* Notice that when the submission reloads in the iframe, it is not a page
error.
Change-Id: If461ecc1121fcae0549f1f451ecaa33edf2aa69e
Reviewed-on: https://gerrit.instructure.com/8046
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
for a user with :read_as_admin but not :read permissions to a course
(odd, but currently happens for designer enrollments in an unpublished
course) the user is redirected from the course "Home" to the course
settings. however, the settings page turns around and makes an xhr
request to /courses/X again for some data. don't redirect that call or
you get an infinite loop.
also, this change cuts out the unnecessary remainder of
CoursesController#show for xhr requests; before we still did all that
work and then just ignored it.
Change-Id: I4b6d39c8bf4e12173a4aa168903d2148f89c7a3d
test-plan:
- create an unpublished course
- enroll a designer in that course (currently only possible via sis or
console)
- while logged in/masquerading as the designed, visit the course's
settings page
- confirm there is *not* a runaway xhr recursion
Reviewed-on: https://gerrit.instructure.com/8039
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Ryan Florence <ryanf@instructure.com>
affects: courses api
test plan:
* make request to courses#index or courses#show api and
verify that json response includes account_id field.
Change-Id: I77db3e7adc99c7150dab691e0091f4a32801c486
Reviewed-on: https://gerrit.instructure.com/8026
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
fixes#6636
there seems to be a race condition where this redirect will sometimes
happen before the LDB notices the params telling it to go into
fullscreen lockdown mode. so we just avoid the redirect, which isn't
necessary since we didn't make a post request anyway.
test plan: in the LDB, go to a quiz that requires the LDB, ensure that
it goes fullscreen.
Change-Id: Idf94ae56ce5e695f51fe75da9d98611f60164666
Reviewed-on: https://gerrit.instructure.com/7564
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>
We were just finding the first CommunicationChannel, which might even be
retired. This will make sure we get an active one and it's an email one.
test plan:
* (perform all these steps on a new account, and don't go into the
notification preferences screen)
* Create a new account with an email address X
* Add a new, secondary email address Y to your account
* Delete the email address X
* As another user, start a conversation with the user created above.
* Verify that you received the email notification at email address Y and not
X.
Change-Id: I7b16001918c5f39b5929e0f520069838a10bcf66
Reviewed-on: https://gerrit.instructure.com/8035
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Zach Wily <zach@instructure.com>
When importing, urls starting with '/' were flagged as not
relative. This commit changes the relative url detection
to use the URI library. There are also a lot of unit tests
added for the ImportedHtmlConverter.
Test Plan:
* Import a package that has a relative mage url starting with '/' on a wiki page
* The image should render when the import is complete
closes#6813
Change-Id: I008bb603b1abb53ae8753b44413cac8b9b1f4245
Reviewed-on: https://gerrit.instructure.com/8022
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
test plan:
* create a course, assignment, and submissions
* add a ta limited to a section
* download the submissions zip for an assignment
* it should only include submissions for students
in the section the ta is a part of
Change-Id: I68e580301ad284186ec25742bc5869ae031baa69
Reviewed-on: https://gerrit.instructure.com/8015
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
test plan:
* create a user via the UI
* add another pseudonym
* delete the extra pseudonym
* import a new user via SIS import with the login of the pseudonym
you deleted
* it should create a separate user for the SIS user, instead of
resurrected the deleted pseudonym onto the original user
Change-Id: Ia1ff1490bae82fc4daef3f91b05293d7c79cc50b
Reviewed-on: https://gerrit.instructure.com/7975
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
course api now returns html_url with properly scoped domain.
affected areas: courses api
test plan:
* make request to courses#index or courses#show, passing
'html_url' as part of include[]; verify that a course
url with proper domain name is returned.
Change-Id: I5bff251723693e86c6bb2b2249853960ec486265
Reviewed-on: https://gerrit.instructure.com/7829
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: JT Olds <jt@instructure.com>
find_by_* does a data type check, so doesn't work with non-integer ids
test plan:
* log in as site admin
* access an account on a different shard
* masquerade as someone
* unmasquerade
Change-Id: I672d96560d1598801a51f3e5252b32bcbd365fc7
Reviewed-on: https://gerrit.instructure.com/7817
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
Somehow the after_attachment_saved callback in Attachment was getting called
on non-scribdable attachments with a workflow_state of 'processing'. We were
previously only skipping non-scribdable attachments if the workflow_state was
'pending_upload'. Now we will skip scribd processing for any non-scribdable
attachment, no matter it's state.
Also changed UnzipAttachment to only create a scribd job if there are any
scribdable attachments.
test plan:
* Since we're not sure how the attachments originally got in the state
described above, it's hard to describe repro steps. However, if you upload
some images, they should not get scribd jobs created for them.
Change-Id: I7381af3f0928c2decf4f224834780cc90fbbc103
Reviewed-on: https://gerrit.instructure.com/8028
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
Nokogiri now converts html entities to the actual utf-8 characters, and
the order of html attribute output changed in some cases
Change-Id: I4dce79c5b9e335c9fdec2f9affe97f8f11cc3c5e
Reviewed-on: https://gerrit.instructure.com/8010
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
added validate text to common and cleaned up course_settings to use common method
Change-Id: I01db65bbe03b117cde59f9a1a5b857c7960acaf3
Reviewed-on: https://gerrit.instructure.com/7976
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Bryan Madsen <bryan@instructure.com>
if the site admin account has a global stylesheet or
javascript include, include those files in all other
accounts.
affects: application layout
test plan:
* run specs
Change-Id: I87031fef39eac7ec77e318257c26b5378819a669
Reviewed-on: https://gerrit.instructure.com/7963
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>
When a user didn't have any notification policies, default values are used to
decide when to send them notifications to their communication channel. Since
Conversation Message didn't have a default set, it used 'daily'.
It looks like there's another issue where anything except for 'immediately'
would not get sent at all. (It wouldn't get batched up for the digest email).
See communication_channel.rb:216. That should probably be fixed separately.
This fix will just address the common use case where people expect to get
emails about Conversations without having to do anything extra.
test plan:
* create a new user, verify the email address, etc, but do not save any
notification preferences.
* as another user, start a conversation with the new user.
* verify that the new user got a Message or email about the conversation
message.
Change-Id: Ib198ba528a80a8b1576d236bfbd91b3cc72980db
Reviewed-on: https://gerrit.instructure.com/7991
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
A bug in the upgraded nokogiri library was causing nil access errors
when trying to truncate html.
See https://github.com/tenderlove/nokogiri/issues/595
test plan: Send a discussion topic message to the dashboard that is more
than 50 words long. verify that it is truncated with ..., and no error
is generated.
Change-Id: I35523d125e50beac82ed8cf7214a64d6ce0d7001
Reviewed-on: https://gerrit.instructure.com/7989
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
test plan:
* run the migration
* redo the migration
* do a batch mode sis import
Change-Id: I0204256f3825ffe0b4d252bbb83a0604d7a58942
Reviewed-on: https://gerrit.instructure.com/7904
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Reviewed-by: Ben Chobot <bench@instructure.com>
Updating to nokogiri 1.5.0 requires updating to sanitize 2.0.3 as well.
Since the API for sanitize changed, we take this opportunity to remove
the monkey patching in config/initializers, and use the actual
transformers plugin interface for sanitize.
The changes to html in the specs are due to nokogiri making a couple
changes around empty tags -- html5 wants <img> , not <img />
test plan: The existing specs exercise both gems, to ensure
compatibility.
Change-Id: Id04d017dda056e03205b373ac9bfbf71bd338cb9
Reviewed-on: https://gerrit.instructure.com/7988
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
This is in anticipation of upgrading to sanitize 2.x, so we can upgrade
nokogiri to 1.5.x
test plan: n/a
Change-Id: Ie59459dc2c4baeb48aca54b7ec30384239e06354
Reviewed-on: https://gerrit.instructure.com/7987
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
semanticDateRange returns html, so the handlebars template needs to use {{{}}}
so the html isn't escaped.
test plan:
* Create an appointment group as a teacher
* Reserve one of the slots as a student
* Verify that the green box in the appointment list shows the time chosen
correctly without any of the html.
Change-Id: If4f4732b5dea9e169c7c88701b3c7c10faf69720
Reviewed-on: https://gerrit.instructure.com/7977
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Tested-by: Zach Wily <zach@instructure.com>
this spec was failing sometimes because
the link still had the :hover state applied
this just makes sure to mouse off of it
Change-Id: Ifb5cf36b1d525625946159a03f012afa8084bc61
Reviewed-on: https://gerrit.instructure.com/7973
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Bryan Madsen <bryan@instructure.com>
when a user is enrolled in one or more courses, we now give the query
planner a more efficient way of finding rows in the course subselect in
User#messageable_users. the new condition is redundant (and less
restrictive than the course_sql ones), but it ensures the planner will
use the index on enrollments.course_id and then filter on the other
fields.
the performance gains are most relevant in mysql when there are multiple
conditions in course_sql (from ~4s down to <0.01s)
test plan:
ensure the various course_sql paths work (e.g. section visibility).
existing specs should already cover these different flows
Change-Id: I11d41daa1134146abd2b099be24e873d47ef567b
Reviewed-on: https://gerrit.instructure.com/7450
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Jacob Fugal <jacob@instructure.com>
Jake Sorce