Skip to content

feat(aih): cohort access ability for workshops #470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 12, 2025
5 changes: 5 additions & 0 deletions .changeset/wide-years-cut.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@coursebuilder/utils-auth': patch
---

add canCreate to abilityForResource type
70 changes: 48 additions & 22 deletions apps/ai-hero/src/ability/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,26 +177,12 @@ export function getAbilityRules(options: GetAbilityOptions = {}) {
}
})
}

// Entitlement-based permissions
options.user.entitlements?.forEach((entitlement) => {
if (entitlement.type === 'cohort_content_access') {
can('read', 'Content', {
id: { $in: entitlement.metadata.contentIds },
status: 'published',
})
}

if (entitlement.type === 'cohort_discord_role') {
can('invite', 'Discord')
}
})
}

can('read', 'Content', {
createdAt: { $lte: new Date() },
status: { $in: ['review', 'published'] },
})
// can('read', 'Content', {
// createdAt: { $lte: new Date() },
// status: { $in: ['review', 'published'] },
// })
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented these out since I think they were a problem, we might consider changing from "content" to "CohortContent" or something more specific 🤔


return rules
}
Expand All @@ -210,13 +196,24 @@ type ViewerAbilityInput = {
isSolution?: boolean
country?: string
purchases?: Purchase[]
entitlementTypes?: {
id: string
name: string
}[]
}

