fix: fetchComponent when shared name prefix or suffix (#198)

* fix: fetchComponent when common name prefix

* test: 422 responses when upserting components

* fix: finding matching preset when names aren't unique

Author: angelikatyborska

src/commands/components/pull/actions.test.ts

@@ -24,6 +24,16 @@ const mockedComponents = [{
   color: null,
   internal_tags_list: ['tag'],
   internal_tag_ids: [1],
+}, {
+  name: 'name-2',
+  display_name: 'Name 2',
+  created_at: '2021-08-09T12:00:00Z',
+  updated_at: '2021-08-09T12:00:00Z',
+  id: 12346,
+  schema: { type: 'object' },
+  color: null,
+  internal_tags_list: [],
+  internal_tag_ids: [],
 }];
 
 const handlers = [
@@ -70,6 +80,16 @@ describe('pull components actions', () => {
       color: null,
       internal_tags_list: ['tag'],
       internal_tag_ids: [1],
+    }, {
+      name: 'name-2',
+      display_name: 'Name 2',
+      created_at: '2021-08-09T12:00:00Z',
+      updated_at: '2021-08-09T12:00:00Z',
+      id: 12346,
+      schema: { type: 'object' },
+      color: null,
+      internal_tags_list: [],
+      internal_tag_ids: [],
     }];
 
     const result = await fetchComponents('12345', 'valid-token', 'eu');
@@ -94,6 +114,25 @@ describe('pull components actions', () => {
     expect(result).toEqual(mockResponse.components[0]);
   });
 
+  it('should choose the right component when multiple names match', async () => {
+    const mockResponse = {
+      components: [{
+        name: 'name-2',
+        display_name: 'Name 2',
+        created_at: '2021-08-09T12:00:00Z',
+        updated_at: '2021-08-09T12:00:00Z',
+        id: 12346,
+        schema: { type: 'object' },
+        color: null,
+        internal_tags_list: [],
+        internal_tag_ids: [],
+      }],
+    };
+    // searching for 'name-2' would match both 'component-name-2' and 'name-2'
+    const result = await fetchComponent('12345', 'name-2', 'valid-token', 'eu');
+    expect(result).toEqual(mockResponse.components[0]);
+  });
+
   it('should throw an masked error for invalid token', async () => {
     await expect(fetchComponents('12345', 'invalid-token', 'eu')).rejects.toThrow(
       expect.objectContaining({

src/commands/components/pull/actions.ts

@@ -34,7 +34,7 @@ export const fetchComponent = async (space: string, componentName: string, token
         Authorization: token,
       },
     });
-    return response.components?.[0];
+    return response.components?.find(c => c.name === componentName);
   }
   catch (error) {
     handleAPIError('pull_components', error as Error, `Failed to fetch component ${componentName}`);

src/commands/components/push/actions.test.ts

@@ -17,6 +17,18 @@ const mockComponent: SpaceComponent = {
   internal_tag_ids: ['1'],
 };
 
+const mockComponentExisting: SpaceComponent = {
+  name: 'component-name-2',
+  display_name: 'Component Name @',
+  created_at: '2021-08-09T12:00:00Z',
+  updated_at: '2021-08-09T12:00:00Z',
+  id: 12346,
+  schema: { type: 'object' },
+  color: null,
+  internal_tags_list: [],
+  internal_tag_ids: ['1'],
+};
+
 const mockComponentGroup: SpaceComponentGroup = {
   name: 'group-name',
   uuid: 'group-uuid',
@@ -25,6 +37,14 @@ const mockComponentGroup: SpaceComponentGroup = {
   parent_uuid: 'parent-uuid',
 };
 
+const mockComponentGroupExisting: SpaceComponentGroup = {
+  name: 'group-name-2',
+  uuid: 'group-uuid-2',
+  id: 2,
+  parent_id: 0,
+  parent_uuid: 'parent-uuid',
+};
+
 const mockComponentPreset: SpaceComponentPreset = {
   id: 1,
   name: 'preset-name',
@@ -39,17 +59,59 @@ const mockComponentPreset: SpaceComponentPreset = {
   description: '',
 };
 
+const mockComponentPresetExisting: SpaceComponentPreset = {
+  id: 2,
+  name: 'preset-name-2',
+  component_id: 1,
+  preset: { field: 'value' },
+  space_id: 12345,
+  created_at: '2021-08-09T12:00:00Z',
+  updated_at: '2021-08-09T12:00:00Z',
+  image: '',
+  color: '',
+  icon: '',
+  description: '',
+};
+
+const mockComponentPresetExistingDifferentComponent: SpaceComponentPreset = {
+  id: 3,
+  // component present names aren't globally unique, they're only unique for each component
+  name: 'preset-name-2',
+  component_id: 2,
+  preset: { field: 'value' },
+  space_id: 12345,
+  created_at: '2021-08-09T12:00:00Z',
+  updated_at: '2021-08-09T12:00:00Z',
+  image: '',
+  color: '',
+  icon: '',
+  description: '',
+};
+
 const mockInternalTag: SpaceComponentInternalTag = {
   id: 1,
   name: 'tag-name',
+  object_type: 'component',
+};
+
+const mockInternalTagExisting: SpaceComponentInternalTag = {
+  id: 2,
+  name: 'tag-name-2',
+  object_type: 'component',
 };
 
 const handlers = [
   // Component handlers
   http.post('https://api.storyblok.com/v1/spaces/12345/components', async ({ request }) => {
     const token = request.headers.get('Authorization');
     if (token === 'valid-token') {
-      return HttpResponse.json({ component: mockComponent });
+      const body: any = await request.json();
+      if (body.id === mockComponentExisting.id) {
+        return HttpResponse.json({ name: ['has already been taken'] }, { status: 422 });
+      }
+      else {
+        return HttpResponse.json({ component: mockComponent });
+      }
     }
     return new HttpResponse('Unauthorized', { status: 401 });
   }),
@@ -60,12 +122,32 @@ const handlers = [
     }
     return new HttpResponse('Unauthorized', { status: 401 });
   }),
+  http.get('https://api.storyblok.com/v1/spaces/12345/components', async ({ request }) => {
+    const token = request.headers.get('Authorization');
+    if (token === 'valid-token') {
+      return HttpResponse.json({ components: [mockComponentExisting] });
+    }
+    return new HttpResponse('Unauthorized', { status: 401 });
+  }),
+  http.put('https://api.storyblok.com/v1/spaces/12345/components/12346', async ({ request }) => {
+    const token = request.headers.get('Authorization');
+    if (token === 'valid-token') {
+      return HttpResponse.json({ component: mockComponentExisting });
+    }
+    return new HttpResponse('Unauthorized', { status: 401 });
+  }),
 
   // Component group handlers
   http.post('https://api.storyblok.com/v1/spaces/12345/component_groups', async ({ request }) => {
     const token = request.headers.get('Authorization');
     if (token === 'valid-token') {
-      return HttpResponse.json({ component_group: mockComponentGroup });
+      const body: any = await request.json();
+      if (body.id === mockComponentPresetExisting.id) {
+        return HttpResponse.json({ name: ['has already been taken'] }, { status: 422 });
+      }
+      else {
+        return HttpResponse.json({ component_group: mockComponentGroup });
+      }
     }
     return new HttpResponse('Unauthorized', { status: 401 });
   }),
@@ -76,12 +158,32 @@ const handlers = [
     }
     return new HttpResponse('Unauthorized', { status: 401 });
   }),
+  http.get('https://api.storyblok.com/v1/spaces/12345/component_groups', async ({ request }) => {
+    const token = request.headers.get('Authorization');
+    if (token === 'valid-token') {
+      return HttpResponse.json({ component_groups: [mockComponentGroupExisting] });
+    }
+    return new HttpResponse('Unauthorized', { status: 401 });
+  }),
+  http.put('https://api.storyblok.com/v1/spaces/12345/component_groups/2', async ({ request }) => {
+    const token = request.headers.get('Authorization');
+    if (token === 'valid-token') {
+      return HttpResponse.json({ component_group: mockComponentGroupExisting });
+    }
+    return new HttpResponse('Unauthorized', { status: 401 });
+  }),
 
   // Component preset handlers
   http.post('https://api.storyblok.com/v1/spaces/12345/presets', async ({ request }) => {
     const token = request.headers.get('Authorization');
     if (token === 'valid-token') {
-      return HttpResponse.json({ preset: mockComponentPreset });
+      const body: any = await request.json();
+      if (body.preset.id === mockComponentPresetExisting.id || body.preset.id === mockComponentPresetExistingDifferentComponent.id) {
+        return HttpResponse.json({ name: ['has already been taken'] }, { status: 422 });
+      }
+      else {
+        return HttpResponse.json({ preset: mockComponentPreset });
+      }
     }
     return new HttpResponse('Unauthorized', { status: 401 });
   }),
@@ -92,12 +194,39 @@ const handlers = [
     }
     return new HttpResponse('Unauthorized', { status: 401 });
   }),
+  http.get('https://api.storyblok.com/v1/spaces/12345/presets', async ({ request }) => {
+    const token = request.headers.get('Authorization');
+    if (token === 'valid-token') {
+      return HttpResponse.json({ presets: [mockComponentPresetExisting, mockComponentPresetExistingDifferentComponent] });
+    }
+    return new HttpResponse('Unauthorized', { status: 401 });
+  }),
+  http.put('https://api.storyblok.com/v1/spaces/12345/presets/2', async ({ request }) => {
+    const token = request.headers.get('Authorization');
+    if (token === 'valid-token') {
+      return HttpResponse.json({ preset: mockComponentPresetExisting });
+    }
+    return new HttpResponse('Unauthorized', { status: 401 });
+  }),
+  http.put('https://api.storyblok.com/v1/spaces/12345/presets/3', async ({ request }) => {
+    const token = request.headers.get('Authorization');
+    if (token === 'valid-token') {
+      return HttpResponse.json({ preset: mockComponentPresetExistingDifferentComponent });
+    }
+    return new HttpResponse('Unauthorized', { status: 401 });
+  }),
 
   // Internal tag handlers
   http.post('https://api.storyblok.com/v1/spaces/12345/internal_tags', async ({ request }) => {
     const token = request.headers.get('Authorization');
     if (token === 'valid-token') {
-      return HttpResponse.json({ internal_tag: mockInternalTag });
+      const body: any = await request.json();
+      if (body.id === mockInternalTagExisting.id) {
+        return HttpResponse.json({ name: ['has already been taken'] }, { status: 422 });
+      }
+      else {
+        return HttpResponse.json({ internal_tag: mockInternalTag });
+      }
     }
     return new HttpResponse('Unauthorized', { status: 401 });
   }),
@@ -108,6 +237,20 @@ const handlers = [
     }
     return new HttpResponse('Unauthorized', { status: 401 });
   }),
