o365-moodle
o365-moodle copied to clipboard
Admin tool to clear application or system tokens
https://github.com/microsoft/o365-moodle/blob/6a210db3779a6da9db544139ccb4e0d12873d199/local/o365/db/upgrade.php#L649
This should have never made into the codebase because everybody with a decent directory will have to fetch again at 200 user per hour unless they increase the frequency for a nice have "additional profile fields" which would occur on next login of the user.
Office365-Maintenance-Reset-Token.zip
This change need to be merge ASAP and upgrade step as noted removed.
Hi @tlock,
Thank you for raising the issue. I understand the root of your concern, but the user sync task already handles it.
The code change was made on purpose and in our tests, it does what it's supposed to do.
- The delta token needs to be cleared because the delta user Graph API call now requires more user properties to be included in the return value. The properties returned by the delta user API are defined in the first call, and kept in the delta token. Any change to it will require making a fresh request without passing in delta token.
- After the delta token is cleared, the sync user task will make calls to the user delta API to make get users. You are right that each call will only return a maximum of 200 users, but the task has logic to make the call multiple times, until the all users are returned.
- In the first call, no delta token is passed, which will ensure it returns the first batch of users from the full list of users.
- If
nextLinkis returned, it means there are more users to return, and theskiptokenis stored. Theskiptokenis used in a new call to the delta user API to return the next batch of users. - If
deltaLinkis returned, it means it reaches the last batch of users, and thedeltatokenis stored.
The logic ensures it will not process only 200 users in a task run.
On a separate note, could you confirm based on which version was your patch file created please. It cannot be applied in the latest code in MOODLE_39_STABLE or MOODLE_310_STABLE branch.
Regards, Lai
Hi Lai,
The code was based on 3.8.0.1 and detected this issue while looking at the 3.10 code.
In regard the raised issue, I didn't noticed the loop to get more than 200 users per cron run. That itself is great, but running the sync previously from start to finish was hours which concerns me because I don't see any guard again the cron job running over the top every hour.
As discussed originally, force re-sync needs to controlled and additional fields will not be included to till that has occur by running the maintenance operation to "Hard reset" the tokens.
Therefore, I have commented out the problem line and will address separately.
Regards, Tim
Hi @tlock,
Am I right in understanding this issue can be closed?
Regards, Lai
Hi @weilai-irl ,
Are you able to implement the code as suggested, then I'd be happy for this to be closed.
Hi @tlock,
As explained in my first comment, unsetting the task_usersync_lastdeltatoken config variable to force a full user sync is a necessary step during upgrade in order to receive additional profile fields from Azure AD in Graph API call. This is due to the design of the user: delta Graph API. In reality, this will cause the first run of the user sync task after the plugin upgrade to take longer time to finish, but it will not be only processing 200 users per run.
If the purpose of your proposed change is to provide a way for Moodle site admins to manually reset the task_usersync_lastdeltatoken config variable, this would be categorised as a feature request, and will be scheduled for a future release. Or please update your patch to be based on a more recent version of the code and possibly create a PR, so that it can be reviewed and merged more easily.
Regards, Lai
Hi @weilai-irl,
I will update the patch with the latest code in due course and post here.