export function defineRulesForPurchases(
viewerAbilityInput: ViewerAbilityInput,
) {
const { can, rules } = new AbilityBuilder<AppAbility>(createMongoAbility)
const { user, country, purchases = [], module } = viewerAbilityInput
const {
user,
country,
purchases = [],
module,
lesson,
entitlementTypes,
} = viewerAbilityInput

if (user) {
can('update', 'User', {
Expand Down Expand Up @@ -266,9 +263,10 @@ export function defineRulesForPurchases(
return { valid: false, reason: 'unknown' }
})

if (userHasPurchaseWithAccess.some((purchase) => purchase.valid)) {
can('read', 'Content')
}
// LEGACY: non-entitlement based access
// if (userHasPurchaseWithAccess.some((purchase) => purchase.valid)) {
// can('read', 'Content')
// }

if (
userHasPurchaseWithAccess.some(
Expand Down Expand Up @@ -313,6 +311,34 @@ export function defineRulesForPurchases(
can('read', 'Content')
}

const cohortEntitlementType = entitlementTypes?.find(
(entitlement) => entitlement.name === 'cohort_content_access',
)

// check workshop in cohort
if (user?.entitlements && module?.id) {
user.entitlements.forEach((entitlement) => {
if (entitlement.type === cohortEntitlementType?.id) {
can('read', 'Content', {
id: { $in: entitlement.metadata.contentIds },
})
}
})
}

// lesson check
// TODO: validate
const lessonModule = module?.resources?.find(
(resource) => resource.resourceId === lesson?.id,
)
if (user?.entitlements && lessonModule) {
user.entitlements.forEach((entitlement) => {
if (entitlement.type === cohortEntitlementType?.id) {
can('read', 'Content', { id: { $in: entitlement.metadata.contentIds } })
}
})
}
Comment on lines +330 to +340
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Address TODO comment and consider potential redundancy

The lesson-level check correctly uses $in operator as suggested in previous reviews. However, there are two issues to address:

  1. The "TODO: validate" comment should be addressed with specific validation requirements
  2. Consider potential redundancy between module-level and lesson-level checks - a user might get duplicate permissions if they have access to both a module and its lessons

Consider adding validation for entitlement.metadata.contentIds to ensure it's always an array:

if (entitlement.type === cohortEntitlementType?.id) {
+   // Ensure contentIds is an array before using $in operator
+   const contentIds = Array.isArray(entitlement.metadata.contentIds) 
+     ? entitlement.metadata.contentIds 
+     : [entitlement.metadata.contentIds];
-   can('read', 'Content', { id: { $in: entitlement.metadata.contentIds } })
+   can('read', 'Content', { id: { $in: contentIds } })
}

Also consider adding a comment explaining the purpose of the separate module and lesson level checks.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// TODO: validate
const lessonModule = module?.resources?.find(
(resource) => resource.resourceId === lesson?.id,
)
if (user?.entitlements && lessonModule) {
user.entitlements.forEach((entitlement) => {
if (entitlement.type === cohortEntitlementType?.id) {
can('read', 'Content', { id: { $in: entitlement.metadata.contentIds } })
}
})
}
// TODO: validate
const lessonModule = module?.resources?.find(
(resource) => resource.resourceId === lesson?.id,
)
if (user?.entitlements && lessonModule) {
user.entitlements.forEach((entitlement) => {
if (entitlement.type === cohortEntitlementType?.id) {
// Ensure contentIds is an array before using $in operator
const contentIds = Array.isArray(entitlement.metadata.contentIds)
? entitlement.metadata.contentIds
: [entitlement.metadata.contentIds];
can('read', 'Content', { id: { $in: contentIds } })
}
})
}


return rules
}

Expand Down
4 changes: 2 additions & 2 deletions apps/ai-hero/src/app/(commerce)/products/page.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react'
import Link from 'next/link'
import { getActiveSelfPacedProducts } from '@/lib/products-query'
import { getProducts } from '@/lib/products-query'
import { getServerAuthSession } from '@/server/auth'

import {
Expand All @@ -13,7 +13,7 @@ import {

export default async function EventIndexPage() {
const { ability } = await getServerAuthSession()
const products = await getActiveSelfPacedProducts()
const products = await getProducts()

return (
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react'
import { WorkshopResourceList } from '@/app/(content)/workshops/_components/workshop-resource-list'
import LayoutClient from '@/components/layout-client'
import { ActiveHeadingProvider } from '@/hooks/use-active-heading'
import { getAbilityForResource } from '@/utils/get-current-ability-rules'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Unused import detected

The getAbilityForResource function is imported but not used in this file. Consider removing this import if it's not needed.

-import { getAbilityForResource } from '@/utils/get-current-ability-rules'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { getAbilityForResource } from '@/utils/get-current-ability-rules'
// (import removed as it was unused)

import { MenuIcon } from 'lucide-react'

import {
Expand All @@ -25,35 +26,24 @@ const LessonLayout = async (props: {
<ActiveHeadingProvider>
<LayoutClient>
<div className="flex min-w-0">
<React.Suspense
fallback={
<div className="flex w-full max-w-sm flex-shrink-0 flex-col gap-2 border-l p-5">
<Skeleton className="mb-8 h-8 w-full bg-gray-800" />
{new Array(10).fill(null).map((_, i) => (
<Skeleton key={i} className="h-8 w-full bg-gray-800" />
))}
</div>
}
>
<WorkshopResourceList
currentLessonSlug={params.lesson}
className="hidden lg:block"
/>
<Sheet>
<SheetTrigger className="bg-card/80 fixed bottom-5 right-5 z-50 flex items-center gap-2 rounded border p-3 backdrop-blur-md lg:hidden">
<MenuIcon className="size-4" /> Lessons
</SheetTrigger>
<SheetContent className="px-0">
<SheetHeader>
<SheetTitle className="sr-only">Workshop Contents</SheetTitle>
</SheetHeader>
<WorkshopResourceList
currentLessonSlug={params.lesson}
className="border-r-0 border-t-0"
/>
</SheetContent>
</Sheet>
</React.Suspense>
<WorkshopResourceList
currentLessonSlug={params.lesson}
className="hidden lg:block"
/>
<Sheet>
<SheetTrigger className="bg-card/80 fixed bottom-5 right-5 z-50 flex items-center gap-2 rounded border p-3 backdrop-blur-md lg:hidden">
<MenuIcon className="size-4" /> Lessons
</SheetTrigger>
<SheetContent className="px-0">
<SheetHeader>
<SheetTitle className="sr-only">Workshop Contents</SheetTitle>
</SheetHeader>
<WorkshopResourceList
currentLessonSlug={params.lesson}
className="border-r-0 border-t-0"
/>
</SheetContent>
</Sheet>
<div className="min-w-0 flex-1">{children}</div>
</div>
</LayoutClient>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ export function VideoOverlayWorkshopPricing({
>
<Pricing.Product className="w-full">
<Pricing.ProductImage />
<Pricing.Details className="px-0">
<Pricing.Details className="px-0 pt-0">
<Pricing.Name className="mb-5" />
<Pricing.LiveQuantity />
<Pricing.Price />
<Pricing.TeamToggle />
<Pricing.TeamQuantityInput />
<Pricing.BuyButton />
<Pricing.BuyButton>Enroll Now</Pricing.BuyButton>
<Pricing.GuaranteeBadge />
<Pricing.LiveRefundPolicy />
<Pricing.SaleCountdown className="py-4" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { CldImage } from '@/components/cld-image'
import { findSectionIdForLessonSlug, NavigationResource } from '@/lib/workshops'
import { api } from '@/trpc/react'
import { cn } from '@/utils/cn'
import { subject } from '@casl/ability'
import { Check, Lock, PanelLeftClose, PanelLeftOpen, Pen } from 'lucide-react'
import { useMeasure } from 'react-use'

Expand All @@ -30,7 +31,7 @@ import {
import { AutoPlayToggle } from '../../_components/autoplay-toggle'

type Props = {
currentLessonSlug?: string | null
currentLessonSlug?: string
currentSectionSlug?: string | null
className?: string
wrapperClassName?: string
Expand All @@ -40,7 +41,6 @@ type Props = {
}

export function WorkshopResourceList(props: Props) {
const params = useParams()
const wrapperClassName =
'wrapperClassName' in props ? props.wrapperClassName : ''
const className = 'className' in props ? props.className : ''
Expand All @@ -53,17 +53,23 @@ export function WorkshopResourceList(props: Props) {
const { moduleProgress } = useModuleProgress()

const { data: abilityRules, status: abilityStatus } =
api.ability.getCurrentAbilityRules.useQuery({
moduleId: workshopNavigation?.id,
})
api.ability.getCurrentAbilityRules.useQuery(
{
moduleId: workshopNavigation?.id,
lessonId: props.currentLessonSlug,
},
{
enabled: !!workshopNavigation?.id,
},
)

const ability = createAppAbility(abilityRules || [])

const sectionId = findSectionIdForLessonSlug(
workshopNavigation,
props.currentLessonSlug,
)

const ability = createAppAbility(abilityRules || [])

const scrollAreaRef = React.useRef<HTMLDivElement>(null)

React.useEffect(() => {
Expand Down Expand Up @@ -155,7 +161,9 @@ export function WorkshopResourceList(props: Props) {
{resources.map((resource: NavigationResource, i: number) => {
const isActiveGroup =
(resource.type === 'section' || resource.type === 'lesson') &&
resource.resources.some((item) => params.lesson === item.slug)
resource.resources.some(
(item) => props.currentLessonSlug === item.slug,
)

return resource.type === 'section' ? (
// sections
Expand All @@ -179,6 +187,7 @@ export function WorkshopResourceList(props: Props) {
return (
<LessonResource
lesson={item}
moduleId={workshopNavigation.id}
index={index}
moduleProgress={moduleProgress}
ability={ability}
Expand All @@ -199,6 +208,7 @@ export function WorkshopResourceList(props: Props) {
lesson={resource}
index={i}
moduleProgress={moduleProgress}
moduleId={workshopNavigation.id}
ability={ability}
abilityStatus={abilityStatus}
key={resource.id}
Expand Down Expand Up @@ -245,13 +255,15 @@ const LessonResource = ({
ability,
abilityStatus,
className,
moduleId,
}: {
lesson: NavigationResource
moduleProgress?: ModuleProgress | null
index: number
ability: AppAbility
abilityStatus: 'error' | 'success' | 'pending'
className?: string
moduleId: string
}) => {
const params = useParams()
const pathname = usePathname()
Expand All @@ -273,6 +285,8 @@ const LessonResource = ({
(progress) => progress.resourceId === lesson.id && progress.completedAt,
)
// if solution of a resource is active, or resource of a section is active
const canView = ability.can('read', subject('Content', { id: moduleId }))
const canCreate = ability.can('create', 'Content')

return (
<li
Expand Down Expand Up @@ -322,7 +336,7 @@ const LessonResource = ({
<span className="w-full text-base">{lesson.title}</span>
{abilityStatus === 'success' && (
<>
{ability.can('read', 'Content') || index === 0 ? null : (
{canView || index === 0 ? null : (
<Lock
className="absolute right-5 w-3 text-gray-500"
aria-label="locked"
Expand All @@ -332,7 +346,7 @@ const LessonResource = ({
)}
</Link>

{ability.can('create', 'Content') ? (
{canCreate ? (
<Button
asChild
variant="outline"
Expand Down
3 changes: 0 additions & 3 deletions apps/ai-hero/src/components/navigation/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ const Navigation = () => {
setIsMobileMenuOpen(false)
}, [pathname])

const { data: abilityRules, status: abilityStatus } =
api.ability.getCurrentAbilityRules.useQuery()

const { data: sessionData, status: sessionStatus } = useSession()
const { data: subscriber, status } =
api.ability.getCurrentSubscriberFromCookie.useQuery()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import {
import { inngest } from '@/inngest/inngest.server'
import { getCohort } from '@/lib/cohorts-query'
import { and, eq } from 'drizzle-orm'
import { v4 as uuidv4 } from 'uuid'

import { guid } from '@coursebuilder/adapter-drizzle/mysql'
import { NEW_PURCHASE_CREATED_EVENT } from '@coursebuilder/core/inngest/commerce/event-new-purchase-created'

export const postCohortPurchaseWorkflow = inngest.createFunction(
Expand Down Expand Up @@ -103,7 +105,7 @@ export const postCohortPurchaseWorkflow = inngest.createFunction(
if (cohortContentAccessEntitlementType && cohortResource?.resources) {
await step.run(`add user to cohort via entitlement`, async () => {
for (const resource of cohortResource.resources || []) {
const entitlementId = `${cohortContentAccessEntitlementType.id}-${resource.resource.id}`
const entitlementId = `${resource.resource.id}-${guid()}`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making sure these are all unique

await db.insert(entitlements).values({
id: entitlementId,
entitlementType: cohortContentAccessEntitlementType.id,
Expand Down
14 changes: 12 additions & 2 deletions apps/ai-hero/src/lib/cohorts-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
contentResource,
contentResourceResource,
entitlements as entitlementsTable,
entitlementTypes,
organizationMemberships,
} from '@/db/schema'
import { env } from '@/env.mjs'
Expand Down Expand Up @@ -88,10 +89,19 @@ export async function checkCohortAccess(
if (!membership) {
return null // User is not a member of the organization
}

const cohortEntitlementType = await db.query.entitlementTypes.findFirst({
where: eq(entitlementTypes.name, 'cohort_content_access'),
})

if (!cohortEntitlementType) {
return null // Cohort entitlement type not found
}

const validEntitlements = await db.query.entitlements.findMany({
where: and(
eq(entitlementsTable.organizationMembershipId, membership.id), // Use membershipId
eq(entitlementsTable.entitlementType, 'cohort_content_access'),
eq(entitlementsTable.entitlementType, cohortEntitlementType.id),
or(
isNull(entitlementsTable.expiresAt),
gt(entitlementsTable.expiresAt, sql`CURRENT_TIMESTAMP`),
Expand All @@ -101,7 +111,7 @@ export async function checkCohortAccess(
})

const cohortEntitlement = validEntitlements.find(
(e) => e.metadata?.cohortSlug === cohortSlug,
(e) => e.sourceType === 'cohort',
)

if (!cohortEntitlement || !cohortEntitlement.metadata) return null
Expand Down
Loading