handle reserved formats properly; test trying to delete a format that…

… is in use (#1103)
bmlt-enabled · Oct 26, 2024 · c303aeb · c303aeb
1 parent 6b46940
commit c303aeb
Show file tree

Hide file tree

Showing 6 changed files with 91 additions and 10 deletions.
diff --git a/src/resources/js/components/FormatForm.svelte b/src/resources/js/components/FormatForm.svelte
@@ -12,6 +12,7 @@
   import { translations } from '../stores/localization';
 
   export let selectedFormat: Format | null;
+  export let reservedFormatKeys: string[];
 
   const globalSettings = settings;
   const mappings = globalSettings.languageMapping;
@@ -123,6 +124,18 @@
               if (!$data[lang + '_key'] && ($data[lang + '_name'] || $data[lang + '_description'])) {
                 k.push($translations.keyIsRequired);
               }
+              // For English translations only: check that we're not trying to set the key of some other format to one of the
+              // reserved keys HY, TC, or VM.  If the format already exists and is one of HY, TC, or VM, it's OK to edit the
+              // name or description -- the condition includes a check to allow this.  (The key will be read-only.)
+              if (lang === 'en' && initialValues[lang + '_key'] !== $data[lang + '_key'] && reservedFormatKeys.includes($data[lang + '_key'])) {
+                k.push($translations.keyIsReserved);
+              }
+              // a translation is required for all languages for the reserved formats HY, TC, and VM -- it's an error
+              // if we're trying to delete one of these (just need to check the key, since if the key is there the name and
+              // description will be required)
+              if (reservedFormatKeys.includes(initialValues.en_key) && !$data[lang + '_key']) {
+                k.push($translations.formatTranslationIsRequired);
+              }
               errorObject[lang + '_key'] = k.join(' ');
               const n = error?.errors[lang + '_name'] ?? [];
               errorObject[lang + '_name'] = n.join(' ');
@@ -156,6 +169,15 @@
     return t ? t : $translations.getString(title, 'en');
   }
 
+  function isReservedKey(lang: string): boolean {
+    if (lang === 'en') {
+      const e = selectedFormatTranslations.find((t) => t.language === 'en');
+      return e !== undefined && reservedFormatKeys.includes(e.key);
+    } else {
+      return false;
+    }
+  }
+
   $: isDirty.set(formIsDirty(initialValues, $data));
 </script>
 
@@ -174,7 +196,7 @@
             <span slot="header">{mappings[lang]}</span>
             <div>
               <Label for="{lang}_key" class="mb-2">{getLabel('keyTitle', lang)}</Label>
-              <Input type="text" id="{lang}_key" name="{lang}_key" />
+              <Input type="text" id="{lang}_key" name="{lang}_key" disabled={isReservedKey(lang)} />
               <Helper class="mb-2" color="red">
                 {#if $errors[lang + '_key']}
                   {$errors[lang + '_key']}

diff --git a/src/resources/js/components/FormatModal.svelte b/src/resources/js/components/FormatModal.svelte
@@ -9,6 +9,7 @@
 
   export let showModal: boolean;
   export let selectedFormat: Format | null;
+  export let reservedFormatKeys: string[];
 
   let showConfirmModal = false;
   let forceClose = false;
@@ -51,7 +52,7 @@
 
 <Modal bind:open={showModal} size="sm" class="modal-content">
   <div class="p-4">
-    <FormatForm {selectedFormat} on:saved />
+    <FormatForm {selectedFormat} {reservedFormatKeys} on:saved />
   </div>
 </Modal>
 

diff --git a/src/resources/js/routes/Formats.svelte b/src/resources/js/routes/Formats.svelte
@@ -14,6 +14,8 @@
   import type { Format } from 'bmlt-root-server-client';
   import FormatForm from '../components/FormatForm.svelte';
 
+  const reservedFormatKeys = ['HY', 'TC', 'VM'];
+
   let isLoaded = false;
   let formats: Format[] = [];
   let filteredFormats: Format[] = [];
@@ -78,6 +80,10 @@
     showModal = false;
   }
 
+  function isReserved(f: Format): boolean {
+    return f.translations.find((t) => t.language === 'en' && reservedFormatKeys.includes(t.key)) !== undefined;
+  }
+
   // Get the name of the format in the current language.  If no translation for that language is available, return the name in
   // English; if no English version, then just pick the first one in the array.  If that doesn't exist either then return a blank.
   // This last case only arises when trying to create a format with no translations; the UI signals an error if that happens.