+  http.get('https://api.storyblok.com/v1/spaces/12345/internal_tags', async ({ request }) => {
+    const token = request.headers.get('Authorization');
+    if (token === 'valid-token') {
+      return HttpResponse.json({ internal_tags: [mockInternalTagExisting] });
+    }
+    return new HttpResponse('Unauthorized', { status: 401 });
+  }),
+  http.put('https://api.storyblok.com/v1/spaces/12345/internal_tags/2', async ({ request }) => {
+    const token = request.headers.get('Authorization');
+    if (token === 'valid-token') {
+      return HttpResponse.json({ internal_tag: mockInternalTagExisting });
+    }
+    return new HttpResponse('Unauthorized', { status: 401 });
+  }),
 ];
 
 const server = setupServer(...handlers);
@@ -136,6 +279,11 @@ describe('push components actions', () => {
       expect(result).toEqual(mockComponent);
     });
 
+    it('should upsert existing component successfully with a valid token', async () => {
+      const result = await upsertComponent('12345', mockComponentExisting, 'valid-token', 'eu');
+      expect(result).toEqual(mockComponentExisting);
+    });
+
     it('should throw an error for invalid token', async () => {
       await expect(pushComponent('12345', mockComponent, 'invalid-token', 'eu')).rejects.toThrow(
         expect.objectContaining({
@@ -179,6 +327,11 @@ describe('push components actions', () => {
       const result = await upsertComponentGroup('12345', mockComponentGroup, 'valid-token', 'eu');
       expect(result).toEqual(mockComponentGroup);
     });
+
+    it('should upsert existing component group successfully with a valid token', async () => {
+      const result = await upsertComponentGroup('12345', mockComponentGroupExisting, 'valid-token', 'eu');
+      expect(result).toEqual(mockComponentGroupExisting);
+    });
   });
 
   describe('component preset', () => {
@@ -196,6 +349,16 @@ describe('push components actions', () => {
       const result = await upsertComponentPreset('12345', mockComponentPreset, 'valid-token', 'eu');
       expect(result).toEqual(mockComponentPreset);
     });
+
+    it('should upsert existing component preset successfully with a valid token', async () => {
+      const result = await upsertComponentPreset('12345', mockComponentPresetExisting, 'valid-token', 'eu');
+      expect(result).toEqual(mockComponentPresetExisting);
+    });
+
+    it('should upsert existing component preset with a duplicated name successfully with a valid token', async () => {
+      const result = await upsertComponentPreset('12345', mockComponentPresetExistingDifferentComponent, 'valid-token', 'eu');
+      expect(result).toEqual(mockComponentPresetExistingDifferentComponent);
+    });
   });
 
   describe('component internal tag', () => {
@@ -213,6 +376,11 @@ describe('push components actions', () => {
       const result = await upsertComponentInternalTag('12345', mockInternalTag, 'valid-token', 'eu');
       expect(result).toEqual(mockInternalTag);
     });
+
+    it('should upsert existing component internal tag successfully with a valid token', async () => {
+      const result = await upsertComponentInternalTag('12345', mockInternalTagExisting, 'valid-token', 'eu');
+      expect(result).toEqual(mockInternalTagExisting);
+    });
   });
 
   describe('readComponentsFiles', () => {

src/commands/components/push/actions.ts

@@ -181,7 +181,7 @@ export const upsertComponentPreset = async (space: string, preset: Partial<Space
       if (responseData?.name?.[0] === 'has already been taken') {
         // Find existing preset by name
         const existingPresets = await fetchComponentPresets(space, token, region);
-        const existingPreset = existingPresets?.find(p => p.name === preset.name);
+        const existingPreset = existingPresets?.find(p => p.name === preset.name && p.component_id === preset.component_id);
         if (existingPreset) {
           // Update existing preset
           return await updateComponentPreset(space, existingPreset.id, { preset }, token, region);