Fixes: CNVS-36268
Test plan:
- Sync a PowerSchool Agent you have previously synced to Canvas
before
- In your Canvas instance navigate to the SIS Imports page
- Upon the import completing comfirm the warning messages have a
more descriptive message as:
"An existing Canvas user with the SIS ID"
OR
"An existing Canvas user with the Canvas ID"
Change-Id: I3fc86e5d0de77862cfcb0741625c872dd580f882
Reviewed-on: https://gerrit.instructure.com/108368
Tested-by: Jenkins
Reviewed-by: Brad Humphrey <brad@instructure.com>
QA-Review: Mark McDermott <mmcdermott@instructure.com>
Product-Review: Brad Humphrey <brad@instructure.com>
this is needed for sis plugins
refs CNVS-35708
test plan
- specs should pass
Change-Id: Id24acc1253b8d9f70c744e5e175a0fb47ce518e4
Reviewed-on: https://gerrit.instructure.com/107526
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
fixes CNVS-35716
test plan
- sis import of users should work
Change-Id: I06a8ff352c7b780cec4edda81820af53937ea16b
Reviewed-on: https://gerrit.instructure.com/107507
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
fixes CNVS-33458
test plan
- specs should pass
Change-Id: Ia63bc50ae91e5742a3a3684e500f470a7ba672fe
Reviewed-on: https://gerrit.instructure.com/95770
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
fixes CNVS-32336
test plan
- run an invalid sis import with invalid login_id
- it should display warnings and not an error on
/sis_imports page
Change-Id: I708ee42c359a154bad6574417daac44839317177
Reviewed-on: https://gerrit.instructure.com/91767
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Benjamin Christian Nelson <bcnelson@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
caused by sis user deletion and course state changes
also DRY up the enrollment recalculation a little
closes #CNVS-31060
Change-Id: Ife8b2a95223c423f2faec79f3f9d5af5377ce51e
Reviewed-on: https://gerrit.instructure.com/88307
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
test plan:
- create a user with a SIS ID
- add this user as an admin in an account
- delete the user via SIS import
- ensure the user is no longer an admin in the account
- also test when the user is a subaccount admin
and ensure the subaccount membership is deleted
when the user is deleted
- ensure that an admin in multiple root accounts/shards
is only deleted from one of these accounts via a
SIS import (the domain the SIS import was run from)
fixes CNVS-30275
Change-Id: I288e10219fe39905fd271d02df1dc0aeb9814e76
Reviewed-on: https://gerrit.instructure.com/84852
Tested-by: Jenkins
Reviewed-by: Alex Boyd <aboyd@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Test Plan:
Create a user.csv file to import via SIS Imports with a row
containing an invalid login_id.
Note that the message is not "unique_id is invalid"
Note that the message contains contextual login_id and
sis_id / user_id information
Fixes: SIS-1732
Change-Id: I16f47d57f4dbe2550d979e6f6552d164a8709631
Reviewed-on: https://gerrit.instructure.com/71056
Product-Review: Linda Feng <lfeng@instructure.com>
Tested-by: Jenkins
Reviewed-by: Max Stahl <mstahl@instructure.com>
Reviewed-by: Nick Houle <nhoule@instructure.com>
QA-Review: Kausty Saxena <kausty@instructure.com>
test plan:
* run a sis import that changes a user but does not
update their email address
* the imported items count should include a User
closes #CNVS-14648
Change-Id: I67dfdf167c8eed77d3b4839fed5dbf905bcf513a
Reviewed-on: https://gerrit.instructure.com/65222
Reviewed-by: Tyler Pickett <tpickett@instructure.com>
Tested-by: Jenkins
QA-Review: Clare Strong <clare@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
In testing SIS integrations we encountered a situation where seed data
from a partner had 1771 users with the same email address. Near the end
of the import process this lead to executing 5310 separate queries (3 x
1770) to determine how to handle the seemingly duplicated
CommunicationChannels.
The first query, existence of a user for a communication channel is
redundant because database constraints enforce that the user record
exist and that the CommunicationChannel user_id field not be NULL. This
check was removed.
The second query, existence of active pseudonyms for the other
communication channel's user, can be combined for all relevant users
into a single count query with a grouping by the user_id.
The third query, similar to the second with the addition of checking
the root account and presence of a SIS id, is similarly combined.
These three changes yield a 10x performance improvement in the mentioned
pathological case.
Test Plan:
- Run a user import with a repeated email address and ensure the system
ends with all communication channels in the correct state.
Change-Id: I5695cdf342249780d37780f61d56f8257f3f8452
Reviewed-on: https://gerrit.instructure.com/65262
Reviewed-by: Brad Humphrey <brad@instructure.com>
QA-Review: Kausty Saxena <kausty@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
Tested-by: Tyler Pickett <tpickett@instructure.com>
refs CNVS-22801
Test plan:
* Create a csv file with the following contents
user_id,login_id,first_name,last_name,email,status
user_1,user1,User,Uno,BAD_EMAIL_ADDRESS,active
* Try to import this csv file
* Ensure that the import succeeds (i.e. the user is created)
* Ensure there is an import warning due to "BAD_EMAIL_ADDRESS"
not being a valid email address
Change-Id: Iac85a894bbbbdd69cd4f88f6f893975f6c6436c5
Reviewed-on: https://gerrit.instructure.com/63990
Reviewed-by: Mark Severson <markse@instructure.com>
Tested-by: Jenkins
QA-Review: Adrian Russell <arussell@instructure.com>
Product-Review: Linda Feng <lfeng@instructure.com>
fixes CNVS-12892
test plan
- enroll a user in a course
- add them in a group in the course
- delete user through sis
- the group_membership should also be deleted
Change-Id: I86065eaaebb271594f16f3b60a586f42c897158c
Reviewed-on: https://gerrit.instructure.com/64854
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: August Thornton <august@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
test plan:
* create an auth provider
* do an sis import without auth provider; it should still work
* specify the auth provider by name in the import; it should work
* specify the auth provider by id in the import; it should work
Change-Id: I71f381a1bc140c3992cac6eb45bb077e5bd26baa
Reviewed-on: https://gerrit.instructure.com/59558
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
refs #CNVS-21596
Change-Id: I5dedaab90a2abe6bf288ff30401c9b31629b45b2
Reviewed-on: https://gerrit.instructure.com/59220
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
This email only went to site admins, and isn't useful.
This commit stops new notifications being created, a subsequent commit
will remove the no longer used code and data.
refs CNVS-15219
Change-Id: Ibd8c88c7310cf3dcfb06cf0c782f2b3cf571d843
Reviewed-on: https://gerrit.instructure.com/40375
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Brian Palmer <brianp@instructure.com>
Add full & sortable name fields to SIS User Import
Change-Id: Ib8617572802ca69913b422d22301299ab75c211f
Reviewed-on: https://gerrit.instructure.com/34548
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
fixes CNVS-12963
test plan:
* set up a trust from account A to account B
* enable sis imports for account A and account B
* add a user to account B with an SIS ID
* add a course to account A with an SIS ID
* do an sis import to account A for enrollments as user
that is an admin in both account A and account B;
specify the course from account A as the target,
the user from account B, and add another column
root_account with account B's domain
* it should succeed, and enroll the user from account B
* unenroll the user manually
* do the import as a user that's an admin in account A,
but not account B - it should fail citing permissions
* remove the trust link from A to B
* do the import as the user that's an admin in both;
it should fail citing no usable login
Change-Id: Ie5b7b71bfe563da9c49d3aa2321586994634ccb5
Reviewed-on: https://gerrit.instructure.com/32133
QA-Review: August Thornton <august@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Both fields are optional. If absent, the user's name and sortable_name
will be set in the same manner as before.
The full_name field takes precedence over first_name and last_name. The
sortable_name field takes precedence over the inferred value.
The display name (short_name) field is optional. If absent, short_name
will behave as before. If present, it will explicitly set short_name to
the given value, unless it's a stuck SIS field of course.
Previously, if first and last names were not given, the user's name will
be a single space. The behavior is the same now, except when short_name
is present. In that case, an error will be thrown.
refs CNVS-11680
test plan
- on mysql run user import
- should not fail
Change-Id: I3c6ccc99aae493ba87fc5c2ebaf8e789dfca0338
Reviewed-on: https://gerrit.instructure.com/31543
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
fixes CNVS-11680
test plan
- import user.csv with a different case than
existing user's unique_id
- no import warning errors should be shown
Change-Id: I64a3a84f6874ec88c4f28bc046f80cad9219fc43
Reviewed-on: https://gerrit.instructure.com/31453
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Eric Adams <eadams@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: August Thornton <august@instructure.com>
fixes SIS-162
TestPlan
----------
- import user.csv with a different case than existing user' unique_id
- no import warning errors should be shown
Change-Id: I653bcc7c2ace9953fe74ada0c0ab7da32c65662c
Reviewed-on: https://gerrit.instructure.com/30151
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Duane Johnson <duane@instructure.com>
Product-Review: Duane Johnson <duane@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Reviewed-by: Eric Adams <eadams@instructure.com>
Test Plan:
- Import a CSV with integration_id column in any
of: account, course, section, term, user
- The integration_id should be available in the
console for objects imported in this way (e.g.
user = User.find(123); user.integration_id)
Fixes SIS-108
Change-Id: Ie7f0765c2ec7222d8c7ba7dfbf78c858812e62d3
Reviewed-on: https://gerrit.instructure.com/27672
Tested-by: Jenkins <jenkins@instructure.com>
Tested-by: Ken Romney <kromney@instructure.com>
Reviewed-by: Ken Romney <kromney@instructure.com>
QA-Review: Ken Romney <kromney@instructure.com>
Product-Review: Ken Romney <kromney@instructure.com>
fixes CNVS-7266
test plan:
* create an SIS user with a login id, then delete them
* create a non-SIS user with the same login id
* use an SIS import to resurrect the original user
* the SIS import should succeed, but warn that it skipped the user
cause someone else had his login information
Change-Id: I2102107fefb1d20fae6046ebe3a7e0c03416dcc9
Reviewed-on: https://gerrit.instructure.com/22867
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
When unhandled ActiveRecord validation errors occur during SIS import
for a single record, it can prevent an entire batch from importing.
This change catches validation errors in Enrollment and
GroupMembership objects and re-raises them as ImportErrors so that
batch imports can continue.
Test Plan:
- create a batch import in which a user is enrolled in a class,
and is also in a group within that enrollment
- add something else to the batch (to test that the rest of the
batch is imported even when the error, below, occurs)
- import the batch
- mark the enrollment as deleted
- re-import the batch
- rather than failing to import the batch, there should be an
error message indicating the group user had a validation error
fixes CNVS-4226
Change-Id: I87078bd263f2b2487c5a8d41ff41c0ff71cbed95
Reviewed-on: https://gerrit.instructure.com/18271
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
QA-Review: Clare Hetherington <clare@instructure.com>
fixes #CNVS-2172
found every place I could in the codebase
that passed some kind of input into a finder
that looked up a pseudonym with an sis_user_id.
Then wrote specs to pass in integers (which in
all cases turned up the error described
in the ticket), and
made sure to do explicit casts on the inputs
until the specs didn't break anymore.
TEST PLAN (unsure of how to trigger from UI):
1) Take a pseudonym that already exists and has
an sis_user_id that could be an integer (is
made up of all numeric characters).
2) In the console, try to create a new
pseudonym with the same sis_user_id, but use
an integer instead of a string.
3) you SHOULD NOT get a postgres error
telling you that you need to do an
explicit type cast
4) you SHOULD get a validation error telling
you that 2 pseudonyms cannot have the same
sis_user_id.
Change-Id: Ic69da6a40609a50234a5388af02dd1fbeb021214
Reviewed-on: https://gerrit.instructure.com/16925
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
QA-Review: Clare Hetherington <clare@instructure.com>
refs #11188
test plan: sis import a user with an email. then do another sis import
that only changes the user's email. the change should show up
immediately in the ui and in sis csv export reports.
Change-Id: I14efdf052ea7d2688ef07fff6ce25f76c6deb034
Reviewed-on: https://gerrit.instructure.com/15479
Reviewed-by: Rob Orton <rob@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
fixes#8402
test plan:
* create a user via SIS with no e-mail
* add an e-mail address and remove it from the user's profile
* add the same e-mail address, and confirm it this time
* re-import the user via SIS, with the e-mail address this time,
but differing in case
* it should succeed, and the e-mail address should be update to
the new case
Change-Id: Id54f413bdf6b3c066530c53715141c7507d18f9b
Reviewed-on: https://gerrit.instructure.com/10552
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
fixes#7159
test plan:
* import two users with the same e-mail into the same account
* the user should not get a merge notification e-mail
Change-Id: Ic9f3a3948df1e2a877fcd8e79f3d05d86a2d1e5b
Reviewed-on: https://gerrit.instructure.com/8774
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@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>
test plan: run the migration
Change-Id: I75184edd64ddee384de485c443d4c7eee45cc26f
Reviewed-on: https://gerrit.instructure.com/7895
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
so that it's case insensitive
also fix some latent CommunicationChannel.find_or_initialize_by_path
discovered by specs for this
test plan:
* run the specs
* use forgot password with a username that differs in case
* self-register a new user multiple times (without confirming)
with e-mail addresses that differ in case
Change-Id: I476325f591c997fc8d50d5f38480177f732f07a7
Reviewed-on: https://gerrit.instructure.com/7724
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
test plan:
* import a user via SIS
* do another SIS import that lists the same user, and a new user
* the e-mail addresses for each user should be correct
Change-Id: I0a95804056e38bdfd36f24c0c78187d601940451
Reviewed-on: https://gerrit.instructure.com/7833
Reviewed-by: Zach Wily <zach@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
test plan:
* create a user via SIS import with an e-mail address
* add a different e-mail address to the user
* re-import the user via SIS, and set their e-mail address
to the second e-mail you added, but with a different case
* the user should have a single e-mail address, the one set
in the second SIS import
Change-Id: Iab6b16e0b37cfb8caac3faa453be570b99621a9e
Reviewed-on: https://gerrit.instructure.com/7784
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>
* expose sortable name directly to the user
* don't downcase it
* use a LOWER(sortable_name) index for postgres
* set sortable name as "last_name, first_name" explicitly for SIS imports
* populate sortable name intelligently in the UI
Change-Id: I476641f4817e27a11b573d91f102c5a74d3eba26
Reviewed-on: https://gerrit.instructure.com/6512
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>
Reviewed-by: Ryan Florence <ryanf@instructure.com>
fixes#5573, #5572, #5753
* communication channels are now only unique within a single user
* UserList changes
* Always resolve pseudonym#unique_ids
* Support looking up by SMS CCs
* Option to either require e-mails match an existing CC,
or e-mails that don't match a Pseudonym will always be
returned unattached (relying on better merging behavior
to not have a gazillion accounts created)
* Method to return users, creating new ones (*without* a
Pseudonym) if necessary. (can't create with a pseudonym,
since Pseudonym#unique_id is still unique, I can't have
multiple outstanding users with the same unique_id)
* EnrollmentsFromUserList is mostly gutted, now using UserList's
functionality directy.
* Use UserList for adding account admins, removing the now
unused Account#add_admin => User#find_by_email/User#assert_by_email
codepath
* Update UsersController#create to not worry about duplicate
communication channels
* Remove AccountsController#add_user, and just use
UsersController#create
* Change SIS::UserImporter to send out a merge opportunity
e-mail if a conflicting CC is found (but still create the CC)
* In /profile, don't worry about conflicting CCs (the CC confirmation
process will now allow merging)
* Remove CommunicationChannelsController#try_merge and #merge
* For the non-simple case of CoursesController#enrollment_invitation
redirect to /register (CommunicationsChannelController#confirm)
* Remove CoursesController#transfer_enrollment
* Move PseudonymsController#registration_confirmation to
CommunicationChannelsController#confirm (have to be able to
register an account without a Pseudonym yet)
* Fold the old direct confirm functionality in, if there are
no available merge opportunities
* Allow merging the new account with the currently logged in user
* Allow changing the Pseudonym#unique_id when registering a new
account (since there might be conflicts)
* Display a list of merge opportunities based on conflicting
communication channels
* Provide link(s) to log in as the other user,
redirecting back to the registration page after login is
complete (to complete the merge as the current user)
* Remove several assert_* methods that are no longer needed
* Update PseudonymSessionsController a bit to deal with the new
way of dealing with conflicting CCs (especially CCs from LDAP),
and to redirect back to the registration/confirmation page when
attempting to do a merge
* Expose the open_registration setting; use it to control if
inviting users to a course is able to create new users
Change-Id: If2f38818a71af656854d3bf8431ddbf5dcb84691
Reviewed-on: https://gerrit.instructure.com/6149
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Landon Wilkins