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.

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

[BUG]: GitHub App Installation DataStructure's account property is missing data fields #305

Closed
1 task done
Jman420 opened this issue Apr 23, 2023 · 12 comments · Fixed by octokit/openapi#388
Closed
1 task done
Labels
Status: Blocked Some technical or requirement is blocking the issue Type: Bug Something isn't working as documented

Comments

@Jman420
Copy link

Jman420 commented Apr 23, 2023

What happened?

Upgrading to the latest version of Octokit OpenAPI Types has removed a large number of fields from the account property of the InstallationFunctionOptions type. Previously this property contained the union of both simple-user and enterprise schemas, but the latest version seems to have changed this to only the intersection of the two types.

Latest version snippet from types.d.ts :

installation: {
      /**
       * @description The ID of the installation.
       * @example 1
       */
      id: number;
      account:
        | (
            | components["schemas"]["simple-user"]
            | components["schemas"]["enterprise"]
          )
        | null;
      /**
       * @description Describe whether all repositories have been selected or there's a selection involved
       * @enum {string}
       */
      repository_selection: "all" | "selected";
      /**
       * Format: uri
       * @example https://api.github.com/installations/1/access_tokens
       */
      access_tokens_url: string;
      /**
       * Format: uri
       * @example https://api.github.com/installation/repositories
       */
      repositories_url: string;
      /**
       * Format: uri
       * @example https://github.com/organizations/github/settings/installations/1
       */
      html_url: string;
      /** @example 1 */
      app_id: number;
      /** @description The ID of the user or organization this token is being scoped to. */
      target_id: number;
      /** @example Organization */
      target_type: string;
      permissions: components["schemas"]["app-permissions"];
      events: string[];
      /** Format: date-time */
      created_at: string;
      /** Format: date-time */
      updated_at: string;
      /** @example config.yaml */
      single_file_name: string | null;
      /** @example true */
      has_multiple_single_files?: boolean;
      /**
       * @example [
       *   "config.yml",
       *   ".github/issue_TEMPLATE.md"
       * ]
       */
      single_file_paths?: string[];
      /** @example github-actions */
      app_slug: string;
      suspended_by: components["schemas"]["nullable-simple-user"];
      /** Format: date-time */
      suspended_at: string | null;
      /** @example "test_13f1e99741e3e004@d7e1eb0bc0a1ba12.com" */
      contact_email?: string | null;
    };

Previous version :

installation: {
      /**
       * @description The ID of the installation.
       * @example 1
       */
      id: number;
      account:
        | (Partial<components["schemas"]["simple-user"]> &
            Partial<components["schemas"]["enterprise"]>)
        | null;
      /**
       * @description Describe whether all repositories have been selected or there's a selection involved
       * @enum {string}
       */
      repository_selection: "all" | "selected";
      /**
       * Format: uri
       * @example https://api.github.com/installations/1/access_tokens
       */
      access_tokens_url: string;
      /**
       * Format: uri
       * @example https://api.github.com/installation/repositories
       */
      repositories_url: string;
      /**
       * Format: uri
       * @example https://github.com/organizations/github/settings/installations/1
       */
      html_url: string;
      /** @example 1 */
      app_id: number;
      /** @description The ID of the user or organization this token is being scoped to. */
      target_id: number;
      /** @example Organization */
      target_type: string;
      permissions: components["schemas"]["app-permissions"];
      events: string[];
      /** Format: date-time */
      created_at: string;
      /** Format: date-time */
      updated_at: string;
      /** @example config.yaml */
      single_file_name: string | null;
      /** @example true */
      has_multiple_single_files?: boolean;
      /**
       * @example [
       *   "config.yml",
       *   ".github/issue_TEMPLATE.md"
       * ]
       */
      single_file_paths?: string[];
      /** @example github-actions */
      app_slug: string;
      suspended_by: components["schemas"]["nullable-simple-user"];
      /** Format: date-time */
      suspended_at: string | null;
      /** @example "test_13f1e99741e3e004@d7e1eb0bc0a1ba12.com" */
      contact_email?: string | null;
    };

Versions

octokit@2.0.14
node@18.16.0

Working OpenAPI-Types : @octokit/openapi-types@16.0.0
Broken OpenAPI-Types : @octokit/openapi-types@17.0.0

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Jman420 Jman420 added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels Apr 23, 2023
@wolfy1339
Copy link
Member

This is actually not a bug from us, this comes from GitHub.

I suspect they changed an allOf to a oneOf or a anyOf in the schema.

You can file an issue here: https://github.com/github/rest-api-description/

@wolfy1339 wolfy1339 added Status: Blocked Some technical or requirement is blocking the issue and removed Status: Triage This is being looked at and prioritized labels Apr 23, 2023
@Jman420
Copy link
Author

Jman420 commented Apr 23, 2023

@wolfy1339 Thanks for the guidance. Gonna dig in and open an issue over there. Would you like this issue closed once I open one in the github repo?

@wolfy1339
Copy link
Member

This one can stay open for tracking on our side. You can also link back to here in the issue you create in the GitHub repo

@larshp
Copy link

larshp commented May 15, 2023

loooks like octokit/types.ts#534 is a related issue

@Jman420
Copy link
Author

Jman420 commented Aug 22, 2023

This issue doesn't appear to be getting addressed by GitHub. Is there any recommendation that I can patch the issue in this repo while we wait for GitHub to fix the documentation issue?

@wolfy1339
Copy link
Member

We can possibly add an override for this scenario in the octokit/openapi repo.

@Jman420
Copy link
Author

Jman420 commented Aug 22, 2023

I'm happy to draft a fix if someone can point me in the right direction. Just not sure where the right place is...

@wolfy1339
Copy link
Member

There isn't really any docs on the subject. You will have to copy from the other overrides.

The override goes here, in this folder:
https://github.com/octokit/openapi/tree/main/scripts/overrides

You also need to edit https://github.com/octokit/openapi/blob/main/scripts/overrides/index.mjs to add the override.

@Jman420
Copy link
Author

Jman420 commented Aug 22, 2023

Got it. No worries about docs... those examples should be good to get me off the ground. Thanks!

@gr2m
Copy link
Contributor

gr2m commented Aug 23, 2023

There isn't really any docs on the subject

that's a good call out, we should absolutely add a note about overrides to https://github.com/octokit/openapi/blob/main/CONTRIBUTING.md

@Jman420
Copy link
Author

Jman420 commented Sep 3, 2023

@wolfy1339 or @gr2m Are there any pointers you can give me for creating the actual override json? Can I copy an original from somewhere or generate it from the original spec to start making my modifications?

I figured out how I need to add the entries to index.mjs and that I need two override files (one with references & one with the references replaced with the referenced spec). But not sure where to get an original file to start editing for the overrides.

@wolfy1339
Copy link
Member

You can copy the definition from the OpenAPI spec and make modifications to that.

You can follow the general structure of the other files

wolfy1339 pushed a commit to octokit/openapi that referenced this issue Sep 22, 2023
Add override for `/app/installations/` response for account property to be an `allOf` list instead of `anyOf`

Resolves octokit/openapi-types.ts#305
@github-project-automation github-project-automation bot moved this from 🔥 Backlog to ✅ Done in 🧰 Octokit Active Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Blocked Some technical or requirement is blocking the issue Type: Bug Something isn't working as documented
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants