Devin/feature opt in system 1765208720 #25
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { _generateMetadata } from "app/_utils"; | ||
| import { headers, cookies } from "next/headers"; | ||
| import { redirect } from "next/navigation"; | ||
|
|
||
| import { getServerSession } from "@calcom/features/auth/lib/getServerSession"; | ||
|
|
||
| import { buildLegacyRequest } from "@lib/buildLegacyCtx"; | ||
|
|
||
| import FeaturesView from "~/settings/my-account/features-view"; | ||
|
|
||
| export const generateMetadata = async () => | ||
| await _generateMetadata( | ||
| (t) => t("features"), | ||
| (t) => t("features_description"), | ||
| undefined, | ||
| undefined, | ||
| "/settings/my-account/features" | ||
| ); | ||
|
|
||
| const Page = async () => { | ||
| const session = await getServerSession({ req: buildLegacyRequest(await headers(), await cookies()) }); | ||
| const userId = session?.user?.id; | ||
| const redirectUrl = "/auth/login?callbackUrl=/settings/my-account/features"; | ||
|
|
||
| if (!userId) { | ||
| return redirect(redirectUrl); | ||
| } | ||
|
|
||
| return <FeaturesView />; | ||
| }; | ||
|
|
||
| export default Page; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import { _generateMetadata, getTranslate } from "app/_utils"; | ||
|
|
||
| import { Resource } from "@calcom/features/pbac/domain/types/permission-registry"; | ||
| import { getResourcePermissions } from "@calcom/features/pbac/lib/resource-permissions"; | ||
| import SettingsHeader from "@calcom/features/settings/appDir/SettingsHeader"; | ||
| import { MembershipRole } from "@calcom/prisma/enums"; | ||
|
|
||
| import { validateUserHasOrg } from "../actions/validateUserHasOrg"; | ||
|
|
||
| import OrganizationFeaturesView from "~/settings/organizations/organization-features-view"; | ||
|
|
||
| export const generateMetadata = async () => | ||
| await _generateMetadata( | ||
| (t) => t("features"), | ||
| (t) => t("organization_features_description"), | ||
| undefined, | ||
| undefined, | ||
| "/settings/organizations/features" | ||
| ); | ||
|
|
||
| const Page = async () => { | ||
| const t = await getTranslate(); | ||
|
|
||
| const session = await validateUserHasOrg(); | ||
|
|
||
| const { canRead } = await getResourcePermissions({ | ||
| userId: session.user.id, | ||
| teamId: session.user.profile.organizationId, | ||
| resource: Resource.Feature, | ||
| userRole: session.user.org.role, | ||
| fallbackRoles: { | ||
| read: { | ||
| roles: [MembershipRole.MEMBER, MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }, | ||
| update: { | ||
| roles: [MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| if (!canRead) { | ||
| return ( | ||
| <SettingsHeader | ||
| title={t("features")} | ||
| description={t("organization_features_description")} | ||
| borderInShellHeader={true}> | ||
| <div className="border-subtle border-x px-4 py-8 sm:px-6"> | ||
| <p className="text-subtle text-sm">{t("no_permission_to_view")}</p> | ||
| </div> | ||
| </SettingsHeader> | ||
| ); | ||
| } | ||
|
|
||
| return <OrganizationFeaturesView organizationId={session.user.profile.organizationId} />; | ||
| }; | ||
|
|
||
| export default Page; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,18 @@ | ||||||||||||||||||||||||||||||
| import { _generateMetadata } from "app/_utils"; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import TeamFeaturesView from "~/settings/teams/team-features-view"; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export const generateMetadata = async ({ params }: { params: Promise<{ id: string }> }) => | ||||||||||||||||||||||||||||||
| await _generateMetadata( | ||||||||||||||||||||||||||||||
| (t) => t("features"), | ||||||||||||||||||||||||||||||
| (t) => t("team_features_description"), | ||||||||||||||||||||||||||||||
| undefined, | ||||||||||||||||||||||||||||||
| undefined, | ||||||||||||||||||||||||||||||
| `/settings/teams/${(await params).id}/features` | ||||||||||||||||||||||||||||||
|
Comment on lines
+5
to
+11
There was a problem hiding this comment. Suggestion: The Severity Level: Minor
Suggested change
Why it matters? ⭐The current code types Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/features/page.tsx
**Line:** 5:11
**Comment:**
*Type Error: The `generateMetadata` function incorrectly types and treats `params` as a `Promise`, but in Next.js `generateMetadata` receives a plain `params` object; this mismatch can break the expected contract, confuse callers and tooling, and makes the implementation harder to reason about even though `await` on a non-promise currently happens to work.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. |
||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const Page = async () => { | ||||||||||||||||||||||||||||||
| return <TeamFeaturesView />; | ||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||
|
Comment on lines
+14
to
+16
There was a problem hiding this comment. Add server-side authentication and authorization checks. Unlike the user-level features page (my-account/features/page.tsx lines 20-27), this team features page lacks authentication and authorization validation. Server components should verify:
Relying solely on client-side checks or route middleware may not be sufficient for sensitive settings pages. 🔎 Recommended server-side auth patternBased on the my-account features page implementation: +import { headers, cookies } from "next/headers";
+import { redirect } from "next/navigation";
+
+import { getServerSession } from "@calcom/features/auth/lib/getServerSession";
+
+import { buildLegacyRequest } from "@lib/buildLegacyCtx";
+
import TeamFeaturesView from "~/settings/teams/team-features-view";
export const generateMetadata = async ({ params }: { params: Promise<{ id: string }> }) =>
await _generateMetadata(
(t) => t("features"),
(t) => t("team_features_description"),
undefined,
undefined,
`/settings/teams/${(await params).id}/features`
);
-const Page = async () => {
+const Page = async ({ params }: { params: Promise<{ id: string }> }) => {
+ const { id: teamId } = await params;
+ const session = await getServerSession({ req: buildLegacyRequest(await headers(), await cookies()) });
+
+ if (!session?.user?.id) {
+ return redirect(`/auth/login?callbackUrl=/settings/teams/${teamId}/features`);
+ }
+
+ // TODO: Add team membership/permission check here
+ // e.g., verify session.user has access to this teamId
+
return <TeamFeaturesView />;
}; |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export default Page; | ||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| "use client"; | ||
|
|
||
| import SettingsHeader from "@calcom/features/settings/appDir/SettingsHeader"; | ||
| import { useLocale } from "@calcom/lib/hooks/useLocale"; | ||
| import { trpc } from "@calcom/trpc/react"; | ||
| import { SettingsToggle } from "@calcom/ui/components/form"; | ||
| import { showToast } from "@calcom/ui/components/toast"; | ||
| import { SkeletonContainer, SkeletonText } from "@calcom/ui/components/skeleton"; | ||
|
|
||
| const SkeletonLoader = () => { | ||
| return ( | ||
| <SkeletonContainer> | ||
| <div className="border-subtle space-y-6 border-x px-4 py-8 sm:px-6"> | ||
| <SkeletonText className="h-8 w-full" /> | ||
| <SkeletonText className="h-8 w-full" /> | ||
| <SkeletonText className="h-8 w-full" /> | ||
| </div> | ||
| </SkeletonContainer> | ||
| ); | ||
| }; | ||
|
|
||
| const FeaturesView = () => { | ||
| const { t } = useLocale(); | ||
| const utils = trpc.useUtils(); | ||
|
|
||
| const { data: features, isLoading } = trpc.viewer.featureManagement.listForUser.useQuery(); | ||
|
|
||
| const setFeatureEnabledMutation = trpc.viewer.featureManagement.setUserFeatureEnabled.useMutation({ | ||
| onSuccess: () => { | ||
| utils.viewer.featureManagement.listForUser.invalidate(); | ||
| showToast(t("settings_updated_successfully"), "success"); | ||
| }, | ||
| onError: () => { | ||
| showToast(t("error_updating_settings"), "error"); | ||
| }, | ||
| }); | ||
|
|
||
| if (isLoading) { | ||
| return ( | ||
| <SettingsHeader | ||
| title={t("features")} | ||
| description={t("features_description")} | ||
| borderInShellHeader={true}> | ||
| <SkeletonLoader /> | ||
| </SettingsHeader> | ||
| ); | ||
| } | ||
|
|
||
| const userControlledFeatures = features?.filter((f) => f.globallyEnabled) ?? []; | ||
|
|
||
| return ( | ||
| <SettingsHeader title={t("features")} description={t("features_description")} borderInShellHeader={true}> | ||
| <div className="border-subtle border-x px-4 py-8 sm:px-6"> | ||
| {userControlledFeatures.length === 0 ? ( | ||
| <p className="text-subtle text-sm">{t("no_features_available")}</p> | ||
| ) : ( | ||
| <div className="space-y-6"> | ||
| {userControlledFeatures.map((feature) => ( | ||
| <SettingsToggle | ||
| key={feature.slug} | ||
|
Comment on lines
+52
to
+60
There was a problem hiding this comment. View Logic DuplicationThe UI rendering logic for feature toggles is duplicated across user, team, and organization views. Extracting a shared Standards
|
||
| toggleSwitchAtTheEnd={true} | ||
| title={feature.slug} | ||
|
There was a problem hiding this comment. Using the feature Consider adding a |
||
| description={feature.description || t("no_description_available")} | ||
| disabled={setFeatureEnabledMutation.isPending} | ||
| checked={feature.enabled} | ||
| onCheckedChange={(checked) => { | ||
| setFeatureEnabledMutation.mutate({ | ||
| featureSlug: feature.slug, | ||
| enabled: checked, | ||
| }); | ||
| }} | ||
| /> | ||
| ))} | ||
| </div> | ||
| )} | ||
| </div> | ||
| </SettingsHeader> | ||
| ); | ||
| }; | ||
|
|
||
| export default FeaturesView; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| "use client"; | ||
|
|
||
| import SettingsHeader from "@calcom/features/settings/appDir/SettingsHeader"; | ||
| import { useLocale } from "@calcom/lib/hooks/useLocale"; | ||
| import { trpc } from "@calcom/trpc/react"; | ||
| import { SettingsToggle } from "@calcom/ui/components/form"; | ||
| import { showToast } from "@calcom/ui/components/toast"; | ||
| import { SkeletonContainer, SkeletonText } from "@calcom/ui/components/skeleton"; | ||
|
|
||
| interface OrganizationFeaturesViewProps { | ||
| organizationId: number; | ||
| } | ||
|
|
||
| const SkeletonLoader = () => { | ||
| return ( | ||
| <SkeletonContainer> | ||
| <div className="border-subtle space-y-6 border-x px-4 py-8 sm:px-6"> | ||
| <SkeletonText className="h-8 w-full" /> | ||
| <SkeletonText className="h-8 w-full" /> | ||
| <SkeletonText className="h-8 w-full" /> | ||
| </div> | ||
| </SkeletonContainer> | ||
| ); | ||
| }; | ||
|
|
||
| const OrganizationFeaturesView = ({ organizationId }: OrganizationFeaturesViewProps) => { | ||
| const { t } = useLocale(); | ||
| const utils = trpc.useUtils(); | ||
|
|
||
| const { data: features, isLoading } = trpc.viewer.featureManagement.listForOrganization.useQuery({ | ||
| organizationId, | ||
| }); | ||
|
|
||
| const setFeatureEnabledMutation = trpc.viewer.featureManagement.setOrganizationFeatureEnabled.useMutation({ | ||
| onSuccess: () => { | ||
| utils.viewer.featureManagement.listForOrganization.invalidate({ organizationId }); | ||
| showToast(t("settings_updated_successfully"), "success"); | ||
| }, | ||
| onError: () => { | ||
| showToast(t("error_updating_settings"), "error"); | ||
| }, | ||
| }); | ||
|
|
||
| if (isLoading) { | ||
| return ( | ||
| <SettingsHeader | ||
| title={t("features")} | ||
| description={t("organization_features_description")} | ||
| borderInShellHeader={true}> | ||
| <SkeletonLoader /> | ||
| </SettingsHeader> | ||
| ); | ||
| } | ||
|
|
||
| const orgControlledFeatures = features?.filter((f) => f.globallyEnabled) ?? []; | ||
|
|
||
| return ( | ||
| <SettingsHeader | ||
| title={t("features")} | ||
| description={t("organization_features_description")} | ||
| borderInShellHeader={true}> | ||
| <div className="border-subtle border-x px-4 py-8 sm:px-6"> | ||
| {orgControlledFeatures.length === 0 ? ( | ||
| <p className="text-subtle text-sm">{t("no_features_available")}</p> | ||
| ) : ( | ||
| <div className="space-y-6"> | ||
| {orgControlledFeatures.map((feature) => ( | ||
| <SettingsToggle | ||
| key={feature.slug} | ||
| toggleSwitchAtTheEnd={true} | ||
| title={feature.slug} | ||
| description={feature.description || t("no_description_available")} | ||
| disabled={setFeatureEnabledMutation.isPending} | ||
| checked={feature.enabled} | ||
| onCheckedChange={(checked) => { | ||
| setFeatureEnabledMutation.mutate({ | ||
| organizationId, | ||
| featureSlug: feature.slug, | ||
| enabled: checked, | ||
| }); | ||
| }} | ||
| /> | ||
| ))} | ||
| </div> | ||
| )} | ||
| </div> | ||
| </SettingsHeader> | ||
| ); | ||
| }; | ||
|
|
||
| export default OrganizationFeaturesView; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Suggestion:
validateUserHasOrgonly guarantees that the user has anorganizationId, not thatsession.user.orgis non-null, so directly accessingsession.user.org.rolecan throw a runtime error whenorgis missing; use optional chaining with a safe default role to avoid a null/undefined property access. [null pointer]Severity Level: Minor⚠️
Why it matters? ⭐
The suggestion is correct and pragmatic: if validateUserHasOrg guarantees only an organizationId but not a populated session.user.org object, accessing session.user.org.role could throw at runtime. Using optional chaining with a sensible fallback (MembershipRole.MEMBER) prevents a potential undefined property access and provides a safe default role for permission checks. MembershipRole is already imported in the file, so the change is minimal and fixes a real robustness issue rather than being purely cosmetic.
Prompt for AI Agent 🤖