Skip to content

Commit 35424a9

Browse files
authored
Merge pull request #148 from OneNoteDev/feature/patch-permission-optimization
Remove unecessary subsequent PATCH permission checks
2 parents 6278bd9 + 1c5ffac commit 35424a9

File tree

3 files changed

+23
-10
lines changed

3 files changed

+23
-10
lines changed

src/scripts/clipperUI/clipper.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@ class ClipperClass extends ComponentBase<ClipperState, {}> {
612612
let userInfoReturned = updatedUser && !!updatedUser.user;
613613
if (userInfoReturned) {
614614
// Sign in succeeded
615+
Clipper.storeValue(ClipperStorageKeys.hasPatchPermissions, "true");
615616
Clipper.logger.logUserFunnel(Log.Funnel.Label.AuthSignInCompleted);
616617
}
617618
handleSignInEvent.setCustomProperty(Log.PropertyName.Custom.UserInformationReturned, userInfoReturned);

src/scripts/clipperUI/saveToOneNote.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -371,17 +371,28 @@ export class SaveToOneNote {
371371
});
372372
}
373373

374-
private static checkIfUserHasPermissionToPatch(): Promise<any> {
375-
let patchPermissionCheckEvent = new Log.Event.PromiseEvent(Log.Event.Label.PatchPermissionCheck);
374+
private static checkIfUserHasPermissionToPatch(): Promise<void> {
376375
return new Promise<any>((resolve, reject) => {
377-
SaveToOneNote.getPages({ top: 1, sectionId: this.clipperState.saveLocation }).then((getPagesResponse) => {
378-
resolve(getPagesResponse);
379-
}, (error) => {
380-
patchPermissionCheckEvent.setStatus(Log.Status.Failed);
381-
patchPermissionCheckEvent.setFailureInfo({ error: error });
382-
reject(error);
383-
}).then(() => {
384-
Clipper.logger.logEvent(patchPermissionCheckEvent);
376+
Clipper.getStoredValue(ClipperStorageKeys.hasPatchPermissions, (hasPermissions) => {
377+
// We have checked their permissions successfully in the past, or the user signed in on this device (with the latest scope)
378+
if (hasPermissions) {
379+
resolve();
380+
} else {
381+
// As of v3.2.9, we have added a new scope for MSA to allow for PATCHing, however currently-logged-in users will not have
382+
// this scope, so this call is a workaround to check for permissions, but is very unperformant. We need to investigate a
383+
// quicker way of doing this ... perhaps exposing an endpoint that we can use for this sole purpose.
384+
let patchPermissionCheckEvent = new Log.Event.PromiseEvent(Log.Event.Label.PatchPermissionCheck);
385+
SaveToOneNote.getPages({ top: 1, sectionId: this.clipperState.saveLocation }).then(() => {
386+
Clipper.storeValue(ClipperStorageKeys.hasPatchPermissions, "true");
387+
resolve();
388+
}, (error) => {
389+
patchPermissionCheckEvent.setStatus(Log.Status.Failed);
390+
patchPermissionCheckEvent.setFailureInfo({ error: error });
391+
reject(error);
392+
}).then(() => {
393+
Clipper.logger.logEvent(patchPermissionCheckEvent);
394+
});
395+
}
385396
});
386397
});
387398
}

src/scripts/storage/clipperStorageKeys.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export module ClipperStorageKeys {
55
export var displayLanguageOverride = "displayLocaleOverride";
66
export var doNotPromptRatings = "doNotPromptRatings";
77
export var flightingInfo = "flightingInfo";
8+
export var hasPatchPermissions = "hasPatchPermissions";
89
export var lastBadRatingDate = "lastBadRatingDate";
910
export var lastBadRatingVersion = "lastBadRatingVersion";
1011
export var lastClippedDate = "lastClippedDate";

0 commit comments

Comments
 (0)