From 4063a9cb625276246d60eaf3da6a067b95112b0b Mon Sep 17 00:00:00 2001 From: ikusteu Date: Sat, 8 Jul 2023 19:28:29 +0200 Subject: [PATCH 1/4] Remove all mentions of deprecated categories --- .../src/__tests__/firestoreRules.test.ts | 80 ++--------- .../client/src/__tests__/migrations.test.ts | 129 +----------------- .../bookings/__tests__/bookings.test.ts | 77 +---------- .../src/store/selectors/bookings/slots.ts | 43 +----- packages/functions/src/migrations.ts | 83 ----------- .../shared/src/enums/deprecated/firestore.ts | 5 - packages/shared/src/enums/deprecated/index.ts | 1 - packages/shared/src/index.ts | 2 - packages/shared/src/types/firestore.ts | 5 +- packages/shared/src/types/misc.ts | 9 -- packages/translations/src/dict/en.json | 5 - packages/translations/src/dict/it.json | 5 - packages/translations/src/translations.ts | 6 +- .../Forms/CustomerForm/SectionAdminOnly.tsx | 14 +- .../CustomerForm/__tests__/FormAdmin.test.tsx | 9 +- packages/ui/src/Forms/SlotForm/SlotForm.tsx | 11 +- .../SlotForm/__tests__/SlotForm.test.tsx | 16 +-- 17 files changed, 30 insertions(+), 470 deletions(-) delete mode 100644 packages/shared/src/enums/deprecated/firestore.ts delete mode 100644 packages/shared/src/enums/deprecated/index.ts diff --git a/packages/client/src/__tests__/firestoreRules.test.ts b/packages/client/src/__tests__/firestoreRules.test.ts index d72737f74..ccc9e31d7 100644 --- a/packages/client/src/__tests__/firestoreRules.test.ts +++ b/packages/client/src/__tests__/firestoreRules.test.ts @@ -14,7 +14,6 @@ import { SlotType, Customer, sanitizeCustomer, - DeprecatedCategory, } from "@eisbuk/shared"; import { defaultCustomerFormValues } from "@/lib/data"; @@ -96,8 +95,6 @@ describe("Firestore rules", () => { }); describe("Slots rules", () => { - // const getSlotDocPath(organization, baseSlot.id) = [getSlotsPath(organization), baseSlot.id].join("/"); - testWithEmulator("should not allow access to unauth user", async () => { const { db, organization } = await getTestEnv({ auth: false, @@ -150,6 +147,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow create/update if slot type not valid", async () => { @@ -162,6 +160,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow create/update if slot category not valid", async () => { @@ -177,43 +176,7 @@ describe("Firestore rules", () => { ); } ); - testWithEmulator( - 'should allow updating a slot with "adults" category, but disallow new creating new slots with said category', - () => { - async () => { - const slotWithAdults = { - ...baseSlot, - categories: [DeprecatedCategory.Adults], - id: "slot-with-adults", - }; - const { db, organization } = await getTestEnv({ - setup: (db, { organization }) => - Promise.all([ - setDoc( - doc(db, getSlotDocPath(organization, "slot-with-adults")), - slotWithAdults - ), - ]), - }); - - // The deprecated category already exists in the slot, we should allow it to stay there - await assertSucceeds( - setDoc(doc(db, getSlotDocPath(organization, "slot-with-adults")), { - ...baseSlot, - categories: [Category.Competitive, DeprecatedCategory.Adults], - }) - ); - // We're not allowing the creation of new slots with deprecated values - await assertFails( - setDoc(doc(db, getSlotDocPath(organization, "new-slot")), { - ...baseSlot, - categories: [Category.Competitive, DeprecatedCategory.Adults], - }) - ); - }; - } - ); /** * @TODO as firestore.rules don't allow loops or iterations * we need to apply this when we agree on maximum number of intervals per slot @@ -341,6 +304,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow update access to non-admins", async () => { @@ -367,6 +331,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow delete access to non-admins (as it's handled through cloud functions)", async () => { @@ -479,6 +444,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow create/update of booking subscribing to non-existing interval", async () => { @@ -504,6 +470,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow create/update of invalid booking entry", async () => { @@ -532,6 +499,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow customer to subscribe to slot not supporting their category", async () => { @@ -625,6 +593,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should allow create/update with empty strings as values of optional strings (as is in CustomerForm in production)", async () => { @@ -647,6 +616,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow create/update if 'certificateExpiration' provided, but not a valid date", async () => { @@ -668,6 +638,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow create/update if birthday provided, but not a valid date", async () => { @@ -689,6 +660,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow create/update if phone provided but not valid", async () => { @@ -752,6 +724,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow create/update if email provided but not valid", async () => { @@ -773,6 +746,7 @@ describe("Firestore rules", () => { ); } ); + testWithEmulator( "should not allow create/update if invalid category", async () => { @@ -786,33 +760,7 @@ describe("Firestore rules", () => { ); } ); - testWithEmulator( - 'should allow updating customer in "adults" category, but disallow creation of new customers in (non-spacific) "adults" category', - async () => { - const { db, organization } = await getTestEnv({ - setup: (db, { organization }) => - setDoc(doc(db, getCustomerDocPath(organization, saul.id)), { - ...saul, - categories: [DeprecatedCategory.Adults], - }), - }); - // Should allow updating, even though category = "adults" - await assertSucceeds( - setDoc(doc(db, getCustomerDocPath(organization, saul.id)), { - ...saul, - categories: [DeprecatedCategory.Adults], - name: "Jimmy", - }) - ); - // Should disallow creation of new customers with category "adults" - await assertFails( - setDoc(doc(db, getCustomerDocPath(organization, "new-customer")), { - ...saul, - categories: [DeprecatedCategory.Adults], - }) - ); - } - ); + testWithEmulator("should allow `extendedDate` update", async () => { const { db, organization } = await getTestEnv({ setup: (db, { organization }) => diff --git a/packages/client/src/__tests__/migrations.test.ts b/packages/client/src/__tests__/migrations.test.ts index 029f1c093..588c98029 100644 --- a/packages/client/src/__tests__/migrations.test.ts +++ b/packages/client/src/__tests__/migrations.test.ts @@ -5,12 +5,7 @@ import { describe, expect } from "vitest"; import { httpsCallable, FunctionsError } from "@firebase/functions"; -import { - HTTPSErrors, - sanitizeCustomer, - Category, - DeprecatedCategory, -} from "@eisbuk/shared"; +import { HTTPSErrors, sanitizeCustomer } from "@eisbuk/shared"; import { CloudFunction } from "@eisbuk/shared/ui"; import { @@ -21,7 +16,6 @@ import { unprunedMonth, } from "@eisbuk/testing/migrations"; import * as customers from "@eisbuk/testing/customers"; -import { baseSlot } from "@eisbuk/testing/slots"; import { saul } from "@eisbuk/testing/customers"; import { functions, adminDb } from "@/__testSetup__/firestoreSetup"; @@ -32,7 +26,6 @@ import { getBookingsDocPath, getBookingsPath, getCustomerDocPath, - getSlotDocPath, getSlotsByDayPath, } from "@/utils/firestore"; @@ -128,126 +121,6 @@ describe("Migrations", () => { }); }); - describe("'migrateSlotsCategoriesToExplicitMinors'", () => { - testWithEmulator( - 'should replace "pre-competitive" and "course" category entries with corresponging "-minor" category entries, while leaving the existing categories as they are', - async () => { - const { organization } = await setUpOrganization(); - const courseSlot = { - ...baseSlot, - categories: [Category.Competitive, DeprecatedCategory.Course], - id: "course-slot", - }; - const preCompetitiveSlot = { - ...baseSlot, - categories: [Category.Competitive, DeprecatedCategory.PreCompetitive], - id: "pre-competitive-slot", - }; - // Edge case, if the category already exists, shouldn't be duplicated - const courseMinorsSlot = { - ...baseSlot, - categories: [Category.CourseMinors, DeprecatedCategory.Course], - id: "pre-competitive-minors-slot", - }; - - const courseSlotRef = adminDb.doc( - getSlotDocPath(organization, courseSlot.id) - ); - const preCompetitiveSlotRef = adminDb.doc( - getSlotDocPath(organization, preCompetitiveSlot.id) - ); - const courseMinorsSlotRef = adminDb.doc( - getSlotDocPath(organization, courseMinorsSlot.id) - ); - - await Promise.all([ - courseSlotRef.set(courseSlot), - preCompetitiveSlotRef.set(preCompetitiveSlot), - courseMinorsSlotRef.set(courseMinorsSlot), - ]); - - await invokeFunction(CloudFunction.MigrateCategoriesToExplicitMinors)({ - organization, - }); - - const [resCourse, resPreCompetitive, resCourseMinors] = - await Promise.all([ - courseSlotRef.get(), - preCompetitiveSlotRef.get(), - courseMinorsSlotRef.get(), - ]); - expect(resCourse.data()).toEqual({ - ...courseSlot, - categories: [Category.Competitive, Category.CourseMinors], - }); - expect(resPreCompetitive.data()).toEqual({ - ...preCompetitiveSlot, - categories: [Category.Competitive, Category.PreCompetitiveMinors], - }); - expect(resCourseMinors.data()).toEqual({ - ...courseMinorsSlot, - categories: [Category.CourseMinors], - }); - } - ); - - testWithEmulator( - 'should replace "pre-competitive" and "course" category entries with corresponging "-minor" category entries, in customer documents', - async () => { - const { organization } = await setUpOrganization(); - - const courseCustomer = { - ...saul, - categories: [DeprecatedCategory.Course], - id: "course-customer", - }; - const preCompetitiveCustomer = { - ...saul, - categories: [DeprecatedCategory.PreCompetitive], - id: "pre-competitive-customer", - }; - - const courseCustomerRef = adminDb.doc( - getCustomerDocPath(organization, courseCustomer.id) - ); - const preCompetitiveCustomerRef = adminDb.doc( - getCustomerDocPath(organization, preCompetitiveCustomer.id) - ); - - await Promise.all([ - courseCustomerRef.set(courseCustomer), - preCompetitiveCustomerRef.set(preCompetitiveCustomer), - ]); - - await invokeFunction(CloudFunction.MigrateCategoriesToExplicitMinors)({ - organization, - }); - - const [resCourse, resPreCompetitive] = await Promise.all([ - courseCustomerRef.get(), - preCompetitiveCustomerRef.get(), - ]); - expect(resCourse.data()).toEqual({ - ...courseCustomer, - categories: [Category.CourseMinors], - }); - expect(resPreCompetitive.data()).toEqual({ - ...preCompetitiveCustomer, - categories: [Category.PreCompetitiveMinors], - }); - } - ); - - testWithEmulator("should not allow calls to non-admins", async () => { - const { organization } = await setUpOrganization({ doLogin: false }); - await expect( - invokeFunction(CloudFunction.MigrateCategoriesToExplicitMinors)({ - organization, - }) - ).rejects.toThrow(HTTPSErrors.Unauth); - }); - }); - describe("customersToPluralCategories", () => { testWithEmulator( "should change customer's category field into an array instead of scalar", diff --git a/packages/client/src/store/selectors/bookings/__tests__/bookings.test.ts b/packages/client/src/store/selectors/bookings/__tests__/bookings.test.ts index 502452646..f5e39169a 100644 --- a/packages/client/src/store/selectors/bookings/__tests__/bookings.test.ts +++ b/packages/client/src/store/selectors/bookings/__tests__/bookings.test.ts @@ -4,7 +4,6 @@ import { DateTime } from "luxon"; import { BookingSubCollection, Category, - DeprecatedCategory, sanitizeCustomer, OrgSubCollection, SlotsByDay, @@ -44,7 +43,7 @@ const setupBookingsTest = ({ date, slotsByDay, }: { - category: Category | DeprecatedCategory; + category: Category; date: DateTime; slotsByDay: NonNullable; }): ReturnType => { @@ -80,80 +79,6 @@ describe("Selectors ->", () => { const res = getSlotsForCustomer(store.getState()); expect(res).toEqual(expectedMonthCustomer); }); - - const courseAdultsSlot = { - ...baseSlot, - id: "course", - categories: [Category.CourseAdults], - }; - const preCompetitiveAdultsSlot = { - ...baseSlot, - id: "pre-competitive", - categories: [Category.PreCompetitiveAdults], - }; - const adultsSlot = { - ...baseSlot, - id: "adults", - categories: [DeprecatedCategory.Adults as unknown as Category], - }; - const competitiveSlot = { - ...baseSlot, - id: "competitive", - categories: [Category.Competitive], - }; - - test('should display both "pre-competitive-adults" and "course-adults" slots to unsorted "adults" customers', () => { - const date = DateTime.fromISO(baseSlot.date); - const currentMonthString = baseSlot.date.substring(0, 7); - const store = setupBookingsTest({ - category: DeprecatedCategory.Adults, - date, - slotsByDay: { - [currentMonthString]: { - ["2021-03-01"]: { - [courseAdultsSlot.id]: courseAdultsSlot, - [preCompetitiveAdultsSlot.id]: preCompetitiveAdultsSlot, - [competitiveSlot.id]: competitiveSlot, - }, - }, - }, - }); - const res = getSlotsForCustomer(store.getState()); - // Should only filter the "competitive" slot - expect(res).toEqual({ - ["2021-03-01"]: { - [courseAdultsSlot.id]: courseAdultsSlot, - [preCompetitiveAdultsSlot.id]: preCompetitiveAdultsSlot, - }, - }); - }); - - test('should display slots with category "adults" to both "pre-competitive-adults" and "course-adults" customers', () => { - const date = DateTime.fromISO(baseSlot.date); - const currentMonthString = baseSlot.date.substring(0, 7); - const store = setupBookingsTest({ - category: Category.CourseAdults, - date, - slotsByDay: { - [currentMonthString]: { - /** @TODO_TESTS hardcoded test date, make this a bit more concise */ - ["2021-03-01"]: { - [courseAdultsSlot.id]: courseAdultsSlot, - [preCompetitiveAdultsSlot.id]: preCompetitiveAdultsSlot, - [adultsSlot.id]: adultsSlot, - [competitiveSlot.id]: competitiveSlot, - }, - }, - }, - }); - const res = getSlotsForCustomer(store.getState()); - expect(res).toEqual({ - ["2021-03-01"]: { - [courseAdultsSlot.id]: courseAdultsSlot, - [adultsSlot.id]: adultsSlot, - }, - }); - }); }); describe("Test 'getCanBook' selector", () => { diff --git a/packages/client/src/store/selectors/bookings/slots.ts b/packages/client/src/store/selectors/bookings/slots.ts index 6e345ff71..40b343d74 100644 --- a/packages/client/src/store/selectors/bookings/slots.ts +++ b/packages/client/src/store/selectors/bookings/slots.ts @@ -1,8 +1,6 @@ import { Category, SlotsById, - DeprecatedCategory, - CategoryUnion, SlotInterface, CustomerBookingEntry, SlotsByDay, @@ -18,10 +16,6 @@ import { getBookingsCustomer } from "./customer"; import { isEmpty } from "@/utils/helpers"; import { comparePeriods } from "@/utils/sort"; -interface CategoryFilter { - (categories: CategoryUnion[]): boolean; -} - /** * Get subscribed slots from state * @param state Local Redux Store @@ -41,35 +35,6 @@ export const getAttendedSlots = ( ): Record> => state.firestore.data?.attendedSlots || {}; -/** - * An util higher order function returning the condition for category filtering - * based on received category. - * @param category category (of a customer we're filtering for) - * @returns a filtering function used check if slot's categories contain the category or should be filtered out - */ -const createCategoryFilter = ( - customerCategory: CategoryUnion[] -): CategoryFilter => - // For unspecified "adults" we're showing all the "adult-" prefixed category slots - customerCategory.includes(DeprecatedCategory.Adults) - ? (categories) => - categories.includes(DeprecatedCategory.Adults) || - categories.includes(Category.CourseAdults) || - categories.includes(Category.PreCompetitiveAdults) - : // For customers belonging to "adult-" prefixed category, we're showing the specific category slots - // as well as general "adult" slots - customerCategory.some((cat) => - [Category.PreCompetitiveAdults, Category.CourseAdults].includes( - cat as Category - ) - ) - ? (categories) => - categories.includes(DeprecatedCategory.Adults) || - categories.some((slotCat) => customerCategory.includes(slotCat)) - : // For all non adult categories, we're only displaying the slot if it contains that exact category - (categories) => - categories.some((slotCat) => customerCategory.includes(slotCat)); - /** * A helper function we're using to filter out slots not within provided category. * @param slotsRecord record of slots keyed by slotId @@ -78,17 +43,15 @@ const createCategoryFilter = ( */ const filterSlotsByCategory = ( slotsRecord: SlotsById, - category: CategoryUnion[] + category: Category[] ): [SlotsById, boolean] => { let isEmptyWhenFiltered = true; - const categoryFilter = createCategoryFilter(category); - const filteredRecord = Object.keys(slotsRecord).reduce((acc, slotId) => { const slot = slotsRecord[slotId]; - const categories = slot.categories as CategoryUnion[]; + const categories = slot.categories; - if (categoryFilter(categories)) { + if (categories.some((slotCat) => category.includes(slotCat))) { isEmptyWhenFiltered = false; return { ...acc, [slotId]: slot }; } diff --git a/packages/functions/src/migrations.ts b/packages/functions/src/migrations.ts index 12d4dc21f..f46b5d19a 100644 --- a/packages/functions/src/migrations.ts +++ b/packages/functions/src/migrations.ts @@ -3,12 +3,8 @@ import { FieldValue } from "@google-cloud/firestore"; import admin from "firebase-admin"; import { - Category, Collection, - DeprecatedCategory, OrgSubCollection, - SlotInterface, - CategoryUnion, defaultEmailTemplates, OrganizationData, CustomerFull, @@ -116,85 +112,6 @@ export const deleteOrphanedBookings = functions return { success: true }; }); -export const migrateCategoriesToExplicitMinors = functions - .region(__functionsZone__) - .https.onCall(async ({ organization }, { auth }) => { - if (!(await checkUser(organization, auth))) throwUnauth(); - - const batch = admin.firestore().batch(); - - const orgRef = admin - .firestore() - .collection(Collection.Organizations) - .doc(organization); - const slotsRef = orgRef.collection(OrgSubCollection.Slots); - const customersRef = orgRef.collection(OrgSubCollection.Customers); - - const [allSlots, allCustomers] = await Promise.all([ - slotsRef.get(), - customersRef.get(), - ]); - - // Enqueue slot updates - allSlots.forEach((slot) => { - const updatedCategories: CategoryUnion[] = []; - - const data = slot.data() as SlotInterface; - const categories = data.categories as CategoryUnion[]; - - // Update slot only if needed: if at least one of the categories needs updating - let shouldUpdate = false; - categories.forEach((c) => { - let category = c; - switch (category) { - case DeprecatedCategory.Course: - category = Category.CourseMinors; - shouldUpdate = true; - break; - case DeprecatedCategory.PreCompetitive: - category = Category.PreCompetitiveMinors; - shouldUpdate = true; - break; - default: - } - // Avoid duplicating of the categories - if (!updatedCategories.includes(category)) { - updatedCategories.push(category); - } - }); - - if (shouldUpdate) { - batch.set(slot.ref, { categories: updatedCategories }, { merge: true }); - } - }); - - // Enqueue customer updates - allCustomers.forEach((customer) => { - const data = customer.data() as CustomerFull; - const categories = data.categories as CategoryUnion[]; - - categories.forEach((category, i) => { - switch (category) { - case DeprecatedCategory.Course: - categories[i] = Category.CourseMinors; - break; - case DeprecatedCategory.PreCompetitive: - categories[i] = Category.PreCompetitiveMinors; - break; - default: - // No changes needed, no updates batched - return; - } - }); - - batch.set(customer.ref, { categories }, { merge: true }); - }); - - await batch.commit(); - - return { success: true }; - }); - export const customersToPluralCategories = functions .region(__functionsZone__) .https.onCall(async ({ organization }, { auth }) => { diff --git a/packages/shared/src/enums/deprecated/firestore.ts b/packages/shared/src/enums/deprecated/firestore.ts deleted file mode 100644 index 00044b579..000000000 --- a/packages/shared/src/enums/deprecated/firestore.ts +++ /dev/null @@ -1,5 +0,0 @@ -export enum DeprecatedCategory { - Adults = "adults", // still supporting this with certain fallbacks - PreCompetitive = "pre-competitive", // replaced by "pre-competitive-minors" - Course = "course", // replaced by "course-minors" -} diff --git a/packages/shared/src/enums/deprecated/index.ts b/packages/shared/src/enums/deprecated/index.ts deleted file mode 100644 index 981328a96..000000000 --- a/packages/shared/src/enums/deprecated/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from "./firestore"; diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 14ef6a677..ab4fbf1a5 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -9,8 +9,6 @@ export * from "./types/utils"; export * from "./utils"; export * from "./data/templates"; -export * from "./enums/deprecated"; - import italianNames from "./assets/italian-names.json"; import italianSurnames from "./assets/italian-surnames.json"; diff --git a/packages/shared/src/types/firestore.ts b/packages/shared/src/types/firestore.ts index 75aacc15c..cc78f11ee 100644 --- a/packages/shared/src/types/firestore.ts +++ b/packages/shared/src/types/firestore.ts @@ -7,7 +7,6 @@ import { Collection, DeliveryQueue, } from "../enums/firestore"; -import { DeprecatedCategory } from "../enums/deprecated"; /** * Organization data record included in each organization (other than nested collections) @@ -155,7 +154,7 @@ export interface SlotInterface { /** * Athlete categories this slot is suitable for. */ - categories: (Category | DeprecatedCategory)[]; + categories: Category[]; /** * Intervals available for booking. Keyed by start-end time. * @@ -217,7 +216,7 @@ export interface CustomerBase { export interface Customer extends CustomerBase { id: string; secretKey: string; - categories: (Category | DeprecatedCategory)[]; + categories: Category[]; extendedDate?: string | null; } /** diff --git a/packages/shared/src/types/misc.ts b/packages/shared/src/types/misc.ts index a46b2fea0..3045682e8 100644 --- a/packages/shared/src/types/misc.ts +++ b/packages/shared/src/types/misc.ts @@ -1,14 +1,5 @@ import * as React from "react"; -import { Category } from "../enums/firestore"; -import { DeprecatedCategory } from "../enums/deprecated/firestore"; - -/** - * A union type of deprecated and currently supported categories. - * We're mostily using this for category function params. - */ -export type CategoryUnion = Category | DeprecatedCategory; - /** * A type alias for svgr loaded SVG as React component */ diff --git a/packages/translations/src/dict/en.json b/packages/translations/src/dict/en.json index 6d116d49a..d478c36c5 100644 --- a/packages/translations/src/dict/en.json +++ b/packages/translations/src/dict/en.json @@ -77,11 +77,6 @@ "CourseMinors": "course-minors", "Competitive": "competitive" }, - "DeprecatedCategory": { - "Adults": "adults", - "PreCompetitive": "pre-competitive", - "Course": "course" - }, "FormTitle": { "NewCustomer": "New Athlete", diff --git a/packages/translations/src/dict/it.json b/packages/translations/src/dict/it.json index a28754f4a..b3eb0bf7d 100644 --- a/packages/translations/src/dict/it.json +++ b/packages/translations/src/dict/it.json @@ -76,11 +76,6 @@ "CourseMinors": "corso-ragazzi", "Competitive": "agonismo" }, - "DeprecatedCategory": { - "Adults": "adulti", - "PreCompetitive": "pre-agonismo", - "Course": "corso" - }, "FormTitle": { "NewCustomer": "Nuovo Atleta", diff --git a/packages/translations/src/translations.ts b/packages/translations/src/translations.ts index a898cc3e3..8c79baf6f 100644 --- a/packages/translations/src/translations.ts +++ b/packages/translations/src/translations.ts @@ -1,4 +1,4 @@ -import { Category, SlotType, DeprecatedCategory } from "@eisbuk/shared"; +import { Category, SlotType } from "@eisbuk/shared"; // #region navigation export enum NavigationLabel { @@ -82,10 +82,6 @@ export const CategoryLabel = { [Category.CourseAdults]: "Category.CourseAdults", [Category.CourseMinors]: "Category.CourseMinors", [Category.Competitive]: "Category.Competitive", - /** @TODO This should be removed in the future and is here for temporary backwards compatibility */ - [DeprecatedCategory.Adults]: "DeprecatedCategory.Adults", - [DeprecatedCategory.PreCompetitive]: "DeprecatedCategory.PreCompetitive", - [DeprecatedCategory.Course]: "DeprecatedCategory.Course", }; // #endregion dataEntries diff --git a/packages/ui/src/Forms/CustomerForm/SectionAdminOnly.tsx b/packages/ui/src/Forms/CustomerForm/SectionAdminOnly.tsx index 2dfa01ba4..e0ed9776b 100644 --- a/packages/ui/src/Forms/CustomerForm/SectionAdminOnly.tsx +++ b/packages/ui/src/Forms/CustomerForm/SectionAdminOnly.tsx @@ -10,14 +10,14 @@ import i18n, { CategoryLabel, } from "@eisbuk/translations"; import { Identification } from "@eisbuk/svg"; -import { Category, DeprecatedCategory } from "@eisbuk/shared"; +import { Category } from "@eisbuk/shared"; import FormSection from "../FormSection"; import FormField, { FormFieldVariant, FormFieldWitdh } from "../FormField"; import Checkbox from "../../Checkbox"; export interface AdminOnlyFields { - categories: Array; + categories: Category[]; subscriptionNumber: string; } @@ -35,17 +35,11 @@ const SectionAdminOnly: React.FC = ({ const { errors } = useFormikContext(); - type CategoryString = DeprecatedCategory | Category; - const deprecatedCategories: CategoryString[] = - Object.values(DeprecatedCategory); - const availableCategories = ( - Object.values(Category) as CategoryString[] - ).concat(deprecatedCategories); + const availableCategories = Object.values(Category); const categoryOptions = availableCategories.map((category) => ({ value: category, label: t(CategoryLabel[category]), - disabled: deprecatedCategories.includes(category), })); return ( @@ -70,7 +64,7 @@ const SectionAdminOnly: React.FC = ({ className="col-span-2" key={category.label} component={Checkbox} - disabled={contextProps.disabled || category.disabled} + disabled={contextProps.disabled} /> ))} diff --git a/packages/ui/src/Forms/CustomerForm/__tests__/FormAdmin.test.tsx b/packages/ui/src/Forms/CustomerForm/__tests__/FormAdmin.test.tsx index 5ec2e7b18..17d8c8f1e 100644 --- a/packages/ui/src/Forms/CustomerForm/__tests__/FormAdmin.test.tsx +++ b/packages/ui/src/Forms/CustomerForm/__tests__/FormAdmin.test.tsx @@ -8,7 +8,7 @@ import i18n, { CustomerFormTitle, CustomerLabel, } from "@eisbuk/translations"; -import { Category, DeprecatedCategory } from "@eisbuk/shared"; +import { Category } from "@eisbuk/shared"; import { CustomerForm } from "../index"; @@ -95,13 +95,6 @@ describe("CustomerForm", () => { false ); }); - // Deprecated categories, however, while shown, should be disabled - Object.values(DeprecatedCategory).forEach((category) => { - expect(screen.getByLabelText(category)).toHaveProperty( - "disabled", - true - ); - }); // Clicking cancel should disable the fields again userEvent.click(screen.getAllByText(t(ActionButton.Cancel))[0]); diff --git a/packages/ui/src/Forms/SlotForm/SlotForm.tsx b/packages/ui/src/Forms/SlotForm/SlotForm.tsx index d98941e0b..e16971289 100644 --- a/packages/ui/src/Forms/SlotForm/SlotForm.tsx +++ b/packages/ui/src/Forms/SlotForm/SlotForm.tsx @@ -5,7 +5,6 @@ import * as Yup from "yup"; import { Category, - DeprecatedCategory, SlotInterface, SlotInterfaceLoose, SlotInterval, @@ -109,17 +108,11 @@ const SlotForm: React.FC = ({ : t(SlotFormTitle.NewSlot); const titleDate = t(DateFormat.DayMonth, { date: DateTime.fromISO(date) }); - type CategoryString = DeprecatedCategory | Category; - const deprecatedCategories: CategoryString[] = - Object.values(DeprecatedCategory); - const availableCategories = ( - Object.values(Category) as CategoryString[] - ).concat(deprecatedCategories); + const availableCategories = Object.values(Category); const categoryOptions = availableCategories.map((category) => ({ value: category, label: t(CategoryLabel[category]), - disabled: deprecatedCategories.includes(category), })); const typeOptions = Object.values(SlotType).map((type) => ({ @@ -216,7 +209,7 @@ const SlotForm: React.FC = ({ className="col-span-2" key={category.label} component={Checkbox} - disabled={disableCategories || category.disabled} + disabled={disableCategories} /> ))} diff --git a/packages/ui/src/Forms/SlotForm/__tests__/SlotForm.test.tsx b/packages/ui/src/Forms/SlotForm/__tests__/SlotForm.test.tsx index dc688211f..d5d0379be 100644 --- a/packages/ui/src/Forms/SlotForm/__tests__/SlotForm.test.tsx +++ b/packages/ui/src/Forms/SlotForm/__tests__/SlotForm.test.tsx @@ -10,12 +10,7 @@ import { } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { - Category, - DeprecatedCategory, - fromISO, - SlotType, -} from "@eisbuk/shared"; +import { Category, fromISO, SlotType } from "@eisbuk/shared"; import i18n, { SlotFormTitle, SlotTypeLabel, @@ -76,15 +71,6 @@ describe("SlotForm", () => { // intervals field screen.getByTestId(__timeIntervalFieldId__); }); - - test("should render a deprecated categories for backwards compatibility, but disable them for selection", () => { - Object.values(DeprecatedCategory).forEach((label) => { - const checkbox = screen.getByLabelText( - i18n.t(CategoryLabel[label]) as string - ); - expect(checkbox).toHaveProperty("disabled", true); - }); - }); }); describe("Test edit slot form", () => { From cea567e9505aa58288035b4bf9fb243692edb5cf Mon Sep 17 00:00:00 2001 From: ikusteu Date: Sat, 8 Jul 2023 20:46:41 +0200 Subject: [PATCH 2/4] Clean up cloud functions a bit: * Remove 'customersToPluralCategories' migration * Remove 'customersToPluralCategories' and 'migrateCategoriesToExplicitMinors' from cloud functions emun (as they no longer exist) --- packages/client/src/pages/debug/index.tsx | 23 +-------------- packages/functions/src/migrations.ts | 34 ----------------------- packages/shared/src/ui/enums/misc.ts | 2 -- 3 files changed, 1 insertion(+), 58 deletions(-) diff --git a/packages/client/src/pages/debug/index.tsx b/packages/client/src/pages/debug/index.tsx index 636fe45f0..ab3787b09 100644 --- a/packages/client/src/pages/debug/index.tsx +++ b/packages/client/src/pages/debug/index.tsx @@ -120,28 +120,6 @@ const DebugPage: React.FC = () => { -
- - Migrate categories to explicit minors - -
-
- - Migrate categories to array - -
{ Populate Default Email Templates
+
{ - if (!(await checkUser(organization, auth))) throwUnauth(); - - const batch = admin.firestore().batch(); - - const orgRef = admin - .firestore() - .collection(Collection.Organizations) - .doc(organization); - const customersRef = orgRef.collection(OrgSubCollection.Customers); - - const allCustomers = await customersRef.get(); - - allCustomers.forEach((customer) => { - const data = customer.data(); - - if (!data.category) return; - const { category, ...noCategoryData } = data; - if (!Array.isArray(category)) { - functions.logger.info(`Converted customer: ${data.id}`); - - batch.set(customer.ref, { ...noCategoryData, categories: [category] }); - } else { - batch.set(customer.ref, { ...noCategoryData, categories: category }); - } - }); - - await batch.commit(); - - return { success: true }; - }); - export const populateDefaultEmailTemplates = functions .region(__functionsZone__) .https.onCall(async ({ organization }, { auth }) => { diff --git a/packages/shared/src/ui/enums/misc.ts b/packages/shared/src/ui/enums/misc.ts index f7123da1c..6280bd613 100644 --- a/packages/shared/src/ui/enums/misc.ts +++ b/packages/shared/src/ui/enums/misc.ts @@ -18,8 +18,6 @@ export enum CloudFunction { PruneSlotsByDay = "pruneSlotsByDay", DeleteOrphanedBookings = "deleteOrphanedBookings", - MigrateCategoriesToExplicitMinors = "migrateCategoriesToExplicitMinors", - CustomersToPluralCategories = "customersToPluralCategories", PopulateDefaultEmailTemplates = "populateDefaultEmailTemplates", RemoveInvalidCustomerPhones = "removeInvalidCustomerPhones", } From 11d027b254e1d7f8cb359865f3df1aff9331106e Mon Sep 17 00:00:00 2001 From: ikusteu Date: Sun, 9 Jul 2023 11:10:40 +0200 Subject: [PATCH 3/4] Remove stale migration tests --- .../client/src/__tests__/migrations.test.ts | 61 ------------------- 1 file changed, 61 deletions(-) diff --git a/packages/client/src/__tests__/migrations.test.ts b/packages/client/src/__tests__/migrations.test.ts index 588c98029..46e4f3a81 100644 --- a/packages/client/src/__tests__/migrations.test.ts +++ b/packages/client/src/__tests__/migrations.test.ts @@ -16,7 +16,6 @@ import { unprunedMonth, } from "@eisbuk/testing/migrations"; import * as customers from "@eisbuk/testing/customers"; -import { saul } from "@eisbuk/testing/customers"; import { functions, adminDb } from "@/__testSetup__/firestoreSetup"; @@ -120,64 +119,4 @@ describe("Migrations", () => { ).rejects.toThrow(HTTPSErrors.Unauth); }); }); - - describe("customersToPluralCategories", () => { - testWithEmulator( - "should change customer's category field into an array instead of scalar", - async () => { - const { organization } = await setUpOrganization(); - - const customer = { - ...saul, - category: saul.categories[0], - id: "course-customer", - }; - const customerRef = adminDb.doc( - getCustomerDocPath(organization, customer.id) - ); - await customerRef.set(customer); - - await invokeFunction(CloudFunction.CustomersToPluralCategories)({ - organization, - }); - - const { category, ...newCustomer } = customer; - - expect((await customerRef.get()).data()).toEqual({ - ...newCustomer, - categories: [category], - }); - } - ); - // Normally when you try to convert an array, firestore throws an INTERNAL error - testWithEmulator( - "should not throw an error if category is already an array", - async () => { - const { organization } = await setUpOrganization(); - - const customer = { - ...saul, - id: "course-customer", - }; - const customerRef = adminDb.doc( - getCustomerDocPath(organization, customer.id) - ); - await customerRef.set(customer); - - await expect( - invokeFunction(CloudFunction.CustomersToPluralCategories)({ - organization, - }) - ).resolves.not.toThrow(); - } - ); - testWithEmulator("should not allow calls to non-admins", async () => { - const { organization } = await setUpOrganization({ doLogin: false }); - await expect( - invokeFunction(CloudFunction.CustomersToPluralCategories)({ - organization, - }) - ).rejects.toThrow(HTTPSErrors.Unauth); - }); - }); }); From 69e7369f395b6779041617a0d0ffa91490bc4d3d Mon Sep 17 00:00:00 2001 From: ikusteu Date: Sun, 9 Jul 2023 11:15:40 +0200 Subject: [PATCH 4/4] Remove support for "adults" category in firestore rules --- packages/firestore/firestore.rules | 39 +++--------------------------- 1 file changed, 3 insertions(+), 36 deletions(-) diff --git a/packages/firestore/firestore.rules b/packages/firestore/firestore.rules index f3c325592..376dfcd6f 100644 --- a/packages/firestore/firestore.rules +++ b/packages/firestore/firestore.rules @@ -27,9 +27,6 @@ service cloud.firestore { function getCategories() { return ["course-adults", "pre-competitive-adults", "course-minors", "pre-competitive-minors", "competitive"] } - function getAllCategories() { - return ["course-adults", "pre-competitive-adults", "course-minors", "pre-competitive-minors", "competitive", "adults"] - } function getSlotTypes() { return ["ice", "off-ice"] } @@ -93,18 +90,12 @@ service cloud.firestore { // Only admins can read/delete slots allow read, delete: if isAdmin(organization) // Validate data integrity for create/update - allow update: if isAdmin(organization) && + allow create, update: if isAdmin(organization) && // check date checkValidDate(request.resource.data.date) && // check valid type request.resource.data.type in getSlotTypes() && - // check valid categories (on update, "adults" is still supported, even though deprecated) - request.resource.data.categories.hasOnly(getAllCategories()) - // @TODO check valid intervals - allow create: if isAdmin(organization) && - checkValidDate(request.resource.data.date) && - request.resource.data.type in getSlotTypes() && - // allow slot creation only with currently supported categories + // contains only supported categories request.resource.data.categories.hasOnly(getCategories()) } // #endregion slot-checks @@ -165,30 +156,7 @@ service cloud.firestore { // Only admins can access customer collection allow read, delete: if isAdmin(organization) // Check data integrity - allow update: if ( - isAdmin(organization) && - // Check required fields - checkRequiredString(request.resource.data.name) && - checkRequiredString(request.resource.data.surname) && - // Check optional dates - checkOptionalValidDate(request.resource.data, "birthday") && - checkOptionalValidDate(request.resource.data, "certificateExpiration") && - // Check category (allows updates even if "adults") - request.resource.data.categories.hasOnly(getAllCategories()) && - // Check valid email - ( - !("email" in request.resource.data) || - request.resource.data.email == "" || - checkValidEmail(request.resource.data.email) - ) && - // Check valid phone number - ( - !("phone" in request.resource.data) || - request.resource.data.phone == "" || - checkValidPhoneNumber(request.resource.data.phone) - ) - ) - allow create: if ( + allow create, update: if ( isAdmin(organization) && // Check required fields checkRequiredString(request.resource.data.name) && @@ -196,7 +164,6 @@ service cloud.firestore { // Check optional dates checkOptionalValidDate(request.resource.data, "birthday") && checkOptionalValidDate(request.resource.data, "certificateExpiration") && - // Check category (shouldn't allow creation with "adults" category) request.resource.data.categories.hasOnly(getCategories()) && // Check valid email (