-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Feat: editorial workflow bitbucket gitlab #3014
Conversation
ea39a6c
to
7fb7361
Compare
bf7cf10
to
5413d50
Compare
package.json
Outdated
@@ -24,8 +24,8 @@ | |||
"test:e2e:ci": "run-s build:demo test:e2e:run-ci", | |||
"test:e2e:dev": "run-p clean && start-test develop 8080 test:e2e:exec-dev", | |||
"test:e2e:serve": "http-server dev-test", | |||
"test:e2e:exec": "cypress run", | |||
"test:e2e:exec-ci": "cypress run --record --parallel --ci-build-id $GITHUB_SHA --group 'GitHub CI' displayName: 'Run Cypress tests'", | |||
"test:e2e:exec": "cypress run --browser chrome --headless", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch to headless chrome
}, | ||
description: DEFAULT_PR_BODY, | ||
// eslint-disable-next-line @typescript-eslint/camelcase | ||
close_source_branch: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auto delete source branch on merge
@@ -16,22 +15,25 @@ export default class BitbucketAuthenticationPage extends React.Component { | |||
base_url: PropTypes.string, | |||
siteId: PropTypes.string, | |||
authEndpoint: PropTypes.string, | |||
config: ImmutablePropTypes.map, | |||
config: PropTypes.object.isRequired, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switched to plain JavaScript object for the config
login: user.username, | ||
token: state.token, | ||
// eslint-disable-next-line @typescript-eslint/camelcase | ||
avatar_url: user.links.avatar.href, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes missing avatar image on BitBucket
@@ -113,7 +114,7 @@ export default class GitGatewayAuthenticationPage extends React.Component { | |||
onLogin: PropTypes.func.isRequired, | |||
inProgress: PropTypes.bool.isRequired, | |||
error: PropTypes.node, | |||
config: ImmutablePropTypes.map, | |||
config: PropTypes.object.isRequired, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched to plain JavaScript object
...options, | ||
}; | ||
this.config = config; | ||
this.branch = config.getIn(['backend', 'branch'], 'master').trim(); | ||
this.squash_merges = config.getIn(['backend', 'squash_merges']); | ||
this.branch = config.backend.branch?.trim() || 'master'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched to plain JavaScript object for config
} | ||
entriesByFiles(collection) { | ||
return this.backend.entriesByFiles(collection); | ||
entriesByFolder(folder: string, extension: string, depth: number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each backend had the same code to extract the folder and depth from the collection so I moved that to the core
} | ||
fetchFiles(files) { | ||
return this.backend.fetchFiles(files); | ||
entriesByFiles(files: ImplementationFile[]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each backend had the same code to extract the files from the collection so I moved that to the core
} | ||
getEntry(collection, slug, path) { | ||
return this.backend.getEntry(collection, slug, path); | ||
getEntry(path: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
collection, slug
were not used
} | ||
|
||
const mediaFiles = await Promise.all( | ||
files.map(async file => { | ||
if (client.matchPath(file.path)) { | ||
const { sha: id, path } = file; | ||
const { id, path } = file; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All backends return the same file structure using id
and not sha
} else { | ||
return this.retrieveContent({ path, branch, repoURL, parseText }); | ||
if (!sha) { | ||
sha = await this.getFileSha(path, { repoURL, branch }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always use the sha
to get the file to support caching.
@@ -479,38 +463,10 @@ export default class API { | |||
} | |||
} | |||
|
|||
async getMediaAsBlob(sha: string | null, path: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to a shared file in lib-util
.join('/')}` | ||
: this.repoURL; | ||
|
||
const [fileData, isModification] = await Promise.all([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this easier to read instead of the nesting when using resolvePromiseProperties
} | ||
|
||
isUnpublishedEntryModification(path: string, branch: string) { | ||
isUnpublishedEntryModification(path: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was always called with the default branch
@@ -28,7 +27,7 @@ export default class GitHubAuthenticationPage extends React.Component { | |||
base_url: PropTypes.string, | |||
siteId: PropTypes.string, | |||
authEndpoint: PropTypes.string, | |||
config: ImmutablePropTypes.map, | |||
config: PropTypes.object.isRequired, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As before, switch to plain object for config
return { ...pullRequest, head: { sha: pullRequest.headRefOid } }; | ||
} | ||
|
||
async getPullRequestCommits(number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't required since we switched to compare API
const mergeRequest = await this.getBranchMergeRequest(branch); | ||
const diff = await this.getDifferences(mergeRequest.sha); | ||
const path = diff.find(d => d.old_path.includes(slug))?.old_path as string; | ||
// TODO: get real file id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The media file id is used with Large Media support in git-gateway.
We should be able to get it using aHEAD
request once https://gitlab.com/gitlab-org/gitlab/issues/194897 is resolved
} | ||
|
||
async rebaseMergeRequest(mergeRequest: GitLabMergeRequest) { | ||
let rebase: GitLabMergeRebase = await this.requestJSON({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very useful asynchronous API
@@ -200,7 +202,7 @@ export function persistMedia(file: File, opts: MediaOptions = {}) { | |||
const fileName = sanitizeSlug(file.name.toLowerCase(), state.config.get('slug')); | |||
const existingFile = files.find(existingFile => existingFile.name.toLowerCase() === fileName); | |||
|
|||
const editingDraft = selectEditingWorkflowDraft(state); | |||
const editingDraft = selectEditingDraft(state.entryDraft); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Media files will be persisted in a single commit with the entry regardless if you're using editorial workflow.
@@ -349,7 +275,7 @@ export class Backend { | |||
async entryExist(collection: Collection, path: string, slug: string) { | |||
const unpublishedEntry = | |||
this.implementation.unpublishedEntry && | |||
(await this.implementation.unpublishedEntry(collection, slug).catch(error => { | |||
(await this.implementation.unpublishedEntry(collection.get('name'), slug).catch(error => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The backends only need the collection name. I've aligned all the backends to just receive the relevant arguments instead of the collection immutable object (we have some places we were passing a collection object and other places we were passing a collection string which was confusing).
gitlab git-gateway doesn't support editorial workflow APIs yet. This change makes sure not to call them in simple workflow
49923e6
to
d83f006
Compare
Rebased, fixed merge conflicts. |
* refactor: typescript the backends * feat: support multiple files upload for GitLab and BitBucket * fix: load entry media files from media folder or UI state * chore: cleanup log message * chore: code cleanup * refactor: typescript the test backend * refactor: cleanup getEntry unsued variables * refactor: moved shared backend code to lib util * chore: rename files to preserve history * fix: bind readFile method to API classes * test(e2e): switch to chrome in cypress tests * refactor: extract common api methods * refactor: remove most of immutable js usage from backends * feat(backend-gitlab): initial editorial workflow support * feat(backend-gitlab): implement missing workflow methods * chore: fix lint error * feat(backend-gitlab): support files deletion * test(e2e): add gitlab cypress tests * feat(backend-bitbucket): implement missing editorial workflow methods * test(e2e): add BitBucket backend e2e tests * build: update node version to 12 on netlify builds * fix(backend-bitbucket): extract BitBucket avatar url * test: fix git-gateway AuthenticationPage test * test(e2e): fix some backend tests * test(e2e): fix tests * test(e2e): add git-gateway editorial workflow test * chore: code cleanup * test(e2e): revert back to electron * test(e2e): add non editorial workflow tests * fix(git-gateway-gitlab): don't call unpublishedEntry in simple workflow gitlab git-gateway doesn't support editorial workflow APIs yet. This change makes sure not to call them in simple workflow * refactor(backend-bitbucket): switch to diffstat API instead of raw diff * chore: fix test * test(e2e): add more git-gateway tests * fix: post rebase typescript fixes * test(e2e): fix tests * fix: fix parsing of content key and add tests * refactor: rename test file * test(unit): add getStatues unit tests * chore: update cypress * docs: update beta docs
* refactor: typescript the backends * feat: support multiple files upload for GitLab and BitBucket * fix: load entry media files from media folder or UI state * chore: cleanup log message * chore: code cleanup * refactor: typescript the test backend * refactor: cleanup getEntry unsued variables * refactor: moved shared backend code to lib util * chore: rename files to preserve history * fix: bind readFile method to API classes * test(e2e): switch to chrome in cypress tests * refactor: extract common api methods * refactor: remove most of immutable js usage from backends * feat(backend-gitlab): initial editorial workflow support * feat(backend-gitlab): implement missing workflow methods * chore: fix lint error * feat(backend-gitlab): support files deletion * test(e2e): add gitlab cypress tests * feat(backend-bitbucket): implement missing editorial workflow methods * test(e2e): add BitBucket backend e2e tests * build: update node version to 12 on netlify builds * fix(backend-bitbucket): extract BitBucket avatar url * test: fix git-gateway AuthenticationPage test * test(e2e): fix some backend tests * test(e2e): fix tests * test(e2e): add git-gateway editorial workflow test * chore: code cleanup * test(e2e): revert back to electron * test(e2e): add non editorial workflow tests * fix(git-gateway-gitlab): don't call unpublishedEntry in simple workflow gitlab git-gateway doesn't support editorial workflow APIs yet. This change makes sure not to call them in simple workflow * refactor(backend-bitbucket): switch to diffstat API instead of raw diff * chore: fix test * test(e2e): add more git-gateway tests * fix: post rebase typescript fixes * test(e2e): fix tests * fix: fix parsing of content key and add tests * refactor: rename test file * test(unit): add getStatues unit tests * chore: update cypress * docs: update beta docs
Replaces #2409
Fixes #568
Initial GitLab editorial workflow support (BitBucket coming soon).Still work in progress and needs code cleanup, testing, performance improvements, bug fixes and a rebase with master.Uses merge requests labels for entry status.Wanted to get this out there for people to experiment and give feedback (also on the code) while I work out the details.I'll add comments to the PR next week.BitBucket and GitLab editorial workflow support.
I TypeScripted all the backends, then moved some of the shared code to the
core/lib-util
. Probably we can do more after we align GitHub metadata handling with the other backends.Metadata handling
Limitations:
TODO