Address PR review on data request pages

Require a verified viewer email before creating a rights request and
validate the free-text fields with the same SafeText bounds the console
uses, so this public portal mutation stays safe and bounded.

Move myRightsRequests onto the base Query, drop the now-dead count
loaders, and order the RECTIFICATION enum value before PORTABILITY so
the Postgres sort order matches RightsRequestTypes().

Harden the v2 kit primitives: SegmentedControl keeps equal-width cards
(auto-fill), preserves its selection when the active card is toggled,
and forwards an accessible name; Field associates its label and error
by id/aria instead of wrapping the control in a label. Give the type
group an accessible name, require the name field for non-complaint
types, use a timezone-stable reference year, drop the underreporting
header count, and neutralize the response-deadline copy.

Signed-off-by: Émile Ré <emile@probo.com>
This commit is contained in:
Émile Ré
2026-07-20 09:47:55 +02:00
parent 6623cbc6f2
commit 622f1ba67d
16 changed files with 156 additions and 146 deletions

View File

@@ -132,7 +132,7 @@ export function RequestsPage({ queryRef }: RequestsPageProps) {
return (
<>
<PageHeader title={t("title")} count={requests.length} actions={newRequestButton} />
<PageHeader title={t("title")} actions={newRequestButton} />
<div className={page()}>
<div className={results()}>
<ListErrorBoundary

View File

@@ -34,7 +34,7 @@ import { TextField } from "@probo/ui/src/v2/form/TextField";
import { SegmentedControl } from "@probo/ui/src/v2/SegmentedControl/SegmentedControl";
import { SegmentedControlItem } from "@probo/ui/src/v2/SegmentedControl/SegmentedControlItem";
import { Text } from "@probo/ui/src/v2/typography/Text";
import { type FormEvent, useState } from "react";
import { type FormEvent, useId, useState } from "react";
import { useTranslation } from "react-i18next";
import {
@@ -89,6 +89,7 @@ interface NewRequestFormProps {
function NewRequestForm({ onClose, connectionId, viewerEmail, viewerName }: NewRequestFormProps) {
const { t } = useTranslation("requests");
const [submit, isSubmitting] = useCreateRightsRequest();
const typeLabelId = useId();
const [type, setType] = useState<SubmittableRightsRequestType>("ACCESS");
const [name, setName] = useState(viewerName);
@@ -148,10 +149,11 @@ function NewRequestForm({ onClose, connectionId, viewerEmail, viewerName }: NewR
<DialogBody>
<div className={root()}>
<div className={label()}>
<Text size={2} weight="medium" color="neutral" highContrast>
<Text id={typeLabelId} size={2} weight="medium" color="neutral" highContrast>
{t("dialog.typeLabel")}
</Text>
<SegmentedControl
aria-labelledby={typeLabelId}
value={type}
onValueChange={value => setType(value as SubmittableRightsRequestType)}
>
@@ -173,6 +175,7 @@ function NewRequestForm({ onClose, connectionId, viewerEmail, viewerName }: NewR
<TextField
value={name}
placeholder={t("form.namePlaceholder")}
required={!config.nameOptional}
onChange={e => setName(e.target.value)}
/>
</Field>

View File

@@ -98,7 +98,8 @@ export const rightsRequestFormConfig: Record<
// Human-facing reference derived from the created year and a short suffix of the
// opaque id (display-only; not a stored sequential number).
export function formatRightsRequestReference(id: string, createdAt: string): string {
const year = new Date(createdAt).getFullYear();
// Use UTC so the reference year is stable regardless of the viewer's timezone.
const year = new Date(createdAt).getUTCFullYear();
const suffix = id.replace(/[^a-zA-Z0-9]/g, "").slice(-6).toUpperCase();
return `REQ-${year}-${suffix}`;
}

View File

@@ -35,7 +35,7 @@
"submit": "Submit Request",
"success": {
"title": "Request submitted",
"description": "We'll process your request and respond within 30 days.",
"description": "We'll process your request and respond within the timeframe required by applicable law.",
"close": "Close"
}
},

View File

@@ -35,7 +35,7 @@
"submit": "Envoyer la demande",
"success": {
"title": "Demande envoyée",
"description": "Nous traiterons votre demande et vous répondrons sous 30 jours.",
"description": "Nous traiterons votre demande et vous répondrons dans le délai prévu par la loi applicable.",
"close": "Fermer"
}
},

View File

@@ -19,40 +19,66 @@
// SOFTWARE.
import { ToggleGroup as BaseToggleGroup } from "@base-ui/react/toggle-group";
import type { ReactNode } from "react";
import { type ReactNode, useState } from "react";
import { segmentedControl } from "./variants";
export type SegmentedControlProps = {
// Single selected value (controlled).
value?: string;
"value"?: string;
// Single selected value (uncontrolled).
defaultValue?: string;
"defaultValue"?: string;
// Fired with the newly selected value. Never fired with an empty selection,
// so a value always stays selected (clicking the active item is a no-op).
onValueChange?: (value: string) => void;
disabled?: boolean;
className?: string;
children?: ReactNode;
"onValueChange"?: (value: string) => void;
"disabled"?: boolean;
"className"?: string;
// Accessible name for the group (or reference a visible label via
// `aria-labelledby`), since the control has no intrinsic label.
"aria-label"?: string;
"aria-labelledby"?: string;
"children"?: ReactNode;
};
// Single-select pill group. Wraps Base UI's array-based ToggleGroup with a
// friendlier single-value API.
// friendlier single-value API. Selection is tracked internally (seeded from
// `value`/`defaultValue`) so toggling the active item off — which Base UI
// reports as an empty group value — never clears the selection.
export function SegmentedControl(props: SegmentedControlProps) {
const { value, defaultValue, onValueChange, disabled, className, children } = props;
const {
value,
defaultValue,
onValueChange,
disabled,
className,
children,
"aria-label": ariaLabel,
"aria-labelledby": ariaLabelledby,
} = props;
const { root } = segmentedControl();
const isControlled = value !== undefined;
const [internalValue, setInternalValue] = useState(defaultValue);
const currentValue = isControlled ? value : internalValue;
return (
<BaseToggleGroup
className={root({ className })}
disabled={disabled}
value={value != null ? [value] : undefined}
defaultValue={defaultValue != null ? [defaultValue] : undefined}
aria-label={ariaLabel}
aria-labelledby={ariaLabelledby}
value={currentValue != null ? [currentValue] : []}
onValueChange={(groupValue) => {
const next = groupValue[0];
if (next != null) {
onValueChange?.(next);
// Ignore the empty value emitted when the active item is toggled off,
// preserving the single-selection contract.
if (next == null) {
return;
}
if (!isControlled) {
setInternalValue(next);
}
onValueChange?.(next);
}}
>
{children}

View File

@@ -28,7 +28,7 @@ import { tv } from "tailwind-variants/lite";
// (the pressed state only darkens the border, so selection never shifts layout).
export const segmentedControl = tv({
slots: {
root: "grid grid-cols-[repeat(auto-fit,minmax(9rem,1fr))] gap-1",
root: "grid grid-cols-[repeat(auto-fill,minmax(9rem,1fr))] gap-1",
item: [
"min-w-0 cursor-pointer select-none rounded-3 border border-sand-a6 bg-sand-1 px-4 py-3.5",
"text-center text-2 font-medium text-sand-12 outline-none transition-colors",

View File

@@ -18,32 +18,60 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.
import type { ReactNode } from "react";
import { cloneElement, isValidElement, type ReactElement, type ReactNode, useId } from "react";
import { field } from "./variants";
export type FieldProps = {
// Text shown above the control. The control is nested inside the <label> so
// the association is implicit (no htmlFor / id threading required).
// Text shown above the control, associated with it via `htmlFor`/`id`.
label?: ReactNode;
// Validation / server error shown below the control.
// Validation / server error shown below the control and linked to it via
// `aria-describedby` so assistive technology announces it.
error?: ReactNode;
className?: string;
// A single form control (TextField, Textarea, …). It receives an injected
// `id`, plus `aria-describedby`/`aria-invalid` when an error is present.
children: ReactNode;
};
// Vertical label + control + error grouping for form dialogs.
// Vertical label + control + error grouping for form dialogs. Unlike a raw
// <label> wrapper, the label associates with the control by id, so controls
// whose root is a <div> (and multi-element controls) remain valid and clicks
// never activate an unintended descendant.
export function Field(props: FieldProps) {
const { label, error, className, children } = props;
const { root, label: labelSlot, labelText, error: errorSlot } = field();
const { root, labelText, error: errorSlot } = field();
const generatedId = useId();
const errorId = useId();
const child = isValidElement(children)
? (children as ReactElement<Record<string, unknown>>)
: null;
const existingId = typeof child?.props.id === "string" ? child.props.id : undefined;
const controlId = existingId ?? generatedId;
const control = child
? cloneElement(child, {
"id": controlId,
"aria-describedby": error != null ? errorId : undefined,
"aria-invalid": error != null ? true : undefined,
})
: children;
return (
<div className={root({ className })}>
<label className={labelSlot()}>
{label != null && <span className={labelText()}>{label}</span>}
{children}
</label>
{error != null && <p className={errorSlot()}>{error}</p>}
{label != null && (
<label htmlFor={controlId} className={labelText()}>
{label}
</label>
)}
{control}
{error != null && (
<p id={errorId} className={errorSlot()}>
{error}
</p>
)}
</div>
);
}

View File

@@ -81,7 +81,6 @@ export const textArea = tv({
export const field = tv({
slots: {
root: "flex flex-col gap-1.5",
label: "flex flex-col gap-1.5",
labelText: "text-2 font-medium text-sand-12",
error: "text-1 text-red-a11",
},

View File

@@ -18,7 +18,9 @@
-- OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
-- SOFTWARE.
ALTER TYPE rights_request_type ADD VALUE IF NOT EXISTS 'RECTIFICATION';
-- Insert RECTIFICATION before PORTABILITY so the enum's sort order matches the
-- canonical RightsRequestTypes() ordering used for type-sorted cursors.
ALTER TYPE rights_request_type ADD VALUE IF NOT EXISTS 'RECTIFICATION' BEFORE 'PORTABILITY';
ALTER TYPE rights_request_type ADD VALUE IF NOT EXISTS 'OBJECTION';
ALTER TYPE rights_request_type ADD VALUE IF NOT EXISTS 'COMPLAINT';

View File

@@ -240,44 +240,6 @@ WHERE
return nil
}
func (rrs *RightsRequests) CountByOrganizationIDAndContact(
ctx context.Context,
conn pg.Querier,
scope Scoper,
organizationID gid.GID,
contact string,
) (int, error) {
q := `
SELECT
COUNT(id)
FROM
rights_requests
WHERE
%s
AND organization_id = @organization_id
AND contact = @contact
`
q = fmt.Sprintf(q, scope.SQLFragment())
args := pgx.StrictNamedArgs{
"organization_id": organizationID,
"contact": contact,
}
maps.Copy(args, scope.SQLArguments())
row := conn.QueryRow(ctx, q, args)
var count int
err := row.Scan(&count)
if err != nil {
return 0, fmt.Errorf("cannot count rights requests: %w", err)
}
return count, nil
}
func (rrs *RightsRequests) LoadByOrganizationIDAndContact(
ctx context.Context,
conn pg.Querier,

View File

@@ -14,6 +14,7 @@ import (
"go.probo.inc/probo/pkg/coredata"
"go.probo.inc/probo/pkg/gid"
"go.probo.inc/probo/pkg/mailman"
"go.probo.inc/probo/pkg/page"
"go.probo.inc/probo/pkg/server/api/authn"
"go.probo.inc/probo/pkg/server/api/compliancepage"
"go.probo.inc/probo/pkg/server/api/trust/v1/schema"
@@ -304,6 +305,38 @@ func (r *queryResolver) OidcProviders(ctx context.Context) ([]*types.OIDCProvide
return result, nil
}
// MyRightsRequests is the resolver for the myRightsRequests field.
func (r *queryResolver) MyRightsRequests(ctx context.Context, first *int, after *page.CursorKey, last *int, before *page.CursorKey) (*types.RightsRequestConnection, error) {
pageOrderBy := page.OrderBy[coredata.RightsRequestOrderField]{
Field: coredata.RightsRequestOrderFieldCreatedAt,
Direction: page.OrderDirectionDesc,
}
cursor := types.NewCursor(first, after, last, before, pageOrderBy)
identity := authn.IdentityFromContext(ctx)
if identity == nil {
emptyPage := page.NewPage([]*coredata.RightsRequest{}, cursor)
return types.NewRightsRequestConnection(emptyPage), nil
}
compliancePage := compliancepage.CompliancePageFromContext(ctx)
scope := coredata.NewScopeFromObjectID(compliancePage.OrganizationID)
result, err := r.trust.RightsRequests.ListForOrganizationIDAndContact(
ctx,
scope,
compliancePage.OrganizationID,
identity.EmailAddress.String(),
cursor,
)
if err != nil {
r.logger.ErrorCtx(ctx, "cannot list rights requests", log.Error(err))
return nil, gqlutils.Internal(ctx)
}
return types.NewRightsRequestConnection(result), nil
}
// Mutation returns schema.MutationResolver implementation.
func (r *Resolver) Mutation() schema.MutationResolver { return &mutationResolver{r} }

View File

@@ -30,6 +30,16 @@ type Query {
oidcProviders: [OIDCProviderInfo!]!
@goField(forceResolver: true)
@authentication(required: OPTIONAL)
# The current viewer's own data subject requests for this trust center,
# scoped by their verified email. Returns an empty connection for guests so
# the portal can still render its empty state.
myRightsRequests(
first: Int
after: CursorKey
last: Int
before: CursorKey
): RightsRequestConnection! @goField(forceResolver: true)
}
type OIDCProviderInfo {

View File

@@ -52,18 +52,6 @@ type RightsRequestEdge {
node: RightsRequest!
}
extend type Query {
# The current viewer's own data subject requests for this trust center,
# scoped by their verified email. Returns an empty connection for guests so
# the portal can still render its empty state.
myRightsRequests(
first: Int
after: CursorKey
last: Int
before: CursorKey
): RightsRequestConnection!
}
extend type Mutation {
# Submit a data subject request. Requires a verified viewer; the request is
# attributed to the viewer's email, so no NDA gate applies.

View File

@@ -10,7 +10,6 @@ import (
"go.gearno.de/kit/log"
"go.probo.inc/probo/pkg/coredata"
"go.probo.inc/probo/pkg/page"
"go.probo.inc/probo/pkg/server/api/authn"
"go.probo.inc/probo/pkg/server/api/compliancepage"
"go.probo.inc/probo/pkg/server/api/trust/v1/types"
@@ -21,8 +20,10 @@ import (
// CreateRightsRequest is the resolver for the createRightsRequest field.
func (r *mutationResolver) CreateRightsRequest(ctx context.Context, input types.CreateRightsRequestInput) (*types.CreateRightsRequestPayload, error) {
identity := authn.IdentityFromContext(ctx)
if identity == nil {
return nil, gqlutils.Unauthenticatedf(ctx, "authentication is required to submit a request")
if identity == nil || !identity.EmailAddressVerified {
// The request is attributed to the viewer's email, so the email must be
// verified — an authenticated-but-unverified identity is not enough.
return nil, gqlutils.Unauthenticatedf(ctx, "a verified email is required to submit a request")
}
compliancePage := compliancepage.CompliancePageFromContext(ctx)
@@ -51,35 +52,3 @@ func (r *mutationResolver) CreateRightsRequest(ctx context.Context, input types.
),
}, nil
}
// MyRightsRequests is the resolver for the myRightsRequests field.
func (r *queryResolver) MyRightsRequests(ctx context.Context, first *int, after *page.CursorKey, last *int, before *page.CursorKey) (*types.RightsRequestConnection, error) {
pageOrderBy := page.OrderBy[coredata.RightsRequestOrderField]{
Field: coredata.RightsRequestOrderFieldCreatedAt,
Direction: page.OrderDirectionDesc,
}
cursor := types.NewCursor(first, after, last, before, pageOrderBy)
identity := authn.IdentityFromContext(ctx)
if identity == nil {
emptyPage := page.NewPage([]*coredata.RightsRequest{}, cursor)
return types.NewRightsRequestConnection(emptyPage), nil
}
compliancePage := compliancepage.CompliancePageFromContext(ctx)
scope := coredata.NewScopeFromObjectID(compliancePage.OrganizationID)
result, err := r.trust.RightsRequests.ListForOrganizationIDAndContact(
ctx,
scope,
compliancePage.OrganizationID,
identity.EmailAddress.String(),
cursor,
)
if err != nil {
r.logger.ErrorCtx(ctx, "cannot list rights requests", log.Error(err))
return nil, gqlutils.Internal(ctx)
}
return types.NewRightsRequestConnection(result), nil
}

View File

@@ -29,6 +29,8 @@ import (
"go.probo.inc/probo/pkg/coredata"
"go.probo.inc/probo/pkg/gid"
"go.probo.inc/probo/pkg/page"
"go.probo.inc/probo/pkg/probo"
"go.probo.inc/probo/pkg/validator"
)
// RightsRequestDeadlineDays is the number of days a portal-submitted data
@@ -54,11 +56,26 @@ type (
}
)
// Validate bounds the free-text fields with the same rules the console applies,
// so this public portal mutation can't persist oversized or unsafe input.
func (r *CreateRightsRequest) Validate() error {
v := validator.New()
v.Check(r.DataSubject, "data_subject", validator.SafeText(probo.ContentMaxLength))
v.Check(r.Details, "details", validator.SafeText(probo.ContentMaxLength))
return v.Error()
}
func (s *RightsRequestService) Create(
ctx context.Context,
scope coredata.Scoper,
req *CreateRightsRequest,
) (*coredata.RightsRequest, error) {
if err := req.Validate(); err != nil {
return nil, err
}
now := time.Now()
deadline := now.AddDate(0, 0, RightsRequestDeadlineDays)
@@ -97,34 +114,6 @@ func (s *RightsRequestService) Create(
return request, nil
}
func (s RightsRequestService) CountForOrganizationIDAndContact(
ctx context.Context,
scope coredata.Scoper,
organizationID gid.GID,
contact string,
) (int, error) {
var count int
err := s.svc.pg.WithConn(
ctx,
func(ctx context.Context, conn pg.Querier) (err error) {
requests := coredata.RightsRequests{}
count, err = requests.CountByOrganizationIDAndContact(ctx, conn, scope, organizationID, contact)
if err != nil {
return fmt.Errorf("cannot count rights requests: %w", err)
}
return nil
},
)
if err != nil {
return 0, err
}
return count, nil
}
func (s RightsRequestService) ListForOrganizationIDAndContact(
ctx context.Context,
scope coredata.Scoper,