TykTechnologies / tyk

Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols
Other
9.51k stars 1.07k forks source link

[TT-12195] : Public playground still shows schema when introspection is turned off #6397

Closed jay-deshmukh closed 1 month ago

jay-deshmukh commented 1 month ago

User description

Description

Schema handling for public playground : Do not show schema by default for public playground

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

Checklist


PR Type

Bug fix


Description


Changes walkthrough ๐Ÿ“

Relevant files
Bug fix
api_loader.go
Remove schema from GraphQL playground template data           

gateway/api_loader.go
  • Removed the Schema field from the struct passed to the playground
    template.
  • Updated the struct to only include Url and PathPrefix.
  • +2/-2     
    index.html
    Exclude schema from GraphiQL initialization in playground

    templates/playground/index.html
  • Removed the schema variable initialization.
  • Updated GraphiQlInitInstance initialization to exclude the schema
    field.
  • +0/-2     

    ๐Ÿ’ก PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    github-actions[bot] commented 1 month ago

    API Changes

    no api changes detected
    github-actions[bot] commented 1 month ago

    PR Reviewer Guide ๐Ÿ”

    โฑ๏ธ Estimated effort to review: 2 ๐Ÿ”ต๐Ÿ”ตโšชโšชโšช
    ๐Ÿงช No relevant tests
    ๐Ÿ”’ No security concerns identified
    โšก Key issues to review

    **Possible Bug:** Ensure that removing the schema from the playground does not affect other functionalities that might rely on the schema being present.
    github-actions[bot] commented 1 month ago

    PR Code Suggestions โœจ

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Improve struct initialization by explicitly naming the fields ___ **The struct initialization in playgroundTemplate.ExecuteTemplate should explicitly
    name the fields to avoid future errors if the struct definition changes.** [gateway/api_loader.go [907]](https://github.com/TykTechnologies/tyk/pull/6397/files#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68R907-R907) ```diff -{endpoint, path.Join(endpoint, playgroundPath)} +{Url: endpoint, PathPrefix: path.Join(endpoint, playgroundPath)} ```
    Suggestion importance[1-10]: 9 Why: Explicitly naming the fields in the struct initialization improves code readability and maintainability, and helps prevent future errors if the struct definition changes.
    9
    Enhancement
    Enhance error handling by logging and providing detailed client responses ___ **Consider handling the error from playgroundTemplate.ExecuteTemplate more
    comprehensively by logging the error or providing a more informative response to the
    client.** [gateway/api_loader.go [910]](https://github.com/TykTechnologies/tyk/pull/6397/files#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68R910-R910) ```diff if err != nil { - rw.WriteHeader(http.StatusInternalServerError) + log.Printf("Error executing template: %v", err) + http.Error(rw, "Internal Server Error", http.StatusInternalServerError) ```
    Suggestion importance[1-10]: 8 Why: Logging the error and providing a detailed response to the client improves debugging and user experience, making the system more robust.
    8
    Best practice
    Add error handling for the JavaScript instance creation to improve robustness ___ **Add error handling for the GraphiqlInit instance creation to catch and display
    errors related to initialization or configuration issues.** [templates/playground/index.html [78-81]](https://github.com/TykTechnologies/tyk/pull/6397/files#diff-98c2e4a69b270a6e10203409fff11a7ec1253faeaa0c1dd1b5cec5e137dcbf6fR78-R81) ```diff -const GraphiQlInitInstance = new window.GraphiqlInit({ - containerId: 'root', - schemaUrls: [{url}], - onSubscriptionError: window.showError +try { + const GraphiQlInitInstance = new window.GraphiqlInit({ + containerId: 'root', + schemaUrls: [{url}], + onSubscriptionError: window.showError + }); +} catch (error) { + window.showError(error.message); +} ```
    Suggestion importance[1-10]: 7 Why: Adding error handling for the `GraphiqlInit` instance creation is a good practice that can help catch and display errors related to initialization or configuration issues, improving the robustness of the code.
    7
    sonarcloud[bot] commented 1 month ago

    Quality Gate Passed Quality Gate passed

    Issues
    0 New issues
    0 Accepted issues

    Measures
    0 Security Hotspots
    100.0% Coverage on New Code
    0.0% Duplication on New Code

    See analysis details on SonarCloud

    jay-deshmukh commented 1 month ago

    /release to release-5.4

    tykbot[bot] commented 1 month ago

    Working on it! Note that it can take a few minutes.

    tykbot[bot] commented 1 month ago

    @jay-deshmukh Succesfully merged PR