From a30dbd3afb469abc83199e2d9e19db82eabf408a Mon Sep 17 00:00:00 2001 From: Mark Goho Date: Wed, 25 Mar 2026 16:55:42 -0400 Subject: [PATCH 1/5] replace profile approval timestamp with editing permission Use allowProfileEditing as the runtime gate for member profile access so admins can toggle profile editing on and off without relying on a one-time approval timestamp. Co-Authored-By: Claude Opus 4.6 --- .../plugins/admin-members-plugin.ts | 5 +- .../routes/approve-profile.test.ts | 16 +- .../routes/approve-profile.ts | 13 +- .../routes/link-profile.test.ts | 6 +- .../schemas/member-schemas.ts | 19 +-- .../services/approve-profile.ts | 24 +-- .../admin-members-api/services/interface.ts | 22 +-- .../services/link-profile.ts | 4 +- .../src/backfill-allow-profile-editing.ts | 24 +++ .../routes/create-profile.test.ts | 6 +- .../src/profiles-api/routes/create-profile.ts | 2 +- .../routes/profile-route-handler-factory.ts | 2 +- .../profiles-api/routes/write-profile.test.ts | 6 +- .../src/profiles-api/services/member/index.ts | 10 +- .../profiles-api/services/member/interface.ts | 6 +- .../test-utils/create-profiles-test-plugin.ts | 3 +- functions/src/types/member-document.ts | 9 +- .../admin/services/admin-members.service.ts | 10 +- .../admin-member-detail.html | 32 ++-- .../admin-member-detail.service.ts | 16 +- .../admin-member-detail.spec.ts | 149 +++++++++++------- .../admin-member-detail.ts | 39 ++++- .../src/app/api-types/api-member-response.ts | 2 +- members/src/app/app.routes.ts | 4 + .../profile-editing.guard.integration.spec.ts | 114 ++++++++++++++ .../src/app/guards/profile-editing.guard.ts | 15 ++ members/src/app/header/header.spec.ts | 18 +++ members/src/app/header/header.ts | 6 +- members/src/app/membership/membership.html | 12 +- members/src/app/membership/membership.spec.ts | 30 ++-- members/src/app/membership/membership.ts | 8 +- .../src/app/services/membership.service.ts | 7 +- 32 files changed, 456 insertions(+), 183 deletions(-) create mode 100644 functions/src/backfill-allow-profile-editing.ts create mode 100644 members/src/app/guards/profile-editing.guard.integration.spec.ts create mode 100644 members/src/app/guards/profile-editing.guard.ts diff --git a/functions/src/admin-members-api/plugins/admin-members-plugin.ts b/functions/src/admin-members-api/plugins/admin-members-plugin.ts index 4c21a3e3..f9e1bae4 100644 --- a/functions/src/admin-members-api/plugins/admin-members-plugin.ts +++ b/functions/src/admin-members-api/plugins/admin-members-plugin.ts @@ -27,6 +27,7 @@ import { ActivateMembershipApiResponseSchema, ActivateMembershipBodySchema, ApproveProfileApiResponseSchema, + ApproveProfileBodySchema, CancelMembershipApiResponseSchema, CleanSlateApiResponseSchema, DeleteDraftProfileApiResponseSchema, @@ -194,15 +195,17 @@ export function createAdminMembersPlugin(services?: PartialServices) { // POST /:memberId/profile/approve - Approve member for profile work (served at /api/admin/members/:memberId/profile/approve) .post( "/profile/approve", - async ({ params, adminToken, memberAdminService, logger, set }) => + async ({ params, body, adminToken, memberAdminService, logger, set }) => approveProfileLogic({ memberId: params.memberId, + allowProfileEditing: body.allowProfileEditing, adminUid: getAdminUid(adminToken, logger), memberAdminService, logger, set, }), { + body: ApproveProfileBodySchema, response: ApproveProfileApiResponseSchema, }, ) diff --git a/functions/src/admin-members-api/routes/approve-profile.test.ts b/functions/src/admin-members-api/routes/approve-profile.test.ts index f40a194e..c2bc0424 100644 --- a/functions/src/admin-members-api/routes/approve-profile.test.ts +++ b/functions/src/admin-members-api/routes/approve-profile.test.ts @@ -30,7 +30,7 @@ describe("POST /:memberId/profile/approve", () => { email: "member@example.com", createdAt: Timestamp.now(), membershipActive: true, - profileApprovedAt: Timestamp.now(), + allowProfileEditing: true, }; const mockApproveProfile = mock( @@ -68,7 +68,11 @@ describe("POST /:memberId/profile/approve", () => { `http://localhost/${memberId}/profile/approve`, { method: "POST", - headers, + headers: { + ...headers, + "Content-Type": "application/json", + }, + body: JSON.stringify({ allowProfileEditing: true }), }, ); @@ -106,14 +110,14 @@ describe("POST /:memberId/profile/approve", () => { member?: { uid?: string; email?: string; - profileApprovedAt?: string; + allowProfileEditing?: boolean; isAdmin?: boolean; }; }; expect(body.success).toBe(true); expect(body.member?.uid).toBe("test-member-id"); expect(body.member?.email).toBe("member@example.com"); - expect(body.member?.profileApprovedAt).toBeDefined(); + expect(body.member?.allowProfileEditing).toBe(true); expect(body.member?.isAdmin).toBe(false); }); @@ -126,12 +130,12 @@ describe("POST /:memberId/profile/approve", () => { const body = (await response.json()) as { success?: boolean; member?: { - profileApprovedAt?: string; + allowProfileEditing?: boolean; isAdmin?: boolean; }; }; expect(body.success).toBe(true); - expect(body.member?.profileApprovedAt).toBeDefined(); + expect(body.member?.allowProfileEditing).toBe(true); expect(body.member?.isAdmin).toBe(false); }); diff --git a/functions/src/admin-members-api/routes/approve-profile.ts b/functions/src/admin-members-api/routes/approve-profile.ts index fe231938..4aca7a37 100644 --- a/functions/src/admin-members-api/routes/approve-profile.ts +++ b/functions/src/admin-members-api/routes/approve-profile.ts @@ -7,24 +7,29 @@ import { handleRouteError } from "../../shared-api/utils/route-error-handler.js" export async function approveProfileLogic({ memberId, + allowProfileEditing, adminUid, memberAdminService, logger, set, }: { memberId: string; + allowProfileEditing: boolean; adminUid: string; memberAdminService: MemberAdminService; logger: Logger; set: { status?: number | string }; }): Promise { try { - logger.info("Admin approving member for profile work", { + logger.info("Admin updating member profile editing permission", { adminUid, memberId, }); - const result = await memberAdminService.approveProfile({ memberId }); + const result = await memberAdminService.approveProfile({ + memberId, + allowProfileEditing, + }); let isAdmin = false; try { @@ -42,11 +47,11 @@ export async function approveProfileLogic({ } catch (error: unknown) { return handleRouteError({ error, - operation: "approve member for profile work", + operation: "update member profile editing permission", errorId: ERROR_IDS.API_ADMIN_UPDATE_MEMBER_FAILED, logger, set, - context: { memberId, adminUid }, + context: { memberId, allowProfileEditing, adminUid }, }) as ApproveProfileApiResponse; } } diff --git a/functions/src/admin-members-api/routes/link-profile.test.ts b/functions/src/admin-members-api/routes/link-profile.test.ts index 83740523..4e3aed09 100644 --- a/functions/src/admin-members-api/routes/link-profile.test.ts +++ b/functions/src/admin-members-api/routes/link-profile.test.ts @@ -51,7 +51,7 @@ describe("POST /:memberId/profile/link", () => { membershipExpiresAt: Timestamp.now(), slug: "unlinked-doula-profile", profileCreatedAt: Timestamp.now(), - profileApprovedAt: Timestamp.now(), + allowProfileEditing: true, }; const mockLinkProfile = mock( @@ -164,7 +164,7 @@ describe("POST /:memberId/profile/link", () => { email?: string; membershipActive?: boolean; slug?: string; - profileApprovedAt?: string; + allowProfileEditing?: boolean; isAdmin?: boolean; }; }; @@ -172,7 +172,7 @@ describe("POST /:memberId/profile/link", () => { expect(body.member?.uid).toBe("test-member-id"); expect(body.member?.email).toBe("member@example.com"); expect(body.member?.slug).toBe("unlinked-doula-profile"); - expect(body.member?.profileApprovedAt).toBeDefined(); + expect(body.member?.allowProfileEditing).toBe(true); expect(body.member?.isAdmin).toBe(false); }); diff --git a/functions/src/admin-members-api/schemas/member-schemas.ts b/functions/src/admin-members-api/schemas/member-schemas.ts index 9a65cb51..07a2a328 100644 --- a/functions/src/admin-members-api/schemas/member-schemas.ts +++ b/functions/src/admin-members-api/schemas/member-schemas.ts @@ -88,12 +88,9 @@ export const MemberResponseSchema = t.Object({ description: "Profile creation timestamp (ISO 8601)", }), ), - profileApprovedAt: t.Optional( - t.String({ - format: "date-time", - description: "Profile approval timestamp (ISO 8601)", - }), - ), + allowProfileEditing: t.Boolean({ + description: "Whether the member can currently create or edit a profile", + }), stripeCustomerId: t.Optional( t.String({ description: "Stripe customer ID", @@ -189,9 +186,7 @@ export function toMemberResponse( ...(document.profileCreatedAt !== undefined && { profileCreatedAt: timestampToIso(document.profileCreatedAt), }), - ...(document.profileApprovedAt !== undefined && { - profileApprovedAt: timestampToIso(document.profileApprovedAt), - }), + allowProfileEditing: document.allowProfileEditing ?? false, ...(document.stripeCustomerId !== undefined && { stripeCustomerId: document.stripeCustomerId, }), @@ -394,6 +389,12 @@ export type UpdateMemberApiResponse = Static< typeof UpdateMemberApiResponseSchema >; +export const ApproveProfileBodySchema = t.Object({ + allowProfileEditing: t.Boolean({ + description: "Whether the member can currently create or edit a profile", + }), +}); + /** * POST /api/admin/members/:memberId/profile/approve response - union of success and error. */ diff --git a/functions/src/admin-members-api/services/approve-profile.ts b/functions/src/admin-members-api/services/approve-profile.ts index 2bd68b05..b852bddd 100644 --- a/functions/src/admin-members-api/services/approve-profile.ts +++ b/functions/src/admin-members-api/services/approve-profile.ts @@ -1,4 +1,3 @@ -import { FieldValue } from "firebase-admin/firestore"; import { logger } from "firebase-functions/v2"; import { MEMBERS_COLLECTION } from "../../collections/index.js"; import { ERROR_IDS } from "../../constants/error-ids.js"; @@ -7,28 +6,29 @@ import { MemberFirestoreService } from "../../shared-api/services/member-firesto import type { MemberDocument } from "../../types/member-document.js"; import { verifyMemberExists } from "./verify-member-exists.js"; -export interface ApproveProfileResult { +export interface SetProfileEditingPermissionResult { member: MemberDocument; } /** - * Approve a member to create or edit a profile. - * Sets profileApprovedAt to the current server timestamp. + * Set whether a member can create or edit a profile. * * @param options.memberId - The Firestore document ID of the member + * @param options.allowProfileEditing - Whether profile editing is allowed * @returns The updated member document * @throws NotFoundError if member does not exist */ export async function approveProfile(options: { memberId: string; -}): Promise { - const { memberId } = options; + allowProfileEditing: boolean; +}): Promise { + const { memberId, allowProfileEditing } = options; try { await verifyMemberExists(memberId); await MemberFirestoreService.updateMember(memberId, { - profileApprovedAt: FieldValue.serverTimestamp(), + allowProfileEditing, }); const updatedMemberDocument = await MemberFirestoreService.getMemberByUid( @@ -48,7 +48,10 @@ export async function approveProfile(options: { uid: updatedMemberDocument.id, }; - logger.info("Approved member for profile work", { memberId }); + logger.info("Updated member profile editing permission", { + memberId, + allowProfileEditing, + }); return { member, @@ -58,12 +61,13 @@ export async function approveProfile(options: { throw error; } - logger.error("Failed to approve member for profile work", { + logger.error("Failed to update member profile editing permission", { errorId: ERROR_IDS.API_ADMIN_UPDATE_MEMBER_FAILED, memberId, + allowProfileEditing, error, errorMessage: error instanceof Error ? error.message : "Unknown error", }); - throw new HttpError("Failed to approve member for profile work", 500); + throw new HttpError("Failed to update member profile editing permission", 500); } } diff --git a/functions/src/admin-members-api/services/interface.ts b/functions/src/admin-members-api/services/interface.ts index 01ed195b..a42d4702 100644 --- a/functions/src/admin-members-api/services/interface.ts +++ b/functions/src/admin-members-api/services/interface.ts @@ -6,7 +6,7 @@ import type { DeleteDraftProfileResult } from "./delete-draft-profile.js"; import type { LinkProfileResult } from "./link-profile.js"; import type { ListUnlinkedProfilesResult } from "./list-unlinked-profiles.js"; import type { ProfileData } from "../../profiles-api/schemas/profile-schemas.js"; -import type { ApproveProfileResult } from "./approve-profile.js"; +import type { SetProfileEditingPermissionResult } from "./approve-profile.js"; import type { ReadProfileResult } from "./read-profile.js"; import type { RefundMembershipResult } from "./refund-membership.js"; import type { ToggleProfileDraftResult } from "./toggle-profile-draft.js"; @@ -120,6 +120,17 @@ export interface MemberAdminService { logger: Logger; }): Promise; + /** + * Set whether a member can create or edit a profile. + * + * @param options - Member ID and desired permission state + * @returns Promise resolving to updated member document + */ + approveProfile(options: { + memberId: string; + allowProfileEditing: boolean; + }): Promise; + /** * Check if a user has admin privileges. * @@ -219,15 +230,6 @@ export interface MemberAdminService { */ listUnlinkedProfiles(): Promise; - /** - * Approve a member to create or edit a profile. - * - * @param options - Object containing memberId - * @returns Promise resolving to updated member document - * @throws NotFoundError if member does not exist - */ - approveProfile(options: { memberId: string }): Promise; - /** * Link an unlinked profile to a member account. * Creates a bidirectional relationship between profile and member. diff --git a/functions/src/admin-members-api/services/link-profile.ts b/functions/src/admin-members-api/services/link-profile.ts index 53612cb2..e07dd348 100644 --- a/functions/src/admin-members-api/services/link-profile.ts +++ b/functions/src/admin-members-api/services/link-profile.ts @@ -1,4 +1,4 @@ -import { FieldValue, getFirestore, Timestamp } from "firebase-admin/firestore"; +import { getFirestore, Timestamp } from "firebase-admin/firestore"; import { logger } from "firebase-functions/v2"; import { IMPORT_COLLECTION, @@ -84,7 +84,7 @@ export async function linkProfile(options: { batch.update(memberReference, { slug, profileCreatedAt, - profileApprovedAt: FieldValue.serverTimestamp(), + allowProfileEditing: true, }); await batch.commit(); diff --git a/functions/src/backfill-allow-profile-editing.ts b/functions/src/backfill-allow-profile-editing.ts new file mode 100644 index 00000000..0c6e28c9 --- /dev/null +++ b/functions/src/backfill-allow-profile-editing.ts @@ -0,0 +1,24 @@ +import { getFirestore } from "firebase-admin/firestore"; +import { MEMBERS_COLLECTION } from "./collections/index.js"; + +export async function backfillAllowProfileEditing(): Promise { + const firestore = getFirestore(); + const snapshot = await firestore + .collection(MEMBERS_COLLECTION) + .where("profileApprovedAt", "!=", null) + .get(); + + if (snapshot.empty) { + return; + } + + const batch = firestore.batch(); + + for (const document of snapshot.docs) { + batch.update(document.ref, { + allowProfileEditing: true, + }); + } + + await batch.commit(); +} diff --git a/functions/src/profiles-api/routes/create-profile.test.ts b/functions/src/profiles-api/routes/create-profile.test.ts index 8b0612f7..2ae3e11d 100644 --- a/functions/src/profiles-api/routes/create-profile.test.ts +++ b/functions/src/profiles-api/routes/create-profile.test.ts @@ -73,7 +73,7 @@ describe("POST /:slug (create profile)", () => { if (profileNotApproved) { return Promise.reject( new ForbiddenError( - "Profile work requires admin approval before creating or editing a profile.", + "Profile work requires profile editing to be enabled before creating or editing a profile.", ), ); } @@ -106,7 +106,7 @@ describe("POST /:slug (create profile)", () => { const testApp = createProfilesTestPlugin({ profileMemberService: { - verifyProfileApproved: mockVerifyMembership, + verifyProfileEditingAllowed: mockVerifyMembership, }, profileStoreService: { createProfile: mockCreateProfile, @@ -212,7 +212,7 @@ describe("POST /:slug (create profile)", () => { expect(response.status).toBe(403); const body = (await response.json()) as { error?: string }; - expect(body.error).toContain("admin approval"); + expect(body.error).toContain("profile editing to be enabled"); }); it("should return 403 when user has no slug", async () => { diff --git a/functions/src/profiles-api/routes/create-profile.ts b/functions/src/profiles-api/routes/create-profile.ts index 2849beae..eef2435b 100644 --- a/functions/src/profiles-api/routes/create-profile.ts +++ b/functions/src/profiles-api/routes/create-profile.ts @@ -95,7 +95,7 @@ export async function createProfileLogic({ set: { status?: number | string }; }): Promise { try { - const member = await profileMemberService.verifyProfileApproved(uid); + const member = await profileMemberService.verifyProfileEditingAllowed(uid); const slug = member.slug; if (!slug) { diff --git a/functions/src/profiles-api/routes/profile-route-handler-factory.ts b/functions/src/profiles-api/routes/profile-route-handler-factory.ts index 8ea8954d..a48f3a53 100644 --- a/functions/src/profiles-api/routes/profile-route-handler-factory.ts +++ b/functions/src/profiles-api/routes/profile-route-handler-factory.ts @@ -70,7 +70,7 @@ export function createProfileRouteHandler( set: { status?: number | string }; }): Promise => { try { - const member = await profileMemberService.verifyProfileApproved(uid); + const member = await profileMemberService.verifyProfileEditingAllowed(uid); const slug = member.slug; if (!slug) { diff --git a/functions/src/profiles-api/routes/write-profile.test.ts b/functions/src/profiles-api/routes/write-profile.test.ts index b27d9388..97748315 100644 --- a/functions/src/profiles-api/routes/write-profile.test.ts +++ b/functions/src/profiles-api/routes/write-profile.test.ts @@ -68,7 +68,7 @@ describe("PUT /:slug (update profile)", () => { if (profileNotApproved) { return Promise.reject( new ForbiddenError( - "Profile work requires admin approval before creating or editing a profile.", + "Profile work requires profile editing to be enabled before creating or editing a profile.", ), ); } @@ -92,7 +92,7 @@ describe("PUT /:slug (update profile)", () => { const testApp = createProfilesTestPlugin({ profileMemberService: { - verifyProfileApproved: mockVerifyMembership, + verifyProfileEditingAllowed: mockVerifyMembership, }, profileStoreService: { writeProfile: mockWriteProfile, @@ -201,7 +201,7 @@ describe("PUT /:slug (update profile)", () => { expect(response.status).toBe(403); const body = (await response.json()) as { error?: string }; - expect(body.error).toContain("admin approval"); + expect(body.error).toContain("profile editing to be enabled"); }); it("should return 403 when user has no slug (no profile yet)", async () => { diff --git a/functions/src/profiles-api/services/member/index.ts b/functions/src/profiles-api/services/member/index.ts index 7a4c7e2d..2b0d56e1 100644 --- a/functions/src/profiles-api/services/member/index.ts +++ b/functions/src/profiles-api/services/member/index.ts @@ -64,14 +64,14 @@ async function verifyActiveMembership(uid: string): Promise { } /** - * Verify user has been approved to create or edit a profile. + * Verify user has permission to create or edit a profile. */ -async function verifyProfileApproved(uid: string): Promise { +async function verifyProfileEditingAllowed(uid: string): Promise { const member = await verifyActiveMembership(uid); - if (member.profileApprovedAt === undefined) { + if (member.allowProfileEditing !== true) { throw new ForbiddenError( - "Profile work requires admin approval before creating or editing a profile.", + "Profile work requires profile editing to be enabled before creating or editing a profile.", ); } @@ -200,7 +200,7 @@ async function getMemberBySlug( export const ProfileMemberService: ProfileMemberServiceInterface = { getMemberByUid, verifyActiveMembership, - verifyProfileApproved, + verifyProfileEditingAllowed, checkSlugAvailable, setSlug, setProfileCreatedAt, diff --git a/functions/src/profiles-api/services/member/interface.ts b/functions/src/profiles-api/services/member/interface.ts index 4fc98690..ec1a25b4 100644 --- a/functions/src/profiles-api/services/member/interface.ts +++ b/functions/src/profiles-api/services/member/interface.ts @@ -39,14 +39,14 @@ export interface ProfileMemberService { verifyActiveMembership(uid: string): Promise; /** - * Verify user has been approved to create or edit a profile. + * Verify user has permission to create or edit a profile. * * @param uid - Firebase Auth user ID * @returns Promise with member document * @throws NotFoundError if member not found - * @throws ForbiddenError if profile work is not approved + * @throws ForbiddenError if profile work is not allowed */ - verifyProfileApproved(uid: string): Promise; + verifyProfileEditingAllowed(uid: string): Promise; /** * Check if a slug is available (not already in use as a profile document ID). diff --git a/functions/src/profiles-api/test-utils/create-profiles-test-plugin.ts b/functions/src/profiles-api/test-utils/create-profiles-test-plugin.ts index 806e0865..438b803f 100644 --- a/functions/src/profiles-api/test-utils/create-profiles-test-plugin.ts +++ b/functions/src/profiles-api/test-utils/create-profiles-test-plugin.ts @@ -26,6 +26,7 @@ export const mockMemberDocument: MemberDocument = { membershipExpiresAt: Timestamp.now(), slug: "test-user", profileCreatedAt: Timestamp.now(), + allowProfileEditing: true, }; /** @@ -96,7 +97,7 @@ export function createProfilesTestPlugin(overrides?: { verifyActiveMembership: mock(() => Promise.resolve({ ...mockMemberDocument }), ), - verifyProfileApproved: mock(() => + verifyProfileEditingAllowed: mock(() => Promise.resolve({ ...mockMemberDocument }), ), checkSlugAvailable: mock(() => Promise.resolve({ available: true })), diff --git a/functions/src/types/member-document.ts b/functions/src/types/member-document.ts index 6934d966..1c7ea00a 100644 --- a/functions/src/types/member-document.ts +++ b/functions/src/types/member-document.ts @@ -44,8 +44,13 @@ export interface MemberDocument { */ profileCreatedAt?: Timestamp; /** - * Timestamp when an admin approved the member to create or edit a public doula profile. - * Undefined if the member has not been approved for profile work. + * Whether an admin currently allows the member to create or edit a public doula profile. + * Defaults to false when unset. + */ + allowProfileEditing?: boolean; + /** + * Legacy timestamp when an admin approved the member for profile work. + * Kept only for migration/backfill purposes. */ profileApprovedAt?: Timestamp; stripeCustomerId?: string; diff --git a/members/src/app/admin/services/admin-members.service.ts b/members/src/app/admin/services/admin-members.service.ts index 0a759fd0..902d76c7 100644 --- a/members/src/app/admin/services/admin-members.service.ts +++ b/members/src/app/admin/services/admin-members.service.ts @@ -33,6 +33,10 @@ interface ApiMemberSuccessResponse { } type ApiApproveProfileResponse = ApiMemberSuccessResponse | ApiErrorResponse; + +interface ApproveProfileRequestBody { + allowProfileEditing: boolean; +} type ApiLinkProfileResponse = ApiMemberSuccessResponse | ApiErrorResponse; type ApiListUnlinkedProfilesResult = ApiListUnlinkedProfilesResponse | ApiErrorResponse; @@ -130,9 +134,11 @@ export class AdminMembersService { ); } - async approveProfile(uid: string): Promise { + async approveProfile(uid: string, allowProfileEditing: boolean): Promise { const response = await firstValueFrom( - this.httpClient.post(`/api/admin/members/${uid}/profile/approve`, {}), + this.httpClient.post(`/api/admin/members/${uid}/profile/approve`, { + allowProfileEditing, + } satisfies ApproveProfileRequestBody), ); return toLinkedMember(response); diff --git a/members/src/app/admin/users/admin-member-detail/admin-member-detail.html b/members/src/app/admin/users/admin-member-detail/admin-member-detail.html index bfaf01bb..685864f5 100644 --- a/members/src/app/admin/users/admin-member-detail/admin-member-detail.html +++ b/members/src/app/admin/users/admin-member-detail/admin-member-detail.html @@ -94,13 +94,9 @@

Membership Details

{{ member.slug }}
} -
Profile Approval:
+
Profile Editing:
- @if (member.profileApprovedAt) { - Approved on {{ member.profileApprovedAt | date: 'MMM d, yyyy h:mm a' }} - } @else { - Not approved - } + {{ member.allowProfileEditing ? 'Enabled' : 'Disabled' }}
@@ -114,16 +110,18 @@

Membership Actions

}
- @if (!isProfileApproved()) { - - } + @if (member.membershipActive) {
} - @if (showProfileApprovalPending()) { -
-

Profile approval pending

+ @if (showProfileEditingPending()) { +
+

Profile editing not enabled

- Your membership is active, but profile creation is pending admin approval. Once approved, + Your membership is active, but profile editing has not been enabled yet. Once enabled, you'll be able to create or edit your profile here.

diff --git a/members/src/app/membership/membership.spec.ts b/members/src/app/membership/membership.spec.ts index 62a5c4c8..57308e8c 100644 --- a/members/src/app/membership/membership.spec.ts +++ b/members/src/app/membership/membership.spec.ts @@ -214,7 +214,7 @@ describe('Membership', () => { email: 'jane@example.com', uid: 'user123', membershipActive: true, - profileApprovedAt: new Date(), + allowProfileEditing: true, }, }); @@ -282,7 +282,7 @@ describe('Membership', () => { email: 'jane@example.com', uid: 'user123', membershipActive: true, - profileApprovedAt: new Date(), + allowProfileEditing: true, }, }); @@ -299,7 +299,7 @@ describe('Membership', () => { email: 'jane@example.com', uid: 'user123', membershipActive: true, - profileApprovedAt: new Date(), + allowProfileEditing: true, }, }); @@ -319,7 +319,7 @@ describe('Membership', () => { email: 'jane@example.com', uid: 'user123', membershipActive: true, - profileApprovedAt: new Date(), + allowProfileEditing: true, }, }); @@ -339,7 +339,7 @@ describe('Membership', () => { email: 'jane@example.com', uid: 'user123', membershipActive: true, - profileApprovedAt: new Date(), + allowProfileEditing: true, }, }); @@ -361,7 +361,7 @@ describe('Membership', () => { email: 'jane@example.com', uid: 'user123', membershipActive: true, - profileApprovedAt: new Date(), + allowProfileEditing: true, }, }); @@ -387,7 +387,7 @@ describe('Membership', () => { email: 'jane@example.com', uid: 'user123', membershipActive: true, - profileApprovedAt: new Date(), + allowProfileEditing: true, }, updateMemberNameShouldFail: true, }); @@ -418,7 +418,7 @@ describe('Membership', () => { expect(screen.queryByText('Create Your Doula Profile')).toBeNull(); }); - it('should not show welcome prompt without profile approval', async () => { + it('should not show welcome prompt without profile editing access', async () => { await setup({ isAuthenticated: true, hasUserDocument: true, @@ -433,7 +433,7 @@ describe('Membership', () => { expect(screen.queryByText('Welcome to the Rochester Doula Cooperative!')).toBeNull(); }); - it('should show welcome prompt when profile approval exists and name is missing', async () => { + it('should show welcome prompt when profile editing is enabled and name is missing', async () => { await setup({ isAuthenticated: true, hasUserDocument: true, @@ -442,14 +442,14 @@ describe('Membership', () => { email: 'jane@example.com', uid: 'user123', membershipActive: true, - profileApprovedAt: new Date(), + allowProfileEditing: true, }, }); expect(screen.getByText('Welcome to the Rochester Doula Cooperative!')).toBeVisible(); }); - it('should show create profile banner when profile approval exists', async () => { + it('should show create profile banner when profile editing is enabled', async () => { await setup({ isAuthenticated: true, hasUserDocument: true, @@ -464,10 +464,10 @@ describe('Membership', () => { }); expect(screen.getByText('Create Your Doula Profile')).toBeVisible(); - expect(screen.getByText(/active membership and admin approval/)).toBeVisible(); + expect(screen.getByText(/active membership and profile editing access/)).toBeVisible(); }); - it('should show approval pending message when active member is not approved', async () => { + it('should show profile editing pending message when active member is not enabled', async () => { await setup({ isAuthenticated: true, hasUserDocument: true, @@ -480,8 +480,8 @@ describe('Membership', () => { }, }); - expect(screen.getByText('Profile approval pending')).toBeVisible(); - expect(screen.getByText(/profile creation is pending admin approval/)).toBeVisible(); + expect(screen.getByText('Profile editing not enabled')).toBeVisible(); + expect(screen.getByText(/profile editing has not been enabled yet/)).toBeVisible(); expect(screen.queryByText('Create Your Doula Profile')).toBeNull(); }); }); diff --git a/members/src/app/membership/membership.ts b/members/src/app/membership/membership.ts index ceea45d3..55ba5c83 100644 --- a/members/src/app/membership/membership.ts +++ b/members/src/app/membership/membership.ts @@ -55,7 +55,7 @@ export class Membership { const userDocument = this.userDocument(); return ( userDocument?.membershipActive && - userDocument?.profileApprovedAt !== undefined && + userDocument.allowProfileEditing && !userDocument.slug && !!userDocument.name ); @@ -67,7 +67,7 @@ export class Membership { const userDocument = this.userDocument(); return ( userDocument?.membershipActive && - userDocument?.profileApprovedAt !== undefined && + userDocument.allowProfileEditing && !userDocument.slug && !userDocument.name ); @@ -84,11 +84,11 @@ export class Membership { return userDocument?.name; }); - protected showProfileApprovalPending = computed(() => { + protected showProfileEditingPending = computed(() => { const userDocument = this.userDocument(); return ( userDocument?.membershipActive && - userDocument?.profileApprovedAt === undefined && + !userDocument.allowProfileEditing && !userDocument.slug ); }); diff --git a/members/src/app/services/membership.service.ts b/members/src/app/services/membership.service.ts index 3d37c84e..1ec52888 100644 --- a/members/src/app/services/membership.service.ts +++ b/members/src/app/services/membership.service.ts @@ -17,7 +17,7 @@ export interface Member { membershipExpiresAt?: Date; slug?: string; profileCreatedAt?: Date; - profileApprovedAt?: Date; + allowProfileEditing: boolean; stripeCustomerId?: string; stripeSubscriptionId?: string; subscriptionStatus?: SubscriptionStatus; @@ -72,6 +72,7 @@ export class MembershipService { // Computed properties for easy access to specific member document fields membershipActive = computed(() => this.userDocument()?.membershipActive ?? false); + allowProfileEditing = computed(() => this.userDocument()?.allowProfileEditing ?? false); hasProfile = computed(() => { return !!this.userDocument()?.profileCreatedAt; }); @@ -410,9 +411,7 @@ export class MembershipService { ...(apiResponse.profileCreatedAt !== undefined && { profileCreatedAt: new Date(apiResponse.profileCreatedAt), }), - ...(apiResponse.profileApprovedAt !== undefined && { - profileApprovedAt: new Date(apiResponse.profileApprovedAt), - }), + allowProfileEditing: apiResponse.allowProfileEditing, ...(apiResponse.stripeCustomerId !== undefined && { stripeCustomerId: apiResponse.stripeCustomerId, }), From 8a8684bc109ff9de42149776e6e96582765c8ba7 Mon Sep 17 00:00:00 2001 From: Mark Goho Date: Wed, 25 Mar 2026 17:55:43 -0400 Subject: [PATCH 2/5] fix: restore profile editing guards and API contract Preserve auth checks on profile routes while returning allowProfileEditing consistently across the member API and related tests. Also clean up admin-member-detail coverage and make the backfill safer for existing approved members. Co-Authored-By: Claude Opus 4.6 --- .../routes/approve-profile.test.ts | 4 ++-- .../src/backfill-allow-profile-editing.ts | 23 +++++++++++-------- .../src/members-api/routes/members.test.ts | 6 ++++- .../src/members-api/schemas/member-schemas.ts | 4 ++++ .../active-members-table.spec.ts | 1 + .../admin-member-detail.spec.ts | 23 +++++++++++-------- members/src/app/app.routes.ts | 13 ++++++----- .../create-profile-wizard.spec.ts | 8 +++++++ .../personal-info-step.spec.ts | 1 + .../src/app/edit-profile/edit-profile.spec.ts | 2 ++ members/src/app/membership/membership.spec.ts | 2 +- .../profile-form-utilities.spec.ts | 4 ++++ 12 files changed, 62 insertions(+), 29 deletions(-) diff --git a/functions/src/admin-members-api/routes/approve-profile.test.ts b/functions/src/admin-members-api/routes/approve-profile.test.ts index c2bc0424..cdd70a89 100644 --- a/functions/src/admin-members-api/routes/approve-profile.test.ts +++ b/functions/src/admin-members-api/routes/approve-profile.test.ts @@ -6,7 +6,7 @@ import { } from "../../shared-api/errors/http-error.js"; import { handleRequest } from "../../test-utils/handle-request.js"; import type { MemberDocument } from "../../types/member-document.js"; -import type { ApproveProfileResult } from "../services/approve-profile.js"; +import type { SetProfileEditingPermissionResult } from "../services/approve-profile.js"; import { createAdminTestPlugin } from "../test-utils/create-admin-test-plugin.js"; describe("POST /:memberId/profile/approve", () => { @@ -34,7 +34,7 @@ describe("POST /:memberId/profile/approve", () => { }; const mockApproveProfile = mock( - (): Promise => { + (): Promise => { if (memberNotFound) { return Promise.reject( new NotFoundError(`Member with ID ${memberId} not found`), diff --git a/functions/src/backfill-allow-profile-editing.ts b/functions/src/backfill-allow-profile-editing.ts index 0c6e28c9..c8c1f9fe 100644 --- a/functions/src/backfill-allow-profile-editing.ts +++ b/functions/src/backfill-allow-profile-editing.ts @@ -1,24 +1,29 @@ -import { getFirestore } from "firebase-admin/firestore"; +import { getFirestore, Timestamp } from "firebase-admin/firestore"; import { MEMBERS_COLLECTION } from "./collections/index.js"; +const PROFILE_APPROVAL_CUTOFF = new Timestamp(0, 1); +const MAX_BATCH_SIZE = 500; + export async function backfillAllowProfileEditing(): Promise { const firestore = getFirestore(); const snapshot = await firestore .collection(MEMBERS_COLLECTION) - .where("profileApprovedAt", "!=", null) + .where("profileApprovedAt", ">", PROFILE_APPROVAL_CUTOFF) .get(); if (snapshot.empty) { return; } - const batch = firestore.batch(); + for (let index = 0; index < snapshot.docs.length; index += MAX_BATCH_SIZE) { + const batch = firestore.batch(); - for (const document of snapshot.docs) { - batch.update(document.ref, { - allowProfileEditing: true, - }); - } + for (const document of snapshot.docs.slice(index, index + MAX_BATCH_SIZE)) { + batch.update(document.ref, { + allowProfileEditing: true, + }); + } - await batch.commit(); + await batch.commit(); + } } diff --git a/functions/src/members-api/routes/members.test.ts b/functions/src/members-api/routes/members.test.ts index e892c04f..55be8a7d 100644 --- a/functions/src/members-api/routes/members.test.ts +++ b/functions/src/members-api/routes/members.test.ts @@ -47,6 +47,7 @@ describe("GET /:memberId (authenticated)", () => { createdAt: Timestamp.now(), name: "Test Member", membershipActive: true, + allowProfileEditing: true, }); }); @@ -158,9 +159,12 @@ describe("GET /:memberId (authenticated)", () => { const response = await handleRequest(testApp, request); expect(response.status).toBe(200); - const body = (await response.json()) as MemberDocument; + const body = (await response.json()) as MemberDocument & { + allowProfileEditing: boolean; + }; expect(body.uid).toBe("test-member-id"); expect(body.name).toBe("Test Member"); + expect(body.allowProfileEditing).toBe(true); }); it("should return 404 when member does not exist", async () => { diff --git a/functions/src/members-api/schemas/member-schemas.ts b/functions/src/members-api/schemas/member-schemas.ts index b5c96bf9..02f879ce 100644 --- a/functions/src/members-api/schemas/member-schemas.ts +++ b/functions/src/members-api/schemas/member-schemas.ts @@ -88,6 +88,9 @@ export const MemberResponseSchema = t.Object({ description: "Profile creation timestamp (ISO 8601)", }), ), + allowProfileEditing: t.Boolean({ + description: "Whether the member can currently create or edit a profile", + }), stripeCustomerId: t.Optional( t.String({ description: "Stripe customer ID", @@ -195,6 +198,7 @@ export function toMemberResponse( ...(document.profileCreatedAt !== undefined && { profileCreatedAt: timestampToIso(document.profileCreatedAt), }), + allowProfileEditing: document.allowProfileEditing ?? false, ...(document.stripeCustomerId !== undefined && { stripeCustomerId: document.stripeCustomerId, }), diff --git a/members/src/app/admin/users/active-members-table/active-members-table.spec.ts b/members/src/app/admin/users/active-members-table/active-members-table.spec.ts index beefd003..3f7a686e 100644 --- a/members/src/app/admin/users/active-members-table/active-members-table.spec.ts +++ b/members/src/app/admin/users/active-members-table/active-members-table.spec.ts @@ -13,6 +13,7 @@ function createMockMember(overrides: Partial = {}): ApiMember createdAt: '2024-01-15T00:00:00.000Z', isAdmin: false, membershipActive: false, + allowProfileEditing: false, ...overrides, }; } diff --git a/members/src/app/admin/users/admin-member-detail/admin-member-detail.spec.ts b/members/src/app/admin/users/admin-member-detail/admin-member-detail.spec.ts index 8ab7a4fe..71bdaf56 100644 --- a/members/src/app/admin/users/admin-member-detail/admin-member-detail.spec.ts +++ b/members/src/app/admin/users/admin-member-detail/admin-member-detail.spec.ts @@ -153,7 +153,10 @@ function createMockAdminMembersService({ }), approveProfile: shouldFailApproveProfile ? vi.fn().mockRejectedValue(new Error('Failed')) - : vi.fn().mockResolvedValue(memberToUse), + : vi.fn().mockImplementation(async (_uid: string, allowProfileEditing: boolean) => ({ + ...memberToUse, + allowProfileEditing, + })), listUnlinkedProfiles: overrideListUnlinkedProfiles ?? (shouldFailUnlinkedProfilesLoad @@ -567,7 +570,7 @@ describe('AdminUserDetail', () => { expect(await screen.findByText('test-slug')).toBeVisible(); }); - it('should keep profile approval state independent from lazy profile controls', async () => { + it('should keep profile editing state independent from lazy profile controls', async () => { const member = createMockMember({ slug: 'test-slug', membershipActive: true, @@ -576,7 +579,7 @@ describe('AdminUserDetail', () => { await setup({ member }); - expect(await screen.findByText(/Approved on Mar 15, 2024/)).toBeVisible(); + expect(await screen.findByText('Enabled')).toBeVisible(); expect(await screen.findByRole('button', { name: 'Load Profile Status' })).toBeVisible(); }); @@ -744,7 +747,7 @@ describe('AdminUserDetail', () => { expect(await screen.findByText('Disabled')).toBeVisible(); }); - it('should show profile approval timestamp for approved linked members before profile load', async () => { + it('should show enabled profile editing state for approved linked members before profile load', async () => { const member = createMockMember({ slug: 'test-slug', allowProfileEditing: true, @@ -752,7 +755,7 @@ describe('AdminUserDetail', () => { await setup({ member }); - expect(await screen.findByText(/Approved on Mar 15, 2024/)).toBeVisible(); + expect(await screen.findByText('Enabled')).toBeVisible(); }); it('should still show link section approval helper for unlinked members without approval', async () => { @@ -833,7 +836,7 @@ describe('AdminUserDetail', () => { expect(await screen.findByText('Disabled')).toBeVisible(); }); - it('should display approved status for linked profile members', async () => { + it('should display enabled status for linked profile members', async () => { const member = createMockMember({ slug: 'test-slug', allowProfileEditing: true, @@ -841,7 +844,7 @@ describe('AdminUserDetail', () => { await setup({ member }); - expect(await screen.findByText(/Approved on Mar 15, 2024/)).toBeVisible(); + expect(await screen.findByText('Enabled')).toBeVisible(); }); it('should show no profile approval pending message for approved members', async () => { @@ -1037,7 +1040,7 @@ describe('AdminUserDetail', () => { const { user, mockAdminMembersService } = await setup({ member }); await user.click(await screen.findByRole('button', { name: 'Enable Profile Editing' })); - await user.click(screen.getByRole('button', { name: 'Enable Profile Editing' })); + await user.click(screen.getByRole('button', { name: 'Cancel' }).nextElementSibling as HTMLButtonElement); expect(mockAdminMembersService.approveProfile).toHaveBeenCalledWith('test-uid-123', true); expect(await screen.findByText('Profile editing enabled successfully')).toBeVisible(); @@ -1049,7 +1052,7 @@ describe('AdminUserDetail', () => { const { user, mockAdminMembersService } = await setup({ member }); await user.click(await screen.findByRole('button', { name: 'Disable Profile Editing' })); - await user.click(screen.getByRole('button', { name: 'Disable Profile Editing' })); + await user.click(screen.getByRole('button', { name: 'Cancel' }).nextElementSibling as HTMLButtonElement); expect(mockAdminMembersService.approveProfile).toHaveBeenCalledWith('test-uid-123', false); expect(await screen.findByText('Profile editing disabled successfully')).toBeVisible(); @@ -1065,7 +1068,7 @@ describe('AdminUserDetail', () => { }); await user.click(await screen.findByRole('button', { name: 'Enable Profile Editing' })); - await user.click(screen.getByRole('button', { name: 'Enable Profile Editing' })); + await user.click(screen.getByRole('button', { name: 'Cancel' }).nextElementSibling as HTMLButtonElement); expect(await screen.findByText('Failed to update profile editing permission.')).toBeVisible(); }); diff --git a/members/src/app/app.routes.ts b/members/src/app/app.routes.ts index 7db42338..d55ff4cc 100644 --- a/members/src/app/app.routes.ts +++ b/members/src/app/app.routes.ts @@ -7,6 +7,10 @@ import { wizardStepGuard } from './create-profile-wizard/guards/wizard-step.guar // Guards for different authentication states const redirectUnauthorizedToSignIn = () => redirectUnauthorizedTo(['sign-in']); const redirectToMembership = () => redirectLoggedInTo(['membership']); +const authThenProfileEditingGuard = [ + ...canActivate(redirectUnauthorizedToSignIn).canActivate, + profileEditingGuard, +]; export const routes: Routes = [ { path: '', pathMatch: 'full', redirectTo: 'sign-in' }, @@ -19,8 +23,7 @@ export const routes: Routes = [ }, { path: 'profile/create', - ...canActivate(redirectUnauthorizedToSignIn), - canActivate: [profileEditingGuard], + canActivate: authThenProfileEditingGuard, loadComponent: () => import('./create-profile-wizard/create-profile-wizard').then((m) => m.CreateProfileWizard), children: [ @@ -77,15 +80,13 @@ export const routes: Routes = [ { path: 'profile', loadComponent: () => import('./edit-profile/edit-profile').then((m) => m.EditProfile), - ...canActivate(redirectUnauthorizedToSignIn), - canActivate: [profileEditingGuard], + canActivate: authThenProfileEditingGuard, }, { path: 'profile/image', loadComponent: () => import('./edit-profile-image/edit-profile-image').then((m) => m.EditProfileImage), - ...canActivate(redirectUnauthorizedToSignIn), - canActivate: [profileEditingGuard], + canActivate: authThenProfileEditingGuard, }, // Admin routes (require authentication and admin claim) diff --git a/members/src/app/create-profile-wizard/create-profile-wizard.spec.ts b/members/src/app/create-profile-wizard/create-profile-wizard.spec.ts index c96f667c..60395415 100644 --- a/members/src/app/create-profile-wizard/create-profile-wizard.spec.ts +++ b/members/src/app/create-profile-wizard/create-profile-wizard.spec.ts @@ -31,6 +31,7 @@ describe('CreateProfileWizard', () => { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: true, slug: 'test-user', profileCreatedAt: new Date(), }; @@ -52,6 +53,7 @@ describe('CreateProfileWizard', () => { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: true, slug: 'test-user', }; mockMembershipService.userDocument.set(member); @@ -72,6 +74,7 @@ describe('CreateProfileWizard', () => { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: true, slug: 'test-user', }; mockMembershipService.userDocument.set(member); @@ -92,6 +95,7 @@ describe('CreateProfileWizard', () => { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: true, slug: 'test-user', }; mockMembershipService.userDocument.set(member); @@ -112,6 +116,7 @@ describe('CreateProfileWizard', () => { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: true, slug: 'test-user', }; mockMembershipService.userDocument.set(member); @@ -141,6 +146,7 @@ describe('CreateProfileWizard', () => { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: true, slug: 'test-user', } as Member; mockMembershipService.userDocument.set(member); @@ -166,6 +172,7 @@ describe('CreateProfileWizard', () => { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: true, slug: 'test-user', }; mockMembershipService.userDocument.set(member); @@ -219,6 +226,7 @@ async function setup({ userDocumentLoading = false }: SetupOptions = {}) { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: true, slug: 'test-user', }; diff --git a/members/src/app/create-profile-wizard/steps/personal-info-step/personal-info-step.spec.ts b/members/src/app/create-profile-wizard/steps/personal-info-step/personal-info-step.spec.ts index dee10195..af161ec8 100644 --- a/members/src/app/create-profile-wizard/steps/personal-info-step/personal-info-step.spec.ts +++ b/members/src/app/create-profile-wizard/steps/personal-info-step/personal-info-step.spec.ts @@ -65,6 +65,7 @@ async function setup() { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: true, slug: 'test-user', }; diff --git a/members/src/app/edit-profile/edit-profile.spec.ts b/members/src/app/edit-profile/edit-profile.spec.ts index dfe48395..6742526c 100644 --- a/members/src/app/edit-profile/edit-profile.spec.ts +++ b/members/src/app/edit-profile/edit-profile.spec.ts @@ -356,6 +356,7 @@ async function setup({ slug: 'jane-doe', profileCreatedAt: new Date(0), membershipActive: true, + allowProfileEditing: true, }; } else { mockMemberDocument = { @@ -364,6 +365,7 @@ async function setup({ createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: true, }; } diff --git a/members/src/app/membership/membership.spec.ts b/members/src/app/membership/membership.spec.ts index 57308e8c..973b8c20 100644 --- a/members/src/app/membership/membership.spec.ts +++ b/members/src/app/membership/membership.spec.ts @@ -459,7 +459,7 @@ describe('Membership', () => { uid: 'user123', membershipActive: true, name: 'Jane Doe', - profileApprovedAt: new Date(), + allowProfileEditing: true, }, }); diff --git a/members/src/app/shared/profile-form/profile-form-utilities.spec.ts b/members/src/app/shared/profile-form/profile-form-utilities.spec.ts index 8dc66afd..0f09153b 100644 --- a/members/src/app/shared/profile-form/profile-form-utilities.spec.ts +++ b/members/src/app/shared/profile-form/profile-form-utilities.spec.ts @@ -108,6 +108,7 @@ describe('Profile Form Utilities', () => { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: false, slug: 'test-user', }; @@ -126,6 +127,7 @@ describe('Profile Form Utilities', () => { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: false, }; initializeCreateProfileForm(form, member); @@ -146,6 +148,7 @@ describe('Profile Form Utilities', () => { createdAt: new Date(0), isAdmin: false, membershipActive: true, + allowProfileEditing: false, }; initializeCreateProfileForm(form, member); @@ -166,6 +169,7 @@ describe('Profile Form Utilities', () => { createdAt: new Date(0), isAdmin: false, membershipActive: false, + allowProfileEditing: false, }; initializeCreateProfileForm(form, member); From 15222cea25f8fa1c31c2ac97cb60deb03521def7 Mon Sep 17 00:00:00 2001 From: Mark Goho Date: Wed, 25 Mar 2026 18:19:28 -0400 Subject: [PATCH 3/5] fix(members): align profile e2e mocks with editing permission flow Update the profile creation and edit-profile E2E specs to navigate through the membership UI and keep mocked member documents in sync with slug and profile state changes. Co-Authored-By: Claude Opus 4.6 --- members/e2e/tests/create-profile-flow.spec.ts | 107 ++++++++++-------- members/e2e/tests/view-profile.spec.ts | 54 +++++---- 2 files changed, 91 insertions(+), 70 deletions(-) diff --git a/members/e2e/tests/create-profile-flow.spec.ts b/members/e2e/tests/create-profile-flow.spec.ts index d195fde2..84692589 100644 --- a/members/e2e/tests/create-profile-flow.spec.ts +++ b/members/e2e/tests/create-profile-flow.spec.ts @@ -1,5 +1,5 @@ import { test } from '../fixtures/regular-user-auth.fixture'; -import { expect } from '@playwright/test'; +import { expect, type Page } from '@playwright/test'; import type { ProfileData } from '../../src/app/types/profile-data'; import { EditProfilePage } from '../pages/edit-profile.page'; import type { ApiMemberResponse } from '../../src/app/api-types/members-api.types'; @@ -12,7 +12,13 @@ const mockMemberDocument: ApiMemberResponse = { isAdmin: false, subscriptionStart: '2024-01-01T00:00:00.000Z', membershipActive: true, + allowProfileEditing: true, +}; + +const mockCreatedMemberDocument: ApiMemberResponse = { + ...mockMemberDocument, slug: 'test-user', + profileCreatedAt: '2024-01-02T00:00:00.000Z', }; const mockProfileData: ProfileData = { @@ -25,20 +31,25 @@ const mockProfileData: ProfileData = { draft: true, }; -function setupApiMocks(page: import('@playwright/test').Page) { +function createCurrentMemberDocument(): ApiMemberResponse { + return { ...mockMemberDocument }; +} + +let currentMemberDocument = createCurrentMemberDocument(); + +function setupApiMocks(page: Page) { + currentMemberDocument = createCurrentMemberDocument(); + return Promise.all([ - // Mock member document (no profileCreatedAt so wizard doesn't redirect) page.route('**/api/members/*', async (route) => { await (route.request().method() === 'GET' ? route.fulfill({ status: 200, contentType: 'application/json', - body: JSON.stringify(mockMemberDocument), + body: JSON.stringify(currentMemberDocument), }) : route.continue()); }), - - // Mock slug availability check (slug is available) page.route(/\/api\/profiles\/slugs\/check(\?|$)/, async (route) => { await route.fulfill({ status: 200, @@ -46,57 +57,64 @@ function setupApiMocks(page: import('@playwright/test').Page) { body: JSON.stringify({ available: true }), }); }), - - // Mock slug update page.route('**/api/profiles/slugs', async (route) => { - await (route.request().method() === 'POST' - ? route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ success: true, slug: 'test-user' }), - }) - : route.continue()); - }), + if (route.request().method() === 'POST') { + currentMemberDocument = { + ...currentMemberDocument, + slug: 'test-user', + }; + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ success: true, slug: 'test-user' }), + }); + return; + } - // Mock profile creation (POST) and profile fetch (GET) + await route.continue(); + }), page.route('**/api/profiles/test-user', async (route) => { const method = route.request().method(); if (method === 'POST') { + currentMemberDocument = mockCreatedMemberDocument; await route.fulfill({ status: 201, contentType: 'application/json', body: JSON.stringify({ success: true, profile: mockProfileData }), }); - } else if (method === 'GET') { + return; + } + + if (method === 'GET') { await route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(mockProfileData), }); - } else { - await route.continue(); + return; } + + await route.continue(); }), ]); } -async function walkThroughWizard(page: import('@playwright/test').Page) { - await page.goto('/profile/create'); +async function walkThroughWizard(page: Page) { + await page.goto('/membership'); + await page.getByRole('button', { name: 'Create Profile' }).click(); // === Step 1: Personal Info === - await page - .getByRole('heading', { name: /Personal Information/i, level: 2 }) - .waitFor({ state: 'visible' }); - // Name is pre-filled from member document, but we clear and type to trigger slug validator + await page.getByRole('heading', { name: /Personal Information/i, level: 2 }).waitFor({ + state: 'visible', + }); await page.getByLabel(/^Name/i).fill('Test User'); - // Wait for async slug validator to resolve (shows URL preview) await expect(page.getByText(/Your profile URL/i)).toBeVisible({ timeout: 10_000 }); await page.getByRole('button', { name: 'Next' }).click(); // === Step 2: Tags === - await page - .getByRole('heading', { name: /Services & Specialties/i, level: 2 }) - .waitFor({ state: 'visible' }); + await page.getByRole('heading', { name: /Services & Specialties/i, level: 2 }).waitFor({ + state: 'visible', + }); await page.getByLabel('Birth Doula').check(); await page.getByRole('button', { name: 'Next' }).click(); @@ -106,25 +124,24 @@ async function walkThroughWizard(page: import('@playwright/test').Page) { await page.getByRole('button', { name: 'Next' }).click(); // === Step 4: Contact === - await page - .getByRole('heading', { name: /Contact Information/i, level: 2 }) - .waitFor({ state: 'visible' }); + await page.getByRole('heading', { name: /Contact Information/i, level: 2 }).waitFor({ + state: 'visible', + }); await page.getByRole('button', { name: 'Next' }).click(); // === Step 5: Image (skip) === - await page - .getByRole('heading', { name: /Profile Photo/i, level: 2 }) - .waitFor({ state: 'visible' }); + await page.getByRole('heading', { name: /Profile Photo/i, level: 2 }).waitFor({ + state: 'visible', + }); await page.getByRole('button', { name: /Skip for now/i }).click(); - // === Step 6: Preview (triggers profile creation POST on Finish) === - await page - .getByRole('heading', { name: /Preview Your Profile/i, level: 2 }) - .waitFor({ state: 'visible' }); + // === Step 6: Preview === + await page.getByRole('heading', { name: /Preview Your Profile/i, level: 2 }).waitFor({ + state: 'visible', + }); await page.getByRole('button', { name: 'Finish' }).click(); - // Wait for navigation to the edit profile page - await page.waitForURL('/profile', { timeout: 10_000 }); + await expect(page).toHaveURL(/\/profile$/); } test.describe('Create Profile → Edit Profile Flow', () => { @@ -132,11 +149,8 @@ test.describe('Create Profile → Edit Profile Flow', () => { authenticatedUserPage, }) => { await setupApiMocks(authenticatedUserPage); - - // === Walk through the 6-step wizard === await walkThroughWizard(authenticatedUserPage); - // === Profile form should load with data from the created profile === const editProfilePage = new EditProfilePage(authenticatedUserPage); await editProfilePage.waitForProfileForm(); await expect(editProfilePage.titleInput).toHaveValue('Test User'); @@ -147,11 +161,8 @@ test.describe('Create Profile → Edit Profile Flow', () => { authenticatedUserPage, }) => { await setupApiMocks(authenticatedUserPage); - - // === Walk through the 6-step wizard === await walkThroughWizard(authenticatedUserPage); - // === Profile form should load immediately with correct data === const editProfilePage = new EditProfilePage(authenticatedUserPage); await editProfilePage.waitForProfileForm(); await expect(editProfilePage.titleInput).toHaveValue('Test User'); diff --git a/members/e2e/tests/view-profile.spec.ts b/members/e2e/tests/view-profile.spec.ts index 3cfaaaf3..0416b6a5 100644 --- a/members/e2e/tests/view-profile.spec.ts +++ b/members/e2e/tests/view-profile.spec.ts @@ -1,5 +1,5 @@ import { test } from '../fixtures/regular-user-auth.fixture'; -import { expect } from '@playwright/test'; +import { expect, type Page } from '@playwright/test'; import type { ProfileData } from '../../src/app/types/profile-data'; import { EditProfilePage } from '../pages/edit-profile.page'; import type { ApiMemberResponse } from '../../src/app/api-types/members-api.types'; @@ -37,24 +37,14 @@ const mockMemberDocument: ApiMemberResponse = { isAdmin: false, subscriptionStart: '2024-01-01T00:00:00.000Z', membershipActive: true, + allowProfileEditing: true, slug: 'test-user', + profileCreatedAt: '2024-01-02T00:00:00.000Z', }; -test.describe('View Profile', () => { - /** - * Tests the edit profile page with properly mocked API endpoints. - * Uses regular-user-auth.fixture for non-admin user (test-user@doulacooperative.com). - * Mocks: - * - GET /api/members/:memberId (MembershipService - provides membershipActive + slug) - * - GET /api/profiles/:slug (ProfileService - loads profile data) - * - PUT /api/profiles/:slug (ProfileService - saves profile updates) - */ - - test('user views their profile page', async ({ authenticatedUserPage }) => { - const userSlug = 'test-user'; - - // Mock GET /api/members/:memberId for member document lookup - await authenticatedUserPage.route('**/api/members/*', async (route) => { +function setupApiMocks(page: Page, userSlug: string) { + return Promise.all([ + page.route('**/api/members/*', async (route) => { await (route.request().method() === 'GET' ? route.fulfill({ status: 200, @@ -62,10 +52,8 @@ test.describe('View Profile', () => { body: JSON.stringify(mockMemberDocument), }) : route.continue()); - }); - - // Mock GET /api/profiles/:slug - await authenticatedUserPage.route(`**/api/profiles/${userSlug}`, async (route) => { + }), + page.route(`**/api/profiles/${userSlug}`, async (route) => { await (route.request().method() === 'GET' ? route.fulfill({ status: 200, @@ -73,10 +61,32 @@ test.describe('View Profile', () => { body: JSON.stringify(mockProfileData), }) : route.continue()); - }); + }), + ]); +} + +async function goToEditProfile(page: Page) { + await page.goto('/membership'); + await page.getByRole('link', { name: 'Edit Profile' }).click(); +} + +test.describe('View Profile', () => { + /** + * Tests the edit profile page with properly mocked API endpoints. + * Uses regular-user-auth.fixture for non-admin user (test-user@doulacooperative.com). + * Mocks: + * - GET /api/members/:memberId (MembershipService - provides membershipActive + slug) + * - GET /api/profiles/:slug (ProfileService - loads profile data) + * - PUT /api/profiles/:slug (ProfileService - saves profile updates) + */ + + test('user views their profile page', async ({ authenticatedUserPage }) => { + const userSlug = 'test-user'; + + await setupApiMocks(authenticatedUserPage, userSlug); const editProfilePage = new EditProfilePage(authenticatedUserPage); - await editProfilePage.goto(); + await goToEditProfile(authenticatedUserPage); await editProfilePage.waitForProfileForm(); // === Verify Page Structure === From 0e43b6e79a63eea375d954967c31434566063a5d Mon Sep 17 00:00:00 2001 From: Mark Goho Date: Thu, 26 Mar 2026 10:12:54 -0400 Subject: [PATCH 4/5] fix(members): add profile editing flag to e2e member mocks Update the remaining E2E member API mocks to include allowProfileEditing so the CI e2e typecheck matches the current API contract. Co-Authored-By: Claude Opus 4.6 --- members/e2e/tests/admin-member-detail.spec.ts | 1 + members/e2e/tests/admin-members.spec.ts | 3 +++ members/e2e/tests/cancel-membership.spec.ts | 1 + members/e2e/tests/claim-profile.spec.ts | 1 + members/e2e/tests/newsletter-preference.spec.ts | 1 + 5 files changed, 7 insertions(+) diff --git a/members/e2e/tests/admin-member-detail.spec.ts b/members/e2e/tests/admin-member-detail.spec.ts index fd1c274e..de6793e8 100644 --- a/members/e2e/tests/admin-member-detail.spec.ts +++ b/members/e2e/tests/admin-member-detail.spec.ts @@ -14,6 +14,7 @@ const mockMember: ApiMemberResponse = { createdAt: '2024-01-15T10:30:00.000Z', isAdmin: false, membershipActive: true, + allowProfileEditing: false, subscriptionStart: '2024-01-15T10:30:00.000Z', membershipExpiresAt: '2025-01-15T10:30:00.000Z', slug: 'test-member', diff --git a/members/e2e/tests/admin-members.spec.ts b/members/e2e/tests/admin-members.spec.ts index ebff6890..b9e80ba1 100644 --- a/members/e2e/tests/admin-members.spec.ts +++ b/members/e2e/tests/admin-members.spec.ts @@ -18,6 +18,7 @@ const mockMembers: ApiMemberResponse[] = [ createdAt: '2024-01-15T10:30:00.000Z', isAdmin: false, membershipActive: true, + allowProfileEditing: false, subscriptionStart: '2024-01-15T10:30:00.000Z', membershipExpiresAt: '2025-01-15T10:30:00.000Z', }, @@ -28,6 +29,7 @@ const mockMembers: ApiMemberResponse[] = [ createdAt: '2024-02-20T14:15:00.000Z', isAdmin: false, membershipActive: false, + allowProfileEditing: false, subscriptionStart: '2024-02-20T14:15:00.000Z', membershipExpiresAt: '2024-08-20T14:15:00.000Z', }, @@ -37,6 +39,7 @@ const mockMembers: ApiMemberResponse[] = [ createdAt: '2024-03-10T09:00:00.000Z', isAdmin: false, membershipActive: true, + allowProfileEditing: false, subscriptionStart: '2024-03-10T09:00:00.000Z', }, ]; diff --git a/members/e2e/tests/cancel-membership.spec.ts b/members/e2e/tests/cancel-membership.spec.ts index 155c8505..6fdd7137 100644 --- a/members/e2e/tests/cancel-membership.spec.ts +++ b/members/e2e/tests/cancel-membership.spec.ts @@ -19,6 +19,7 @@ const mockActiveStripeMember: ApiMemberResponse = { isAdmin: false, subscriptionStart: '2024-01-15T00:00:00.000Z', membershipActive: true, + allowProfileEditing: true, stripeCustomerId: 'cus_test123', stripeSubscriptionId: 'sub_test456', subscriptionStatus: 'active', diff --git a/members/e2e/tests/claim-profile.spec.ts b/members/e2e/tests/claim-profile.spec.ts index 5071125f..0df5dfb9 100644 --- a/members/e2e/tests/claim-profile.spec.ts +++ b/members/e2e/tests/claim-profile.spec.ts @@ -25,6 +25,7 @@ test.describe('Claim Profile Flow', () => { isAdmin: false, subscriptionStart: '2024-01-01T00:00:00.000Z', membershipActive: true, + allowProfileEditing: false, // No slug - profile not set up yet }; diff --git a/members/e2e/tests/newsletter-preference.spec.ts b/members/e2e/tests/newsletter-preference.spec.ts index f3f0f799..4990fc79 100644 --- a/members/e2e/tests/newsletter-preference.spec.ts +++ b/members/e2e/tests/newsletter-preference.spec.ts @@ -14,6 +14,7 @@ const mockMemberDocumentBase: ApiMemberResponse = { isAdmin: false, subscriptionStart: '2024-01-01T00:00:00.000Z', membershipActive: true, + allowProfileEditing: true, slug: 'test-user', }; From c4d37e43116342b1d9ad07a507e03002ce33b35b Mon Sep 17 00:00:00 2001 From: Mark Goho Date: Thu, 26 Mar 2026 11:31:58 -0400 Subject: [PATCH 5/5] fix: resolve profile editing guard and backfill rollout issues Prevent profile-editing guard redirects while member data is still loading, strengthen the admin profile-editing route tests, and convert the editing-permission backfill into a runnable script with clear local-emulator behavior. Co-Authored-By: Claude Opus 4.6 --- functions/package.json | 3 +- .../plugins/admin-members-plugin.ts | 2 +- .../routes/approve-profile.test.ts | 205 +++++++++++------- .../admin-members-api/services/interface.ts | 1 + .../scripts/backfill-allow-profile-editing.ts | 116 ++++++++++ .../admin-member-detail.service.ts | 2 +- .../admin-member-detail.spec.ts | 17 +- .../profile-editing.guard.integration.spec.ts | 104 ++++++++- .../src/app/guards/profile-editing.guard.ts | 20 +- 9 files changed, 364 insertions(+), 106 deletions(-) create mode 100644 functions/src/scripts/backfill-allow-profile-editing.ts diff --git a/functions/package.json b/functions/package.json index f50bd19a..9b315811 100644 --- a/functions/package.json +++ b/functions/package.json @@ -15,7 +15,8 @@ "cleanup-test-data": "bun run src/scripts/cleanup-test-data.ts", "test-webhook": "bun run src/scripts/test-stripe-webhook.ts", "sync-profile-timestamps": "bun run src/scripts/sync-profile-timestamps.ts", - "seed-claim-test": "bun run src/scripts/seed-claim-test.ts" + "seed-claim-test": "bun run src/scripts/seed-claim-test.ts", + "backfill-allow-profile-editing": "bun run src/scripts/backfill-allow-profile-editing.ts" }, "engines": { "node": "24" diff --git a/functions/src/admin-members-api/plugins/admin-members-plugin.ts b/functions/src/admin-members-api/plugins/admin-members-plugin.ts index f9e1bae4..1e705a7f 100644 --- a/functions/src/admin-members-api/plugins/admin-members-plugin.ts +++ b/functions/src/admin-members-api/plugins/admin-members-plugin.ts @@ -192,7 +192,7 @@ export function createAdminMembersPlugin(services?: PartialServices) { response: ToggleProfileDraftApiResponseSchema, }, ) - // POST /:memberId/profile/approve - Approve member for profile work (served at /api/admin/members/:memberId/profile/approve) + // POST /:memberId/profile/approve - Enable or disable profile editing permission (served at /api/admin/members/:memberId/profile/approve) .post( "/profile/approve", async ({ params, body, adminToken, memberAdminService, logger, set }) => diff --git a/functions/src/admin-members-api/routes/approve-profile.test.ts b/functions/src/admin-members-api/routes/approve-profile.test.ts index cdd70a89..80ad4397 100644 --- a/functions/src/admin-members-api/routes/approve-profile.test.ts +++ b/functions/src/admin-members-api/routes/approve-profile.test.ts @@ -11,6 +11,7 @@ import { createAdminTestPlugin } from "../test-utils/create-admin-test-plugin.js describe("POST /:memberId/profile/approve", () => { interface SetupOptions { + body?: Record; memberId?: string; authToken?: string | null; memberNotFound?: boolean; @@ -19,22 +20,21 @@ describe("POST /:memberId/profile/approve", () => { } function setup({ + body = { allowProfileEditing: true }, memberId = "test-member-id", authToken = "admin-token", memberNotFound = false, serverError = false, isAdminLookupFails = false, }: SetupOptions = {}) { - const defaultMember: MemberDocument = { - uid: memberId, - email: "member@example.com", - createdAt: Timestamp.now(), - membershipActive: true, - allowProfileEditing: true, - }; - const mockApproveProfile = mock( - (): Promise => { + ({ + memberId: approvedMemberId, + allowProfileEditing, + }: { + memberId: string; + allowProfileEditing: boolean; + }): Promise => { if (memberNotFound) { return Promise.reject( new NotFoundError(`Member with ID ${memberId} not found`), @@ -43,7 +43,14 @@ describe("POST /:memberId/profile/approve", () => { if (serverError) { return Promise.reject(new Error("Firestore unavailable")); } - return Promise.resolve({ member: defaultMember }); + const member: MemberDocument = { + uid: approvedMemberId, + email: "member@example.com", + createdAt: Timestamp.now(), + membershipActive: true, + allowProfileEditing, + }; + return Promise.resolve({ member }); }, ); @@ -59,104 +66,144 @@ describe("POST /:memberId/profile/approve", () => { }, }); - const headers: Record = {}; + const headers: Record = { + "Content-Type": "application/json", + }; if (authToken) { headers["Authorization"] = `Bearer ${authToken}`; } - const request = new Request( - `http://localhost/${memberId}/profile/approve`, - { - method: "POST", - headers: { - ...headers, - "Content-Type": "application/json", - }, - body: JSON.stringify({ allowProfileEditing: true }), - }, - ); + const request = new Request(`http://localhost/${memberId}/profile/approve`, { + method: "POST", + headers, + body: JSON.stringify(body), + }); return { testApp, request }; } - it("should return 401 when no authorization header is provided", async () => { - const { testApp, request } = setup({ authToken: null }); + describe("Authentication", () => { + it("should return 401 when no authorization header is provided", async () => { + const { testApp, request } = setup({ authToken: null }); + + const response = await handleRequest(testApp, request); - const response = await handleRequest(testApp, request); + expect(response.status).toBe(401); + const body = (await response.json()) as { error?: string }; + expect(body.error).toBe("Missing Authorization header"); + }); + + it("should return 403 when non-admin user tries to approve profile work", async () => { + const { testApp, request } = setup({ authToken: "non-admin-token" }); - expect(response.status).toBe(401); - const body = (await response.json()) as { error?: string }; - expect(body.error).toBe("Missing Authorization header"); + const response = await handleRequest(testApp, request); + + expect(response.status).toBe(403); + const body = (await response.json()) as { error?: string }; + expect(body.error).toBe("Admin privileges required"); + }); }); - it("should return 403 when non-admin user tries to approve profile work", async () => { - const { testApp, request } = setup({ authToken: "non-admin-token" }); + describe("Input validation", () => { + it("should return 422 when allowProfileEditing is missing", async () => { + const { testApp, request } = setup({ body: {} }); + + const response = await handleRequest(testApp, request); + + expect(response.status).toBe(422); + }); - const response = await handleRequest(testApp, request); + it("should return 422 when allowProfileEditing is not a boolean", async () => { + const { testApp, request } = setup({ + body: { allowProfileEditing: "yes" }, + }); - expect(response.status).toBe(403); - const body = (await response.json()) as { error?: string }; - expect(body.error).toBe("Admin privileges required"); + const response = await handleRequest(testApp, request); + + expect(response.status).toBe(422); + }); }); - it("should return success with updated member data", async () => { - const { testApp, request } = setup(); + describe("Success", () => { + it("should return success with updated member data", async () => { + const { testApp, request } = setup(); + + const response = await handleRequest(testApp, request); + + expect(response.status).toBe(200); + const body = (await response.json()) as { + success?: boolean; + member?: { + uid?: string; + email?: string; + allowProfileEditing?: boolean; + isAdmin?: boolean; + }; + }; + expect(body.success).toBe(true); + expect(body.member?.uid).toBe("test-member-id"); + expect(body.member?.email).toBe("member@example.com"); + expect(body.member?.allowProfileEditing).toBe(true); + expect(body.member?.isAdmin).toBe(false); + }); - const response = await handleRequest(testApp, request); + it("should return success when profile editing is disabled", async () => { + const { testApp, request } = setup({ + body: { allowProfileEditing: false }, + }); - expect(response.status).toBe(200); - const body = (await response.json()) as { - success?: boolean; - member?: { - uid?: string; - email?: string; - allowProfileEditing?: boolean; - isAdmin?: boolean; + const response = await handleRequest(testApp, request); + + expect(response.status).toBe(200); + const body = (await response.json()) as { + success?: boolean; + member?: { + allowProfileEditing?: boolean; + }; }; - }; - expect(body.success).toBe(true); - expect(body.member?.uid).toBe("test-member-id"); - expect(body.member?.email).toBe("member@example.com"); - expect(body.member?.allowProfileEditing).toBe(true); - expect(body.member?.isAdmin).toBe(false); - }); + expect(body.success).toBe(true); + expect(body.member?.allowProfileEditing).toBe(false); + }); - it("should still return success when isAdmin lookup fails after approval", async () => { - const { testApp, request } = setup({ isAdminLookupFails: true }); + it("should still return success when isAdmin lookup fails after approval", async () => { + const { testApp, request } = setup({ isAdminLookupFails: true }); - const response = await handleRequest(testApp, request); + const response = await handleRequest(testApp, request); - expect(response.status).toBe(200); - const body = (await response.json()) as { - success?: boolean; - member?: { - allowProfileEditing?: boolean; - isAdmin?: boolean; + expect(response.status).toBe(200); + const body = (await response.json()) as { + success?: boolean; + member?: { + allowProfileEditing?: boolean; + isAdmin?: boolean; + }; }; - }; - expect(body.success).toBe(true); - expect(body.member?.allowProfileEditing).toBe(true); - expect(body.member?.isAdmin).toBe(false); + expect(body.success).toBe(true); + expect(body.member?.allowProfileEditing).toBe(true); + expect(body.member?.isAdmin).toBe(false); + }); }); - it("should return 404 when member not found", async () => { - const { testApp, request } = setup({ memberNotFound: true }); + describe("Error handling", () => { + it("should return 404 when member not found", async () => { + const { testApp, request } = setup({ memberNotFound: true }); - const response = await handleRequest(testApp, request); + const response = await handleRequest(testApp, request); - expect(response.status).toBe(404); - const body = (await response.json()) as { error?: string }; - expect(body.error).toContain("not found"); - }); + expect(response.status).toBe(404); + const body = (await response.json()) as { error?: string }; + expect(body.error).toContain("not found"); + }); - it("should return 500 for unexpected errors", async () => { - const { testApp, request } = setup({ serverError: true }); + it("should return 500 for unexpected errors", async () => { + const { testApp, request } = setup({ serverError: true }); - const response = await handleRequest(testApp, request); + const response = await handleRequest(testApp, request); - expect(response.status).toBe(500); - const body = (await response.json()) as { error?: string }; - expect(body.error).toBeDefined(); - expect(body.error).not.toContain("Firestore unavailable"); + expect(response.status).toBe(500); + const body = (await response.json()) as { error?: string }; + expect(body.error).toBeDefined(); + expect(body.error).not.toContain("Firestore unavailable"); + }); }); }); diff --git a/functions/src/admin-members-api/services/interface.ts b/functions/src/admin-members-api/services/interface.ts index a42d4702..b326e0d5 100644 --- a/functions/src/admin-members-api/services/interface.ts +++ b/functions/src/admin-members-api/services/interface.ts @@ -125,6 +125,7 @@ export interface MemberAdminService { * * @param options - Member ID and desired permission state * @returns Promise resolving to updated member document + * @throws NotFoundError if member does not exist */ approveProfile(options: { memberId: string; diff --git a/functions/src/scripts/backfill-allow-profile-editing.ts b/functions/src/scripts/backfill-allow-profile-editing.ts new file mode 100644 index 00000000..43baaeea --- /dev/null +++ b/functions/src/scripts/backfill-allow-profile-editing.ts @@ -0,0 +1,116 @@ +/* eslint-disable unicorn/no-process-exit */ +/* eslint-disable unicorn/prefer-top-level-await */ + +import { getApps, initializeApp } from "firebase-admin/app"; +import { getFirestore, Timestamp } from "firebase-admin/firestore"; +import { Socket } from "node:net"; +import { MEMBERS_COLLECTION } from "../collections/index.js"; + +const USE_EMULATOR = process.env["USE_EMULATOR"] !== "false"; +const PROFILE_APPROVAL_CUTOFF = new Timestamp(0, 1); +const MAX_BATCH_SIZE = 500; + +function isEmulatorReachable(port: number, host = "127.0.0.1"): Promise { + return new Promise((resolve) => { + const socket = new Socket(); + + const cleanup = () => { + socket.removeAllListeners(); + socket.destroy(); + }; + + socket.setTimeout(500); + socket.once("connect", () => { + cleanup(); + resolve(true); + }); + socket.once("timeout", () => { + cleanup(); + resolve(false); + }); + socket.once("error", () => { + cleanup(); + resolve(false); + }); + + socket.connect(port, host); + }); +} + +async function configureFirestoreTarget(): Promise { + if (!USE_EMULATOR) { + return true; + } + + const emulatorReady = await isEmulatorReachable(8090); + if (!emulatorReady) { + console.warn( + "⚠️ Firestore emulator is not running on 127.0.0.1:8090. Start the emulator to run this backfill locally.", + ); + return false; + } + + process.env["FIRESTORE_EMULATOR_HOST"] = "127.0.0.1:8090"; + process.env["GCLOUD_PROJECT"] = "doula-cooperative"; + console.log("🔧 Using Firebase Firestore Emulator at 127.0.0.1:8090"); + return true; +} + +/** + * Backfill allowProfileEditing for members who were previously approved via + * profileApprovedAt before the explicit boolean permission field existed. + * + * Timestamp(0, 1) acts as a sentinel for "any real approval timestamp" while + * excluding documents where profileApprovedAt was never set. + */ +async function backfillAllowProfileEditing(): Promise { + const targetReady = await configureFirestoreTarget(); + if (!targetReady) { + return; + } + + if (getApps().length === 0) { + initializeApp({ + projectId: "doula-cooperative", + }); + } + + const firestore = getFirestore(); + const snapshot = await firestore + .collection(MEMBERS_COLLECTION) + .where("profileApprovedAt", ">", PROFILE_APPROVAL_CUTOFF) + .get(); + + if (snapshot.empty) { + console.log("✅ No members require profile editing backfill."); + return; + } + + console.log(`Found ${snapshot.docs.length} members to backfill.`); + + let updatedCount = 0; + for (let index = 0; index < snapshot.docs.length; index += MAX_BATCH_SIZE) { + const batch = firestore.batch(); + + for (const document of snapshot.docs.slice(index, index + MAX_BATCH_SIZE)) { + batch.update(document.ref, { + allowProfileEditing: true, + }); + updatedCount++; + } + + await batch.commit(); + process.stdout.write("."); + } + + console.log(`\n✅ Backfilled profile editing permission for ${updatedCount} members.`); +} + +backfillAllowProfileEditing() + .then(() => { + process.exit(0); + }) + .catch((error: unknown) => { + console.error("❌ Failed to backfill profile editing permission:", error); + process.exit(1); + }); diff --git a/members/src/app/admin/users/admin-member-detail/admin-member-detail.service.ts b/members/src/app/admin/users/admin-member-detail/admin-member-detail.service.ts index e719de4f..d2ca6675 100644 --- a/members/src/app/admin/users/admin-member-detail/admin-member-detail.service.ts +++ b/members/src/app/admin/users/admin-member-detail/admin-member-detail.service.ts @@ -204,7 +204,7 @@ export class AdminMemberDetailService { } /** - * Delete a draft profile for the current member + * Set whether the current member can create or edit a profile. */ async approveProfile(uid: string, allowProfileEditing: boolean): Promise { this.actionInProgress.set(true); diff --git a/members/src/app/admin/users/admin-member-detail/admin-member-detail.spec.ts b/members/src/app/admin/users/admin-member-detail/admin-member-detail.spec.ts index 71bdaf56..79fae247 100644 --- a/members/src/app/admin/users/admin-member-detail/admin-member-detail.spec.ts +++ b/members/src/app/admin/users/admin-member-detail/admin-member-detail.spec.ts @@ -1,5 +1,5 @@ import { Router } from '@angular/router'; -import { render, screen, waitFor } from '@testing-library/angular'; +import { render, screen, waitFor, within } from '@testing-library/angular'; import userEvent from '@testing-library/user-event'; import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; import type { ApiMemberResponse } from '../../../api-types/api-member-response'; @@ -1040,10 +1040,11 @@ describe('AdminUserDetail', () => { const { user, mockAdminMembersService } = await setup({ member }); await user.click(await screen.findByRole('button', { name: 'Enable Profile Editing' })); - await user.click(screen.getByRole('button', { name: 'Cancel' }).nextElementSibling as HTMLButtonElement); + const dialog = await screen.findByRole('dialog'); + await user.click(within(dialog).getByRole('button', { name: 'Enable Profile Editing' })); expect(mockAdminMembersService.approveProfile).toHaveBeenCalledWith('test-uid-123', true); - expect(await screen.findByText('Profile editing enabled successfully')).toBeVisible(); + expect(await screen.findByRole('status')).toHaveTextContent('Profile editing enabled successfully'); }); it('should disable profile editing after confirmation', async () => { @@ -1052,10 +1053,11 @@ describe('AdminUserDetail', () => { const { user, mockAdminMembersService } = await setup({ member }); await user.click(await screen.findByRole('button', { name: 'Disable Profile Editing' })); - await user.click(screen.getByRole('button', { name: 'Cancel' }).nextElementSibling as HTMLButtonElement); + const dialog = await screen.findByRole('dialog'); + await user.click(within(dialog).getByRole('button', { name: 'Disable Profile Editing' })); expect(mockAdminMembersService.approveProfile).toHaveBeenCalledWith('test-uid-123', false); - expect(await screen.findByText('Profile editing disabled successfully')).toBeVisible(); + expect(await screen.findByRole('status')).toHaveTextContent('Profile editing disabled successfully'); }); it('should show error when updating profile editing permission fails', async () => { @@ -1068,9 +1070,10 @@ describe('AdminUserDetail', () => { }); await user.click(await screen.findByRole('button', { name: 'Enable Profile Editing' })); - await user.click(screen.getByRole('button', { name: 'Cancel' }).nextElementSibling as HTMLButtonElement); + const dialog = await screen.findByRole('dialog'); + await user.click(within(dialog).getByRole('button', { name: 'Enable Profile Editing' })); - expect(await screen.findByText('Failed to update profile editing permission.')).toBeVisible(); + expect(await screen.findByRole('alert')).toHaveTextContent('Failed to update profile editing permission.'); }); it('should display subscription dates', async () => { diff --git a/members/src/app/guards/profile-editing.guard.integration.spec.ts b/members/src/app/guards/profile-editing.guard.integration.spec.ts index 5be44401..9eb85a4f 100644 --- a/members/src/app/guards/profile-editing.guard.integration.spec.ts +++ b/members/src/app/guards/profile-editing.guard.integration.spec.ts @@ -1,8 +1,8 @@ -import { ChangeDetectionStrategy, Component } from '@angular/core'; -import { signal } from '@angular/core'; +import { ChangeDetectionStrategy, Component, signal, type WritableSignal } from '@angular/core'; import { provideRouter, RouterOutlet } from '@angular/router'; -import { render, screen } from '@testing-library/angular'; +import { render, screen, waitFor } from '@testing-library/angular'; import { describe, expect, it } from 'vitest'; +import type { ResourceStatus } from '@angular/core'; import { MembershipService } from '../services/membership.service'; import { profileEditingGuard } from './profile-editing.guard'; @@ -27,6 +27,13 @@ class MockEditProfilePage {} }) class MockCreateProfilePage {} +@Component({ + selector: 'app-mock-edit-profile-image-page', + template: '

Edit Profile Image Page

', + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class MockEditProfileImagePage {} + @Component({ selector: 'app-mock-root', template: '', @@ -47,6 +54,11 @@ const routes = [ component: MockCreateProfilePage, canActivate: [profileEditingGuard], }, + { + path: 'profile/image', + component: MockEditProfileImagePage, + canActivate: [profileEditingGuard], + }, ]; describe('profileEditingGuard - Integration Tests', () => { @@ -81,26 +93,94 @@ describe('profileEditingGuard - Integration Tests', () => { expect(screen.getByText('Create Profile Page')).toBeVisible(); }); + + it('should wait for member document loading before allowing guarded routes', async () => { + const { navigate, resourceStatus } = await setup({ + membershipActive: true, + allowProfileEditing: true, + resourceStatus: 'loading', + }); + + const navigation = navigate('/profile'); + + expect(screen.queryByText('Edit Profile Page')).toBeNull(); + expect(screen.queryByText('Membership Page')).toBeNull(); + + resourceStatus.set('resolved'); + await navigation; + + await waitFor(() => { + expect(screen.getByText('Edit Profile Page')).toBeVisible(); + }); + }); + + it('should redirect when member document finishes loading without editing permission', async () => { + const { navigate, resourceStatus, memberDocument } = await setup({ + membershipActive: true, + allowProfileEditing: true, + resourceStatus: 'loading', + }); + + const navigation = navigate('/profile'); + memberDocument.set({ + uid: 'user123', + email: 'jane@example.com', + createdAt: new Date(), + isAdmin: false, + membershipActive: true, + allowProfileEditing: false, + }); + resourceStatus.set('resolved'); + await navigation; + + await waitFor(() => { + expect(screen.getByText('Membership Page')).toBeVisible(); + }); + }); + + it('should redirect guarded routes when member document load errors', async () => { + const { navigate } = await setup({ resourceStatus: 'error' }); + + await navigate('/profile'); + + expect(screen.getByText('Membership Page')).toBeVisible(); + }); + + it('should guard the profile image route', async () => { + const { navigate } = await setup({ membershipActive: true, allowProfileEditing: true }); + + await navigate('/profile/image'); + + expect(screen.getByText('Edit Profile Image Page')).toBeVisible(); + }); }); interface SetupOptions { membershipActive?: boolean; allowProfileEditing?: boolean; + resourceStatus?: ResourceStatus; } async function setup({ membershipActive = false, allowProfileEditing = false, + resourceStatus = 'resolved', }: SetupOptions = {}) { + const memberDocument = signal({ + uid: 'user123', + email: 'jane@example.com', + createdAt: new Date(), + isAdmin: false, + membershipActive, + allowProfileEditing, + }); + const statusSignal: WritableSignal = signal(resourceStatus); + const mockMembershipService = { - userDocument: signal({ - uid: 'user123', - email: 'jane@example.com', - createdAt: new Date(), - isAdmin: false, - membershipActive, - allowProfileEditing, - }), + userDocument: memberDocument, + userDocumentResource: { + status: statusSignal, + }, }; const { navigate } = await render(MockApp, { @@ -110,5 +190,5 @@ async function setup({ ], }); - return { navigate }; + return { navigate, resourceStatus: statusSignal, memberDocument }; } diff --git a/members/src/app/guards/profile-editing.guard.ts b/members/src/app/guards/profile-editing.guard.ts index 64665325..8c989195 100644 --- a/members/src/app/guards/profile-editing.guard.ts +++ b/members/src/app/guards/profile-editing.guard.ts @@ -1,15 +1,25 @@ import { inject } from '@angular/core'; +import { toObservable } from '@angular/core/rxjs-interop'; import { type CanActivateFn, Router } from '@angular/router'; +import { firstValueFrom } from 'rxjs'; +import { filter, map } from 'rxjs/operators'; import { MembershipService } from '../services/membership.service'; export const profileEditingGuard: CanActivateFn = () => { const membershipService = inject(MembershipService); const router = inject(Router); - const member = membershipService.userDocument(); - if (member?.membershipActive && member.allowProfileEditing) { - return true; - } + return firstValueFrom( + toObservable(membershipService.userDocumentResource.status).pipe( + filter((status) => !['idle', 'loading', 'reloading'].includes(status)), + map(() => { + const member = membershipService.userDocument(); + if (member?.membershipActive && member.allowProfileEditing) { + return true; + } - return router.createUrlTree(['/membership']); + return router.createUrlTree(['/membership']); + }), + ), + ); };