Repository: Redocly/redocly-cli
Pull request #890 (2022-09-29 18:46:05)

fix: push destination parsing
## What/Why/How?

## Reference
https://github.com/Redocly/workflows/issues/2498

## Testing

## Screenshots (optional)

## Check yourself

- [x] Code is linted
- [ ] Tested with redoc/reference-docs/workflows
- [x] All new/updated code is covered with tests

## Security

- [x] Security impact of change has been considered
- [x] Code follows company security practices and guidelines


github-actions[bot]: <!-- jest coverage report action for options with hash 3339ae53a38c44382c0fb4491bdb282d -->

## Coverage report 



| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | Category   | Percentage | Covered / Total |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :--------- | :--------- | :-------------: |
|                                                                           🟡                                                                           | Statements | 73.17%     |    3267/4465    |
|                                                                           🟡                                                                           | Branches   | 64.84%     |    1829/2821    |
|                                                                           🟡                                                                           | Functions  | 65.5%      |     545/832     |
|                                                                           🟡                                                                           | Lines      | 73.08%     |    3035/4153    |


<details><summary>Show files with reduced coverage 🔻</summary>
<br/>

| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | File                                                        | Statements | Branches                                                       | Functions                                                      | Lines  |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :---------------------------------------------------------- | :--------- | :------------------------------------------------------------- | :------------------------------------------------------------- | :----- |
|                                                                           🟡                                                                           | <div title="cli/src/commands/push.ts">`...` / push.ts</div> | 71.14%     | <div title="Base coverage is: 61.05%">62.14% (+1.08% 🔼)</div> | <div title="Base coverage is: 59.09%">57.14% (-1.95% 🔻)</div> | 73.38% |

</details>

## Test suite run success
505 tests passing in 85 suites.

<p align="right">Report generated by <a href="https://github.com/ArtiomTr/jest-coverage-report-action">🧪jest coverage report action</a> from d14bfe731b2a23486a1edfb50f7988cbaf49fa9c</p>

RomanHotsiy: @AntonKozachuk please add a description to PRs. Thanks.

Patch:
diff --git a/packages/cli/src/commands/push.ts b/packages/cli/src/commands/push.ts
index ab15f346..fdd9520c 100644
--- a/packages/cli/src/commands/push.ts
+++ b/packages/cli/src/commands/push.ts
@@ -55,10 +55,10 @@ export async function handlePush(argv: PushArgs): Promise<void> {
   const batchId = argv['batch-id'];
   const batchSize = argv['batch-size'];
 
-  if (
-    destination &&
-    !(validateDestination(destination) || validateDestinationWithoutOrganization(destination))
-  ) {
+  const isValidDestination =
+    destination && Object.values(validateDestination(destination) || {}).every(Boolean);
+
+  if (!isValidDestination) {
     exitWithError(
       `Destination argument value is not valid, please use the right format: ${yellow(
         '<@organization-id/api-name@api-version>'
@@ -66,7 +66,7 @@ export async function handlePush(argv: PushArgs): Promise<void> {
     );
   }
 
-  const [organizationId, name, version] = getDestinationProps(destination, config.organization);
+  const { organizationId, name, version } = getDestinationProps(destination, config.organization);
 
   if (!organizationId) {
     return exitWithError(
@@ -302,24 +302,26 @@ function hashFiles(filePaths: { filePath: string }[]) {
 }
 
 function validateDestination(destination: string) {
-  const regexp = /^@+([a-zA-Z0-9-_.& ]+)\/+([^@\/]+)@([^@\/]+)$/;
-  return regexp.test(destination);
-}
+  const regexp = /^(@(?<organizationId>\w+)\/)?(?<name>.*)@(?<version>[\w\.\-\_]+)$/;
 
-function validateDestinationWithoutOrganization(destination: string) {
-  const regexp = /^()([^@\/]+)@([^@\/]+)$/;
-  return regexp.test(destination);
+  return destination?.match(regexp)?.groups;
 }
 
 export function getDestinationProps(
   destination: string | undefined,
   organization: string | undefined
 ) {
-  return destination && validateDestination(destination)
-    ? destination.substring(1).split(/[@\/]/)
-    : destination && validateDestinationWithoutOrganization(destination)
-    ? [organization, ...destination.split('@')]
-    : [organization];
+  const groups = destination && validateDestination(destination);
+  if (groups) {
+    groups.name && (groups.name = encodeURIComponent(groups.name));
+    return {
+      organizationId: groups.organizationId || organization,
+      name: groups.name,
+      version: groups.version,
+    };
+  } else {
+    return { organizationId: organization, name: undefined, version: undefined };
+  }
 }
 
 type BarePushArgs = Omit<PushArgs, 'api' | 'destination' | 'branchName'> & {


Test patch:
diff --git a/packages/cli/src/__tests__/commands/push.test.ts b/packages/cli/src/__tests__/commands/push.test.ts
index cbc08783..4b802d93 100644
--- a/packages/cli/src/__tests__/commands/push.test.ts
+++ b/packages/cli/src/__tests__/commands/push.test.ts
@@ -177,24 +177,30 @@ describe('transformPush', () => {
 
 describe('getDestinationProps', () => {
   it('should get valid destination props for the full destination syntax', () => {
-    expect(getDestinationProps('@testing_org/main@v1', 'org-from-config')).toEqual([
-      'testing_org',
-      'main',
-      'v1',
-    ]);
+    expect(getDestinationProps('@testing_org/main@v1', 'org-from-config')).toEqual({
+      organizationId: 'testing_org',
+      name: 'main',
+      version: 'v1',
+    });
   });
   it('should fallback the organizationId from a config for the short destination syntax', () => {
-    expect(getDestinationProps('main@v1', 'org-from-config')).toEqual([
-      'org-from-config',
-      'main',
-      'v1',
-    ]);
+    expect(getDestinationProps('main@v1', 'org-from-config')).toEqual({
+      organizationId: 'org-from-config',
+      name: 'main',
+      version: 'v1',
+    });
   });
   it('should fallback the organizationId from a config if no destination provided', () => {
-    expect(getDestinationProps(undefined, 'org-from-config')).toEqual(['org-from-config']);
+    expect(getDestinationProps(undefined, 'org-from-config')).toEqual({
+      organizationId: 'org-from-config',
+    });
   });
   it('should return empty organizationId if there is no one found', () => {
-    expect(getDestinationProps('main@v1', undefined)).toEqual([, 'main', 'v1']);
+    expect(getDestinationProps('main@v1', undefined)).toEqual({
+      organizationId: undefined,
+      name: 'main',
+      version: 'v1',
+    });
   });
 });