-
Notifications
You must be signed in to change notification settings - Fork 34
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
Changes from all commits
05a130c
51d49b5
f619121
d50a2a3
7631623
2bbf031
8d1e131
af3677f
9cdf186
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@coursebuilder/utils-auth': patch | ||
--- | ||
|
||
add canCreate to abilityForResource type |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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'] }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
// }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
return rules | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -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', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -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( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Consider adding validation for 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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
return rules | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused import detected The -import { getAbilityForResource } from '@/utils/get-current-ability-rules' 📝 Committable suggestion
Suggested change
|
||||||
import { MenuIcon } from 'lucide-react' | ||||||
|
||||||
import { | ||||||
|
@@ -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> | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
|
@@ -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()}` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
There was a problem hiding this comment.
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 🤔