Skip to content

Commit

Permalink
Fix errors setting Google Drive permissions not being reported
Browse files Browse the repository at this point in the history
If an error occurred when attemping to change file permissions after selecting a
file from Google Drive, the LMS frontend incorrectly assumed the attempt had
succeeded. This is because the `request.execute` API doesn't work as the code
expected. It always invokes its first argument with the response. The
`request.then` API is the correct Promise-like API. The argument passed to the
rejection handler then needs to be processed to turn the result into an `Error`
as expected by the calling code.
  • Loading branch information
robertknight committed Jun 14, 2024
1 parent 1799b89 commit 0bba5e3
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
18 changes: 15 additions & 3 deletions lms/static/scripts/frontend_apps/utils/google-picker-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,21 @@ export class GooglePickerClient {
fileId: docId,
resource: body,
});
return new Promise((resolve, reject) => {
// @ts-expect-error - We're missing types for this.
request.execute(resolve, reject);
const result = new Promise((resolve, reject) => {
// @ts-expect-error - We're missing types for this, but see
// https://github.com/google/google-api-javascript-client/blob/96cf6057f03c5e56179e83869828a06c47ce571b/docs/reference.md.
//
// For the types of arguments passed to resolve/reject, see
// https://github.com/google/google-api-javascript-client/blob/96cf6057f03c5e56179e83869828a06c47ce571b/docs/promises.md#using-promises-1.
request.then(resolve, reject);
});

try {
await result;
} catch (response) {
throw new Error(
response.result?.error?.message ?? 'Unable to make file public',
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ describe('GooglePickerClient', () => {

beforeEach(() => {
createPermission = fakeGoogleLibs.client.drive.permissions.create;
const apiRequest = { execute: resolve => resolve() };
const apiRequest = { then: resolve => resolve() };
createPermission.returns(apiRequest);
});

Expand Down Expand Up @@ -394,8 +394,14 @@ describe('GooglePickerClient', () => {
const client = createClient();
await client.requestAuthorization();
createPermission.returns({
execute: (_, reject) =>
reject(new Error('Changing permissions failed')),
then: (_, reject) =>
reject({
result: {
error: {
message: 'Changing permissions failed',
},
},
}),
});
let err;
try {
Expand Down

0 comments on commit 0bba5e3

Please sign in to comment.