Skip to content

Commit

Permalink
Support "category" query param
Browse files Browse the repository at this point in the history 
Closes MAT-633

Additionally fix an issue with thumbnail_url for dev

Test Plan:
- Checkout the associated canvas-lms commit (ask Weston)
- In Canvas, open the buttons and icons tray
- Navigate to the "Image" section
- Choose "Course Images"
> Verify previously created buttons and icons
  do not appear in the list of images

Change-Id: Ib8e6b6ec2b5bae58d799eae75bf190e16c0bc4d7
Reviewed-on: https://gerrit.instructure.com/c/canvas-rce-api/+/283765
Tested-by: Service Cloud Jenkins <[email protected]>
Reviewed-by: Jacob DeWar <[email protected]>
QA-Review: Jacob DeWar <[email protected]>
Product-Review: Weston Dransfield <[email protected]>
  • Loading branch information
@westonkd
westonkd committed Feb 1, 2022
1 parent d40f142 commit ad7865c
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 37 deletions.
16 changes: 9 additions & 7 deletions app/api/documents.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,20 @@
const packageBookmark = require("./packageBookmark");
const { getArrayQueryParam } = require("../utils/object");
const { getSearch } = require("../utils/search");
const { optionalQuery } = require("../utils/optionalQuery");

function getContentTypes(query) {
const list = getArrayQueryParam(query.content_types);
if (list && list.length) {
return "&" + list.map(t => `content_types[]=${t}`).join("&");
return "&" + list.map((t) => `content_types[]=${t}`).join("&");
}
return "";
}

function getNotContentTypes(query) {
const list = getArrayQueryParam(query.exclude_content_types);
if (list && list.length) {
return "&" + list.map(t => `exclude_content_types[]=${t}`).join("&");
return "&" + list.map((t) => `exclude_content_types[]=${t}`).join("&");
}
return "";
}
Expand All @@ -46,7 +47,7 @@ const validSortFields = [
"created_at",
"updated_at",
"content_type",
"user"
"user",
];

