From ad5c47edac875abc55c92cd79341a98ee5bb1556 Mon Sep 17 00:00:00 2001 From: Nathan Levesque Date: Thu, 29 Aug 2024 09:47:32 -0400 Subject: [PATCH 1/4] Fix syntax that a pre-commit check was failing (#1480) --- data_fixtures/migrations/0001_add_testimonial_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_fixtures/migrations/0001_add_testimonial_data.py b/data_fixtures/migrations/0001_add_testimonial_data.py index ad9001688a..544a61547b 100644 --- a/data_fixtures/migrations/0001_add_testimonial_data.py +++ b/data_fixtures/migrations/0001_add_testimonial_data.py @@ -242,7 +242,7 @@ def load_fixtures(apps, schema_editor): Save the image from fixture so alternate sizes are generated """ with Path.open( - f"{settings.BASE_DIR}/testimonials/fixtures/avatars/{fixture["attestant_name"]}.png", + f"{settings.BASE_DIR}/testimonials/fixtures/avatars/{fixture['attestant_name']}.png", "rb", ) as imagefile: testimonial.avatar.save("avatar.png", File(imagefile), save=True) From 87fc79b00458e516ab7c269aa289e65f43449a62 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 29 Aug 2024 10:48:40 -0400 Subject: [PATCH 2/4] add to list dialog updates (#1463) * WIP * rework api endpoints and implement them * remove the modal after we're done with it * await setting new list parents before dismissing modal * remove circularprogress for now * fix tests * try and fix warning about api parameter * try and fix python tests again * remove id parameter because it broke everything * remove extraneous API endpoints * simplify api signature * remove extraneous debugging line * filter user list manipulation based on the current user and properly rebuild position index * restrict learning path editing to learning path editor * add reordering logic to learning path setting too * fix js tests * fix schema annotations * fix annotations and response types * override get_serializer_class to satisfy checks * regenerate spec * fix invalidations * revise invalidations based on feedback * only specify parameters where necessary * fix backwards classes * add learning path permission class instead of manual check * add some tests for the new API endpoints * use get_serializer_class * properly use get_serializer_class * throw an unauthorized error if you try to assign courses to userlists that don't belong to you * fix test by checking for raising of permission error * fix rebase issue yet again * add a test to ensure accidental unassignment does not happen * pass in course to userlist assignment function --- frontends/api/src/generated/v1/api.ts | 375 ++++++++++++++++ .../api/src/hooks/learningResources/index.ts | 47 ++- frontends/api/src/test-utils/urls.ts | 18 +- .../Dialogs/AddToListDialog.test.tsx | 80 ++-- .../Dialogs/AddToListDialog.tsx | 399 ++++++------------ .../Checkbox/CheckboxChoiceField.tsx | 16 +- learning_resources/urls.py | 6 +- learning_resources/views.py | 133 ++++++ learning_resources/views_learningpath_test.py | 26 ++ learning_resources/views_userlist_test.py | 69 ++- openapi/specs/v1.yaml | 96 +++++ 11 files changed, 936 insertions(+), 329 deletions(-) diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index 4b6095b59a..f004b56fbe 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -3300,6 +3300,38 @@ export interface PatchedLearningPathResourceRequest { completeness?: number } +/** + * CRUD serializer for LearningResourceRelationship + * @export + * @interface PatchedLearningResourceRelationshipRequest + */ +export interface PatchedLearningResourceRelationshipRequest { + /** + * + * @type {number} + * @memberof PatchedLearningResourceRelationshipRequest + */ + position?: number + /** + * + * @type {RelationTypeEnum} + * @memberof PatchedLearningResourceRelationshipRequest + */ + relation_type?: RelationTypeEnum + /** + * + * @type {number} + * @memberof PatchedLearningResourceRelationshipRequest + */ + parent?: number + /** + * + * @type {number} + * @memberof PatchedLearningResourceRelationshipRequest + */ + child?: number +} + /** * Serializer for UserListRelationship model * @export @@ -10560,6 +10592,68 @@ export const LearningResourcesApiAxiosParamCreator = function ( options: localVarRequestOptions, } }, + /** + * Set Learning Path Relationships on a given Learning Resource. + * @summary Set Learning Path Relationships + * @param {number} id id of the learning resource + * @param {Array} [learning_path_id] id of the parent learning path + * @param {PatchedLearningResourceRelationshipRequest} [PatchedLearningResourceRelationshipRequest] + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + learningResourcesLearningPathsPartialUpdate: async ( + id: number, + learning_path_id?: Array, + PatchedLearningResourceRelationshipRequest?: PatchedLearningResourceRelationshipRequest, + options: RawAxiosRequestConfig = {}, + ): Promise => { + // verify required parameter 'id' is not null or undefined + assertParamExists("learningResourcesLearningPathsPartialUpdate", "id", id) + const localVarPath = + `/api/v1/learning_resources/{id}/learning_paths/`.replace( + `{${"id"}}`, + encodeURIComponent(String(id)), + ) + // use dummy base URL string because the URL constructor only accepts absolute URLs. + const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL) + let baseOptions + if (configuration) { + baseOptions = configuration.baseOptions + } + + const localVarRequestOptions = { + method: "PATCH", + ...baseOptions, + ...options, + } + const localVarHeaderParameter = {} as any + const localVarQueryParameter = {} as any + + if (learning_path_id) { + localVarQueryParameter["learning_path_id"] = learning_path_id + } + + localVarHeaderParameter["Content-Type"] = "application/json" + + setSearchParams(localVarUrlObj, localVarQueryParameter) + let headersFromBaseOptions = + baseOptions && baseOptions.headers ? baseOptions.headers : {} + localVarRequestOptions.headers = { + ...localVarHeaderParameter, + ...headersFromBaseOptions, + ...options.headers, + } + localVarRequestOptions.data = serializeDataIfNeeded( + PatchedLearningResourceRelationshipRequest, + localVarRequestOptions, + configuration, + ) + + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + } + }, /** * Get a paginated list of learning resources. * @summary List @@ -10746,6 +10840,67 @@ export const LearningResourcesApiAxiosParamCreator = function ( ...options.headers, } + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + } + }, + /** + * Set User List Relationships on a given Learning Resource. + * @summary Set User List Relationships + * @param {number} id id of the learning resource + * @param {Array} [userlist_id] id of the parent user list + * @param {PatchedUserListRelationshipRequest} [PatchedUserListRelationshipRequest] + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + learningResourcesUserlistsPartialUpdate: async ( + id: number, + userlist_id?: Array, + PatchedUserListRelationshipRequest?: PatchedUserListRelationshipRequest, + options: RawAxiosRequestConfig = {}, + ): Promise => { + // verify required parameter 'id' is not null or undefined + assertParamExists("learningResourcesUserlistsPartialUpdate", "id", id) + const localVarPath = `/api/v1/learning_resources/{id}/userlists/`.replace( + `{${"id"}}`, + encodeURIComponent(String(id)), + ) + // use dummy base URL string because the URL constructor only accepts absolute URLs. + const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL) + let baseOptions + if (configuration) { + baseOptions = configuration.baseOptions + } + + const localVarRequestOptions = { + method: "PATCH", + ...baseOptions, + ...options, + } + const localVarHeaderParameter = {} as any + const localVarQueryParameter = {} as any + + if (userlist_id) { + localVarQueryParameter["userlist_id"] = userlist_id + } + + localVarHeaderParameter["Content-Type"] = "application/json" + + setSearchParams(localVarUrlObj, localVarQueryParameter) + let headersFromBaseOptions = + baseOptions && baseOptions.headers ? baseOptions.headers : {} + localVarRequestOptions.headers = { + ...localVarHeaderParameter, + ...headersFromBaseOptions, + ...options.headers, + } + localVarRequestOptions.data = serializeDataIfNeeded( + PatchedUserListRelationshipRequest, + localVarRequestOptions, + configuration, + ) + return { url: toPathString(localVarUrlObj), options: localVarRequestOptions, @@ -10931,6 +11086,46 @@ export const LearningResourcesApiFp = function (configuration?: Configuration) { configuration, )(axios, operationBasePath || basePath) }, + /** + * Set Learning Path Relationships on a given Learning Resource. + * @summary Set Learning Path Relationships + * @param {number} id id of the learning resource + * @param {Array} [learning_path_id] id of the parent learning path + * @param {PatchedLearningResourceRelationshipRequest} [PatchedLearningResourceRelationshipRequest] + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async learningResourcesLearningPathsPartialUpdate( + id: number, + learning_path_id?: Array, + PatchedLearningResourceRelationshipRequest?: PatchedLearningResourceRelationshipRequest, + options?: RawAxiosRequestConfig, + ): Promise< + ( + axios?: AxiosInstance, + basePath?: string, + ) => AxiosPromise> + > { + const localVarAxiosArgs = + await localVarAxiosParamCreator.learningResourcesLearningPathsPartialUpdate( + id, + learning_path_id, + PatchedLearningResourceRelationshipRequest, + options, + ) + const index = configuration?.serverIndex ?? 0 + const operationBasePath = + operationServerMap[ + "LearningResourcesApi.learningResourcesLearningPathsPartialUpdate" + ]?.[index]?.url + return (axios, basePath) => + createRequestFunction( + localVarAxiosArgs, + globalAxios, + BASE_PATH, + configuration, + )(axios, operationBasePath || basePath) + }, /** * Get a paginated list of learning resources. * @summary List @@ -11044,6 +11239,46 @@ export const LearningResourcesApiFp = function (configuration?: Configuration) { configuration, )(axios, operationBasePath || basePath) }, + /** + * Set User List Relationships on a given Learning Resource. + * @summary Set User List Relationships + * @param {number} id id of the learning resource + * @param {Array} [userlist_id] id of the parent user list + * @param {PatchedUserListRelationshipRequest} [PatchedUserListRelationshipRequest] + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async learningResourcesUserlistsPartialUpdate( + id: number, + userlist_id?: Array, + PatchedUserListRelationshipRequest?: PatchedUserListRelationshipRequest, + options?: RawAxiosRequestConfig, + ): Promise< + ( + axios?: AxiosInstance, + basePath?: string, + ) => AxiosPromise> + > { + const localVarAxiosArgs = + await localVarAxiosParamCreator.learningResourcesUserlistsPartialUpdate( + id, + userlist_id, + PatchedUserListRelationshipRequest, + options, + ) + const index = configuration?.serverIndex ?? 0 + const operationBasePath = + operationServerMap[ + "LearningResourcesApi.learningResourcesUserlistsPartialUpdate" + ]?.[index]?.url + return (axios, basePath) => + createRequestFunction( + localVarAxiosArgs, + globalAxios, + BASE_PATH, + configuration, + )(axios, operationBasePath || basePath) + }, } } @@ -11142,6 +11377,26 @@ export const LearningResourcesApiFactory = function ( ) .then((request) => request(axios, basePath)) }, + /** + * Set Learning Path Relationships on a given Learning Resource. + * @summary Set Learning Path Relationships + * @param {LearningResourcesApiLearningResourcesLearningPathsPartialUpdateRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + learningResourcesLearningPathsPartialUpdate( + requestParameters: LearningResourcesApiLearningResourcesLearningPathsPartialUpdateRequest, + options?: RawAxiosRequestConfig, + ): AxiosPromise> { + return localVarFp + .learningResourcesLearningPathsPartialUpdate( + requestParameters.id, + requestParameters.learning_path_id, + requestParameters.PatchedLearningResourceRelationshipRequest, + options, + ) + .then((request) => request(axios, basePath)) + }, /** * Get a paginated list of learning resources. * @summary List @@ -11191,6 +11446,26 @@ export const LearningResourcesApiFactory = function ( .learningResourcesRetrieve(requestParameters.id, options) .then((request) => request(axios, basePath)) }, + /** + * Set User List Relationships on a given Learning Resource. + * @summary Set User List Relationships + * @param {LearningResourcesApiLearningResourcesUserlistsPartialUpdateRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + learningResourcesUserlistsPartialUpdate( + requestParameters: LearningResourcesApiLearningResourcesUserlistsPartialUpdateRequest, + options?: RawAxiosRequestConfig, + ): AxiosPromise> { + return localVarFp + .learningResourcesUserlistsPartialUpdate( + requestParameters.id, + requestParameters.userlist_id, + requestParameters.PatchedUserListRelationshipRequest, + options, + ) + .then((request) => request(axios, basePath)) + }, } } @@ -11334,6 +11609,34 @@ export interface LearningResourcesApiLearningResourcesItemsRetrieveRequest { readonly learning_resource_id: number } +/** + * Request parameters for learningResourcesLearningPathsPartialUpdate operation in LearningResourcesApi. + * @export + * @interface LearningResourcesApiLearningResourcesLearningPathsPartialUpdateRequest + */ +export interface LearningResourcesApiLearningResourcesLearningPathsPartialUpdateRequest { + /** + * id of the learning resource + * @type {number} + * @memberof LearningResourcesApiLearningResourcesLearningPathsPartialUpdate + */ + readonly id: number + + /** + * id of the parent learning path + * @type {Array} + * @memberof LearningResourcesApiLearningResourcesLearningPathsPartialUpdate + */ + readonly learning_path_id?: Array + + /** + * + * @type {PatchedLearningResourceRelationshipRequest} + * @memberof LearningResourcesApiLearningResourcesLearningPathsPartialUpdate + */ + readonly PatchedLearningResourceRelationshipRequest?: PatchedLearningResourceRelationshipRequest +} + /** * Request parameters for learningResourcesList operation in LearningResourcesApi. * @export @@ -11474,6 +11777,34 @@ export interface LearningResourcesApiLearningResourcesRetrieveRequest { readonly id: number } +/** + * Request parameters for learningResourcesUserlistsPartialUpdate operation in LearningResourcesApi. + * @export + * @interface LearningResourcesApiLearningResourcesUserlistsPartialUpdateRequest + */ +export interface LearningResourcesApiLearningResourcesUserlistsPartialUpdateRequest { + /** + * id of the learning resource + * @type {number} + * @memberof LearningResourcesApiLearningResourcesUserlistsPartialUpdate + */ + readonly id: number + + /** + * id of the parent user list + * @type {Array} + * @memberof LearningResourcesApiLearningResourcesUserlistsPartialUpdate + */ + readonly userlist_id?: Array + + /** + * + * @type {PatchedUserListRelationshipRequest} + * @memberof LearningResourcesApiLearningResourcesUserlistsPartialUpdate + */ + readonly PatchedUserListRelationshipRequest?: PatchedUserListRelationshipRequest +} + /** * LearningResourcesApi - object-oriented interface * @export @@ -11573,6 +11904,28 @@ export class LearningResourcesApi extends BaseAPI { .then((request) => request(this.axios, this.basePath)) } + /** + * Set Learning Path Relationships on a given Learning Resource. + * @summary Set Learning Path Relationships + * @param {LearningResourcesApiLearningResourcesLearningPathsPartialUpdateRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof LearningResourcesApi + */ + public learningResourcesLearningPathsPartialUpdate( + requestParameters: LearningResourcesApiLearningResourcesLearningPathsPartialUpdateRequest, + options?: RawAxiosRequestConfig, + ) { + return LearningResourcesApiFp(this.configuration) + .learningResourcesLearningPathsPartialUpdate( + requestParameters.id, + requestParameters.learning_path_id, + requestParameters.PatchedLearningResourceRelationshipRequest, + options, + ) + .then((request) => request(this.axios, this.basePath)) + } + /** * Get a paginated list of learning resources. * @summary List @@ -11625,6 +11978,28 @@ export class LearningResourcesApi extends BaseAPI { .learningResourcesRetrieve(requestParameters.id, options) .then((request) => request(this.axios, this.basePath)) } + + /** + * Set User List Relationships on a given Learning Resource. + * @summary Set User List Relationships + * @param {LearningResourcesApiLearningResourcesUserlistsPartialUpdateRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof LearningResourcesApi + */ + public learningResourcesUserlistsPartialUpdate( + requestParameters: LearningResourcesApiLearningResourcesUserlistsPartialUpdateRequest, + options?: RawAxiosRequestConfig, + ) { + return LearningResourcesApiFp(this.configuration) + .learningResourcesUserlistsPartialUpdate( + requestParameters.id, + requestParameters.userlist_id, + requestParameters.PatchedUserListRelationshipRequest, + options, + ) + .then((request) => request(this.axios, this.basePath)) + } } /** diff --git a/frontends/api/src/hooks/learningResources/index.ts b/frontends/api/src/hooks/learningResources/index.ts index e2deba4a0c..4519d09720 100644 --- a/frontends/api/src/hooks/learningResources/index.ts +++ b/frontends/api/src/hooks/learningResources/index.ts @@ -5,7 +5,11 @@ import { useQuery, useQueryClient, } from "@tanstack/react-query" -import { learningpathsApi, userListsApi } from "../../clients" +import { + learningpathsApi, + learningResourcesApi, + userListsApi, +} from "../../clients" import type { LearningResourcesApiLearningResourcesListRequest as LRListRequest, TopicsApiTopicsListRequest as TopicsListRequest, @@ -28,6 +32,8 @@ import type { PlatformsApiPlatformsListRequest, FeaturedApiFeaturedListRequest as FeaturedListParams, PaginatedLearningResourceList, + LearningResourcesApiLearningResourcesUserlistsPartialUpdateRequest, + LearningResourcesApiLearningResourcesLearningPathsPartialUpdateRequest, } from "../../generated/v1" import learningResources, { invalidateResourceQueries, @@ -319,6 +325,43 @@ const useUserListRelationshipCreate = () => { }) } +const useLearningResourceSetUserListRelationships = () => { + const queryClient = useQueryClient() + return useMutation({ + mutationFn: ( + params: LearningResourcesApiLearningResourcesUserlistsPartialUpdateRequest, + ) => learningResourcesApi.learningResourcesUserlistsPartialUpdate(params), + onSettled: (_response, _err, vars) => { + invalidateResourceQueries(queryClient, vars.id, { + skipFeatured: false, + }) + vars.userlist_id?.forEach((userlistId) => { + invalidateUserListQueries(queryClient, userlistId) + }) + }, + }) +} + +const useLearningResourceSetLearningPathRelationships = () => { + const queryClient = useQueryClient() + return useMutation({ + mutationFn: ( + params: LearningResourcesApiLearningResourcesLearningPathsPartialUpdateRequest, + ) => + learningResourcesApi.learningResourcesLearningPathsPartialUpdate(params), + onSettled: (_response, _err, vars) => { + invalidateResourceQueries(queryClient, vars.id, { + skipFeatured: false, + }) + vars.learning_path_id?.forEach((learningPathId) => { + invalidateResourceQueries(queryClient, learningPathId, { + skipFeatured: false, + }) + }) + }, + }) +} + const useUserListRelationshipDestroy = () => { const queryClient = useQueryClient() return useMutation({ @@ -454,6 +497,8 @@ export { useLearningpathRelationshipCreate, useLearningpathRelationshipDestroy, useLearningResourcesSearch, + useLearningResourceSetUserListRelationships, + useLearningResourceSetLearningPathRelationships, useUserListList, useUserListsDetail, useUserListCreate, diff --git a/frontends/api/src/test-utils/urls.ts b/frontends/api/src/test-utils/urls.ts index c4ca6079db..9a38165d5b 100644 --- a/frontends/api/src/test-utils/urls.ts +++ b/frontends/api/src/test-utils/urls.ts @@ -34,7 +34,15 @@ const { MITOL_API_BASE_URL: API_BASE_URL } = APP_SETTINGS // eslint-disable-next-line @typescript-eslint/no-explicit-any const query = (params: any) => { if (!params || Object.keys(params).length === 0) return "" - return `?${new URLSearchParams(params).toString()}` + const queryString = new URLSearchParams() + for (const [key, value] of Object.entries(params)) { + if (Array.isArray(value)) { + value.forEach((v) => queryString.append(key, String(v))) + } else { + queryString.append(key, String(value)) + } + } + return `?${queryString.toString()}` } const queryify = (params: unknown) => { @@ -67,6 +75,14 @@ const learningResources = { `${API_BASE_URL}/api/v1/learning_resources/${params.id}/`, featured: (params?: Params) => `${API_BASE_URL}/api/v1/featured/${query(params)}`, + setLearningPathRelationships: ( + params?: Params, + ) => + `${API_BASE_URL}/api/v1/learning_resources/${params?.id}/learning_paths/${params?.learning_path_id ? query({ learning_path_id: params?.learning_path_id }) : ""}`, + setUserListRelationships: ( + params?: Params, + ) => + `${API_BASE_URL}/api/v1/learning_resources/${params?.id}/userlists/${params?.userlist_id ? query({ userlist_id: params?.userlist_id }) : ""}`, } const offerors = { diff --git a/frontends/mit-learn/src/page-components/Dialogs/AddToListDialog.test.tsx b/frontends/mit-learn/src/page-components/Dialogs/AddToListDialog.test.tsx index 074d7a1f3a..c6f9e4755f 100644 --- a/frontends/mit-learn/src/page-components/Dialogs/AddToListDialog.test.tsx +++ b/frontends/mit-learn/src/page-components/Dialogs/AddToListDialog.test.tsx @@ -154,7 +154,7 @@ const addToUserList = ( describe.each([ListType.LearningPath, ListType.UserList])( "AddToListDialog", (listType: string) => { - test("List is checked iff resource is in list", async () => { + test("List is checked if resource is in list", async () => { const index = faker.number.int({ min: 0, max: 2 }) if (listType === ListType.LearningPath) { setupLearningPaths({ inLists: [index] }) @@ -169,62 +169,57 @@ describe.each([ListType.LearningPath, ListType.UserList])( expect(checkboxes[2].checked).toBe(index === 2) }) - test("Clicking an unchecked list adds item to that list", async () => { + test("Clicking an unchecked list and clicking save adds item to that list", async () => { let title = "" - let id = -1 - let addToListUrl = "" + let setRelationshipsUrl = "" if (listType === ListType.LearningPath) { const { resource, lists } = setupLearningPaths() const list = faker.helpers.arrayElement(lists) - addToListUrl = urls.learningPaths.resources({ - learning_resource_id: list.id, - }) + setRelationshipsUrl = + urls.learningResources.setLearningPathRelationships({ + id: resource.id, + learning_path_id: [list.id], + }) const newRelationship = addToLearningPath(resource, list) - setMockResponse.post(addToListUrl, newRelationship) + setMockResponse.patch(setRelationshipsUrl, newRelationship) setMockResponse.get( urls.learningResources.details({ id: resource.id }), resource, ) title = list.title - id = list.id } else if (listType === ListType.UserList) { const { resource, lists } = setupUserLists() const list = faker.helpers.arrayElement(lists) - addToListUrl = urls.userLists.resources({ - userlist_id: list.id, + setRelationshipsUrl = urls.learningResources.setUserListRelationships({ + id: resource.id, + userlist_id: [list.id], }) const newRelationship = addToUserList(resource, list) - setMockResponse.post(addToListUrl, newRelationship) + setMockResponse.patch(setRelationshipsUrl, newRelationship) setMockResponse.get( urls.learningResources.details({ id: resource.id }), resource, ) title = list.title - id = list.id } - const listButton = await screen.findByRole("button", { name: title }) - const checkbox = - within(listButton).getByRole("checkbox") + const checkbox = await screen.findByLabelText(title) expect(checkbox.checked).toBe(false) - await user.click(listButton) + await user.click(checkbox) - expect(makeRequest).toHaveBeenCalledWith( - "post", - addToListUrl, - expect.objectContaining({ - parent: id, - }), - ) + const saveButton = await screen.findByRole("button", { name: "Save" }) + await user.click(saveButton) + + expect(makeRequest).toHaveBeenCalledWith("patch", setRelationshipsUrl, {}) }) - test("Clicking a checked list removes item from that list", async () => { + test("Clicking a checked list and clicking save removes item from that list", async () => { const index = faker.number.int({ min: 0, max: 2 }) let title = "" - let removalUrl = "" + let setRelationshipUrl = "" if (listType === ListType.LearningPath) { const { resource, lists, parents } = setupLearningPaths({ inLists: [index], @@ -234,11 +229,12 @@ describe.each([ListType.LearningPath, ListType.UserList])( invariant(relationship) title = list.title - removalUrl = urls.learningPaths.resourceDetails({ - id: relationship.id, - learning_resource_id: relationship.parent, - }) - setMockResponse.delete(removalUrl) + setRelationshipUrl = + urls.learningResources.setLearningPathRelationships({ + id: relationship.child, + }) + setMockResponse.patch(setRelationshipUrl, relationship) + setMockResponse.get( urls.learningResources.details({ id: resource.id }), resource, @@ -252,28 +248,28 @@ describe.each([ListType.LearningPath, ListType.UserList])( invariant(relationship) title = list.title - removalUrl = urls.userLists.resourceDetails({ - id: relationship.id, - userlist_id: relationship.parent, + setRelationshipUrl = urls.learningResources.setUserListRelationships({ + id: relationship.child, }) - setMockResponse.delete(removalUrl) + setMockResponse.patch(setRelationshipUrl, relationship) setMockResponse.get( urls.userLists.details({ id: resource.id }), resource, ) } - const listButton = await screen.findByRole("button", { name: title }) - const checkbox = - within(listButton).getByRole("checkbox") + const checkbox = await screen.findByLabelText(title) expect(checkbox.checked).toBe(true) - await user.click(listButton) + await user.click(checkbox) + + const saveButton = await screen.findByRole("button", { name: "Save" }) + await user.click(saveButton) - expect(makeRequest).toHaveBeenCalledWith("delete", removalUrl, undefined) + expect(makeRequest).toHaveBeenCalledWith("patch", setRelationshipUrl, {}) }) - test("Clicking 'Create a new list' opens the create list dialog", async () => { + test("Clicking 'Create New list' opens the create list dialog", async () => { let createList = null if (listType === ListType.LearningPath) { // Don't actually open the 'Create List' modal, or we'll need to mock API responses. @@ -289,7 +285,7 @@ describe.each([ListType.LearningPath, ListType.UserList])( setupUserLists() } const button = await screen.findByRole("button", { - name: "Create a new list", + name: "Create New List", }) expect(createList).not.toHaveBeenCalled() await user.click(button) diff --git a/frontends/mit-learn/src/page-components/Dialogs/AddToListDialog.tsx b/frontends/mit-learn/src/page-components/Dialogs/AddToListDialog.tsx index f52c8cdffe..a9642557a7 100644 --- a/frontends/mit-learn/src/page-components/Dialogs/AddToListDialog.tsx +++ b/frontends/mit-learn/src/page-components/Dialogs/AddToListDialog.tsx @@ -1,18 +1,14 @@ -import React, { useState } from "react" +import React, { useCallback, useId } from "react" import { Dialog, - Chip, - MuiCheckbox, - List, - ListItem, - ListItemButton, - ListItemText, LoadingSpinner, Typography, styled, + CheckboxChoiceField, + Button, } from "ol-components" -import { RiLockLine, RiLockUnlockLine, RiAddLine } from "@remixicon/react" +import { RiAddLine } from "@remixicon/react" import * as NiceModal from "@ebay/nice-modal-react" @@ -23,178 +19,32 @@ import { } from "api" import { + useLearningResourceSetUserListRelationships, useLearningPathsList, useLearningResourcesDetail, - useLearningpathRelationshipCreate, - useLearningpathRelationshipDestroy, useUserListList, - useUserListRelationshipCreate, - useUserListRelationshipDestroy, + useLearningResourceSetLearningPathRelationships, } from "api/hooks/learningResources" import { manageListDialogs } from "@/page-components/ManageListDialogs/ManageListDialogs" import { ListType } from "api/constants" - -const useLearningPathRequestRecord = () => { - const [pending, setPending] = useState>( - new Map(), - ) - const key = (resource: LearningResource, list: LearningPathResource) => - `${resource.id}-${list.id}` - const get = (resource: LearningResource, list: LearningPathResource) => - pending.get(key(resource, list)) - const set = ( - resource: LearningResource, - list: LearningPathResource, - value: "delete" | "add", - ) => { - setPending((current) => new Map(current).set(key(resource, list), value)) - } - const clear = (resource: LearningResource, list: LearningPathResource) => { - setPending((current) => { - const next = new Map(current) - next.delete(key(resource, list)) - return next - }) - } - return { get, set, clear } -} - -const useUserListRequestRecord = () => { - const [pending, setPending] = useState>( - new Map(), - ) - const key = (resource: LearningResource, list: UserList) => - `${resource.id}-${list.id}` - const get = (resource: LearningResource, list: UserList) => - pending.get(key(resource, list)) - const set = ( - resource: LearningResource, - list: UserList, - value: "delete" | "add", - ) => { - setPending((current) => new Map(current).set(key(resource, list), value)) - } - const clear = (resource: LearningResource, list: UserList) => { - setPending((current) => { - const next = new Map(current) - next.delete(key(resource, list)) - return next - }) - } - return { get, set, clear } -} - -const useToggleItemInLearningPath = (resource?: LearningResource) => { - const requestRecord = useLearningPathRequestRecord() - const addTo = useLearningpathRelationshipCreate() - const deleteFrom = useLearningpathRelationshipDestroy() - const handleAdd = async (list: LearningPathResource) => { - if (!resource) return - requestRecord.set(resource, list, "add") - try { - await addTo.mutateAsync({ child: resource.id, parent: list.id }) - } finally { - requestRecord.clear(resource, list) - } - } - const handleRemove = async (list: LearningPathResource) => { - if (!resource) return - requestRecord.set(resource, list, "delete") - const relationship = resource.learning_path_parents?.find( - ({ parent }) => parent === list.id, - ) - if (!relationship) return // should not happen - try { - await deleteFrom.mutateAsync(relationship) - } finally { - requestRecord.clear(resource, list) - } - } - - const isChecked = (list: LearningPathResource): boolean => - resource?.learning_path_parents?.some(({ parent }) => parent === list.id) ?? - false - - const isAdding = (list: LearningPathResource) => - !!resource && requestRecord.get(resource, list) === "add" - const isRemoving = (list: LearningPathResource) => - !!resource && requestRecord.get(resource, list) === "delete" - - const handleToggle = (list: LearningPathResource) => async () => { - return isChecked(list) ? handleRemove(list) : handleAdd(list) - } - return { handleToggle, isChecked, isAdding, isRemoving } -} - -const useToggleItemInUserList = (resource?: LearningResource) => { - const requestRecord = useUserListRequestRecord() - const addTo = useUserListRelationshipCreate() - const deleteFrom = useUserListRelationshipDestroy() - const handleAdd = async (list: UserList) => { - if (!resource) return - requestRecord.set(resource, list, "add") - try { - await addTo.mutateAsync({ child: resource.id, parent: list.id }) - } finally { - requestRecord.clear(resource, list) - } - } - const handleRemove = async (list: UserList) => { - if (!resource) return - requestRecord.set(resource, list, "delete") - const relationship = resource.user_list_parents?.find( - ({ parent }) => parent === list.id, - ) - if (!relationship) return // should not happen - try { - await deleteFrom.mutateAsync(relationship) - } finally { - requestRecord.clear(resource, list) - } - } - - const isChecked = (list: UserList): boolean => - resource?.user_list_parents?.some(({ parent }) => parent === list.id) ?? - false - - const isAdding = (list: UserList) => - !!resource && requestRecord.get(resource, list) === "add" - const isRemoving = (list: UserList) => - !!resource && requestRecord.get(resource, list) === "delete" - - const handleToggle = (list: UserList) => async () => { - return isChecked(list) ? handleRemove(list) : handleAdd(list) - } - return { handleToggle, isChecked, isAdding, isRemoving } -} +import { useFormik } from "formik" const ResourceTitle = styled.span({ fontStyle: "italic", }) -const Listing = styled(List)` - & .MuiListItem-root:not(.add-to-list-new) { - padding: 0; - } - - .MuiListItemButton-root:not(.add-to-list-new) { - padding-top: 0; - padding-bottom: 0; - } -` +const CheckboxContainer = styled.div({ + padding: "28px 0", +}) -type PrivacyChipProps = { - publicOption: string - selectedOption: string | undefined -} -const PrivacyChip: React.FC = ({ - publicOption, - selectedOption, -}) => { - const isPublic = selectedOption === publicOption - const icon = isPublic ? : - return -} +const ButtonContainer = styled.div({ + display: "flex", + gap: "12px", + alignItems: "flex-start", + button: { + width: "50%", + }, +}) type AddToListDialogInnerProps = { listType: ListType @@ -209,12 +59,76 @@ const AddToListDialogInner: React.FC = ({ isReady, }) => { const modal = NiceModal.useModal() + const handleCreate = useCallback(() => { + if (listType === ListType.LearningPath) { + manageListDialogs.upsertLearningPath() + } else if (listType === ListType.UserList) { + manageListDialogs.upsertUserList() + } + }, [listType]) + const { + isLoading: isSavingUserListRelationships, + mutateAsync: setUserListRelationships, + } = useLearningResourceSetUserListRelationships() + const { + isLoading: isSavingLearningPathRelationships, + mutateAsync: setLearningPathRelationships, + } = useLearningResourceSetLearningPathRelationships() + const isSaving = + isSavingLearningPathRelationships || isSavingUserListRelationships let dialogTitle = "Add to list" if (listType === ListType.LearningPath) { dialogTitle = "Add to Learning List" } else if (listType === ListType.UserList) { dialogTitle = "Add to User List" } + const listChoices = lists.map((list) => ({ + value: list.id.toString(), + label: list.title, + })) + const learningPathValues = lists + .map((list) => + resource?.learning_path_parents?.some(({ parent }) => parent === list.id) + ? list.id.toString() + : null, + ) + .filter((value) => value !== null) + const userListValues = lists + .map((list) => + resource?.user_list_parents?.some(({ parent }) => parent === list.id) + ? list.id.toString() + : null, + ) + .filter((value) => value !== null) + const formId = useId() + const formik = useFormik({ + enableReinitialize: true, + validateOnChange: false, + validateOnBlur: false, + initialValues: { + learning_paths: learningPathValues, + user_lists: userListValues, + }, + onSubmit: async (values) => { + if (resource) { + if (listType === ListType.LearningPath) { + const newParents = values.learning_paths.map((id) => parseInt(id)) + await setLearningPathRelationships({ + id: resource.id, + learning_path_id: newParents, + }) + } else if (listType === ListType.UserList) { + const newParents = values.user_lists.map((id) => parseInt(id)) + await setUserListRelationships({ + id: resource.id, + userlist_id: newParents, + }) + } + modal.remove() + } + }, + }) + return ( = ({ {...NiceModal.muiDialogV5(modal)} > {isReady ? ( - <> - +
+ Adding {resource?.title} - {listType === ListType.LearningPath ? ( - - + {listType === ListType.LearningPath ? ( + - - manageListDialogs.upsertLearningPath()} - > - - - - - - ) : null} - {listType === ListType.UserList ? ( - - - - manageListDialogs.upsertUserList()} - > - - - - - - ) : null} - + ) : null} + + + + + +
) : ( )} @@ -267,84 +186,6 @@ const AddToListDialogInner: React.FC = ({ ) } -type LearningPathToggleListProps = { - resource: LearningResource | undefined - lists: LearningPathResource[] -} -const LearningPathToggleList: React.FC = ({ - resource, - lists, -}) => { - const { handleToggle, isChecked, isAdding, isRemoving } = - useToggleItemInLearningPath(resource) - return lists.map((list) => { - const checked = isChecked(list) - const adding = isAdding(list) - const removing = isRemoving(list) - const disabled = adding || removing - return ( - - } - > - - - - - - ) - }) -} - -type UserListToggleListProps = { - resource: LearningResource | undefined - lists: UserList[] -} -const UserListToggleList: React.FC = ({ - resource, - lists, -}) => { - const { handleToggle, isChecked, isAdding, isRemoving } = - useToggleItemInUserList(resource) - return lists.map((list) => { - const checked = isChecked(list) - const adding = isAdding(list) - const removing = isRemoving(list) - const disabled = adding || removing - return ( - - - - - - - ) - }) -} - type AddToListDialogProps = { resourceId: number } diff --git a/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.tsx b/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.tsx index 146438c660..6c5b5482f2 100644 --- a/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.tsx +++ b/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.tsx @@ -5,13 +5,14 @@ import FormLabel from "@mui/material/FormLabel" import styled from "@emotion/styled" export type CheckboxChoiceFieldProps = { - label: React.ReactNode // We could make this optional, but we should demand one of (label, aria-label, aria-labelledby) + label?: React.ReactNode // We could make this optional, but we should demand one of (label, aria-label, aria-labelledby) value?: string[] name: string choices: Omit[] values?: string[] onChange?: CheckboxProps["onChange"] className?: string + vertical?: boolean } const Container = styled.div(({ theme }) => ({ @@ -23,6 +24,11 @@ const Container = styled.div(({ theme }) => ({ }, })) +const VerticalContainer = styled(Container)({ + gap: "18px", + flexDirection: "column", +}) + const Label = styled(FormLabel)(({ theme }) => ({ marginTop: "0", marginBottom: "16px", @@ -38,17 +44,19 @@ const CheckboxChoiceField: React.FC = ({ values, onChange, className, + vertical = false, }) => { const isChecked = (choice: CheckboxProps) => choice.value ? (values?.includes(choice.value) ?? false) : false + const _Container = vertical ? VerticalContainer : Container return ( - - + {label && } + <_Container> {choices.map((choice) => { return ( = ({ /> ) })} - + ) } diff --git a/learning_resources/urls.py b/learning_resources/urls.py index f50cd0c5c4..6a770de2a5 100644 --- a/learning_resources/urls.py +++ b/learning_resources/urls.py @@ -24,7 +24,11 @@ views.LearningResourceContentFilesViewSet, basename="learning_resource_content_files_api", ) - +learning_resource_set_list_relationships = router.register( + r"learning_resources", + views.LearningResourceListRelationshipViewSet, + basename="learning_resource_relationships_api", +) router.register(r"courses", views.CourseViewSet, basename="courses_api") nested_courses_router = NestedSimpleRouter( diff --git a/learning_resources/views.py b/learning_resources/views.py index 77d5d8b6f3..adabf4954d 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -15,6 +15,7 @@ from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_view from rest_framework import views, viewsets +from rest_framework.decorators import action from rest_framework.filters import OrderingFilter from rest_framework.generics import get_object_or_404 from rest_framework.pagination import LimitOffsetPagination @@ -26,6 +27,7 @@ from channels.models import Channel from learning_resources import permissions from learning_resources.constants import ( + LearningResourceRelationTypes, LearningResourceType, PlatformType, PrivacyLevel, @@ -52,6 +54,7 @@ UserListRelationship, ) from learning_resources.permissions import ( + HasLearningPathPermissions, HasUserListItemPermissions, HasUserListPermissions, is_learning_path_editor, @@ -376,6 +379,136 @@ class ResourceListItemsViewSet(NestedViewSetMixin, viewsets.ReadOnlyModelViewSet ordering = ["position", "-child__last_modified"] +@extend_schema( + parameters=[ + OpenApiParameter( + name="id", + type=OpenApiTypes.INT, + location=OpenApiParameter.PATH, + description="id of the learning resource", + ), + ], +) +class LearningResourceListRelationshipViewSet(viewsets.GenericViewSet): + """ + Viewset for managing relationships between Learning Resources + and User Lists / Learning Paths + """ + + permission_classes = (AnonymousAccessReadonlyPermission,) + filter_backends = [MultipleOptionsFilterBackend] + queryset = LearningResourceRelationship.objects.select_related("parent", "child") + http_method_names = ["patch"] + + def get_serializer_class(self): + if self.action == "userlists": + return UserListRelationshipSerializer + elif self.action == "learning_paths": + return LearningResourceRelationshipSerializer + return super().get_serializer_class() + + @extend_schema( + summary="Set User List Relationships", + description="Set User List Relationships on a given Learning Resource.", + parameters=[ + OpenApiParameter( + name="userlist_id", + type=OpenApiTypes.INT, + many=True, + location=OpenApiParameter.QUERY, + description="id of the parent user list", + ), + ], + responses={200: UserListRelationshipSerializer(many=True)}, + ) + @action(detail=True, methods=["patch"], name="Set User List Relationships") + def userlists(self, request, *args, **kwargs): # noqa: ARG002 + """ + Set User List relationships for a given Learning Resource + """ + learning_resource_id = kwargs.get("pk") + user_list_ids = request.query_params.getlist("userlist_id") + if ( + UserList.objects.filter(pk__in=user_list_ids) + .exclude(author=request.user) + .exists() + ): + msg = "User does not have permission to add to the selected user list(s)" + raise PermissionError(msg) + current_relationships = UserListRelationship.objects.filter( + parent__author=request.user, child_id=learning_resource_id + ) + current_relationships.exclude(parent_id__in=user_list_ids).delete() + for userlist_id in user_list_ids: + last_index = 0 + for index, relationship in enumerate( + UserListRelationship.objects.filter( + parent__author=request.user, parent__id=userlist_id + ).order_by("position") + ): + relationship.position = index + relationship.save() + last_index = index + UserListRelationship.objects.create( + parent_id=userlist_id, + child_id=learning_resource_id, + position=last_index + 1, + ) + SerializerClass = self.get_serializer_class() + serializer = SerializerClass(current_relationships, many=True) + return Response(serializer.data) + + @extend_schema( + summary="Set Learning Path Relationships", + description="Set Learning Path Relationships on a given Learning Resource.", + parameters=[ + OpenApiParameter( + name="learning_path_id", + type=OpenApiTypes.INT, + many=True, + location=OpenApiParameter.QUERY, + description="id of the parent learning path", + ), + ], + responses={200: LearningResourceRelationshipSerializer(many=True)}, + ) + @action( + detail=True, + methods=["patch"], + permission_classes=[HasLearningPathPermissions], + name="Set Learning Path Relationships", + ) + def learning_paths(self, request, *args, **kwargs): # noqa: ARG002 + """ + Set Learning Path relationships for a given Learning Resource + """ + learning_resource_id = kwargs.get("pk") + learning_path_ids = request.query_params.getlist("learning_path_id") + current_relationships = LearningResourceRelationship.objects.filter( + child_id=learning_resource_id + ) + current_relationships.exclude(parent_id__in=learning_path_ids).delete() + for learning_path_id in learning_path_ids: + last_index = 0 + for index, relationship in enumerate( + LearningResourceRelationship.objects.filter( + parent__id=learning_path_id + ).order_by("position") + ): + relationship.position = index + relationship.save() + last_index = index + LearningResourceRelationship.objects.create( + parent_id=learning_path_id, + child_id=learning_resource_id, + relation_type=LearningResourceRelationTypes.LEARNING_PATH_ITEMS, + position=last_index + 1, + ) + SerializerClass = self.get_serializer_class() + serializer = SerializerClass(current_relationships, many=True) + return Response(serializer.data) + + @extend_schema_view( create=extend_schema(summary="Learning Path Resource Relationship Add"), destroy=extend_schema(summary="Learning Path Resource Relationship Remove"), diff --git a/learning_resources/views_learningpath_test.py b/learning_resources/views_learningpath_test.py index b310e30282..9d479771dc 100644 --- a/learning_resources/views_learningpath_test.py +++ b/learning_resources/views_learningpath_test.py @@ -399,3 +399,29 @@ def test_get_resource_learning_paths(user_client, user, is_editor): resp.data.get("learning_path_parents"), key=lambda item: item["id"] ) assert response_data == expected + + +def test_set_learning_path_relationships(client, staff_user): + """Test the learning_paths endpoint for setting multiple userlist relationships""" + course = factories.CourseFactory.create() + learning_paths = factories.LearningPathFactory.create_batch(3, author=staff_user) + previous_learning_path = factories.LearningPathFactory.create(author=staff_user) + factories.LearningPathRelationshipFactory.create( + parent=previous_learning_path.learning_resource, child=course.learning_resource + ) + url = reverse( + "lr:v1:learning_resource_relationships_api-learning-paths", + args=[course.learning_resource.id], + ) + client.force_login(staff_user) + resp = client.patch( + f"{url}?{"".join([f"learning_path_id={learning_path.learning_resource.id}&" for learning_path in learning_paths])}" + ) + assert resp.status_code == 200 + for learning_path in learning_paths: + assert course.learning_resource.learning_path_parents.filter( + parent__id=learning_path.learning_resource.id + ).exists() + assert not course.learning_resource.learning_path_parents.filter( + parent__id=previous_learning_path.learning_resource.id + ).exists() diff --git a/learning_resources/views_userlist_test.py b/learning_resources/views_userlist_test.py index e30a0993ef..d404b19741 100644 --- a/learning_resources/views_userlist_test.py +++ b/learning_resources/views_userlist_test.py @@ -5,7 +5,7 @@ from learning_resources import factories from learning_resources.constants import PrivacyLevel -from learning_resources.models import UserList +from learning_resources.models import UserList, UserListRelationship from main.factories import UserFactory # pylint:disable=redefined-outer-name, use-maxsplit-arg @@ -283,3 +283,70 @@ def test_get_resource_user_lists(client, user, is_author, is_unlisted): assert item.get("child") == course.learning_resource.id else: assert items_json == [] + + +def test_set_userlist_relationships(client, user): + """Test the userlists endpoint for setting multiple userlist relationships""" + course = factories.CourseFactory.create() + userlists = factories.UserListFactory.create_batch(3, author=user) + previous_list = factories.UserListFactory.create(author=user) + factories.UserListRelationshipFactory.create( + parent=previous_list, child=course.learning_resource + ) + url = reverse( + "lr:v1:learning_resource_relationships_api-userlists", + args=[course.learning_resource.id], + ) + client.force_login(user) + resp = client.patch( + f"{url}?{"".join([f"userlist_id={userlist.id}&" for userlist in userlists])}" + ) + assert resp.status_code == 200 + for userlist in userlists: + assert userlist.resources.filter(id=course.learning_resource.id).exists() + assert not previous_list.resources.filter(id=course.learning_resource.id).exists() + + +def test_set_userlist_relationships_unauthorized(client, user): + """Test the userlists endpoint for unauthorized users""" + course = factories.CourseFactory.create() + userlists = factories.UserListFactory.create_batch(3) + url = reverse( + "lr:v1:learning_resource_relationships_api-userlists", + args=[course.learning_resource.id], + ) + client.force_login(user) + with pytest.raises(PermissionError): + client.patch( + f"{url}?{"".join([f"userlist_id={userlist.id}&" for userlist in userlists])}" + ) + for userlist in userlists: + assert not userlist.resources.filter(id=course.learning_resource.id).exists() + + +def test_set_userlist_relationships_empty_list(client, user): + """Test that sending an empty list in the request does not unassign the wrong userlists""" + + def assign_userlists(course, userlists): + for userlist in userlists: + factories.UserListRelationshipFactory.create( + parent=userlist, child=course.learning_resource + ) + + course = factories.CourseFactory.create() + unowned_userlists = factories.UserListFactory.create_batch(3) + owned_userlists = factories.UserListFactory.create_batch(3, author=user) + assign_userlists(course, unowned_userlists) + assign_userlists(course, owned_userlists) + assert ( + UserListRelationship.objects.filter(child=course.learning_resource).count() == 6 + ) + url = reverse( + "lr:v1:learning_resource_relationships_api-userlists", + args=[course.learning_resource.id], + ) + client.force_login(user) + client.patch(url) + assert ( + UserListRelationship.objects.filter(child=course.learning_resource).count() == 3 + ) diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 750b7ea45c..6857715e85 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -2131,6 +2131,88 @@ paths: schema: $ref: '#/components/schemas/LearningResourceRelationship' description: '' + /api/v1/learning_resources/{id}/learning_paths/: + patch: + operationId: learning_resources_learning_paths_partial_update + description: Set Learning Path Relationships on a given Learning Resource. + summary: Set Learning Path Relationships + parameters: + - in: path + name: id + schema: + type: integer + description: id of the learning resource + required: true + - in: query + name: learning_path_id + schema: + type: array + items: + type: integer + description: id of the parent learning path + tags: + - learning_resources + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/PatchedLearningResourceRelationshipRequest' + application/x-www-form-urlencoded: + schema: + $ref: '#/components/schemas/PatchedLearningResourceRelationshipRequest' + multipart/form-data: + schema: + $ref: '#/components/schemas/PatchedLearningResourceRelationshipRequest' + responses: + '200': + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/LearningResourceRelationship' + description: '' + /api/v1/learning_resources/{id}/userlists/: + patch: + operationId: learning_resources_userlists_partial_update + description: Set User List Relationships on a given Learning Resource. + summary: Set User List Relationships + parameters: + - in: path + name: id + schema: + type: integer + description: id of the learning resource + required: true + - in: query + name: userlist_id + schema: + type: array + items: + type: integer + description: id of the parent user list + tags: + - learning_resources + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/PatchedUserListRelationshipRequest' + application/x-www-form-urlencoded: + schema: + $ref: '#/components/schemas/PatchedUserListRelationshipRequest' + multipart/form-data: + schema: + $ref: '#/components/schemas/PatchedUserListRelationshipRequest' + responses: + '200': + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/UserListRelationship' + description: '' /api/v1/learning_resources_search/: get: operationId: learning_resources_search_retrieve @@ -9547,6 +9629,20 @@ components: completeness: type: number format: double + PatchedLearningResourceRelationshipRequest: + type: object + description: CRUD serializer for LearningResourceRelationship + properties: + position: + type: integer + maximum: 2147483647 + minimum: 0 + relation_type: + $ref: '#/components/schemas/RelationTypeEnum' + parent: + type: integer + child: + type: integer PatchedUserListRelationshipRequest: type: object description: Serializer for UserListRelationship model From 34039f6314532a4f16607cc394dfc7ea75a55ad3 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 29 Aug 2024 13:22:54 -0400 Subject: [PATCH 3/4] Remove Pathways (Coming Soon) (#1482) * remove pathways link * require href on nav links --- .../src/page-components/Header/Header.tsx | 7 ------- .../components/NavDrawer/NavDrawer.stories.tsx | 7 ------- .../components/NavDrawer/NavDrawer.test.tsx | 18 ++---------------- .../src/components/NavDrawer/NavDrawer.tsx | 8 +++----- 4 files changed, 5 insertions(+), 35 deletions(-) diff --git a/frontends/mit-learn/src/page-components/Header/Header.tsx b/frontends/mit-learn/src/page-components/Header/Header.tsx index 216d7a0a6f..fa851d266b 100644 --- a/frontends/mit-learn/src/page-components/Header/Header.tsx +++ b/frontends/mit-learn/src/page-components/Header/Header.tsx @@ -13,7 +13,6 @@ import { RiSearch2Line, RiPencilRulerLine, RiStackLine, - RiSignpostLine, RiBookMarkedLine, RiPresentationLine, RiNodeTree, @@ -189,12 +188,6 @@ const navData: NavData = { "A series of courses for in-depth learning across a range of topics", href: SEARCH_PROGRAM, }, - { - title: "Pathways", - icon: , - description: - "Achieve your learning goals with a curated collection of courses", - }, { title: "Learning Materials", icon: , diff --git a/frontends/ol-components/src/components/NavDrawer/NavDrawer.stories.tsx b/frontends/ol-components/src/components/NavDrawer/NavDrawer.stories.tsx index 98e258caac..1421e4d897 100644 --- a/frontends/ol-components/src/components/NavDrawer/NavDrawer.stories.tsx +++ b/frontends/ol-components/src/components/NavDrawer/NavDrawer.stories.tsx @@ -24,13 +24,6 @@ const NavDrawerDemo = () => { title: "Link but no description", href: "https://ocw.mit.edu", }, - { - title: "Description, but no link", - description: "This item has a description, but no link", - }, - { - title: "Title only", - }, ], }, ], diff --git a/frontends/ol-components/src/components/NavDrawer/NavDrawer.test.tsx b/frontends/ol-components/src/components/NavDrawer/NavDrawer.test.tsx index c9a65b3993..3a1e998789 100644 --- a/frontends/ol-components/src/components/NavDrawer/NavDrawer.test.tsx +++ b/frontends/ol-components/src/components/NavDrawer/NavDrawer.test.tsx @@ -25,13 +25,6 @@ describe("NavDrawer", () => { title: "Link but no description", href: "https://ocw.mit.edu", }, - { - title: "Description, but no link", - description: "This item has a description, but no link", - }, - { - title: "Title only", - }, ], }, ], @@ -45,14 +38,7 @@ describe("NavDrawer", () => { const descriptions = screen.getAllByTestId("nav-link-description") expect(links).toHaveLength(3) expect(icons).toHaveLength(1) - expect(titles).toHaveLength(5) - expect(descriptions).toHaveLength(3) - let linksComingSoon = 0 - Array.prototype.forEach.call(titles, (title) => { - if (title.textContent?.includes("(Coming Soon)")) { - linksComingSoon++ - } - }) - expect(linksComingSoon === 2).toBeTruthy() + expect(titles).toHaveLength(3) + expect(descriptions).toHaveLength(2) }) }) diff --git a/frontends/ol-components/src/components/NavDrawer/NavDrawer.tsx b/frontends/ol-components/src/components/NavDrawer/NavDrawer.tsx index aee4fbdfac..95d711dd49 100644 --- a/frontends/ol-components/src/components/NavDrawer/NavDrawer.tsx +++ b/frontends/ol-components/src/components/NavDrawer/NavDrawer.tsx @@ -108,7 +108,7 @@ export interface NavItem { title: string icon?: string | ReactElement description?: string - href?: string + href: string } const NavItem: React.FC = (props) => { @@ -127,7 +127,7 @@ const NavItem: React.FC = (props) => { - {title} {href ? "" : "(Coming Soon)"} + {title} {description ? ( @@ -137,12 +137,10 @@ const NavItem: React.FC = (props) => { ) - return href ? ( + return ( {navItem} - ) : ( - navItem ) } From 17bb83c995d6875c10c5f0b67d7298cc74d5195b Mon Sep 17 00:00:00 2001 From: Doof Date: Thu, 29 Aug 2024 17:32:31 +0000 Subject: [PATCH 4/4] Release 0.17.12 --- RELEASE.rst | 7 +++++++ main/settings.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/RELEASE.rst b/RELEASE.rst index 648084089d..dc8226eda8 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,13 @@ Release Notes ============= +Version 0.17.12 +--------------- + +- Remove Pathways (Coming Soon) (#1482) +- add to list dialog updates (#1463) +- Fix syntax that a pre-commit check was failing (#1480) + Version 0.17.11 (Released August 29, 2024) --------------- diff --git a/main/settings.py b/main/settings.py index ed68435d21..cf48dc1a9a 100644 --- a/main/settings.py +++ b/main/settings.py @@ -33,7 +33,7 @@ from main.settings_pluggy import * # noqa: F403 from openapi.settings_spectacular import open_spectacular_settings -VERSION = "0.17.11" +VERSION = "0.17.12" log = logging.getLogger()