gr2m / octokit-plugin-create-pull-request

Octokit plugin to create a pull request with multiple file changes
MIT License
104 stars 28 forks source link

Unable to update/add files when updating a pull request #107

Closed st3phhays closed 2 years ago

st3phhays commented 2 years ago

I am running into an issue where I am unable to add new files or update an existing file in a pull request that I have created with this plugin. To reproduce:

First run the code in the base example on the README (update your TOKEN):

const { Octokit } = require("@octokit/core");
const { createPullRequest } = require("octokit-plugin-create-pull-request");
const MyOctokit = Octokit.plugin(createPullRequest);

const TOKEN = "secret123"; // create token at https://github.com/settings/tokens/new?scopes=repo
const octokit = new MyOctokit({
  auth: TOKEN,
});

// Returns a normal Octokit PR response
// See https://octokit.github.io/rest.js/#octokit-routes-pulls-create
octokit
  .createPullRequest({
    owner: "user-or-org-login",
    repo: "repo-name",
    title: "pull request title",
    body: "pull request description",
    head: "pull-request-branch-name",
    base: "main" /* optional: defaults to default branch */,
    update: true /* optional: set to `true` to enable updating existing pull requests */,
    forceFork: false /* optional: force creating fork even when user has write rights */,
    changes: [
      {
        /* optional: if `files` is not passed, an empty commit is created instead */
        files: {
          "path/to/file1.txt": "Content for file1",
          "path/to/file2.png": {
            content: "_base64_encoded_content_",
            encoding: "base64",
          },
          // deletes file if it exists,
          "path/to/file3.txt": null,
          // updates file based on current content
          "path/to/file4.txt": ({ exists, encoding, content }) => {
            // do not create the file if it does not exist
            if (!exists) return null;

            return Buffer.from(content, encoding)
              .toString("utf-8")
              .toUpperCase();
          },
          "path/to/file5.sh": {
            content: "echo Hello World",
            encoding: "utf-8",
            // one of the modes supported by the git tree object
            // https://developer.github.com/v3/git/trees/#tree-object
            mode: "100755",
          },
        },
        commit:
          "creating file1.txt, file2.png, deleting file3.txt, updating file4.txt (if it exists), file5.sh",
      },
    ],
  })
  .then((pr) => console.log(pr.data.number));

This should create a PR successfully. Next, add a new file to this code. Note how I have added a new file1-1.txt file.

const { Octokit } = require("@octokit/core");
const { createPullRequest } = require("octokit-plugin-create-pull-request");
const MyOctokit = Octokit.plugin(createPullRequest);

const TOKEN = "secret123"; // create token at https://github.com/settings/tokens/new?scopes=repo
const octokit = new MyOctokit({
  auth: TOKEN,
});

// Returns a normal Octokit PR response
// See https://octokit.github.io/rest.js/#octokit-routes-pulls-create
octokit
  .createPullRequest({
    owner: "user-or-org-login",
    repo: "repo-name",
    title: "pull request title",
    body: "pull request description",
    head: "pull-request-branch-name",
    base: "main" /* optional: defaults to default branch */,
    update: true /* optional: set to `true` to enable updating existing pull requests */,
    forceFork: false /* optional: force creating fork even when user has write rights */,
    changes: [
      {
        /* optional: if `files` is not passed, an empty commit is created instead */
        files: {
          "path/to/file1.txt": "Content for file1",
          "path/to/file1-1.txt": "Content for file1-1",
          "path/to/file2.png": {
            content: "_base64_encoded_content_",
            encoding: "base64",
          },
          // deletes file if it exists,
          "path/to/file3.txt": null,
          // updates file based on current content
          "path/to/file4.txt": ({ exists, encoding, content }) => {
            // do not create the file if it does not exist
            if (!exists) return null;

            return Buffer.from(content, encoding)
              .toString("utf-8")
              .toUpperCase();
          },
          "path/to/file5.sh": {
            content: "echo Hello World",
            encoding: "utf-8",
            // one of the modes supported by the git tree object
            // https://developer.github.com/v3/git/trees/#tree-object
            mode: "100755",
          },
        },
        commit:
          "creating file1.txt, file2.png, deleting file3.txt, updating file4.txt (if it exists), file5.sh",
      },
    ],
  })
  .then((pr) => console.log(pr.data.number));

Running this again does not add the new file1-1.txt file. I am also unable to update an existing files content as well if it was added during the first push of the PR.

I have tested updating the Title of the PR, and that does seem to work perfectly.

Here is my repo I'm tesitng this out on: https://github.com/craftastack/craftastack/pull/1

st3phhays commented 2 years ago

I think when I ran the first initial PR, I had update: false, and then I changed it to update: true later. I can not recall, however, I'm not sure if that matters or not.

gr2m commented 2 years ago

Great issue, thanks for sharing all the code! I should be able to have a look at it on Monday

st3phhays commented 2 years ago

Awesome, I appreciate it!

st3phhays commented 2 years ago

@gr2m So weird thing, I also had setforceFork: true, but when I set it back to forceFork: false, then updating of the commit worked!

gr2m commented 2 years ago

I think when I ran the first initial PR, I had update: false, and then I changed it to update: true later. I can not recall, however, I'm not sure if that matters or not.

without setting update: true, an error is thrown if a pull request already exists for the given head branch.


While looking into what the problem might be, I found a bug related to forks, it's fixed via https://github.com/gr2m/octokit-plugin-create-pull-request/pull/108. But it doesn't look like you ran into it.

I was thus far not able to reproduce the problem you ran into. I realize instead of update: true we should have called it relpace: true, as that's what the code does. But with your example code, you didn't remove previous entries of the files map, you only added to it, right?

I'm sorry I'm not sure what else to try. If you find a way to reliably reproduce the problem, I'd be happy to look further into it

st3phhays commented 2 years ago

I think what I did was first ran it with forceFork: false, and then later ran it with forceFork: true and update:true, and in that case a branch had already been created and it didn't want to update files. Either way.. I'm up and running now. I switched it back to forceFork:false, and all good now! If I run into this again with a new PR I'll update back with more info.

Thanks for taking a look and the awesome work on octokit.js, very cool all that it can do!

gr2m commented 2 years ago

glad it's working now 👍🏼