Skip to content

Commit 2844afd

Browse files
therealbradclaude
andauthored
enhancement: dialog polish, share link password policy, and review field fixes (#228)
* enhancement: dialog polish, share link password policy, and review field fixes - Static height for tabbed dialogs (ShareDialog, EditProject) - CreateProjectWizard template step uses full height, removed Selected/Use Default buttons - Share link passwords now validate against admin password policy with PasswordStrengthIndicator on create and edit dialogs - Generate wizard review hides fields with null/empty values - Updated hardcoded '4 characters' strings to policy-aware messages - Updated E2E tests for new password validation messages Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: strip HTML from upgrade notification plain text message The upgrade notification message field contained raw HTML tags from the notification definitions. When rendered in the daily digest email via Handlebars (which escapes HTML by default), the tags appeared as literal text. Strip HTML for the plain text message field; the rich HTML is preserved separately in data.htmlContent for in-app display. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add plain text stripping tests for upgrade notification messages Verify that HTML tags are properly stripped from notification messages to prevent raw HTML appearing in digest emails. Tests confirm all notification messages produce clean plain text after stripping. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: fix prettier formatting on 4 files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: multi-pass HTML stripping for CodeQL compliance and lint fixes Loop stripHtml until no tags remain to handle nested/malformed tags (CodeQL CWE-20). Fix unused variable and eslint-disable directive. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0332fe5 commit 2844afd

13 files changed

Lines changed: 228 additions & 90 deletions

File tree

testplanit/app/[locale]/admin/projects/CreateProjectWizard.tsx

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,7 +1251,7 @@ export function CreateProjectWizard({
12511251

12521252
case WizardStep.TEMPLATES:
12531253
return (
1254-
<div className="space-y-4">
1254+
<div className="space-y-4 flex flex-col h-full">
12551255
<Alert>
12561256
<AlertDescription className="flex items-center gap-1">
12571257
<AlertCircle className="h-4 w-4" />
@@ -1268,7 +1268,7 @@ export function CreateProjectWizard({
12681268
</Alert>
12691269
)}
12701270

1271-
<ScrollArea className="h-[400px] border rounded-lg">
1271+
<ScrollArea className="flex-1 border rounded-lg">
12721272
<div className="p-4 space-y-4">
12731273
{templates?.map((template) => {
12741274
const isSelected = localSelectedTemplates.includes(
@@ -1343,26 +1343,6 @@ export function CreateProjectWizard({
13431343
})}
13441344
</div>
13451345
</ScrollArea>
1346-
1347-
<div className="flex items-center justify-between text-sm text-muted-foreground">
1348-
<span>
1349-
{t("admin.projects.wizard.labels.selected")}:{" "}
1350-
{localSelectedTemplates.length}
1351-
</span>
1352-
<Button
1353-
variant="outline"
1354-
size="sm"
1355-
onClick={() => {
1356-
const defaultTemplate = templates?.find((t) => t.isDefault);
1357-
if (defaultTemplate) {
1358-
setLocalSelectedTemplates([defaultTemplate.id]);
1359-
setValue("selectedTemplates", [defaultTemplate.id]);
1360-
}
1361-
}}
1362-
>
1363-
{t("admin.projects.wizard.actions.selectDefault")}
1364-
</Button>
1365-
</div>
13661346
</div>
13671347
);
13681348

testplanit/app/[locale]/admin/projects/EditProject.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ export function EditProjectModal({
767767
}
768768
}}
769769
>
770-
<DialogContent className="sm:max-w-[600px] lg:max-w-[1000px]">
770+
<DialogContent className="sm:max-w-[600px] lg:max-w-[1000px] h-[90vh] flex flex-col overflow-hidden">
771771
<Form {...form}>
772772
<form onSubmit={handleSubmit(onSubmit)} className="space-y-4 mt-4">
773773
<DialogHeader>

testplanit/app/[locale]/projects/repository/[projectId]/GenerateTestCasesWizard.tsx

Lines changed: 50 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -820,40 +820,57 @@ const GeneratedTestCaseCard = memo(function GeneratedTestCaseCard({
820820

821821
const renderFieldList = (isEdit: boolean) => (
822822
<div className="mt-3 border-t pt-3 space-y-4">
823-
{selectedTemplateFields.map((field: any) => {
824-
const displayName = field.caseField.displayName;
825-
const fieldId = field.caseField.id.toString();
826-
const fieldType = field.caseField.type.type;
827-
828-
const commonProps = {
829-
fieldType,
830-
caseId: `generated-${testCase.id}`,
831-
template,
832-
fieldId: field.caseField.id,
833-
session,
834-
projectId,
835-
previousFieldValue: undefined,
836-
fieldValue: testCase.fieldValues[displayName],
837-
stepsForDisplay: fieldType === "Steps" ? stepsForDisplay : undefined,
838-
explicitFieldNameForSteps:
839-
fieldType === "Steps" ? fieldId : undefined,
840-
} as const;
841-
842-
return (
843-
<div key={`field-${field.caseField.id}`} className="space-y-2">
844-
<div className="font-medium text-sm text-primary border-b border-muted-foreground/50 pb-1">
845-
{displayName}
823+
{selectedTemplateFields
824+
.filter((field: any) => {
825+
// In read-only mode, skip fields with no value
826+
if (!isEdit) {
827+
const val =
828+
field.caseField.type.type === "Steps"
829+
? getTestCaseSteps(testCase, selectedTemplateFields)
830+
: testCase.fieldValues[field.caseField.displayName];
831+
return (
832+
val != null &&
833+
val !== "" &&
834+
!(Array.isArray(val) && val.length === 0)
835+
);
836+
}
837+
return true;
838+
})
839+
.map((field: any) => {
840+
const displayName = field.caseField.displayName;
841+
const fieldId = field.caseField.id.toString();
842+
const fieldType = field.caseField.type.type;
843+
844+
const commonProps = {
845+
fieldType,
846+
caseId: `generated-${testCase.id}`,
847+
template,
848+
fieldId: field.caseField.id,
849+
session,
850+
projectId,
851+
previousFieldValue: undefined,
852+
fieldValue: testCase.fieldValues[displayName],
853+
stepsForDisplay:
854+
fieldType === "Steps" ? stepsForDisplay : undefined,
855+
explicitFieldNameForSteps:
856+
fieldType === "Steps" ? fieldId : undefined,
857+
} as const;
858+
859+
return (
860+
<div key={`field-${field.caseField.id}`} className="space-y-2">
861+
<div className="font-medium text-sm text-primary border-b border-muted-foreground/50 pb-1">
862+
{displayName}
863+
</div>
864+
<FieldValueRenderer
865+
{...commonProps}
866+
isEditMode={isEdit}
867+
isSubmitting={false}
868+
control={control}
869+
errors={errors}
870+
/>
846871
</div>
847-
<FieldValueRenderer
848-
{...commonProps}
849-
isEditMode={isEdit}
850-
isSubmitting={false}
851-
control={control}
852-
errors={errors}
853-
/>
854-
</div>
855-
);
856-
})}
872+
);
873+
})}
857874
</div>
858875
);
859876

testplanit/app/actions/upgrade-notifications.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,15 +90,27 @@ export async function checkUpgradeNotifications(): Promise<{
9090
? pendingNotifications[0].notification.title
9191
: `What's New in TestPlanIt`;
9292

93+
// Strip HTML tags for the plain text message field
94+
// Loop until no tags remain to handle nested/malformed tags (CodeQL CWE-20)
95+
const stripHtml = (html: string) => {
96+
let result = html;
97+
let prev;
98+
do {
99+
prev = result;
100+
result = result.replace(/<[^>]*>/g, "");
101+
} while (result !== prev);
102+
return result.replace(/\s+/g, " ").trim();
103+
};
104+
93105
let message: string;
94106
if (pendingNotifications.length === 1) {
95-
message = pendingNotifications[0].notification.message;
107+
message = stripHtml(pendingNotifications[0].notification.message);
96108
} else {
97109
// Combine multiple notifications into a single message
98110
message = pendingNotifications
99111
.map(
100112
({ version, notification }) =>
101-
`**${notification.title}** (v${version})\n${notification.message}`
113+
`**${notification.title}** (v${version})\n${stripHtml(notification.message)}`
102114
)
103115
.join("\n\n");
104116
}

testplanit/components/reports/ShareDialog.tsx

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,15 @@ import { Asterisk, Calendar as CalendarIcon, Loader2 } from "lucide-react";
3333
import { useSession } from "next-auth/react";
3434
import { useTranslations } from "next-intl";
3535
import { useMemo, useState } from "react";
36-
import { useCreateShareLink } from "~/lib/hooks";
36+
import {
37+
useCreateShareLink,
38+
useFindFirstRegistrationSettings,
39+
} from "~/lib/hooks";
3740
import { cn } from "~/utils";
41+
import {
42+
PasswordStrengthIndicator,
43+
type PasswordPolicy,
44+
} from "@/components/PasswordStrengthIndicator";
3845

3946
interface ShareDialogProps {
4047
open: boolean;
@@ -55,6 +62,16 @@ export function ShareDialog({
5562
const tCommon = useTranslations("common");
5663
const tAuth = useTranslations("auth.signup.errors");
5764
const { data: session } = useSession();
65+
const { data: registrationSettings } = useFindFirstRegistrationSettings();
66+
const policy: PasswordPolicy | null = registrationSettings
67+
? {
68+
minPasswordLength: registrationSettings.minPasswordLength ?? 8,
69+
requireUppercase: registrationSettings.requireUppercase ?? false,
70+
requireLowercase: registrationSettings.requireLowercase ?? false,
71+
requireNumbers: registrationSettings.requireNumbers ?? false,
72+
requiredSpecialChars: registrationSettings.requiredSpecialChars ?? null,
73+
}
74+
: null;
5875
const [activeTab, setActiveTab] = useState<"create" | "list">("create");
5976

6077
// Form state
@@ -91,10 +108,24 @@ export function ShareDialog({
91108
return;
92109
}
93110

94-
// Validate password for PASSWORD_PROTECTED mode
95-
if (mode === "PASSWORD_PROTECTED" && (!password || password.length < 4)) {
96-
setPasswordError(tAuth("passwordRequired"));
97-
return;
111+
// Validate password for PASSWORD_PROTECTED mode against admin policy
112+
if (mode === "PASSWORD_PROTECTED") {
113+
if (!password) {
114+
setPasswordError(tAuth("passwordRequired"));
115+
return;
116+
}
117+
const minLen = policy?.minPasswordLength ?? 8;
118+
const meetsPolicy =
119+
password.length >= minLen &&
120+
(!policy?.requireUppercase || /[A-Z]/.test(password)) &&
121+
(!policy?.requireLowercase || /[a-z]/.test(password)) &&
122+
(!policy?.requireNumbers || /\d/.test(password)) &&
123+
(!policy?.requiredSpecialChars ||
124+
[...policy.requiredSpecialChars].some((c) => password.includes(c)));
125+
if (!meetsPolicy) {
126+
setPasswordError(tCommon("errors.passwordDoesNotMeetPolicy"));
127+
return;
128+
}
98129
}
99130

100131
if (mode === "PASSWORD_PROTECTED" && password !== confirmPassword) {
@@ -194,7 +225,7 @@ export function ShareDialog({
194225

195226
return (
196227
<Dialog open={open} onOpenChange={onOpenChange}>
197-
<DialogContent className="max-w-4xl max-h-[90vh] overflow-y-auto">
228+
<DialogContent className="max-w-4xl h-[90vh] flex flex-col overflow-hidden">
198229
<DialogHeader>
199230
<DialogTitle>{t("dialogTitle")}</DialogTitle>
200231
<DialogDescription>{t("dialogDescription")}</DialogDescription>
@@ -203,8 +234,9 @@ export function ShareDialog({
203234
<Tabs
204235
value={activeTab}
205236
onValueChange={(v) => setActiveTab(v as "create" | "list")}
237+
className="flex-1 flex flex-col min-h-0"
206238
>
207-
<TabsList className="grid w-full grid-cols-2">
239+
<TabsList className="grid w-full grid-cols-2 shrink-0">
208240
<TabsTrigger data-testid="share-tab-create" value="create">
209241
{t("tabs.create")}
210242
</TabsTrigger>
@@ -213,7 +245,10 @@ export function ShareDialog({
213245
</TabsTrigger>
214246
</TabsList>
215247

216-
<TabsContent value="create" className="space-y-4 mt-4">
248+
<TabsContent
249+
value="create"
250+
className="space-y-4 mt-4 flex-1 min-h-0 overflow-y-auto"
251+
>
217252
{error && (
218253
<Alert variant="destructive">
219254
<AlertDescription>{error}</AlertDescription>
@@ -321,6 +356,10 @@ export function ShareDialog({
321356
required
322357
className={passwordError ? "border-destructive" : ""}
323358
/>
359+
<PasswordStrengthIndicator
360+
password={password}
361+
policy={policy}
362+
/>
324363
{passwordError && (
325364
<p className="text-sm text-destructive">{passwordError}</p>
326365
)}

testplanit/components/share/EditShareLinkDialog.tsx

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,15 @@ import { format } from "date-fns";
2626
import { Asterisk, Calendar as CalendarIcon, Loader2 } from "lucide-react";
2727
import { useTranslations } from "next-intl";
2828
import { useEffect, useState } from "react";
29-
import { useUpdateShareLink } from "~/lib/hooks";
29+
import {
30+
useUpdateShareLink,
31+
useFindFirstRegistrationSettings,
32+
} from "~/lib/hooks";
3033
import { cn } from "~/utils";
34+
import {
35+
PasswordStrengthIndicator,
36+
type PasswordPolicy,
37+
} from "@/components/PasswordStrengthIndicator";
3138

3239
interface EditShareLinkDialogProps {
3340
open: boolean;
@@ -44,6 +51,16 @@ export function EditShareLinkDialog({
4451
}: EditShareLinkDialogProps) {
4552
const t = useTranslations("reports.shareDialog");
4653
const tCommon = useTranslations("common");
54+
const { data: registrationSettings } = useFindFirstRegistrationSettings();
55+
const policy: PasswordPolicy | null = registrationSettings
56+
? {
57+
minPasswordLength: registrationSettings.minPasswordLength ?? 8,
58+
requireUppercase: registrationSettings.requireUppercase ?? false,
59+
requireLowercase: registrationSettings.requireLowercase ?? false,
60+
requireNumbers: registrationSettings.requireNumbers ?? false,
61+
requiredSpecialChars: registrationSettings.requiredSpecialChars ?? null,
62+
}
63+
: null;
4764

4865
// Form state
4966
const [mode, setMode] = useState<ShareLinkMode>(shareLink.mode);
@@ -94,13 +111,24 @@ export function EditShareLinkDialog({
94111
mode === "PASSWORD_PROTECTED" &&
95112
(modeChanged || !shareLink.passwordHash)
96113
) {
97-
if (!password || password.length < 4) {
98-
setPasswordError("Password must be at least 4 characters long.");
114+
if (!password) {
115+
setPasswordError(tCommon("errors.passwordRequired"));
116+
return;
117+
}
118+
const minLen = policy?.minPasswordLength ?? 8;
119+
const meetsPolicy =
120+
password.length >= minLen &&
121+
(!policy?.requireUppercase || /[A-Z]/.test(password)) &&
122+
(!policy?.requireLowercase || /[a-z]/.test(password)) &&
123+
(!policy?.requireNumbers || /\d/.test(password)) &&
124+
(!policy?.requiredSpecialChars ||
125+
[...policy.requiredSpecialChars].some((c) => password.includes(c)));
126+
if (!meetsPolicy) {
127+
setPasswordError(tCommon("errors.passwordDoesNotMeetPolicy"));
99128
return;
100129
}
101-
102130
if (password !== confirmPassword) {
103-
setPasswordError("Passwords do not match.");
131+
setPasswordError(tCommon("errors.passwordsDoNotMatch"));
104132
return;
105133
}
106134
}
@@ -294,6 +322,10 @@ export function EditShareLinkDialog({
294322
required={modeChanged || !shareLink.passwordHash}
295323
className={passwordError ? "border-destructive" : ""}
296324
/>
325+
<PasswordStrengthIndicator
326+
password={password}
327+
policy={policy}
328+
/>
297329
{passwordError && (
298330
<p className="text-sm text-destructive">{passwordError}</p>
299331
)}

testplanit/components/share/ShareDialog.test.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@ vi.mock("~/lib/hooks", () => ({
2727
mutateAsync: mockMutateAsync,
2828
isPending: false,
2929
}),
30+
useFindFirstRegistrationSettings: () => ({
31+
data: {
32+
minPasswordLength: 8,
33+
requireUppercase: false,
34+
requireLowercase: false,
35+
requireNumbers: false,
36+
requiredSpecialChars: null,
37+
},
38+
}),
3039
}));
3140

3241
// Mock server actions

testplanit/e2e/tests/auth/signup.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ test.describe("User Signup", () => {
123123

124124
await page.getByRole("button", { name: /sign up/i }).click();
125125

126-
// Should show password length error - look for the actual message (using first() since both fields show the error)
126+
// Should show password policy error
127127
await expect(
128-
page.getByText(/password must be at least \d+ characters/i).first()
128+
page.getByText(/password.*does not meet|security requirements/i).first()
129129
).toBeVisible({ timeout: 5000 });
130130
});
131131

0 commit comments

Comments
 (0)