function getSort(query) {
Expand All @@ -71,16 +72,17 @@ function canvasPath(request) {
let search = getSearch(request.query);
let context = getContext(request.query);
let preview = getPreview(request.query);
const category = optionalQuery(request.query, "category");

return `/api/v1/${context}/${request.query.contextId}/files?per_page=${request.query.per_page}&use_verifiers=0${content_types}${exclude_content_types}${sort}${search}${preview}`;
return `/api/v1/${context}/${request.query.contextId}/files?per_page=${request.query.per_page}&use_verifiers=0${content_types}${exclude_content_types}${sort}${search}${preview}${category}`;
}

const svg_re = /image\/svg/;
function canvasResponseHandler(request, response, canvasResponse) {
response.status(canvasResponse.statusCode);
if (canvasResponse.statusCode === 200) {
const files = canvasResponse.body;
const transformedFiles = files.map(file => {
const transformedFiles = files.map((file) => {
// svg files come back from canvas without a thumbnail
// let's use the file's url
let thumbnail_url = file.thumbnail_url;
Expand All @@ -103,13 +105,13 @@ function canvasResponseHandler(request, response, canvasResponse) {
unlock_at: file.unlock_at,
lock_at: file.lock_at,
date: file.created_at,
uuid: file.uuid
uuid: file.uuid,
};
});

response.send({
files: transformedFiles,
bookmark: packageBookmark(request, canvasResponse.bookmark)
bookmark: packageBookmark(request, canvasResponse.bookmark),
});
} else {
response.send(canvasResponse.body);
Expand Down
7 changes: 4 additions & 3 deletions app/api/files.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ function canvasResponseHandler(request, response, canvasResponse) {
response.status(canvasResponse.statusCode);
if (canvasResponse.statusCode === 200) {
const files = canvasResponse.body;

response.send({
files: files.map(file => {
files: files.map((file) => {
return {
createdAt: file.created_at,
id: file.id,
Expand All @@ -35,10 +36,10 @@ function canvasResponseHandler(request, response, canvasResponse) {
embed: fileEmbed(file),
folderId: file.folder_id,
iframeUrl: file.embedded_iframe_url,
thumbnailUrl: file.thumbnail_url
thumbnailUrl: file.thumbnail_url || file.url,
};
}),
bookmark: packageBookmark(request, canvasResponse.bookmark)
bookmark: packageBookmark(request, canvasResponse.bookmark),
});
} else {
response.send(canvasResponse.body);
Expand Down
7 changes: 7 additions & 0 deletions app/utils/optionalQuery.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
"use strict";

function optionalQuery(query, name) {
return query[name] ? `&${name}=${query[name]}` : "";
}

module.exports = { optionalQuery };
58 changes: 42 additions & 16 deletions test/service/api/documents.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { strictEqual } = require("assert");
const sinon = require("sinon");
const {
canvasPath,
canvasResponseHandler
canvasResponseHandler,
} = require("../../../app/api/documents");

describe("Documents API", () => {
Expand Down Expand Up @@ -49,7 +49,7 @@ describe("Documents API", () => {
contextId: id,
contextType: "course",
content_types: "text,application",
per_page: 50
per_page: 50,
};
const path = canvasPath({ params, query });
strictEqual(
Expand All @@ -65,7 +65,7 @@ describe("Documents API", () => {
contextId: id,
contextType: "course",
content_types: ["text", "application"],
per_page: 50
per_page: 50,
};
const path = canvasPath({ params, query });
strictEqual(
Expand All @@ -81,7 +81,7 @@ describe("Documents API", () => {
contextId: id,
contextType: "course",
exclude_content_types: "text,application",
per_page: 50
per_page: 50,
};
const path = canvasPath({ params, query });
strictEqual(
Expand All @@ -97,7 +97,7 @@ describe("Documents API", () => {
contextId: id,
contextType: "course",
exclude_content_types: ["text", "application"],
per_page: 50
per_page: 50,
};
const path = canvasPath({ params, query });
strictEqual(
Expand All @@ -113,7 +113,7 @@ describe("Documents API", () => {
contextId: id,
contextType: "course",
sort: "name",
per_page: 50
per_page: 50,
};
const path = canvasPath({ params, query });
strictEqual(
Expand All @@ -130,7 +130,7 @@ describe("Documents API", () => {
contextType: "course",
sort: "name",
order: "desc",
per_page: 50
per_page: 50,
};
const path = canvasPath({ params, query });
strictEqual(
Expand All @@ -148,14 +148,40 @@ describe("Documents API", () => {
sort: "name",
order: "desc",
search_term: "foo%20bar", // search comes in uri encoded
per_page: 50
per_page: 50,
};
const path = canvasPath({ params, query });
strictEqual(
path,
`/api/v1/courses/${id}/files?per_page=50&use_verifiers=0&sort=name&order=desc&search_term=foo%20bar`
);
});

describe("with 'category' query", () => {
let id, params, query;

beforeEach(() => {
id = 47;
params = {};
query = {
contextId: id,
contextType: "course",
sort: "name",
order: "desc",
search_term: "foo%20bar", // search comes in uri encoded
per_page: 50,
category: "uncategorized",
};
});

it("sets the 'category' query param", () => {
const path = canvasPath({ params, query });
strictEqual(
path,
`/api/v1/courses/47/files?per_page=50&use_verifiers=0&sort=name&order=desc&search_term=foo%20bar&category=uncategorized`
);
});
});
});

describe("canvasResponseHandler()", () => {
Expand All @@ -167,16 +193,16 @@ describe("Documents API", () => {
get: () => "canvashost",
query: {
contextType: "course",
contextId: "17"
}
contextId: "17",
},
};
response = {
status: sinon.spy(),
send: sinon.spy()
send: sinon.spy(),
};
canvasResponse = {
statusCode: 200,
body: []
body: [],
};
});

Expand Down Expand Up @@ -208,15 +234,15 @@ describe("Documents API", () => {
unlock_at: "tomorrow",
lock_at: "next week",
created_at: "last week",
uuid: "xyzzy"
}
uuid: "xyzzy",
},
];
});

it("transforms the API response for course files", () => {
canvasResponseHandler(request, response, canvasResponse);

sinon.assert.calledWithMatch(response.send, val => {
sinon.assert.calledWithMatch(response.send, (val) => {
return (
val.bookmark === null &&
val.files[0].id === 1 &&
Expand All @@ -242,7 +268,7 @@ describe("Documents API", () => {
request.query.contextType = "user";
canvasResponseHandler(request, response, canvasResponse);

sinon.assert.calledWithMatch(response.send, val => {
sinon.assert.calledWithMatch(response.send, (val) => {
return (
val.bookmark === null &&
val.files[0].id === 1 &&
Expand Down
46 changes: 35 additions & 11 deletions test/service/api/files.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe("Files API", () => {
const query = {
contextType: "course",
contextId: "nomatter",
per_page: 50
per_page: 50,
};
const expectedPath = `/api/v1/folders/${id}/files?per_page=50&include[]=preview_url&use_verifiers=0`;
assert.equal(canvasPath({ params, query }), expectedPath);
Expand All @@ -33,7 +33,7 @@ describe("Files API", () => {
contextType: "course",
contextId: "nomatter",
per_page: 50,
search_term: "banana"
search_term: "banana",
};
const expectedPath = `/api/v1/folders/${id}/files?per_page=50&include[]=preview_url&use_verifiers=0&search_term=banana`;
assert.equal(canvasPath({ params, query }), expectedPath);
Expand All @@ -47,7 +47,7 @@ describe("Files API", () => {
contextId: "nomatter",
per_page: 50,
sort: "created_at",
order: "desc"
order: "desc",
};
const expectedPath = `/api/v1/folders/${id}/files?per_page=50&include[]=preview_url&use_verifiers=0&sort=created_at&order=desc`;
assert.equal(canvasPath({ params, query }), expectedPath);
Expand All @@ -62,11 +62,11 @@ describe("Files API", () => {
request = { get: () => {} };
response = {
status: sinon.spy(),
send: sinon.spy()
send: sinon.spy(),
};
canvasResponse = {
status: 200,
body: []
body: [],
};
});

Expand Down Expand Up @@ -96,14 +96,38 @@ describe("Files API", () => {
url: "someurl",
folder_id: 1,
embedded_iframe_url: "https://canvas.com/foo/bar",
thumbnail_url: "https://canvas.com/foo/bar/thumbnail"
thumbnail_url: "https://canvas.com/foo/bar/thumbnail",
};
});

describe("when no thumbnail_url is given", () => {
beforeEach(() => {
file.thumbnail_url = undefined;
});

it("uses the url", () => {
canvasResponse.body = [file];
canvasResponse.statusCode = 200;
canvasResponseHandler(request, response, canvasResponse);
assert.deepStrictEqual(response.send.firstCall.args[0].files[0], {
createdAt: "2021-08-12T18:30:53Z",
id: 47,
uuid: "123123123asdf",
type: "text/plain",
name: "Foo",
url: "someurl",
embed: { type: "scribd" },
folderId: 1,
iframeUrl: "https://canvas.com/foo/bar",
thumbnailUrl: "someurl",
});
});
});

it("creates files array property with items from response body", () => {
canvasResponse.body = [{}, {}, {}];
canvasResponseHandler(request, response, canvasResponse);
response.send.calledWithMatch(val => {
response.send.calledWithMatch((val) => {
return (
Array.isArray(val.files) &&
val.files.length === canvasResponse.body.length
Expand All @@ -125,31 +149,31 @@ describe("Files API", () => {
embed: { type: "scribd" },
folderId: 1,
iframeUrl: "https://canvas.com/foo/bar",
thumbnailUrl: "https://canvas.com/foo/bar/thumbnail"
thumbnailUrl: "https://canvas.com/foo/bar/thumbnail",
});
});

it("will use fallbacks for name", () => {
file.display_name = undefined;
canvasResponse.body = [file];
canvasResponseHandler(request, response, canvasResponse);
response.send.calledWithMatch(val => {
response.send.calledWithMatch((val) => {
return sinon.match({ name: file.filename }, val[0]);
});
});

it("has bookmark from canvasResponse", () => {
canvasResponse.bookmark = "foo";
canvasResponseHandler(request, response, canvasResponse);
response.send.calledWithMatch(val => {
response.send.calledWithMatch((val) => {
return /foo/.test(val.bookmark);
});
});

it("has null bookmark if canvasResponse does not have one", () => {
canvasResponse.bookmark = undefined;
canvasResponseHandler(request, response, canvasResponse);
response.send.calledWithMatch(val => {
response.send.calledWithMatch((val) => {
return val.bookmark === null;
});
});
Expand Down

0 comments on commit ad7865c

Please sign in to comment.