Fix: Set correct branch when it's not specified in the config
fix #5617
Summary
Currently, the default branch in Github, Gitlab and Bitbucket is set to be main instead of master.
However, when people use Netlify CMS and don't specify the branch name to push content in the config file, Netlify CMS will automatically push to branch master.
Since this branch may not exist, users will get an error on the frontend: "API_ERROR: Branch not found".
Test plan
- In the
admin/config.ymlfile, specify a valid Github repo path to push content to. - Run
yarn start. - Go to Neltify CMS dashboard, create a new post and hit Publish. You shouldn't get an error. Checking back on the repo that you specified in the config, you should see the new post.
Checklist
Please add a x inside each checkbox:
- [x] I have read the contribution guidelines.
- [x] Code is formatted via running
yarn format. - [ ] Tests are passing via running
yarn test. - [ ] The status checks are successful (continuous integration). Those can be seen below.
Please hold off on reviewing until I've pushed the unit tests.
Sorry that it took such a long time to update this PR. Writing a helper that can accommodate all the major git hosts turn out to be more complex than I had expected.
Hello @erezrokah . Let me ask a noob question: How do I rerun the Github action workflow? Or is it only possible for team members?
No noob questions - do you have this button when you open the action?

Regardless, I invited you to collaborate on the repo, long overdue ❤️
Let me walk through what I've done:
- Click on the Action tab of Netlify CMS's repo.
- Look for my failed action and click it.
- I'm directed to the dedicated page for the action but I didn't see any Rerun Jobs button.


I can, but there's no Rerun Jobs button that I can see. Anyway, I thought it's only possible to rerun the full pipeline, not a single job?

Btw, my code changes are ready for review again, if you have time.
Oh dear Erez, please never mind. I can see the Rerun button after accept your invitation. At the time of asking, I didn't have write access yet, so it didn't appear. I should have known it.
@erezrokah I'm so sorry for dropping the ball on this. Do you still need this issue to be worked on? I can get back to it in a few days.
@erezrokah I'm so sorry for dropping the ball on this. Do you still need this issue to be worked on? I can get back to it in a few days.
No worries, I appreciate the work you've done so far. I'd be happy to see this land and merged if you have the time to finalize it.
@erezrokah Thank you for your kind words.
Update on my progress: My changes have led to a bunch of failed unit tests. In my newest commit, I tackled the first failed test I saw named stores and returns user object on success.
What happens under the hood is the test calls an authenticate method. This method calls the endpoint /projects/:id twice:
- the first time for checking if the authenticated user has write access.
- the second time is for getting the default branch name.
However, the test only intercepts the request to projects/:id once through the interceptAuth() function.
function interceptAuth(backend, { userResponse, projectResponse } = {}) {
const api = mockApi(backend);
// ...
api
.get(expectedRepoUrl)
.query(true)
.reply(200, projectResponse || resp.project.success);
}
As a result, the second API request from my codes is never intercepted by Nock, leading to an error: "Nock: No match for request ... "url": "https://gitlab.com/api/v4/projects/foo%2Fbar"".
I circumvented it by repeating the same response twice.
function interceptAuth(backend, { userResponse, projectResponse } = {}) {
// ...
api
.get(expectedRepoUrl).times(2)
.query(true)
.reply(200, projectResponse || resp.project.success);
}
Hope that makes sense.
@erezrokah I'm trying to test if the function getDefaultBranchName is called when the config for each backend misses the value for branch field. Would you mind giving me some pointers?
Here's what I'm unsure about where to start:
- I added
getDefaultBranchNamefunction tonetlify-cms-lib-utilso that each backend can import it, thus avoiding repetition. - However, the function that calls
getDefaultBranchNameisauthenticatemethod defined as a property of each namesake class.
So where should I put the test for checking if getDefaultBranchName is called? In the tests folder for each backend, or in the test folder of netlify-cms-lib-util?
For now, I'm putting the test in netlify-cms-backend-gitlab/src/__tests/gitlab.spec.js and it passed.
// ...
jest.mock('netlify-cms-lib-util', () => ({
...jest.requireActual('netlify-cms-lib-util'),
getDefaultBranchName: jest.fn().mockResolvedValue('master')
}))
// ...
it('getDefaultBranchName is called when branch field value is empty in the config', async () => {
const backendName = defaultConfig.backend.name
backend = resolveBackend(defaultConfig)
interceptAuth(backend)
await backend.authenticate(mockCredentials)
expect(getDefaultBranchName).toHaveBeenCalledTimes(1)
expect(getDefaultBranchName).toHaveBeenCalledWith({ backend: backendName, repo, token: mockCredentials.token })
})
What do you think of it?
@erezrokah Would you mind review this PR again since I've made significant changes to it since the last review?
@erezrokah Thank you for the reformat work. I was doing it but you're so fast.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
@bytrangle are you still interested in moving this forward?
I think this PR is interesting to be merged upstream. I was going to suggest a much more simple change (changing the default value of the branch to main) but I think this PR is better.
@paulRbr yes, we would like to have this merged, but we need to solve those merge conflicts first.
@martinjagodic Hi, I'm interested in working on this again. However, I may have to ask you about resolving merging conflicts later :).
@bytrangle awesome! The conflicts emerged when we renamed the packages from netlify-cms-* to decap-cms-*. If a rename does not suffice, I am happy to answer questions/help out.
Hi @demshy . Is there anything else I need to do?
Sorry @bytrangle I somehow overlooked this one. I added a missing import that made the unit tests fail. I will gladly merge this one into a release this week
