From 212c50768f6fb90781293dd84a393017b96a4561 Mon Sep 17 00:00:00 2001 From: swve Date: Sun, 4 Feb 2024 21:02:07 +0100 Subject: [PATCH] fix: collections and courses remaining bugs --- apps/api/src/security/rbac/rbac.py | 22 ++- apps/api/src/security/rbac/utils.py | 1 - .../services/courses/activities/activities.py | 1 - apps/api/src/services/courses/collections.py | 61 +++++++-- .../(withmenu)/collections/new/page.tsx | 129 ++++++++++-------- .../(withmenu)/course/[courseuuid]/course.tsx | 32 ++--- .../app/orgs/[orgslug]/(withmenu)/error.tsx | 2 +- .../components/StyledElements/Error/Error.tsx | 20 ++- 8 files changed, 155 insertions(+), 113 deletions(-) diff --git a/apps/api/src/security/rbac/rbac.py b/apps/api/src/security/rbac/rbac.py index 150a8f7e..01f5a343 100644 --- a/apps/api/src/security/rbac/rbac.py +++ b/apps/api/src/security/rbac/rbac.py @@ -19,9 +19,8 @@ async def authorization_verify_if_element_is_public( ): element_nature = await check_element_type(element_uuid) # Verifies if the element is public - if element_nature == ("courses" or "collections") and action == "read": + if element_nature == ("courses") and action == "read": if element_nature == "courses": - print("looking for course") statement = select(Course).where( Course.public == True, Course.course_uuid == element_uuid ) @@ -29,20 +28,29 @@ async def authorization_verify_if_element_is_public( if course: return True else: - return False + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights : You don't have the right to perform this action", + ) + + if element_nature == "collections" and action == "read": - if element_nature == "collections": statement = select(Collection).where( Collection.public == True, Collection.collection_uuid == element_uuid ) collection = db_session.exec(statement).first() - if collection: return True else: - return False + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights : You don't have the right to perform this action", + ) else: - return False + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights : You don't have the right to perform this action", + ) # Tested and working diff --git a/apps/api/src/security/rbac/utils.py b/apps/api/src/security/rbac/utils.py index 51835ee9..0b2d707d 100644 --- a/apps/api/src/security/rbac/utils.py +++ b/apps/api/src/security/rbac/utils.py @@ -5,7 +5,6 @@ async def check_element_type(element_id): """ Check if the element is a course, a user, a house or a collection, by checking its prefix """ - print("element_id", element_id) if element_id.startswith("course_"): return "courses" elif element_id.startswith("user_"): diff --git a/apps/api/src/services/courses/activities/activities.py b/apps/api/src/services/courses/activities/activities.py index 58e7de7d..236ac7d6 100644 --- a/apps/api/src/services/courses/activities/activities.py +++ b/apps/api/src/services/courses/activities/activities.py @@ -223,7 +223,6 @@ async def rbac_check( res = await authorization_verify_if_element_is_public( request, course_uuid, action, db_session ) - print('res',res) return res else: res = await authorization_verify_based_on_roles_and_authorship( diff --git a/apps/api/src/services/courses/collections.py b/apps/api/src/services/courses/collections.py index e04e5c3d..8ee257e0 100644 --- a/apps/api/src/services/courses/collections.py +++ b/apps/api/src/services/courses/collections.py @@ -26,7 +26,10 @@ from fastapi import HTTPException, status, Request async def get_collection( - request: Request, collection_uuid: str, current_user: PublicUser, db_session: Session + request: Request, + collection_uuid: str, + current_user: PublicUser, + db_session: Session, ) -> CollectionRead: statement = select(Collection).where(Collection.collection_uuid == collection_uuid) collection = db_session.exec(statement).first() @@ -42,11 +45,23 @@ async def get_collection( ) # get courses in collection - statement = ( + statement_all = ( select(Course) .join(CollectionCourse, Course.id == CollectionCourse.course_id) .distinct(Course.id) ) + + statement_public = ( + select(Course) + .join(CollectionCourse, Course.id == CollectionCourse.course_id) + .where(CollectionCourse.org_id == collection.org_id, Course.public == True) + ) + + if current_user.id == 0: + statement = statement_public + else: + statement = statement_all + courses = db_session.exec(statement).all() collection = CollectionRead(**collection.dict(), courses=courses) @@ -180,7 +195,10 @@ async def update_collection( async def delete_collection( - request: Request, collection_uuid: str, current_user: PublicUser, db_session: Session + request: Request, + collection_uuid: str, + current_user: PublicUser, + db_session: Session, ): statement = select(Collection).where(Collection.collection_uuid == collection_uuid) collection = db_session.exec(statement).first() @@ -216,23 +234,40 @@ async def get_collections( page: int = 1, limit: int = 10, ) -> List[CollectionRead]: - # RBAC check - await rbac_check(request, "collection_x", current_user, "read", db_session) - statement = ( + statement_public = select(Collection).where( + Collection.org_id == org_id, Collection.public == True + ) + statement_all = ( select(Collection).where(Collection.org_id == org_id).distinct(Collection.id) ) + + if current_user.id == 0: + statement = statement_public + else: + statement = statement_all + collections = db_session.exec(statement).all() - - collections_with_courses = [] + for collection in collections: - statement = ( + statement_all = ( select(Course) .join(CollectionCourse, Course.id == CollectionCourse.course_id) .distinct(Course.id) ) + statement_public = ( + select(Course) + .join(CollectionCourse, Course.id == CollectionCourse.course_id) + .where(CollectionCourse.org_id == org_id, Course.public == True) + ) + if current_user.id == 0: + statement = statement_public + else: + # RBAC check + statement = statement_all + courses = db_session.exec(statement).all() collection = CollectionRead(**collection.dict(), courses=courses) @@ -256,8 +291,11 @@ async def rbac_check( res = await authorization_verify_if_element_is_public( request, collection_uuid, action, db_session ) - print('res',res) - return res + if res == False: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights : You are not allowed to read this collection", + ) else: res = await authorization_verify_based_on_roles_and_authorship( request, current_user.id, action, collection_uuid, db_session @@ -276,4 +314,3 @@ async def rbac_check( ## 🔒 RBAC Utils ## - diff --git a/apps/web/app/orgs/[orgslug]/(withmenu)/collections/new/page.tsx b/apps/web/app/orgs/[orgslug]/(withmenu)/collections/new/page.tsx index 85922e14..e4bf8239 100644 --- a/apps/web/app/orgs/[orgslug]/(withmenu)/collections/new/page.tsx +++ b/apps/web/app/orgs/[orgslug]/(withmenu)/collections/new/page.tsx @@ -1,29 +1,27 @@ "use client"; import { useRouter } from "next/navigation"; -import React from "react"; +import React, { useState } from "react"; import { createCollection } from "@services/courses/collections"; import useSWR from "swr"; import { getAPIUrl, getUriWithOrg } from "@services/config/config"; import { revalidateTags, swrFetcher } from "@services/utils/ts/requests"; import { getOrganizationContextInfo } from "@services/organizations/orgs"; +import { useOrg } from "@components/Contexts/OrgContext"; function NewCollection(params: any) { + const org = useOrg() as any; const orgslug = params.params.orgslug; const [name, setName] = React.useState(""); - const [org, setOrg] = React.useState({}) as any; const [description, setDescription] = React.useState(""); const [selectedCourses, setSelectedCourses] = React.useState([]) as any; const router = useRouter(); - const { data: courses, error: error } = useSWR(`${getAPIUrl()}courses/org_slug/${orgslug}/page/1/limit/10`, swrFetcher); + const [isPublic, setIsPublic] = useState('true'); + + const handleVisibilityChange = (e: React.ChangeEvent) => { + setIsPublic(e.target.value); + }; - React.useEffect(() => { - async function getOrg() { - const org = await getOrganizationContextInfo(orgslug, { revalidate: 1800 }); - setOrg(org); - } - getOrg(); - }, []); const handleNameChange = (event: React.ChangeEvent) => { setName(event.target.value); @@ -35,83 +33,94 @@ function NewCollection(params: any) { const handleSubmit = async (e: any) => { e.preventDefault(); - + const collection = { name: name, description: description, courses: selectedCourses, - public: true, + public: isPublic, org_id: org.id, }; await createCollection(collection); - await revalidateTags(["collections"], orgslug); + await revalidateTags(["collections"], org.slug); + // reload the page router.refresh(); - router.prefetch(getUriWithOrg(orgslug, "/collections")); - router.push(getUriWithOrg(orgslug, "/collections")); + + // wait for 2s before reloading the page + setTimeout(() => { + router.push(getUriWithOrg(orgslug, "/collections")); + } + , 1000); }; return ( <>
-
Add new
+
Add new
- + -{!courses ? ( + + + + {!courses ? (

Loading...

) : ( -
+
+

Courses

{courses.map((course: any) => ( -
+
- { + if (e.target.checked) { + setSelectedCourses([...selectedCourses, course.id]); + } + else { + setSelectedCourses(selectedCourses.filter((course_uuid: any) => course_uuid !== course.course_uuid)); + } + }} + className="text-blue-500 rounded focus:ring-2 focus:ring-blue-500" + /> - type="checkbox" - id={course.id} - name={course.name} - value={course.id} -// id is an integer, not a string - - onChange={(e) => { - if (e.target.checked) { - setSelectedCourses([...selectedCourses, course.id]); - } - else { - setSelectedCourses(selectedCourses.filter((course_uuid: any) => course_uuid !== course.course_uuid)); - } - } - } - className="mr-2" -/> - - +
))} -
)} - + - +
diff --git a/apps/web/app/orgs/[orgslug]/(withmenu)/course/[courseuuid]/course.tsx b/apps/web/app/orgs/[orgslug]/(withmenu)/course/[courseuuid]/course.tsx index dca31493..17f9df7b 100644 --- a/apps/web/app/orgs/[orgslug]/(withmenu)/course/[courseuuid]/course.tsx +++ b/apps/web/app/orgs/[orgslug]/(withmenu)/course/[courseuuid]/course.tsx @@ -59,11 +59,11 @@ const CourseClient = (props: any) => { useEffect(() => { getLearningTags(); } - , [org]); + , [org, course]); return ( <> - {!course ? ( + {!course && !org ? ( ) : ( @@ -74,8 +74,8 @@ const CourseClient = (props: any) => {
- {props.course.thumbnail_image ? -
+ {props.course?.thumbnail_image && org ? +
:
@@ -193,19 +193,19 @@ const CourseClient = (props: any) => {
{user &&
- +
Author
{course.authors[0].first_name && course.authors[0].last_name && ( -
-

{course.authors[0].first_name + ' ' + course.authors[0].last_name}

@{course.authors[0].username } -
)} +
+

{course.authors[0].first_name + ' ' + course.authors[0].last_name}

@{course.authors[0].username} +
)} {!course.authors[0].first_name && !course.authors[0].last_name && ( -
-

@{course.authors[0].username}

-
)} -
+
+

@{course.authors[0].username}

+
)} +
} @@ -226,12 +226,4 @@ const CourseClient = (props: any) => { }; -const StyledBox = (props: any) => ( -
- {props.children} -
- -); - - export default CourseClient; diff --git a/apps/web/app/orgs/[orgslug]/(withmenu)/error.tsx b/apps/web/app/orgs/[orgslug]/(withmenu)/error.tsx index 8bac23db..52120114 100644 --- a/apps/web/app/orgs/[orgslug]/(withmenu)/error.tsx +++ b/apps/web/app/orgs/[orgslug]/(withmenu)/error.tsx @@ -17,7 +17,7 @@ export default function Error({ return (
- +
); } \ No newline at end of file diff --git a/apps/web/components/StyledElements/Error/Error.tsx b/apps/web/components/StyledElements/Error/Error.tsx index 63dc470f..b2b8e35f 100644 --- a/apps/web/components/StyledElements/Error/Error.tsx +++ b/apps/web/components/StyledElements/Error/Error.tsx @@ -1,22 +1,20 @@ +import { XCircle } from 'lucide-react' import React from 'react' - function ErrorUI() { return (
-
-
- - - -
-
-

Error

-

Something went wrong

+
+
+ +
+
+

Error

+

Something went wrong

+
-
) }