manually done. the big one was the explicit locale assignment
in set_locale_with_localizer needs undone in a controller callback
then using with_locale everywhere, specs no longer need to be concerned
about being in an uncertain locale
Change-Id: I5a1d2c907a6f52ee4d8c2307b8c789a1f1ea436e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/320112
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
Change-Id: I6abefdfa9fed6dd4525c8786e93efa548b3710f2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/319603
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
Migration-Review: Jacob Burroughs <jburroughs@instructure.com>
why:
* in some situations (like consortia), a user may have a pseudonym that
will work for MSFT sync (has a lookup value), but it may not be on the
same shard as the course getting synced
* search first in the root account, then on the shard as a whole, then
in any shards that cross-shard users came from
closes INTEROP-7179
flag=none
test plan:
* follow the MSFT sync setup instructions in
https://instructure.atlassian.net/wiki/spaces/INTEROP/pages/85989064735
* in the MSFT Sync account settings, change the lookup value to
integration_id
* update all your MSFT users to have an integration_id that matches
their unique_id, with something like
u.pseudonyms.first.update! integration_id: u.pseudonyms.first.unique_id
* add a cross-shard user to your course, one that has a pseudonym/login
in their home account but not one in the root account of the course
(requires MRA)
* and make sure this user also has the integration_id set on its
pseudonym
* without this commit checked out:
* sync the course manually
* in the dev tenant, observe the list of teams and users and find the
team that matches your course name
* the cross-shard user should not be in the team
* with this commit checked out:
* sync the course manually
* the cross-shard user should be in the team in the dev tenant
* edge cases to play around with:
* a user from the same shard as the course that has a pseudonym in a
different root account on the same shard (rare) - the pseudonym that
is from the course's root account should be used
* both pseudonyms for a user have an integration id - the one from
the course's root account should be used
* the local pseudonym doesn't have an integration id - the pseudonym
from the other root account/other shard should be used
Change-Id: I90b285431513d7814fd1ca75f1011cb1a42cdf59
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/315547
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Evan Battaglia <ebattaglia@instructure.com>
QA-Review: Tucker Mcknight <tmcknight@instructure.com>
Product-Review: Alexis Nast <alexis.nast@instructure.com>
includes some minor hash layout fixes that were incompatible with prior rubocop
[skip-stages=Flakey]
Change-Id: I75e903292daa70c84b03600b97fac49ca1155004
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/315786
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
many can be converted to heredocs, and most the rest can be simply
re-arranged to chained method calls without line continuations
[skip-stages=Flakey]
Change-Id: Ib96722c0d8108ed2783129fb909bff7a18617ffd
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/315684
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Aaron Ogata <aogata@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
refs AE-31
flag=none
test plan:
- verify Canvas boots in CD
- verify no influx of new errors in CD
[fsc-timeout=1]
[ignore-stage-results=Flakey Spec Catcher]
Change-Id: I26663e32d9452c198c87b957b026170ee3e4bc93
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/305073
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Aaron Ogata <aogata@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Isaac Moore <isaac.moore@instructure.com>
closes INTEROP-6817
flag=none
Why:
- When a user's primary email changes, if the root account is set up to
use email as the login attribute, mapping which maps the user to the
Microsoft 365 user will be out-of-date.
- We never delete UserMappings except when changing Microsoft Sync
settings at an account level so currently this is almost impossible
for customers to rectify
I didn't want to actually immediately delete a mapping, because if a
CommunicationChannel changes in the middle of a sync, the missing
UserMapping could cause us to remove the user from the Microsoft 365
group, even if the user's primary email has not even really changed (for
instance, if the user added a non-primary email). So instead I use a
needs_updating field.
Test plan:
- Make sure you have an account set up with Microsoft Sync using email
as the login attribute.
- Make sure you have a course set up with Microsoft Sync, ideally with
2+ users. Make sure it is successfully synced so that there are 2+
users in the group visible in Microsoft 365 admin console.
- Look at one of the users who has a account in the Microsoft 365
tenant and change their primary email address to another email address
of a another user on the Microsoft 365 side
- Manually sync the course
(course.microsoft_sync_group.syncer_job.run_synchronously)
- Look in the Microsoft 365 admin console and check that the old user
has been removed and the new user has been added to the group.
- Change the email address again to something not corresponding to any
user on the Microsoft side
- Sync again and see that the user has been removed from the Microsoft
group
- Make sure one of your tests used the API to update the email address
- add a communication channel with the POST API endpoint, e.g.:
tok http://canvas-lms.docker/api/v1/users/123/communication_channels \
communication_channel[type]=email \
communication_channel[address]=someaddress@example.com \
skip_confirmation=true
- remove the old communication channel
tok http://canvas-lms.docker/api/v1/users/123/communication_channels
tok delete http://canvas-lms.docker/api/v1/users/123/communication_channels/456
- Repeat the steps above but update a primary email address from a
SIS import. You should be able to do this by creating a users.csv file
like this and uploading with "SIS Import" in the account menu (and
choose the "Override UI changes" checkbox):
user_id,login_id,first_name,last_name,email,status
sisuser1,sisuser1login,FirstNameOne,LastNameOne,myemailaddress@instructure.com,active
- from a console, set the needs_updating on user's UserMapping to false,
then destroy one of the user's email communication channels with
commchannel.destroy_permanently!
- make sure the UserMapping has gotten needs_updating set to true
- destroy all email communication channels for a user, or make all their
communication channels something without a user on the Microsoft 365
side. Sync the course. The user should be removed from the course
(note that you cannot remove the last course owner), and the
UserMapping should no longer exist at all.
Change-Id: I9e2f3d5e439bebc8c9eaf89b1595908213dc1981
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/294820
Product-Review: Alexis Nast <alexis.nast@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Migration-Review: Ben Rinaca <brinaca@instructure.com>
Reviewed-by: Tucker Mcknight <tmcknight@instructure.com>
QA-Review: Tucker Mcknight <tmcknight@instructure.com>
closes INTEROP-7451
flag=none
If the Microsoft 365 Group has been deleted on the Microsoft side, as
can happen if an admin logs in and deletes the group we create, we
shouldn't save an error report. This is an "expected" / "graceful
cancel" error, because we know we can get into this state. This normally
only happens in the partial sync, because in the full sync we recreate
the group.
To be on the safe side, I am still retrying the partial sync when we get
the 404. This required me to change the StateMachineJob code to handle a
Retry object with GracefulCancelError so it will not treat it as an
unexpected failure the final time.
Test plan:
- For ease of testing set:
Setting.set('microsoft_group_enrollments_syncing_debounce_minutes', '0')
This gets rid of the delay between adding the enrollment and the job
starting. In addition, if you want, you can change the last value of
STANDARD_RETRY_DELAY in lib/microsoft_sync/syncer_steps.rb from 300 to
something shorter.
- Have a course successfully synced with Microsoft Sync
- In Microsoft's admin UI, delete the group
- If using email as the sync login attribute state, make sure you have a
user (that can be added to the course) that has an an email that has
already been confirmed (the CommunicationChannel's workflow_state
should be "active")
- Add such a user (that has a corresponding user on the Microsoft side)
to a course, and update the enrollment to 'active':
e = Enrollment.last # etc.
e.update workflow_state: 'active'
The job will only try to add the user after the enrollment is active.
- Make sure you have jobs running. The job should run and be retried
three times. After one of the intermediate failues, check the
MicrosoftSync::Group. The workflow_state should be `retrying` and the
job_state should mention a GroupNotFoundGracefulCancelError.
- After the last failure, the MicrosoftSync::Group workflow_state should
be `errored`. In the UI, in the Course settings -> Integrations, you
should see the message "The Microsoft 365 Group created by sync no
longer exists..."
- Run a full sync
(course.microsoft_sync_group.syncer_job.run_synchronously) to recreate
the group. Add another user as before (or remove the user, run a full
sync, and add the same user back) and let a partial sync run. Make
sure the partial sync runs successfully and adds the user to the
Microsoft 365 Group.
- Set the debounce setting back to its original value:
Setting.set('microsoft_group_enrollments_syncing_debounce_minutes', '10')
Change-Id: I27b906655255ec5a3384313c529aedb48acd258b
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/293325
Reviewed-by: Tucker Mcknight <tmcknight@instructure.com>
QA-Review: Tucker Mcknight <tmcknight@instructure.com>
Product-Review: Alexis Nast <alexis.nast@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Currently, when we look up email addresses for a user, we just look on
the shard the sync is happening on (the shard the course is on).
CommunicationChannels almost live on the user's home shard, so if there
are cross-shard users, we need to activate their shards before looking
for the CommunicationChannel.
I also modified a spec helper because it was not detecting an error's
call to I18n.t correctly for some reason for my local MRA setup. (I
checked that the new helper also fails when the error class doesn't
actually call I18n.t)
closes INTEROP-7147
flag=none
Test plan:
- Have MRA Canvas set up
- Ensure you have the Microsoft Sync creds set in
config/dynamic_settings.yml
- One on shard, set up Microsoft Sync in the root account and in a
course.
- turn on feature flag
- in account settings enable sync with our test tenant and use "email"
and "UPN" as the local and remote lookup fields
- in Course Settings -> Integrations turn on sync for the course
- Add two users to the course, one in another root account. At least one
should be a teacher. Both should have emails corresponding to users in
our Microsoft test tenant. Set the enrollments' workflow_state to
"active".
- Make communication channels for each users in their respective shards.
Make sure each shard has only the communication channel for the user
local to that shard, i.e., there are no communication channels with
user_id > Shard::IDS_PER_SHARD. Confirm the communication channels:
run `communication_channbel.confirm` on each, which should set the
workflow_state to "active".
- Sync the course
course.microsoft_sync_group.syncer_job.run_synchronously
- The sync should succeed and the users should be visible in the
Microsoft admin console
Change-Id: Ic87be3d9dfe0578370f2f04e8509361b75269961
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/283233
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
Reviewed-by: Mysti Lilla <mysti@instructure.com>
closes INTEROP-7232
flag=none
Prevents a partial sync from masking the error in the UI and making the
user think the sync is working great, when in reality, some users may
have not been added and Team may have not even been created for the
Group.
This solution is more complicated than I would like but I really think
this is the simplest way of avoiding the problem. See the ticket for
discussion and another discarded approach.
I also changed COMPLETE and IGNORE to have their own classes so it
makes the debugging show "finished step with Ignore" instead of
"finished step with Object".
Test plan:
- enable Microsoft Sync in course settings for a course where there
are teachers but they don't exist on the Microsoft side.
- kick off a full sync, e.g. from a console:
group.syncer_job.run_synchronously
- observe that it fails. Look in the UI (course settings ->
integrations) and check that it shows the the correct error,
saying that "no users corresponding to the instructors of the Canvas
course could be found on the Microsoft side."
- add a user to the course -- that *does* exist on the MSFT -- to the
course as a teacher. Set the Enrollment's workflow_state, and (if
necessary) the user's CommunicationChannel's workflow_state, to
"active"
- wait 10 minutes until a partial sync job runs (if you have jobs
running) or just kick off a partial sync with:
group.reload.syncer_job.run_synchronously(:partial)
- from the logs / output you can check that that didn't actually hit
the MS API, and that the step returned with Ignore
- check in the UI to see that the same error is still there
- kick off another full sync. this should run successfully now that
there is a teacher that does exist on the MS side. in the MS admin
console check that the group has the use. Also you can see that a team
has been created for the group: if the Group info sidebar, next to
"Delete Team" under the name of the Group, will be a link "Open in
Teams")
- Now that the error has been cleared, add another user and run partial
sync to make sure that works and it adds the new user
Change-Id: I760cf7a69f52d48866cb11511c30739de119c0dd
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282798
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Karl Lloyd <karl@instructure.com>
Previously, we never deleted the mappings between a Canvas User and
Microsoft user AAD object ID after caching it. So if a Microsoft admin
deleted users from Microsoft 365, some courses would fail to sync
because the AAD object ID we had didn't exist on the Microsoft side, and
ge would never try to re-look it up.
Now, if we try to add a user and the user's AAD object ID doesn't exist
on the Microsoft side, we will notice that and delete the mapping. The
next time a sync is run, we will successfully re-lookup the Canvas user
and add them if a different Microsoft user exists for them.
If we are trying to remove a user from a group and the user doesn't
exist, we will still error, as Microsoft's API doesn't differentiate
between the user not existing, group not existing, or the user just not
being in the group. However this case is not as bad as it can only
happen in a partial sync (since in a full sync we get the full list of
users in the group from the API, and that list will not include deleted
users). So if that happened, a full sync would fix the course/MS group
and allow it to sync again.
This commit:
* Modifies add_users_to_group (patch api -- all users at once) to notice
when the API sends back a 404 that some user does not exist. At this
point we only know that at least one user doesn't exist, so we use the
batch API (all users as separate requests in a batch) to get a list of
all such users.
* Changes add_users_to_group_via_batch to notice when the API returns a
404 saying the user does not exist. To communicate this up, we have
a new method nonexistent_users on GroupMembershipChangeResult
* Changes special_case.rb to support a block tester. Also use an
encapsulate response which can contain a `batch_request_id` for batch
requests. These are used by the new code.
closes INTEROP-7000
Test plan:
* have a course with some users (enrollments must be 'active',
CommunicationChannels must be 'active' if email is used for lookup
key) and sync with Microsoft.
* delete users on MS side
* run a full sync (e.g. group.syncer_job.run_synchronously). sync should
succeed, the usermapping for the user that no longer has a user on the
Microsoft side should be deleted.
* delete the group on MS side.
* add a user (active enrollment, active communicationchannel as before)
with a user on the microsoft side and run a full sync. that should
should recreate the group on the microsoft side (as before)
* delete a user on the MS side and then immediately remove the user from
the course on the Canvas side. run a partial sync (e.g.
group.syncer_job.run_synchronously(:partial)). That should be ignored
(as before) because we just assume the user is not in the group (we
can't distinguish between that and the user not existing when removing
users).
Change-Id: I5230b32d7cbfd1462177e3e597ba7d9633f055d6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278068
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
In the next commit I need add_users_ignore_duplicates to return more
complicate information than just the list of users returned.
Encapsulating that information in an new class instead of returning
a complicate array-hash structure seemed the most clear thing to do.
refs INTEROP-7000
Test plan:
- TODO, should smoke test all previous commits
Change-Id: I65e5ccbbc49ff3eaee8fcdb5e15158a95e4cf133
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277726
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
I found the specs for the groups endpoints really hard to follow with
the multiple layers of shared examples and such. This is my attempt to
simplify it a bit, although it's not perfect.
* Separate out add_users_via_batch specs from outside of the
add_users_ignore_duplicates
* Get rid of boilerplate around succ(), dupe(), etc. by using a table of
canned subresponses -- SAMPLE_RESPONSES_CODES_BODIES_HEADERS -- and
build_batch_response_body()
* Also use table of examples instead of another level of shared examples
for failed batches
* Make explicit the dependencies between shared_examples_for by using
parameters instead of lets for most values
Test plan:
- specs. we can do some overall graph service smoke tests after all my
refactoring
Change-Id: I5456a5e26b747ab8e5d8407954ec99077c1a8156
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277727
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
Finish splitting up GraphService -- remove delegates and call the
methods in the GroupsEndpoints class directly, and rename the methods to
not be redundant.
refs INTEROP-7000
Test plan:
- specs. we can do some overall graph service smoke tests after all my
refactoring
Change-Id: Ia5856e11f4f375da6f7feb353ce144a001a78aee
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277695
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
Remove delegates and call the methods in the *Endpoints classes
directly, for everything but GroupsEndpoints. Renames methods to not be
redundant.
refs INTEROP-7000
Test plan:
- specs. we can do some overall graph service smoke tests after all my
refactoring
Change-Id: I13463e6611abfbcf6c33838e8c28b1b97953a123
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278080
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
Finish splitting up Graph Service Endpoints specs. This more fully
separates the specs for each set of endpoints, not going through a
central GraphService object.
refs INTEROP-7000
Test plan:
- specs. we can do some overall graph service smoke tests after all my
refactoring
Change-Id: Iff0e606b6b1cc3aa72c112c8081d541b317a2107
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277693
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
Split up specs part 1: this splits up the specs in a minimal way,
keeping the central GraphService object in the specs.
refs INTEROP-7000
Test plan:
- specs. we can do some overall graph service smoke tests after all my
refactoring
Change-Id: I150c95aaf87ef30e3bf6e09b8dd2519b6c77f449
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277692
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
This makes classes for the different endpoints and for now delegates
calls to them.
refs INTEROP-7000
Test plan:
- specs. we can do some overall graph service smoke tests after all my
refactoring
Change-Id: I0bc81bae35066c5b94c96e37d8b0de9dbe2e821f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277691
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
This method is unused except for rare debugging and is not that hard to
reproduce when needed. This file is big and complicated enough already.
Test plan:
- specs. we can do some overall graph service smoke tests after all my
refactoring
Change-Id: I525b5a39b45612eea85106e676ec7917822cd64f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277690
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
preparation for future commits.
refs INTEROP-7000
Test plan:
- specs. we can do some overall graph service smoke tests after all my
refactoring
Change-Id: Ibca941f1fa1e3efa6dd6e844b7a4d1787b2701bc
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277689
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
[skip-stages=Flakey]
all manual
the fixes are a little scattered, since the same method doesn't work
everywhere depending on requirements. mostly I changed to `let`, but
some required `stub_const`. For `let`, I eventually settled on
avoiding a dedicated `let` for the class if it's only used one, and
it's a trivial class just to include the module. otherwise there's
a separate `let` for the class, and if there's only one it's named
`klass` instead of something contrived.
Change-Id: I84734c963d4789be3ec3cd852cca623e7c2a08df
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277285
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This was discovered in QA for a previous commit. If a course has over
100 owners, the sync fails, and then we run a partial sync, we can run
into the case where the partial sync tries to add 20 owners but many if
not all or duplicates. If there are >= 81 owners, Microsoft won't
realize this but instead say "there are too many users". Instead of
trusting this message, we should fall back to the batch API -- because
it adds each user individually, it will ignore any that are already in
the group instead of saying there isn't room.
Test plan:
- Have a Microsoft group with >= 82 owners
- From a rails console, get the graph service object and then get a list
of 20 owners in the group.
def all_owners_ids(gs, group_id)
[].tap do |list|
gs.list_group_owners(group_id) do |slice|
list.concat slice.map{|u| u['id']}
end
end
end
group = MicrosoftSync::Group.first
gs = group.syncer_job.steps_object.send:graph_service
owners = all_owners_ids(gs, group.ms_group_id).take(20)
gid = group.ms_group_id
- Remove one of the owners from the group:
one = [owners.last]
gs.remove_group_users_ignore_missing(gid, owners: [owners.last])
- Wait a couple seconds (API is eventually consistent) and check that
the user is no longer in the group:
all_owners_ids(gs, gid).include?(owners.last)
- Add the 20 owners -- 19 of which should already be in the group:
gs.add_users_to_group_ignore_duplicates(gid, owners: owners)
- The function should return a list of the 19 duplicates. You should see
it log that it first tried the 'patch' endpoint and fell back tothe
'post $batch' endpoint.
- Wait a couple seconds and check that the user was added back in.
gs.list_group_owners(gid).map{|g| g['id']}.include?(owners.last)
refs INTEROP-6805
flag=microsoft_group_enrollments_syncing
Change-Id: I4a3dd47edb62b29ba8278182b37894b28604ceda
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/276318
Reviewed-by: Sean Scally <sean.scally@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Evan Battaglia <ebattaglia@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
refs INTEROP-6805
flag=microsoft_group_enrollments_syncing
Test plan:
- have a course with one owner
- remove an owner and add a different owner. recall that enrollments
have to be in an "active" workflow_state to count.
- run a partial sync (`group.syncer_job.run_synchronously :partial`)
- sync should fail, but it should still add the new owner. Check in the
Microsoft admin console. Both owners will exist in the group.
- run a full sync (`group.syncer_job.run_synchronously`). Check in the
MS admin console. The old owner should now be removed.
- Repeat but run a full sync instead of a partial sync the first time.
Change-Id: I0cce5a50a51f40ed36f5eea928638c4782b31937
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/276019
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Michael Ziwisky <mziwisky@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
Reviewed-by: Michael Ziwisky <mziwisky@instructure.com>
Also fix default special_cases ([] instead of {}) in graph_service/http.rb
The diff http.rb doesn't show nicely here, try:
diff --ignore-space-change \
<(git show HEAD^:lib/microsoft_sync/graph_service_http.rb) \
<(git show HEAD:lib/microsoft_sync/graph_service/http.rb)
refs INTEROP-6805
flag=microsoft_group_enrollments_syncing
Test plan:
- run through a sync, make sure it still works
Change-Id: I12d4339c0c1a50417ed23f9476ca12b1b9f711a4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/275802
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Michael Ziwisky <mziwisky@instructure.com>
QA-Review: Michael Ziwisky <mziwisky@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
closes INTEROP-6805
flag=microsoft_group_enrollments_syncing
Test plan:
- note: this is closely based on test plan for last commit
- enable sync for a course
- add 101 teachers that have users in our test tenant. I
made a bunch of test users to use owners you can use. What I did was
generate the list of UPNs with
'echo evantest{001..101}deleteafternov2021@...' and add them to my
course)
- Remember the enrollments have to be all active and the communication
channels 'registered', so I did
`CommunicationChannel.update_all(workflow_state: 'active')`
followed by
`ms_group.course.enrollments.map{|e| e.update workflow_state: 'active'}`
which will add the users to the PartialSyncChanges table.
- run a partial sync (e.g.
`group.syncer_job.run_synchronously :partial`)
- in the course settings check the Microsoft Sync integration, it should
be disabled. Note that it won't show any error because errors are not
shown when the sync is disabled, but trying to enable it will show the
error shown when trying to enable a course with too many enrollments
(added in a previous commit)
- in a console, check that last_error is set on the group
- remove the 'check_for_enrollment_limits' before_action hook in
app/controllers/microsoft_sync/groups_controller.rb
- also remove the 'rescue Errors::OwnersQuotaExceeded' block on
lib/microsoft_sync/syncer_steps.rb:263 so we can be sure we are testing
the step_full_sync_prerequisites code path
- enable the sync again
- run a full sync
- check the course settings page again. Again the sync should be
disabled. And last_error should should have the "too many teachers"
error message
Change-Id: Ied4faf7f997040e224c1cbdcdb7f0c5d24c4e02e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274953
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Michael Ziwisky <mziwisky@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
Reviewed-by: Michael Ziwisky <mziwisky@instructure.com>
* This needs to be done when adding users, either via the endpoint which
adds multiple users, or adding via the "$batch" endpoint (which batch
subrequests which each add one user).
* To ease handling of the ever-growing number of special cases, I
refactored code away from using the "special case block" paradigm to a
"special_cases" hash with matchers (status code + body regex)
More work to rescue the exception and disable sync will be done in
the next commit.
flag=microsoft_group_enrollments_syncing
refs INTEROP-6805
Test plan:
- have a course setup with Microsoft Sync enabled and run a full sync
once.
- add 101 teachers that have users in our test tenant. I
made a bunch of test users to use owners you can use. What I did was
generate the list of UPNs with
'echo evantest{001..101}deleteafternov2021@...' and add them to my
course)
- Remember the enrollments have to be all active and the communication
channels 'registered', so I did
`CommunicationChannel.update_all(workflow_state: 'registered')`
followed by
`ms_group.course.enrollments.map{|e| e.update workflow_state: 'active'}`,
which will add the users to the PartialSyncChanges table.
- run a partial sync (e.g.
`group.syncer_job.run_synchronously :partial`) It should fail with the
Errors::OwnersQuotaExceeded error, coming from the request() call in
add_users_to_group_ignore_duplicates() (not from
add_users_to_group_via_batch())
- run:
MicrosoftSync::UserMapping.to_a.each_slice(20) do |users|
graph_service.add_users_to_group_via_batch(
group.ms_group_id, [], users.map(&:aad_id)
)
end
That should also fail with the same error.
- Note: I didn't add 25,000 members to see the too many members error,
but I assume it looks nearly the same.
Change-Id: I425b7c8f09609db11fed31a475ffac2f77b1d3e7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274798
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Michael Ziwisky <mziwisky@instructure.com>
QA-Review: Evan Battaglia <ebattaglia@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
flag=microsoft_group_enrollments_syncing
refs INTEROP-6805
Test plan:
- modify limits in membership_diff.rb
- for each limit, have a course with more owners or users than the
limits. The users' enrollments must all have a workflow_state of
"active" or "creation_pending"
- in the UI try to enable sync for the course
- it should fail and give an error
- make sure you can enable and disable sync for courses below the limits
Change-Id: I7def5f664dc57e640529bf6a8b3b3d12c16556e4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/273810
Reviewed-by: Mysti Lilla <mysti@instructure.com>
QA-Review: Mysti Lilla <mysti@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Karl Lloyd <karl@instructure.com>
Cody Cutrer