Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions local/o365/classes/feature/usersync/main.php
Original file line number Diff line number Diff line change
Expand Up @@ -274,13 +274,14 @@ protected function apply_photo(
}

if (!empty($muser->picture)) {
// User has no photo. Deleting previous profile photo.
// User had a photo — clear it.
$fs = get_file_storage();
$fs->delete_area_files($context->id, 'user', 'icon');
$DB->set_field('user', 'picture', 0, ['id' => $muser->id]);
$result = true;
} else {
$result = false;
}

$result = false;
} else {
// Both get_photo() and get_photos_batch() guarantee that $photodata is valid
// binary image data at this point (validated via is_valid_photo_binary() in
Expand All @@ -294,10 +295,27 @@ protected function apply_photo(
fwrite($fp, $photodata);
fclose($fp);

// Capture the existing f1 file content hash before process_new_icon
// deletes it. process_new_icon always returns a new file ID even for
// identical image content, so $newpicture != $muser->picture is not a
// reliable content-change indicator — we must compare hashes instead.
$fs = get_file_storage();
$existingfile = $fs->get_file($context->id, 'user', 'icon', 0, '/', 'f1.png')
?: $fs->get_file($context->id, 'user', 'icon', 0, '/', 'f1.jpg');
$existinghash = $existingfile ? $existingfile->get_contenthash() : null;

$newpicture = process_new_icon($context, 'user', 'icon', 0, $tempfile);
if ($newpicture != $muser->picture) {

if ($newpicture) {
// Always update user.picture because process_new_icon deleted the old
// files — the old ID is now stale regardless of content equality.
$DB->set_field('user', 'picture', $newpicture, ['id' => $muser->id]);
$result = true;

// Determine whether content actually changed by comparing hashes.
$newfile = $fs->get_file($context->id, 'user', 'icon', 0, '/', 'f1.png')
?: $fs->get_file($context->id, 'user', 'icon', 0, '/', 'f1.jpg');
$newhash = $newfile ? $newfile->get_contenthash() : null;
$result = ($existinghash !== $newhash);
}

@unlink($tempfile);
Expand Down Expand Up @@ -463,8 +481,8 @@ public function apply_photo_public(
$photodata,
?int $appassignid = null,
?int $currentpicture = null
) {
$this->apply_photo($muserid, $photodata, false, $appassignid, $currentpicture);
): bool {
return $this->apply_photo($muserid, $photodata, false, $appassignid, $currentpicture);
}

/**
Expand Down
36 changes: 18 additions & 18 deletions local/o365/classes/rest/unified.php
Original file line number Diff line number Diff line change
Expand Up @@ -2670,36 +2670,36 @@ public function get_timezone(string $upn) {
* Get timezones for multiple users using batch requests.
* Microsoft Graph allows up to 20 requests per batch.
*
* @param array $upns Array of user principal names
* @return array Associative array with UPN as key and timezone data as value
* @param array $objectids Array of Azure AD object IDs (GUIDs)
* @return array Associative array with objectid as key and timezone data as value
*/
public function get_timezones_batch(array $upns): array {
if (empty($upns)) {
public function get_timezones_batch(array $objectids): array {
if (empty($objectids)) {
return [];
}

$results = [];
$chunks = array_chunk($upns, 20); // Max 20 requests per batch.
$chunks = array_chunk($objectids, 20); // Max 20 requests per batch.

foreach ($chunks as $chunk) {
$batchrequests = [];
$idtoupnmap = [];
$idtoobjectidmap = [];

// Pre-initialise every UPN to false so that UPNs absent from the batch
// response (partial API failure) are treated as "no timezone" rather than
// silently skipped by the caller's isset() / empty() branch logic.
foreach ($chunk as $upn) {
$results[$upn] = false;
// Pre-initialise every objectid to false so that objectids absent from
// the batch response (partial API failure) are treated as "no timezone"
// rather than silently skipped by the caller's isset() / empty() branch.
foreach ($chunk as $objectid) {
$results[$objectid] = false;
}

// Build batch request.
foreach ($chunk as $index => $upn) {
foreach ($chunk as $index => $objectid) {
$requestid = (string)($index + 1);
$idtoupnmap[$requestid] = $upn;
$idtoobjectidmap[$requestid] = $objectid;
$batchrequests[] = [
'id' => $requestid,
'method' => 'GET',
'url' => '/users/' . urlencode($upn) . '/mailboxSettings/timeZone',
'url' => '/users/' . urlencode($objectid) . '/mailboxSettings/timeZone',
];
}

Expand All @@ -2712,19 +2712,19 @@ public function get_timezones_batch(array $upns): array {
if (!empty($batchresponse['responses']) && is_array($batchresponse['responses'])) {
foreach ($batchresponse['responses'] as $individualresponse) {
$requestid = $individualresponse['id'];
$upn = $idtoupnmap[$requestid];
$objectid = $idtoobjectidmap[$requestid];

if ($individualresponse['status'] === 200 && !empty($individualresponse['body'])) {
$results[$upn] = $individualresponse['body'];
$results[$objectid] = $individualresponse['body'];
} else {
// Request failed or returned non-200 status.
$results[$upn] = false;
$results[$objectid] = false;
}
}
}
} catch (moodle_exception $e) {
// Batch call itself failed; pre-initialisation above already set all
// UPNs in this chunk to false, so no additional work needed here.
// objectids in this chunk to false, so no additional work needed here.
debugging('Batch timezone request failed: ' . $e->getMessage(), DEBUG_DEVELOPER);
}
}
Expand Down
Loading