Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: console error messages and better names for requests imported from openapi #3948

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: console error messages and better names for requests imported from openapi #3948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/hoppscotch-common/locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@
"curl": "Import cURL",
"environments_from_gist": "Import From Gist",
"environments_from_gist_description": "Import Hoppscotch Environments From Gist",
"failed": "Error while importing: format not recognized",
"failed": "Import error. Check your browser console for error details.",
"from_file": "Import from File",
"from_gist": "Import from Gist",
"from_gist_description": "Import from Gist URL",
Expand Down
8 changes: 4 additions & 4 deletions packages/hoppscotch-common/locales/es.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,10 @@
"import": {
"collections": "Importar colecciones",
"curl": "Importar cURL",
"environments_from_gist": "Import From Gist",
"environments_from_gist_description": "Import Hoppscotch Environments From Gist",
"failed": "Importación fallida",
"from_file": "Import from File",
"environments_from_gist": "Importar desde Gist",
"environments_from_gist_description": "Importar entornos Hoppscotch desde Gist",
"failed": "Error de importación. Comprueba la consola de su navegador para ver los detalles del error.",
"from_file": "Importar desde archivo",
"from_gist": "Importar desde Gist",
"from_gist_description": "Importar desde URL de Gist",
"from_insomnia": "Importar desde Insomnia",
Expand Down
54 changes: 37 additions & 17 deletions packages/hoppscotch-common/src/helpers/import-export/import/openapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
import {
OpenAPI,
OpenAPIV2,
OpenAPIV3,
OpenAPIV3_1 as OpenAPIV31,
} from "openapi-types"
import SwaggerParser from "@apidevtools/swagger-parser"
import yaml from "js-yaml"
import {
FormDataKeyValue,
HoppCollection,
HoppRESTAuth,
HoppRESTHeader,
HoppRESTParam,
HoppRESTReqBody,
HoppRESTRequest,
HoppRESTRequestVariable,
knownContentTypes,
makeRESTRequest,
HoppCollection,
makeCollection,
HoppRESTRequestVariable,
HoppRESTRequest,
makeRESTRequest,
} from "@hoppscotch/data"
import { pipe, flow } from "fp-ts/function"
import * as A from "fp-ts/Array"
import * as S from "fp-ts/string"
import * as O from "fp-ts/Option"
import * as TE from "fp-ts/TaskEither"
import * as RA from "fp-ts/ReadonlyArray"
import { IMPORTER_INVALID_FILE_FORMAT } from "."
import * as TE from "fp-ts/TaskEither"
import { flow, pipe } from "fp-ts/function"
import * as S from "fp-ts/string"
import yaml from "js-yaml"
import { cloneDeep } from "lodash-es"
import {
OpenAPI,
OpenAPIV2,
OpenAPIV3,
OpenAPIV3_1 as OpenAPIV31,
} from "openapi-types"
import { IMPORTER_INVALID_FILE_FORMAT } from "."

export const OPENAPI_DEREF_ERROR = "openapi/deref_error" as const

Expand Down Expand Up @@ -627,14 +627,27 @@ const convertPathToHoppReqs = (
? openAPIUrl + openAPIPath.slice(1)
: openAPIUrl + openAPIPath

let defaultRequestName = "Untitled Request"

if (info.tags?.length) {
const pathStartIndex =
openAPIPath.indexOf(info.tags[0]) + info.tags[0].length
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here this logic will miss some edge cases,

i belive you're assuming that the tag will always be present in the path, but that won't always be the case.

  1. what happens if the tag is no way related to the path ?
  2. adding the tag length makes sense only if the tag is found in the path, otherwise you're adding the length to -1, which does not makes much sense.

Copy link
Contributor Author

@sawa-ko sawa-ko Apr 10, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. If it does not have a tag then the condition is not met and is saved as a request without a title. What I can do is save it with the name of the controller in the request in case it don't have tags.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I quoted the wrong lines actually. the comment is meant for the next line.

const pathStartIndex =
          openAPIPath.indexOf(info.tags[0]) + info.tags[0].length

and i didn't mean when the tag is not present, i meant when a tag is present but it is not in the url.

like this,

/categories/{categoryId}/items:
    get:
      tags:
        - SomethingNonRelatedToTheURL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about it, and I can't find a way to know from which text index to parse the url as name, any idea? My initial motivation for using the tags was because normally people generate the openapi specs automatically based on the code in their apis, but with the point that the first tag may not be the controller itself, I can't think of another idea.


if (pathStartIndex > 0) {
defaultRequestName =
openAPIPath.slice(pathStartIndex).split("/")?.[1] ??
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when naming the request, here it seems we're considering the first value after the tag only, eg: the current logic will name /categories/<<categoryID>/update as <<categoryID>>, but i would prefer the request to be named, whatever the remaining path is after the tag. in this case <<categoryID>>/update

defaultRequestName
}
}

const res: {
request: HoppRESTRequest
metadata: {
tags: string[]
}
} = {
request: makeRESTRequest({
name: info.operationId ?? info.summary ?? "Untitled Request",
name: info.operationId ?? info.summary ?? defaultRequestName,
method: method.toUpperCase(),
endpoint,

Expand Down Expand Up @@ -753,7 +766,14 @@ export const hoppOpenAPIImporter = (fileContents: string[]) =>

return resultDoc
},
() => IMPORTER_INVALID_FILE_FORMAT
(e) => {
console.error(
(e as { message?: string })?.message ??
"An unexpected error occurred while parsing the OpenApi file."
)

return IMPORTER_INVALID_FILE_FORMAT
}
)
)
}),
Expand Down