@@ -132,7 +138,13 @@
               <TableBodyCell class="whitespace-normal">{getFormatName(format)}</TableBodyCell>
               {#if $authenticatedUser?.type === 'admin'}
                 <TableBodyCell class="text-right">
-                  <Button color="none" on:click={(e) => handleDelete(e, format)} class="text-blue-700 dark:text-blue-500" aria-label={$translations.deleteFormat + ' ' + getFormatName(format)}>
+                  <Button
+                    color="none"
+                    on:click={(e) => handleDelete(e, format)}
+                    class="text-blue-700 dark:text-blue-500"
+                    disabled={isReserved(format)}
+                    aria-label={$translations.deleteFormat + ' ' + getFormatName(format)}
+                  >
                     <TrashBinOutline title={{ id: 'deleteFormat', title: $translations.deleteFormat }} ariaLabel={$translations.deleteFormat} />
                   </Button>
                 </TableBodyCell>
@@ -143,11 +155,11 @@
       </TableSearch>
     {:else}
       <div class="p-2">
-        <FormatForm {selectedFormat} on:saved={onSaved} />
+        <FormatForm {selectedFormat} {reservedFormatKeys} on:saved={onSaved} />
       </div>
     {/if}
   {/if}
 </div>
 
-<FormatModal bind:showModal {selectedFormat} on:saved={onSaved} on:close={closeModal} />
+<FormatModal bind:showModal {selectedFormat} {reservedFormatKeys} on:saved={onSaved} on:close={closeModal} />
 <FormatDeleteModal bind:showDeleteModal {deleteFormat} formatName={deleteFormat ? getFormatName(deleteFormat) : ''} on:deleted={onDeleted} />
diff --git a/src/resources/js/stores/localization.ts b/src/resources/js/stores/localization.ts
@@ -59,6 +59,7 @@ const strings = new LocalizedStrings({
     formatDeleteConflictError: 'Error: The format could not be deleted because it is still associated with meetings.', // TODO: translate
     formatId: 'Format ID', // TODO: translate
     formatsTitle: 'Formate',
+    formatTranslationIsRequired: 'a translation is required for all languages for reserved formats', // TODO: translate
     formatTypeCodes: [
       // TODO: translate
       { name: 'Attendance by non-addicts (Open, Closed)', value: 'OPEN_OR_CLOSED' },
@@ -77,6 +78,7 @@ const strings = new LocalizedStrings({
     idTitle: 'ID',
     invalidUsernameOrPassword: 'Ungültiger Benutzername oder Passwort.',
     keyIsRequired: 'Schlüssel ist erforderlich',
+    keyIsReserved: 'Schlüssel ist reserviert -- bitte verwende etwas anderes',
     keyTitle: 'Schlüssel',
     languageSelectTitle: 'Sprache auswählen',
     latitudeTitle: 'Breitengrad',
@@ -253,6 +255,7 @@ const strings = new LocalizedStrings({
     formatDeleteConflictError: 'Error: The format could not be deleted because it is still associated with meetings.',
     formatId: 'Format ID',
     formatsTitle: 'Formats',
+    formatTranslationIsRequired: 'a translation is required for all languages for reserved formats',
     formatTypeCodes: [
       { name: 'Attendance by non-addicts (Open, Closed)', value: 'OPEN_OR_CLOSED' },
       { name: 'Common Needs and Restrictions (Mens Meeting, LGTBQ, No Children, etc.)', value: 'COMMON_NEEDS_OR_RESTRICTION' },
@@ -270,6 +273,7 @@ const strings = new LocalizedStrings({
     idTitle: 'ID',
     invalidUsernameOrPassword: 'Invalid username or password.',
     keyIsRequired: 'key is required',
+    keyIsReserved: 'key is reserved  -- please use something else',
     keyTitle: 'Key',
     languageSelectTitle: 'Select Language',
     latitudeTitle: 'Latitude',

diff --git a/src/resources/js/tests/Formats.spec.ts b/src/resources/js/tests/Formats.spec.ts
@@ -17,9 +17,9 @@ describe('check content in Formats tab', () => {
     await login('serveradmin', 'Formats');
     expect(await screen.findByRole('heading', { name: 'Formats', level: 2 })).toBeInTheDocument();
     expect(await screen.findByRole('textbox', { name: 'Search' })).toBeInTheDocument();
-    // There should be 6 formats, with 2 cells per format (name and a delete icon)
+    // There should be 8 formats, with 2 cells per format (name and a delete icon)
     const cells = screen.getAllByRole('cell');
-    expect(cells.length).toBe(12);
+    expect(cells.length).toBe(16);
     // check for a couple of representative formats
     expect(screen.getByRole('cell', { name: 'Closed' })).toBeInTheDocument();
     expect(screen.getByRole('cell', { name: 'Basic Text' })).toBeInTheDocument();
@@ -49,15 +49,32 @@ describe('check content in Formats tab', () => {
   });
 
   test('delete a format', async () => {
+    const user = await login('serveradmin', 'Formats');
+    await user.click(await screen.findByRole('button', { name: 'Delete Format Beginners' }));
+    await user.click(await screen.findByRole('checkbox', { name: "Yes, I'm sure." }));
+    await user.click(await screen.findByRole('button', { name: 'Delete' }));
+    expect(mockDeletedFormatId).toBe(25);
+    expect(mockSavedFormatCreate).toBe(null);
+    expect(mockSavedFormatUpdate).toBe(null);
+  });
+
+  test('try to delete a format that is in use', async () => {
     const user = await login('serveradmin', 'Formats');
     await user.click(await screen.findByRole('button', { name: 'Delete Format Basic Text' }));
     await user.click(await screen.findByRole('checkbox', { name: "Yes, I'm sure." }));
     await user.click(await screen.findByRole('button', { name: 'Delete' }));
-    expect(mockDeletedFormatId).toBe(19);
+    expect(screen.getByText(/Error: The format could not be deleted because it is still associated with meetings./)).toBeInTheDocument();
+    expect(mockDeletedFormatId).toBe(null);
     expect(mockSavedFormatCreate).toBe(null);
     expect(mockSavedFormatUpdate).toBe(null);
   });
 
+  test('delete should be disabled for reserved formats', async () => {
+    await login('serveradmin', 'Formats');
+    const d = await screen.findByRole('button', { name: 'Delete Format Virtual Meeting' });
+    expect(d).toBeDisabled();
+  });
+
   // Alas can't do much testing with create format, due to the problem with the Accordion (see comment at top).
   // This at least tests for the error condition of trying to create a format with no translations.
   test('try to create a format with no translations', async () => {

diff --git a/src/resources/js/tests/sharedDataAndMocks.ts b/src/resources/js/tests/sharedDataAndMocks.ts
@@ -311,6 +311,20 @@ export const agnosticFormat: Format = {
   type: 'COMMON_NEEDS_OR_RESTRICTION'
 };
 
+export const virtualMeetingFormat: Format = {
+  id: 24,
+  translations: [{ key: 'VM', language: 'en', name: 'Virtual Meeting', description: 'Meets Virtually' }],
+  worldId: 'VM',
+  type: 'FC2'
+};
+
+export const beginnersFormat: Format = {
+  id: 25,
+  translations: [{ key: 'B', language: 'en', name: 'Beginners', description: 'This meeting is focused on the needs of new members of NA.' }],
+  worldId: 'BEG',
+  type: 'FC3'
+};
+
 export const ruralMeeting: Meeting = {
   busLines: undefined,
   comments: undefined,
@@ -513,7 +527,7 @@ export const bigRegionMeeting: Meeting = {
 
 export const allServiceBodies: ServiceBody[] = [northernZone, bigRegion, smallRegion, riverCityArea, mountainArea, ruralArea];
 
-export const allFormats: Format[] = [agnosticFormat, basicTextFormat, closedFormat, discussionFormat, jtFormat, openFormat];
+export const allFormats: Format[] = [agnosticFormat, basicTextFormat, beginnersFormat, closedFormat, discussionFormat, jtFormat, openFormat, virtualMeetingFormat];
 
 export const allMeetings: Meeting[] = [ruralMeeting, mountainMeeting, riverCityMeeting, bigRegionMeeting, smallRegionMeeting];
 
@@ -573,6 +587,15 @@ function serviceBodyHasDependents(id: number): boolean {
   return false;
 }
 
+function formatInUse(id: number): boolean {
+  for (const m of allMeetings) {
+    if (m.formatIds.includes(id)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 // define a mock authToken that expires 24 hours from now
 // to trigger a call to authRefresh, set the token to expire in 1 hour instead
 function generateMockAuthToken(userId: number): Token {
@@ -795,7 +818,9 @@ async function mockUpdateFormat({ formatId: _, formatUpdate: format }: { formatI
 }
 
 async function mockDeleteFormat({ formatId: id }: { formatId: number }): Promise<void> {
-  // Do we need to check for dependants? If meeting is using format. this should use foreign keys.
+  if (formatInUse(id)) {
+    throw new ResponseError(makeResponse('Conflict', 409, 'Unauthorized'), 'Response returned an error code');
+  }
   mockDeletedFormatId = id;
 }