- Adds support for outputting two top-level claims
in the NRPS v2 per-member `message` array element:
- `https://www.instructure.com/canvas_user_id`
- `https://www.instructure.com/canvas_user_login_id`
- While the `picture` claim is not included here, it
_is_ normally included in a LTI 1.3 launch request, but
with a different lookup mechanims than what NRPS v2 had
been using prevously (`User.avatar_url` rather than
`User.avatar_image_url`). So for launch<->NRPS parity,
the latter has been changed to match the former.
- Since we now need to whitelist the "public" claim
group, we needed a way to exclude the `role_scope_mentor`
claimn because it results in a N+1. So that claim was promoted
to its own group.
- Since claim groups were being re-organized, took the
opportunity to expanded claim group tests to verify
`privacy_level` enforcement.
References LTIA-41
Test Plan
- Place a LTI 1.3-enabled tool in a `Course` with several
active members.
- Invoke the NRPS v2 service for that tool and `Course`.
- Verify correct rendering of the `picture` field in `members`
elements.
- Verify correct rendering of
`https://www.instructure.com/canvas_user_id` and
`https://www.instructure.com/canvas_user_login_id` top-level
claims in each `member`'s `message` array element.
- Adjust the tool's `privacy_policy` to be non-`public`, verify
that neither `https://www.instructure.com/canvas_user_id` and
`https://www.instructure.com/canvas_user_login_id` claim
renders.
Change-Id: Ic3f306591214dd25815e8e4acc6f5c8d3d3aafac
Reviewed-on: https://gerrit.instructure.com/169525
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Previously custom param rules were in `MembershipsProvider`
and claim group rules were in `NamesAndRolesSerializer`. Now
they're all in the latter.
- Also moved `privacy_level` enforcement into
`NamesAndRolesSerializer` since that involves rendering of
one special custom param and all custom param config had
been moved into that class. This obviated the need for
`UserDecorator`, which is now completely removed.
- Simplified LTI message matchers significantly by accepting
an `opts` hash. So now we're back to just two top-level
code paths through those matchers... one for groups and one
for courses. Previously each of those had an alternate path
for `rlid` matching rules.
References LTIA-41
Test Plan:
- This is purely a technical refactoring. No behavioral difference
should be observable.
- To guard against regressions, repeat selections from:
- LTIA-41 tests to verify correct rendering of custom params when
the `rlid` query param is present
- LTIA-13/19 tests to verify correct enforcement of tool
`privacy_level`
- LTIA-24/34 tests to verify correct rendering of the
`lis_person_sourcedid` field.
Change-Id: If07e3886eeb069379b01282686133517446f9f4b
Reviewed-on: https://gerrit.instructure.com/169495
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Expands the `Canvas.xapi.url` and `Caliper.url` custom params into
the per-`member` `message` array in NRPS v2 responses.
- The custom param expansion itself was already present and did not
introduce any N+1s so patch is relatively simple. Biggest change
is that the `VariableExpander` needs to be given 'unwrapped'
AR entities since it does `is_a?` checks, especially on the context.
Refs LTIA-41
Test Plan:
- Confgure a LTI 1.3/Advantage tool with the
`$Canvas.xapi.url` and `$Caliper.url` custom params.
- Place that tool into a `Course` with active members
- Request NRPS v2 for that `Course`, specifying the course's LTI ID
as the value of the `rlid` query parameter.
- Verify that, for all active `Course` members, the expanded
`$Canvas.xapi.url` and `$Caliper.url` params have the following form:
- xAPI: `<base>/api/lti/v1/xapi/:tool_id-:user_id-:course_id-:timestamp-:nonce-:signature`
- Caliper: `<base>/api/lti/v1/caliper/:tool_id-:user_id-:course_id-:timestamp-:nonce-:signature`
Change-Id: I50f1e0470db29edba919cb48342d1b0d6ba0162d
Reviewed-on: https://gerrit.instructure.com/169381
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Expands the `Canvas.user.sisIntegrationId` custom param into
the per-`member` `message` array in NRPS v2 responses.
- The custom param expansion itself was already present and did not
introduce any N+1s b/c `Pseudonym`s are already preloaded, so patch
is simple.
Refs LTIA-41
Test Plan:
- Confgure a LTI 1.3/Advantage tool with the
`$Canvas.user.sisIntegrationId` custom param.
- Place that tool into a `Course` with active members having
at least one user with a primary `Pseudonym` having a value in the
`integration_id` field.
- Request NRPS v2 for that `Course`, specifying the course's LTI ID
as the value of the `rlid` query parameter.
- Verify that, for all active `Course` members, the expanded
`$Canvas.user.sisIntegrationId` custom param
in the per-`member` NRPS v2 `message` array reflects the value
in that `User`'s primary `Pseudonym`'s `integration_id` field
(possibly `null`).
Change-Id: I45d17fbec30523031e42529c09a20d7601cd0633
Reviewed-on: https://gerrit.instructure.com/169341
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Expands the `User.username` and `Canvas.user.loginId` custom params
into the per-`member` `message` array in NRPS v2 responses.
- The custom param expansion itself was already present and caused no
N+1s in a NRPS usage context, so a very simple patch.
Refs LTIA-41
Test Plan:
- Confgure a LTI 1.3/Advantage tool with the `$User.username` and
`$Canvas.user.loginId` custom param2.
- Place that tool into a `Course` with active members.
- Request NRPS v2 for that `Course`, specifying the course's LTI ID
as the value of the `rlid` query parameter.
- Verify that, for all active `Course` members, the expanded
`$User.username` and `$Canvas.user.loginId` custom params in the
per-`member` NRPS v2 `message` array are set to the `member`'s primary
`Pseudonym`'s username/login ID.
Change-Id: I7fbfcc0c49f655ed9a80de7f41d79b41cdf860cc
Reviewed-on: https://gerrit.instructure.com/169338
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Expands the `Person.address.timezone` custom param into
the per-`member` `message` array in NRPS v2 responses.
- The custom param expansion itself was already present but,
since we're rendering multiple users, the implementation
required the same trick as `$Message.locale` where we
temporarily set the time zone to match the current user,
then un-set back to the previous value after the user finishes
rendering.
- Also fix an typo in the nearby `$vnd.Canvas.Person.email.sis`
test setup.
Refs LTIA-41
Test Plan:
- Confgure a LTI 1.3/Advantage tool with the
`$Person.address.timezone` custom param.
- Place that tool into a `Course` with active members where at least
one member has a non-standard time zone preference.
- Request NRPS v2 for that `Course`, specifying the course's LTI ID
as the value of the `rlid` query parameter.
- Verify that, for all active `Course` members, the expanded
`$Person.address.timezone` custom param in the per-`member` NRPS v2
`message` array has the same value as the `member`'s time zone
preference or else the account default.
Change-Id: Ia701e5aa9021b54d42b830da61f41dd977db6b87
Reviewed-on: https://gerrit.instructure.com/169332
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Expands the `Person.email.primary` custom param into
the per-`member` `message` array in NRPS v2 responses.
- The custom param expansion itself was already present and caused no
N+1s in a NRPS usage context, so a very simple patch.
Refs LTIA-41
Test Plan:
- Confgure a LTI 1.3/Advantage tool with the `$Person.email.primary`
custom param.
- Place that tool into a `Course` with active members.
- Request NRPS v2 for that `Course`, specifying the course's LTI ID
as the value of the `rlid` query parameter.
- Verify that, for all active `Course` members, the expanded
`$Person.email.primary` custom param in the per-`member` NRPS v2
`message` array has the same value as the `member`'s top-level
`email` attribute.
Change-Id: I5fd70484906b69c85ff192317e0689ae0503e885
Reviewed-on: https://gerrit.instructure.com/169328
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Expands the `vnd.Canvas.Person.email.sis` custom param into
the per-`member` `message` array in NRPS v2 responses.
- The custom param expansion itself was already present - just
needed an alternate code path to avoid N+1 queries when
`CommunicationChannel` and `Pseudonym` associations are already
pre-loaded into `User`, as they are for NRPS v2.
Refs LTIA-41
Test Plan:
- Confgure a LTI 1.3/Advantage tool with the
`$vnd.Canvas.Person.email.sis` custom param.
- Place that tool into a `Course` with active members having
at least one user with a `Pseudonym` linked to a SIS
`CommunicationChannel` (ideally separate from the `User`'s
primary `Pseudonym`)
- Request NRPS v2 for that `Course`, specifying the course's LTI ID
as the value of the `rlid` query parameter.
- Verify that, for all active `Course` members, the expanded
`$vnd.Canvas.Person.email.sis` custom param
in the per-`member` NRPS v2 `message` array reflects the email
associated with the `User`'s SIS `CommunicationChannel` if present
or `null` otherwise.
Change-Id: I0c88b205608d4bc7b0c2974fe2766d5c59a90f76
Reviewed-on: https://gerrit.instructure.com/169325
QA-Review: Bill Smith <bsmith@instructure.com>
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
- NB since NRPS needs to deal with more than one user but
we're trying to reuse code that assumes it's just dealing
with a single LTI user performing a launch, this involved
some light trickery to temporarily set the current locale
to the preference of the user being rendered, then
restore whatever was set previously.
Refs LTIA-41
Test Plan:
- Confgure a LTI 1.3/Advantage tool with the `$Message.locale`
custom param.
- Place that tool into a course with active members having
at least one user-specific locale preference.
- Request NRPS for that course, specifying the course's LTI ID
as the value of the `rlid` query parameter.
- Verify that, for all active course members, the top-level
`locale` claim and the expanded `$Message.locale` custom param
in the `message` array reflects the user's preferred locale or
the system default locale if the user has not set a preference.
Change-Id: Id16626d7837098c57190dd3fcbcb0f12005a59f7
Reviewed-on: https://gerrit.instructure.com/169315
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Output a smaller version of a LTI 1.3 launch payload for
each NRPS v2 member when the NRPS request includes a `rlid`
query parameter.
- Most claims and custom params from the launch payload are
excluded either because:
- They describe the context and would thus be redundant, or
- They pose performance problems (N+1 queries, usually), or
- They are absent from the spec example, e.g.
`https://purl.imsglobal.org/spec/lti/claim/version`, or
- They require additional development and thus need to be
handled as a separate task.
- See `memberships_provider.rb` ~line 68 for list of
custom params supported in this commit. (More coming later.)
- Vast majority of the patch has to do with tests against
`JwtMessage`, which was modified to allow claims to be turned
on/off via a new white/blacklist mechanism in `AppUtil`.
- Custom param white/blacklisting is handled directly in
`VariableExpander` to satisfy the LTI rule that unsupported
params should just be echoed as-is. This (instead of keeping
all the white/blacklist support in `JwtMessage` ensures
consistent behavior w/r/t `VariableExpander`'s more sophisticated
features, specifically its suport for expanding variables embedded
into larger strings.
Closes LTIA-40
Test Plan
- Configure a LTI 1.3/Advantage tool with the supported set of
custom params listed in `memberships_provider.rb` starting ~line 68.
If using the POST
`/api/lti/accounts/:account_id/developer_keys/tool_configuration`
API, this is done by setting
`tool_configuration.settings.custom_fields` to a JSON object where
keys are the param name to be rendered into LTI payloads and values
are the $-prefixed custom param names themselves. Include several
nonsense entries as well as unsupported entries e.g.:
```
// ... snip ...
"tool_configuration": {
"settings": {
// ... snip ...
"custom_fields": {
"person_name_full": "$Person.name.full",
"person_name_display": "$Person.name.display",
"person_name_family": "$Person.name.family",
"person_name_given": "$Person.name.given",
"canvas_user_isrootaccountadmin": "$Canvas.user.isRootAccountAdmin"
"unsupported_param_1": "$unsupported.param.1",
"unsupported_param_2": "$unsupported.param.2"
}
// ... snip ...
}
// ... snip ...
}
// ... snip ...
```
- Place this tool into a course, ensure the course has several active
members.
- Launch the tool in order to observe the course context's LTI
identifier. Use that identifier as the value of the NRPS `rlid`
parameter, e.g. a GET to:
`/api/lti/courses/1/names_and_roles?rlid=4dde05e8ca1973bcca9bffc13e1548820eee93a3`
- Each `members` array element in the response should have a
`message` array with a single element being the simplified
representation of a LTI 1.3 launch payload, were that user to launch
the context referenced by `rlid`.
- The `message` entry should have two top level claims:
- `"https://purl.imsglobal.org/spec/lti/claim/message_type": "LtiResourceLinkRequest"`
- `"https://purl.imsglobal.org/spec/lti/claim/custom": <object>`
- The `custom` claim should include an entry for each `custom_fields`
key/value pair configured above, with supported entries being
correctly expanded and nonsense and unsupported entries being echoed
as-is.
- Repeat for a group context in the same course (still using the
course's LTI ID as the `rlid` value). Results should be the same,
though scoped to group membership.
Change-Id: If2591c62c494756d65774e3115abeca19935c988
Reviewed-on: https://gerrit.instructure.com/169090
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- When the `rlid` query parameter refers to an `Assignment`,
limit resulting NRPS v2 memberships as follows:
- `Course`: All active non-student enrollments plus all
student enrollments with an active `Submission` for the
assignment.
- `Group`: All active `Group` members with an active
`Submission` for the assignment.
- These results are further narrowed by the specified `role`
query param, if any.
- While refactoring `Group` AR queries to implement the
above, started enforcing a rule that was probably missed
previously: if the specified `role` parameter is not a
`Group`-ish role, treat the `role` as nonsense and return
an empty result set.
- It is an error if the `rlid` refers to an `Assignment`
but that `Assignment` is not bound to the same
`ContextExternalTool` resolved from the current LTI access token
and requested `Course`. As noted in the code, some of the
implementation details may need to change in the near future since
`Tool`<->`Assignment` binding for 1.3 `Tools` is incomplete.
- Changes to tests include a new `let`-managed
`:effective_page_size` ivar. This allows tests to distinguish
between the page size used to limit the query window (the
'effective' size) and the _actual_ response page size for
any given request (`:rsp_page_size`). This was not an
issue historically because the two values just hapened to
always be equal.
Closes LTIA-32
Test Plan:
- In a LTI 1.3/Advantage-enabled `Course`, configure
'Assignment 1' which is assigned to 'Everyone' and 'Assignment 2'
which is assigned to some subset of active student enrollments.
- Via UI or console or direct db access or API access, ensure both
assignments are bound to the Tool as which you plan to make NRPS
v2 requests (this Tool must have access to the `Course` owning
the assignments).
- Via console or direct db access or API access, ensure both
assignments have distinct `lti_context_id` values. These will be
their `rlid`s.
Course NRPS:
- Invoke NRPS v2 specifying the `rlid` for 'Assignment 1':
GET `/api/lti/courses/:course_id/names_and_roles?rlid=:rlid_1
Result should be all active course members across all roles.
- Repeat with a non-student `role` filter, e.g.:
GET `/api/lti/courses/:course_id/names_and_roles?rlid=:rlid_1&role=http%3A%2F%2Fpurl.imsglobal.org%2Fvocab%2Flis%2Fv2%2Fmembership%23Instructor
Result should be limited to active course members having that role.
- Repeat with a student `role` filter, e.g.:
GET `/api/lti/courses/:course_id/names_and_roles?rlid=:rlid_1&role=http%3A%2F%2Fpurl.imsglobal.org%2Fvocab%2Flis%2Fv2%2Fmembership%23Learner
Result should be limited to active course students.
- Invoke NRPS v2 specifying the `rlid` for 'Assignment 2':
GET `/api/lti/courses/:course_id/names_and_roles?rlid=:rlid_2
Result should be all active course members in non-student
roles, plus only those active students to whom 'Assignment 2' was
assigned.
- Repeat with a non-student `role` filter, e.g.:
GET `/api/lti/courses/:course_id/names_and_roles?rlid=:rlid_2&role=http%3A%2F%2Fpurl.imsglobal.org%2Fvocab%2Flis%2Fv2%2Fmembership%23Instructor
Result should be limited to active course members having that role.
- Repeat with a student `role` filter, e.g.:
GET `/api/lti/courses/:course_id/names_and_roles?rlid=:rlid_2&role=http%3A%2F%2Fpurl.imsglobal.org%2Fvocab%2Flis%2Fv2%2Fmembership%23Learner
Result should be limited to active course students to whom
'Assignment 2' was assigned.
- Repeat all of the above with pagination parameters and verify that
results can be paged through, one-by-one if necessary.
- Repeat the `role` filtered requests with nonsense values. Results
should always represent the empty set.
Group NRPS:
- In the same `Course` create a `Group` with several members, some of
which have been assigned to `Assignment 2`.
- Invoke NRPS v2 specifying the `rlid` for 'Assignment 1':
GET `/api/lti/groups/:group_id/names_and_roles?rlid=:rlid_1
Result should be all active group members.
- Repeat with the group leader `role` filter:
GET `/api/lti/groups/:group_id/names_and_roles?rlid=:rlid_1&role=http%3A%2F%2Fpurl.imsglobal.org%2Fvocab%2Flis%2Fv2%2Fmembership%23Manager
Result should be limited to the group leader.
- Repeat with the group member `role` filter, e.g.:
GET `/api/lti/groups/:group_id/names_and_roles?rlid=:rlid_1&role=http%3A%2F%2Fpurl.imsglobal.org%2Fvocab%2Flis%2Fv2%2Fmembership%23Member
Result should be all active group members.
- Invoke NRPS v2 specifying the `rlid` for 'Assignment 2':
GET `/api/lti/groups/:group_id/names_and_roles?rlid=:rlid_2
Result should be limited to only those group members to to whom
'Assignment 2' was assigned.
- Repeat with the group leader `role` filter, e.g.:
GET `/api/lti/groups/:group_id/names_and_roles?rlid=:rlid_2&role=http%3A%2F%2Fpurl.imsglobal.org%2Fvocab%2Flis%2Fv2%2Fmembership%23Manager
Result should be limited to the group leader if he was assigned
`Assignment 2`, else empty.
- Repeat with the group member `role` filter, e.g.:
GET `/api/lti/groups/:group_id/names_and_roles?rlid=:rlid_2&role=http%3A%2F%2Fpurl.imsglobal.org%2Fvocab%2Flis%2Fv2%2Fmembership%23Learner
Result should be limited to only those group members to to whom
'Assignment 2' was assigned.
- Repeat all of the above with pagination parameters and verify that
results can be paged through, one-by-one if necessary.
- Repeat the `role` filtered requests with pure nonsense and
non-group-ish values. Results should always represent the empty set.
- Create `Assignment 3` but bind it to a different
`ContextExternalTool`. Ensure that it too has a distinct
`lti_context_id`. A NRPS request for 'Assignment 3' should fail
with a 401.
Change-Id: I4fc16fc28797779abf3d7609961e68e9a8cdc022
Reviewed-on: https://gerrit.instructure.com/168736
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Pull custom LTI Advantage errors out of `AdvantageServices` and
`MembershipsProvider` into a new `Lti::Ims::AdvantageErrors`
module.
- Arrange those errors into a hierarchy and define default
`status_code` and `api_message` values for each. These can be
used for generic mapping to LTI Advantage API response messages.
(`api_message` is intended to contain a more terse, conservative
description of the problem, as compared to `message` which may
have additional debugging details suitable only for logging.)
- Also change casing on the default LTI Advantage access token to
'Missing access token' from 'Missing Access Token' to better match
convention on other errors.
- Stop trapping system failures during LTI Advantage access token
validation. These aren't actually 'handled errors', so log and
render these 'as usual'. I.e. client will get 500s instead of 401s.
Closes LTIA-39
Test Plan:
- This is primarily a technical refactoring so other than the error
message change noted above, testing would be focused regression
checks.
- Select several negative tests and verify that API error messages
and HTTP status code on the wire are still rendered as expected.
E.g.:
-- Check the response when the NRPS v2 API is invoked without an
access token. Error message should be 'Missing access token' and
status code should be 401.
-- Check the response when the NRPS v2 API is invoked with an
access token with an `exp` in the past. The error message should be
'Access token expired' and status code should be 401.
- For system failures during LTI Advantage access token validation,
try adding `raise StandardError` to the `begin` block in
`AdvantageServices#verify_access_token`. The error response message
will be 'An error occurred' and the status code will be 500.
Change-Id: I0a3eebd03d8ea5d5ff588f81be9187b323470c05
Reviewed-on: https://gerrit.instructure.com/168723
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
The keyboard reordering mechanism for the navigation links under course
had a couple problems:
1) It wasn't clear that the items getting focus were actionable, or what
happened if you did try to action them.
2) You could drop an item on the kabob menu and mess up the page
3) When you pressed space, there was no visual change so you couldn't
tell that you were currently dragging something.
4) It was non-standard for Canvas, which uses the move-to dialogs
exclusively
So I just removed it. We are already covered for a11y with the move-to
dialog.
fixes ADMIN-1550
test plan:
- in the course settings, navigation tab, you should only be able to tab
to the kabob menus and not the items themselves.
- pressing "?" will not bring up a keyboard navigation dialog.
- items can still be reordered via the move-to tray.
Change-Id: Iae4e068506e498b8a16bff5a7b53864fe4cc569a
Reviewed-on: https://gerrit.instructure.com/169667
Tested-by: Jenkins
Reviewed-by: Mysti Sadler <mysti@instructure.com>
QA-Review: Mysti Sadler <mysti@instructure.com>
Product-Review: Jon Willesen <jonw+gerrit@instructure.com>
Change-Id: Id9f0f08feb31fb1a3d49bb291f50654fcea492a5
Reviewed-on: https://gerrit.instructure.com/169899
Reviewed-by: Tucker Mcknight <tmcknight@instructure.com>
Tested-by: Jenkins
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
Closes: CNVS-43279
Test plan:
* turn on RTL
* go to /calendar
* click on one of the course names to open the color picker
* it should appear next to the course name like this:
Change-Id: I7dc24f66c90dd8780d029c6a1d03143f81135a1c
Reviewed-on: https://gerrit.instructure.com/169828
Tested-by: Jenkins
Reviewed-by: Steven Burnett <sburnett@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
closes GRADE-1584
Test Plan
- Go to the account settings for feature flags.
- Verify that there is a setting for Final Grade Override in the
allowed state.
- Go to the course settings for feature flags.
- Verify that the course can toggle the feature flag for Final Grade
Override on or off.
Change-Id: I107671b4f458cb0f9b73db4180d3b101c55cf29e
Reviewed-on: https://gerrit.instructure.com/169729
Tested-by: Jenkins
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Keith Garner <kgarner@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Sidharth Oberoi <soberoi@instructure.com>
closes GRADE-1075
test plan:
A. Setup
1. Create an assignment
* With moderated grading
* Muted
* Worth 10 points
* Two graders
* Teachers as final grader (not admin)
* Assigned to only one or two students
* just for less work in QA
2. Update the assignment
* Change points possible
* Change anonymity settings
3. As each student:
a. Submit to the assignment
b. Maybe re-submit
c. Comment on the submission
4. As each grader:
a. Assign provisional grades
b. Change some of those grades
c. Comment on submissions
d. Change or delete comments
5. As the final grader:
a. Visit the moderation page
b. Assign a custom grade
c. Select remaining provisional grades
d. Post grades
e. Display grades to students
B. Verify
1. Log in as a user with "view audit trail" permission (admin)
2. Open the assignment in SpeedGrader
3. Open the assessment audit trail
4. Verify the audit events are listed
5. Verify an "Unknown User" exists for each user involved
* one for each student
* one for each provisional grader
* one for the final grader
6. Verify the event starting dates are correct
7. Verify the event names are correct
8. Verify the events are in ascending date order
9. Verify the icons are shiny and pleasing to the eye
10. Verify that the red dot and example text look legit
* they are random at this time
Change-Id: Iec81c1065b9ce4964510f1c8c26d6f7bc81f87a1
Reviewed-on: https://gerrit.instructure.com/169830
Tested-by: Jenkins
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: Adrian Packel <apackel@instructure.com>
Product-Review: Sidharth Oberoi <soberoi@instructure.com>
test plan:
* when using "find a rubric" should be able to find
rubrics from cross-shard courses the user is enrolled
as a teacher in
closes #CORE-1934
Change-Id: Ic69c571cf8013af1894d9980df4ccc9e20121766
Reviewed-on: https://gerrit.instructure.com/165954
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Closes PLAT-3907
Test Plan:
- Verify clicking on the app center filter tabs ("All", "Not Installed",
etc.) navigates to the correct tab.
- Verify the active tab has the active style applied to it.
Change-Id: If002637d598d6c4d07a31079c646e49bd249845b
Reviewed-on: https://gerrit.instructure.com/169640
Reviewed-by: Marc Phillips <mphillips@instructure.com>
Tested-by: Jenkins
QA-Review: Marc Phillips <mphillips@instructure.com>
Product-Review: Weston Dransfield <wdransfield@instructure.com>
fixes ADMIN-1561
test plan:
- as a teacher, go to a course's people page
- click on the kabob menu for a person and select "Edit Sections"
- you should be able to tab to the "browse" link and activate it
Change-Id: I6a218f979a28ee8c546b958d68b9460f480f813c
Reviewed-on: https://gerrit.instructure.com/169432
Tested-by: Jenkins
Reviewed-by: Carl Kibler <ckibler@instructure.com>
QA-Review: Daniel Sasaki <dsasaki@instructure.com>
Product-Review: Daniel Sasaki <dsasaki@instructure.com>
closes GRADE-1586
Test plan:
- Create a course with a grading standard (default is fine)
- Add an assignment and issue some grades
- In a Rails console, get a Score object for someone in the course
- Set the Score's override_score to a value
- Test that the effective_final_score method returns the value you
just set
- Test that the effective_final_grade method returns a grade
commensurate with the value you just set
- Clear the override_score or get a different Score object
- Test that the above two methods return the computed final
grade/score as expected since no override has been set
Change-Id: Iffa7a9ce764a9fd2d9d334743f38a3fa3433e15f
Reviewed-on: https://gerrit.instructure.com/169576
Tested-by: Jenkins
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
QA-Review: Derek Bender <djbender@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
This does not remove the old dead code, just all the places
that checked to see if it was enabled.
closes PLAT-3752
Test Plan:
- Regression test on the developer keys page
- Also need to check that creating an oauth 2 token is
not broken (using client credentials)
Change-Id: I89983922a894ff7f20e86c034728d55284c8c668
Reviewed-on: https://gerrit.instructure.com/168271
Tested-by: Jenkins
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Weston Dransfield <wdransfield@instructure.com>
Product-Review: Marc Phillips <mphillips@instructure.com>
closes #CORE-2017
Change-Id: I08f4380bb88531c8a96bf32465dbd44c2c7a3633
Reviewed-on: https://gerrit.instructure.com/169720
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
The aud value was returning an integer. The spec dictates
that the aud is either a string or an array.
fixes PLAT-3925
Test Plan:
- Install the lti 1.3 test tool in a beta sandbox
- Install the beta instance in the 1.3 test tool
- A launch of the test tool should work
Change-Id: Ibc6021a467a90983adcabdf7f79cc26e38493d52
Reviewed-on: https://gerrit.instructure.com/169837
Tested-by: Jenkins
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Weston Dransfield <wdransfield@instructure.com>
Product-Review: Marc Phillips <mphillips@instructure.com>
Fixes: GOOF-594
Notes:
(tl;dr There were base retries already in place but they were failing
because there was no backoff. The aws gem gives us exponential backoff
for free)
- We are using the `google-api-client` v0.8.2 (which is quite out of
date) to execute requests to google to obtain documents. The
`execute!` method that is used has a default of `0` retries. The
method then uses `Retriable.retriable` to retry IFF google returns an
error. The `Retriable.retriable` by default has no exponential
backoff (the :interval parameter) because there are no set retries
passed into the `execute!` method. By passing a resonable number of
retries the built in exponential backoff for this gem will allow us to
bypass the `Rate Limit Exceeded` error.
Test Plan:
- Install the Google Doc plugin
- Verify submitting a document via the plugin for an assignment
works
- Verfiy submitting a document via the plugin for a collaboration
works
Change-Id: Icb2a18c4fa290bead72146e92e1e1d2869ee1915
Reviewed-on: https://gerrit.instructure.com/169471
Tested-by: Jenkins
QA-Review: Pedro Fajardo <pfajardo@instructure.com>
Reviewed-by: Joshua Orr <jgorr@instructure.com>
Reviewed-by: Jeremy Slade <jslade@instructure.com>
Reviewed-by: Bradley Horrocks <bhorrocks@instructure.com>
Product-Review: Jeremy Slade <jslade@instructure.com>
closes OUT-2558
test plan:
- create an assignment with aligned outcomes
- as a student, submit to the assignment
- as a teacher, assess the assignment's rubric
- as a student, confirm the results appear under sLMGB
- an an observer to the student, confirm the results
appear under sLMGB
Change-Id: I66118a01822be921e2fd44187143a12c7c13cd6c
Reviewed-on: https://gerrit.instructure.com/169132
Reviewed-by: Neil Gupta <ngupta@instructure.com>
Reviewed-by: Frank Murphy III <fmurphy@instructure.com>
Tested-by: Jenkins
QA-Review: Dariusz Dzien <ddzien@instructure.com>
Product-Review: Augusto Callejas <acallejas@instructure.com>
Fixes: CNVS-43278
The InstUI icons have Start-* and End-* variants that will flip for us
But the old ones that we use classnames to make still need to be flipped
In rtl. We had this, but I took mistakenly it out in g/149482 when I
also took the stuff for the instUI ones
Test plan:
* turn on RTL
* go to /calendar
* the arrow icons on top should look like this:
https://cl.ly/ac78ae2020de
And not this:
https://cl.ly/c949c7f053c1
Change-Id: I94526906a25070e0a17321a82a243cfddb85dbcc
Reviewed-on: https://gerrit.instructure.com/169820
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
Tested-by: Jenkins
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
fixes: QA-618
Test Plan
Passes Jenkins
Change-Id: Idb34db4dac7b4f7a34e49a41cb2621d8b914014f
Reviewed-on: https://gerrit.instructure.com/169437
Tested-by: Jenkins
Reviewed-by: Jeremy Putnam <jeremyp@instructure.com>
QA-Review: James Butters <jbutters@instructure.com>
Product-Review: James Butters <jbutters@instructure.com>
Closes: CNVS-43269
Test plan:
* turn on RTL in your user settings
* go to courses/x/discussion_topics
* look at the background images
* they should look like: https://cl.ly/2d6d02e66c12
* and not: https://cl.ly/ba59cf4e236b
Change-Id: I0c81f18ed384529f3fef53d26721e8023ecedddb
Reviewed-on: https://gerrit.instructure.com/169734
Tested-by: Jenkins
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
test plan:
* create a question bank
* create a question with an image
* import the bank question into a quiz
* edit the question in the quiz
* copy the quiz
* students in the copied course should be able
to see the image
closes #ADMIN-1568 #QO-426
Change-Id: Ic44b6cb14e9b74f2fbede4de905f0f11039dcaaa
Reviewed-on: https://gerrit.instructure.com/169731
Tested-by: Jenkins
QA-Review: Clay Diffrient <cdiffrient@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Reviewed-by: Brent Burgoyne <bburgoyne@instructure.com>
fixes COMMS-1516
Test Plan:
- Create a discussion
- Reply to the discussion with a long text string
- Notice that the entry does not go outside of the div container
Change-Id: I60840fc7939f3f8b218284a2bd1a3673899e6210
Reviewed-on: https://gerrit.instructure.com/169751
Reviewed-by: Landon Gilbert-Bland <lbland@instructure.com>
Tested-by: Jenkins
QA-Review: Steven Burnett <sburnett@instructure.com>
Product-Review: Steven Burnett <sburnett@instructure.com>
see the following diff, in returnView.on('ready' ...
https://gerrit.instructure.com/#/c/canvas-lms/+/168055/2/app/coffeescripts/external_tools/HomeworkSubmissionLtiContainer.js
the coffeescript used backticks to obscure which "this" applied.
decaffeinate did not handle it correctly. I ran the original
coffeescript through coffeescript.org to get a correct translation.
test plan: you should be able to submit a file to an assignment
via an external tool such as google docs
fixes ADMIN-1557
Change-Id: I883ec906d858798394d09b3ec40deea9619e24b4
Reviewed-on: https://gerrit.instructure.com/169677
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Tested-by: Jenkins
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
closes ADMIN-1515
test plan:
- assignments 2 teacher view should have a "Details" tab showing the
assignment's description
- the description should the description's formatting
- the other tabs just contain placeholder text
Change-Id: Id8823c9ae84feddd585631ab71fec74fe72f7518
Reviewed-on: https://gerrit.instructure.com/168299
QA-Review: Anju Reddy <areddy@instructure.com>
Tested-by: Jenkins
Product-Review: Jon Willesen <jonw+gerrit@instructure.com>
Reviewed-by: Ed Schiebel <eschiebel@instructure.com>
Reviewed-by: Carl Kibler <ckibler@instructure.com>
This reverts commit 7b6fdf5f3e
since people reported that the master build still fails with this errror
Change-Id: I639acd0f6e40a5723cc4d4f6eac4bfc5c56f9115
Reviewed-on: https://gerrit.instructure.com/169684
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
Tested-by: Jenkins
QA-Review: Clay Diffrient <cdiffrient@instructure.com>
Product-Review: Clay Diffrient <cdiffrient@instructure.com>
closes: QUIZ-5421
test plan:
1) with canvas, quiz_lti, quiz_api, live events set up and running
2) in canvas account settings, turn the quizzes next feature flag to allow
3) turn the course feature flag to OFF
4) create a quizzes next quiz that is published with at least 1 question
5) copy the course through the course settings
6) verify the new course has the quiz with the questions inside
Change-Id: Ibbaa63ec52a12bfccf64dc1753e4c40d3a5ef4d0
Reviewed-on: https://gerrit.instructure.com/169275
Tested-by: Jenkins
QA-Review: Hannah Bottalla <hannah@instructure.com>
Reviewed-by: Stephen Kacsmark <skacsmark@instructure.com>
Product-Review: Kevin Dougherty III <jdougherty@instructure.com>
fixes ADMIN-1554
Fixed, but noticed that the checkbox does not get the visual focus ring.
The relevant css is at line 173, app/stylesheets/vendor/bootstrap/_forms.scss
I'm hesitant to change css that will have such a wide effect in this
ticket.
test plan:
- Navigate to the People page of a course with several students
- Create a new group set
- Create two groups to the set
- In the kebab menu next to +Group choose Randomly Assign Students
- Tab through the modal
> Expect that focus does not land on the text
Change-Id: I99e0c9923a5c8f966bf44a8a963084002e57c08e
Reviewed-on: https://gerrit.instructure.com/169452
Tested-by: Jenkins
Reviewed-by: Jon Willesen <jonw+gerrit@instructure.com>
QA-Review: Jon Willesen <jonw+gerrit@instructure.com>
Product-Review: Ed Schiebel <eschiebel@instructure.com>
Closes OUT-2568
Test Plan:
- Enable moderated grading feature flag.
- Create moderated grading assignment with:
- an outcome criterion
- a TA grader
- as the TA, grade a submission
- verify no outcome results were posted in sLMGB.
- as the teacher, post the final grade in the moderated
grading workflow
- verify that the outcome result is now posted in sLMGB.
Change-Id: I753e158f2028fb69fd6094e99bc5b6efe4d20029
Reviewed-on: https://gerrit.instructure.com/169244
Reviewed-by: Neil Gupta <ngupta@instructure.com>
Reviewed-by: Augusto Callejas <acallejas@instructure.com>
Tested-by: Jenkins
QA-Review: Brian Watson <bwatson@instructure.com>
Product-Review: Neil Gupta <ngupta@instructure.com>
Test Plan:
- Introduce a failing JS test
- Run `COVERAGE=1 RUN_TESTS_FIRST=true yarn run test:coverage`
- It should fail
Change-Id: I27c6c876c68ce76be2da85e0e573e2051ade1951
Reviewed-on: https://gerrit.instructure.com/169389
Tested-by: Jenkins
QA-Review: Clay Diffrient <cdiffrient@instructure.com>
Product-Review: Clay Diffrient <cdiffrient@instructure.com>
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Dan McCallum