diff options
| author | Phil Hughes <me@iamphill.com> | 2017-05-04 15:54:16 +0100 |
|---|---|---|
| committer | Phil Hughes <me@iamphill.com> | 2017-05-04 15:54:16 +0100 |
| commit | 79f40f27fd64b9209e1faa528e658d831e2c7f2f (patch) | |
| tree | 5eb1c7837b5180c0071ec75e238c27fe917023ee | |
| parent | 5c91113c5b6edc4fa1d63bc161b791c7e84e644d (diff) | |
| parent | 985737fdcf9b79dadfb72d0c9ed9abf4464559f8 (diff) | |
| download | gitlab-ce-79f40f27fd64b9209e1faa528e658d831e2c7f2f.tar.gz | |
Merge branch 'master' into deploy-keys-load-async
203 files changed, 4346 insertions, 1298 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index aa62a86d31d..7f665f19132 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -264,11 +264,15 @@ spinach mysql 9 10: *spinach-knapsack-mysql static-analysis: <<: *ruby-static-analysis <<: *dedicated-runner + <<: *except-docs stage: test script: - scripts/static-analysis -docs:check:links: +# Documentation checks: +# - Check validity of relative links +# - Make sure cURL examples in API docs use the full switches +docs lint: image: "registry.gitlab.com/gitlab-org/gitlab-build-images:nanoc-bootstrap-ruby-2.4-alpine" stage: test <<: *dedicated-runner @@ -276,6 +280,7 @@ docs:check:links: dependencies: [] before_script: [] script: + - scripts/lint-doc.sh - mv doc/ /nanoc/content/ - cd /nanoc # Build HTML from Markdown diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 73c8a77364b..600dad563a6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,27 +13,29 @@ _This notice should stay as the first item in the CONTRIBUTING.MD file._ <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE --> **Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* -- [Contributor license agreement](#contributor-license-agreement) - [Contribute to GitLab](#contribute-to-gitlab) - [Security vulnerability disclosure](#security-vulnerability-disclosure) - [Closing policy for issues and merge requests](#closing-policy-for-issues-and-merge-requests) - [Helping others](#helping-others) - [I want to contribute!](#i-want-to-contribute) -- [Implement design & UI elements](#implement-design-ui-elements) -- [Release retrospective and kickoff](#release-retrospective-and-kickoff) - - [Retrospective](#retrospective) - - [Kickoff](#kickoff) +- [Workflow labels](#workflow-labels) + - [Type labels (~"feature proposal", ~bug, ~customer, etc.)](#type-labels-feature-proposal-bug-customer-etc) + - [Subject labels (~wiki, ~"container registry", ~ldap, ~api, etc.)](#subject-labels-wiki-container-registry-ldap-api-etc) + - [Team labels (~CI, ~Discussion, ~Edge, ~Frontend, ~Platform, etc.)](#team-labels-ci-discussion-edge-frontend-platform-etc) + - [Priority labels (~Deliverable and ~Stretch)](#priority-labels-deliverable-and-stretch) + - [Label for community contributors (~"Accepting Merge Requests")](#label-for-community-contributors-accepting-merge-requests) +- [Implement design & UI elements](#implement-design--ui-elements) - [Issue tracker](#issue-tracker) - - [Feature proposals](#feature-proposals) - - [Issue tracker guidelines](#issue-tracker-guidelines) - - [Issue weight](#issue-weight) - - [Regression issues](#regression-issues) - - [Technical debt](#technical-debt) - - [Stewardship](#stewardship) + - [Issue triaging](#issue-triaging) + - [Feature proposals](#feature-proposals) + - [Issue tracker guidelines](#issue-tracker-guidelines) + - [Issue weight](#issue-weight) + - [Regression issues](#regression-issues) + - [Technical debt](#technical-debt) + - [Stewardship](#stewardship) - [Merge requests](#merge-requests) - - [Merge request guidelines](#merge-request-guidelines) - - [Contribution acceptance criteria](#contribution-acceptance-criteria) -- [Changes for Stable Releases](#changes-for-stable-releases) + - [Merge request guidelines](#merge-request-guidelines) + - [Contribution acceptance criteria](#contribution-acceptance-criteria) - [Definition of done](#definition-of-done) - [Style guides](#style-guides) - [Code of conduct](#code-of-conduct) @@ -103,34 +105,125 @@ contributing to GitLab. ## Workflow labels -Labelling issues is described in the [GitLab Inc engineering workflow]. +To allow for asynchronous issue handling, we use [milestones][milestones-page] +and [labels][labels-page]. Leads and product managers handle most of the +scheduling into milestones. Labelling is a task for everyone. -## Implement design & UI elements +Most issues will have labels for at least one of the following: -Please see the [UX Guide for GitLab]. +- Type: ~"feature proposal", ~bug, ~customer, etc. +- Subject: ~wiki, ~"container registry", ~ldap, ~api, etc. +- Team: ~CI, ~Discussion, ~Edge, ~Frontend, ~Platform, etc. +- Priority: ~Deliverable, ~Stretch + +All labels, their meaning and priority are defined on the +[labels page][labels-page]. + +If you come across an issue that has none of these, and you're allowed to set +labels, you can _always_ add the team and type, and often also the subject. + +[milestones-page]: https://gitlab.com/gitlab-org/gitlab-ce/milestones +[labels-page]: https://gitlab.com/gitlab-org/gitlab-ce/labels + +### Type labels (~"feature proposal", ~bug, ~customer, etc.) + +Type labels are very important. They define what kind of issue this is. Every +issue should have one or more. + +Examples of type labels are ~"feature proposal", ~bug, ~customer, ~security, +and ~"direction". + +A number of type labels have a priority assigned to them, which automatically +makes them float to the top, depending on their importance. + +Type labels are always lowercase, and can have any color, besides blue (which is +already reserved for subject labels). + +The descriptions on the [labels page][labels-page] explain what falls under each type label. + +### Subject labels (~wiki, ~"container registry", ~ldap, ~api, etc.) + +Subject labels are labels that define what area or feature of GitLab this issue +hits. They are not always necessary, but very convenient. + +If you are an expert in a particular area, it makes it easier to find issues to +work on. You can also subscribe to those labels to receive an email each time an +issue is labelled with a subject label corresponding to your expertise. + +Examples of subject labels are ~wiki, ~"container registry", ~ldap, ~api, +~issues, ~"merge requests", ~labels, and ~"container registry". + +Subject labels are always all-lowercase. + +### Team labels (~CI, ~Discussion, ~Edge, ~Frontend, ~Platform, etc.) + +Team labels specify what team is responsible for this issue. +Assigning a team label makes sure issues get the attention of the appropriate +people. + +The current team labels are ~Build, ~CI, ~Discussion, ~Documentation, ~Edge, +~Frontend, ~Gitaly, ~Platform, ~Prometheus, ~Release, and ~"UX". + +The descriptions on the [labels page][labels-page] explain what falls under the +responsibility of each team. + +Team labels are always capitalized so that they show up as the first label for +any issue. + +### Priority labels (~Deliverable and ~Stretch) -## Release retrospective and kickoff +Priority labels help us clearly communicate expectations of the work for the +release. There are two levels of priority labels: -### Retrospective +- ~Deliverable: Issues that are expected to be delivered in the current + milestone. +- ~Stretch: Issues that are a stretch goal for delivering in the current + milestone. If these issues are not done in the current release, they will + strongly be considered for the next release. -After each release, we have a retrospective call where we discuss what went well, -what went wrong, and what we can improve for the next release. The -[retrospective notes] are public and you are invited to comment on them. -If you're interested, you can even join the -[retrospective call][retro-kickoff-call], on the first working day after the -22nd at 6pm CET / 9am PST. +### Label for community contributors (~"Accepting Merge Requests") -### Kickoff +Issues that are beneficial to our users, 'nice to haves', that we currently do +not have the capacity for or want to give the priority to, are labeled as +~"Accepting Merge Requests", so the community can make a contribution. -Before working on the next release, we have a -kickoff call to explain what we expect to ship in the next release. The -[kickoff notes] are public and you are invited to comment on them. -If you're interested, you can even join the [kickoff call][retro-kickoff-call], -on the first working day after the 7th at 6pm CET / 9am PST.. +Community contributors can submit merge requests for any issue they want, but +the ~"Accepting Merge Requests" label has a special meaning. It points to +changes that: -[retrospective notes]: https://docs.google.com/document/d/1nEkM_7Dj4bT21GJy0Ut3By76FZqCfLBmFQNVThmW2TY/edit?usp=sharing -[kickoff notes]: https://docs.google.com/document/d/1ElPkZ90A8ey_iOkTvUs_ByMlwKK6NAB2VOK5835wYK0/edit?usp=sharing -[retro-kickoff-call]: https://gitlab.zoom.us/j/918821206 +1. We already agreed on, +1. Are well-defined, +1. Are likely to get accepted by a maintainer. + +We want to avoid a situation when a contributor picks an +~"Accepting Merge Requests" issue and then their merge request gets closed, +because we realize that it does not fit our vision, or we want to solve it in a +different way. + +We add the ~"Accepting Merge Requests" label to: + +- Low priority ~bug issues (i.e. we do not add it to the bugs that we want to +solve in the ~"Next Patch Release") +- Small ~"feature proposal" that do not need ~UX / ~"Product work", or for which +the ~UX / ~"Product work" is already done +- Small ~"technical debt" issues + +After adding the ~"Accepting Merge Requests" label, we try to estimate the +[weight](#issue-weight) of the issue. We use issue weight to let contributors +know how difficult the issue is. Additionally: + +- We advertise [~"Accepting Merge Requests" issues with weight < 5][up-for-grabs] + as suitable for people that have never contributed to GitLab before on the + [Up For Grabs campaign](http://up-for-grabs.net) +- We encourage people that have never contributed to any open source project to + look for [~"Accepting Merge Requests" issues with a weight of 1][firt-timers] + +[up-for-grabs]: https://gitlab.com/gitlab-org/gitlab-ce/issues?label_name=Accepting+Merge+Requests&scope=all&sort=weight_asc&state=opened +[firt-timers]: https://gitlab.com/gitlab-org/gitlab-ce/issues?label_name%5B%5D=Accepting+Merge+Requests&scope=all&sort=upvotes_desc&state=opened&weight=1 + +## Implement design & UI elements + +Please see the [UX Guide for GitLab]. ## Issue tracker @@ -154,6 +247,21 @@ If it happens that you know the solution to an existing bug, please first open the issue in order to keep track of it and then open the relevant merge request that potentially fixes it. +### Issue triaging + +Our issue triage policies are [described in our handbook]. You are very welcome +to help the GitLab team triage issues. We also organize [issue bash events] once +every quarter. + +The most important thing is making sure valid issues receive feedback from the +development team. Therefore the priority is mentioning developers that can help +on those issues. Please select someone with relevant experience from the +[GitLab team][team]. If there is nobody mentioned with that expertise look in +the commit history for the affected files to find someone. + +[described in our handbook]: https://about.gitlab.com/handbook/engineering/issues/issue-triage-policies/ +[issue bash events]: https://gitlab.com/gitlab-org/gitlab-ce/issues/17815 + ### Feature proposals To create a feature proposal for CE, open an issue on the @@ -327,13 +435,17 @@ request is as follows: "Description" field. 1. If you are contributing documentation, choose `Documentation` from the "Choose a template" menu and fill in the template. + 1. Mention the issue(s) your merge request solves, using the `Solves #XXX` or + `Closes #XXX` syntax to auto-close the issue(s) once the merge request will + be merged. +1. If you're allowed to, set a relevant milestone and labels 1. If the MR changes the UI it should include *Before* and *After* screenshots 1. If the MR changes CSS classes please include the list of affected pages, `grep css-class ./app -R` -1. Link any relevant [issues][ce-tracker] in the merge request description and - leave a comment on them with a link back to the MR 1. Be prepared to answer questions and incorporate feedback even if requests for this arrive weeks or months after your MR submission + 1. If a discussion has been addressed, select the "Resolve discussion" button + beneath it to mark it resolved. 1. If your MR touches code that executes shell commands, reads or opens files or handles paths to files on disk, make sure it adheres to the [shell command guidelines](doc/development/shell_commands.md) @@ -369,24 +481,6 @@ Please ensure that your merge request meets the contribution acceptance criteria When having your code reviewed and when reviewing merge requests please take the [code review guidelines](doc/development/code_review.md) into account. -### Getting your merge request reviewed, approved, and merged - -There are a few rules to get your merge request accepted: - -1. Your merge request should only be **merged by a [maintainer][team]**. - 1. If your merge request includes only backend changes [^1], it must be - **approved by a [backend maintainer][team]**. - 1. If your merge request includes only frontend changes [^1], it must be - **approved by a [frontend maintainer][team]**. - 1. If your merge request includes frontend and backend changes [^1], it must - be **approved by a [frontend and a backend maintainer][team]**. -1. To lower the amount of merge requests maintainers need to review, you can - ask or assign any [reviewers][team] for a first review. - 1. If you need some guidance (e.g. it's your first merge request), feel free - to ask one of the [Merge request coaches][team]. - 1. The reviewer will assign the merge request to a maintainer once the - reviewer is satisfied with the state of the merge request. - ### Contribution acceptance criteria 1. The change is as small as possible @@ -416,8 +510,7 @@ There are a few rules to get your merge request accepted: 1. If you need polling to support real-time features, please use [polling with ETag caching][polling-etag]. 1. Changes after submitting the merge request should be in separate commits - (no squashing). If necessary, you will be asked to squash when the review is - over, before merging. + (no squashing). 1. It conforms to the [style guides](#style-guides) and the following: - If your change touches a line that does not follow the style, modify the entire line to follow it. This prevents linting tools from generating warnings. @@ -428,19 +521,6 @@ There are a few rules to get your merge request accepted: See the instructions in that document for help if your MR fails the "license-finder" test with a "Dependencies that need approval" error. -## Changes for Stable Releases - -Sometimes certain changes have to be added to an existing stable release. -Two examples are bug fixes and performance improvements. In these cases the -corresponding merge request should be updated to have the following: - -1. A milestone indicating what release the merge request should be merged into. -1. The label "Pick into Stable" - -This makes it easier for release managers to keep track of what still has to be -merged and where changes have to be merged into. -Like all merge requests the target should be master so all bugfixes are in master. - ## Definition of done If you contribute to GitLab please know that changes involve more than just @@ -449,16 +529,16 @@ the feature you contribute through all of these steps. 1. Description explaining the relevancy (see following item) 1. Working and clean code that is commented where needed -1. Unit and integration tests that pass on the CI server +1. [Unit and system tests][testing] that pass on the CI server 1. Performance/scalability implications have been considered, addressed, and tested -1. [Documented][doc-styleguide] in the /doc directory -1. Changelog entry added +1. [Documented][doc-styleguide] in the `/doc` directory +1. [Changelog entry added][changelog], if necessary 1. Reviewed and any concerns are addressed -1. Merged by the project lead -1. Added to the release blog article -1. Added to [the website](https://gitlab.com/gitlab-com/www-gitlab-com/) if relevant +1. Merged by a project maintainer +1. Added to the release blog article, if relevant +1. Added to [the website](https://gitlab.com/gitlab-com/www-gitlab-com/), if relevant 1. Community questions answered -1. Answers to questions radiated (in docs/wiki/etc.) +1. Answers to questions radiated (in docs/wiki/support etc.) If you add a dependency in GitLab (such as an operating system package) please consider updating the following and note the applicability of each in your @@ -481,7 +561,7 @@ merge request: - string literal quoting style **Option A**: single quoted by default 1. [Rails](https://github.com/bbatsov/rails-style-guide) 1. [Newlines styleguide][newlines-styleguide] -1. [Testing](doc/development/testing.md) +1. [Testing][testing] 1. [JavaScript styleguide][js-styleguide] 1. [SCSS styleguide][scss-styleguide] 1. [Shell commands](doc/development/shell_commands.md) created by GitLab @@ -558,6 +638,7 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor [license-finder-doc]: doc/development/licensing.md [GitLab Inc engineering workflow]: https://about.gitlab.com/handbook/engineering/workflow/#labelling-issues [polling-etag]: https://docs.gitlab.com/ce/development/polling.html +[testing]: doc/development/testing.md [^1]: Please note that specs other than JavaScript specs are considered backend code. diff --git a/PROCESS.md b/PROCESS.md index fac3c22e09f..3b97a4e8c75 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -1,35 +1,53 @@ -# GitLab Contributing Process +## GitLab Core Team & GitLab Inc. Contribution Process + +--- + +<!-- START doctoc generated TOC please keep comment here to allow auto update --> +<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE --> +**Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* + +- [Purpose of describing the contributing process](#purpose-of-describing-the-contributing-process) +- [Common actions](#common-actions) + - [Merge request coaching](#merge-request-coaching) +- [Assigning issues](#assigning-issues) +- [Be kind](#be-kind) +- [Feature freeze on the 7th for the release on the 22nd](#feature-freeze-on-the-7th-for-the-release-on-the-22nd) + - [Between the 1st and the 7th](#between-the-1st-and-the-7th) + - [On the 7th](#on-the-7th) + - [After the 7th](#after-the-7th) +- [Release retrospective and kickoff](#release-retrospective-and-kickoff) + - [Retrospective](#retrospective) + - [Kickoff](#kickoff) +- [Copy & paste responses](#copy--paste-responses) + - [Improperly formatted issue](#improperly-formatted-issue) + - [Issue report for old version](#issue-report-for-old-version) + - [Support requests and configuration questions](#support-requests-and-configuration-questions) + - [Code format](#code-format) + - [Issue fixed in newer version](#issue-fixed-in-newer-version) + - [Improperly formatted merge request](#improperly-formatted-merge-request) + - [Inactivity close of an issue](#inactivity-close-of-an-issue) + - [Inactivity close of a merge request](#inactivity-close-of-a-merge-request) + - [Accepting merge requests](#accepting-merge-requests) + - [Only accepting merge requests with green tests](#only-accepting-merge-requests-with-green-tests) + - [Closing down the issue tracker on GitHub](#closing-down-the-issue-tracker-on-github) + +<!-- END doctoc generated TOC please keep comment here to allow auto update --> + +--- ## Purpose of describing the contributing process -Below we describe the contributing process to GitLab for two reasons. So that -contributors know what to expect from maintainers (possible responses, friendly -treatment, etc.). And so that maintainers know what to expect from contributors -(use the latest version, ensure that the issue is addressed, friendly treatment, -etc.). +Below we describe the contributing process to GitLab for two reasons: + +1. Contributors know what to expect from maintainers (possible responses, friendly + treatment, etc.) +1. Maintainers know what to expect from contributors (use the latest version, + ensure that the issue is addressed, friendly treatment, etc.). - [GitLab Inc engineers should refer to the engineering workflow document](https://about.gitlab.com/handbook/engineering/workflow/) ## Common actions -### Issue triaging - -Our issue triage policies are [described in our handbook]. You are very welcome -to help the GitLab team triage issues. We also organize [issue bash events] once -every quarter. - -The most important thing is making sure valid issues receive feedback from the -development team. Therefore the priority is mentioning developers that can help -on those issues. Please select someone with relevant experience from -[GitLab team][team]. If there is nobody mentioned with that expertise -look in the commit history for the affected files to find someone. Avoid -mentioning the lead developer, this is the person that is least likely to give a -timely response. If the involvement of the lead developer is needed the other -core team members will mention this person. - -[described in our handbook]: https://about.gitlab.com/handbook/engineering/issues/issue-triage-policies/ -[issue bash events]: https://gitlab.com/gitlab-org/gitlab-ce/issues/17815 - ### Merge request coaching Several people from the [GitLab team][team] are helping community members to get @@ -37,12 +55,6 @@ their contributions accepted by meeting our [Definition of done][done]. What you can expect from them is described at https://about.gitlab.com/jobs/merge-request-coach/. -## Workflow labels - -Labelling issues is described in the [GitLab Inc engineering workflow]. - -[GitLab Inc engineering workflow]: https://about.gitlab.com/handbook/engineering/workflow/#labelling-issues - ## Assigning issues If an issue is complex and needs the attention of a specific person, assignment is a good option but assigning issues might discourage other people from contributing to that issue. We need all the contributions we can get so this should never be discouraged. Also, an assigned person might not have time for a few weeks, so others should feel free to takeover. @@ -146,6 +158,29 @@ release should have the correct milestone assigned _and_ have the label Merge requests without a milestone and this label will not be merged into any stable branches. +## Release retrospective and kickoff + +### Retrospective + +After each release, we have a retrospective call where we discuss what went well, +what went wrong, and what we can improve for the next release. The +[retrospective notes] are public and you are invited to comment on them. +If you're interested, you can even join the +[retrospective call][retro-kickoff-call], on the first working day after the +22nd at 6pm CET / 9am PST. + +### Kickoff + +Before working on the next release, we have a +kickoff call to explain what we expect to ship in the next release. The +[kickoff notes] are public and you are invited to comment on them. +If you're interested, you can even join the [kickoff call][retro-kickoff-call], +on the first working day after the 7th at 6pm CET / 9am PST.. + +[retrospective notes]: https://docs.google.com/document/d/1nEkM_7Dj4bT21GJy0Ut3By76FZqCfLBmFQNVThmW2TY/edit?usp=sharing +[kickoff notes]: https://docs.google.com/document/d/1ElPkZ90A8ey_iOkTvUs_ByMlwKK6NAB2VOK5835wYK0/edit?usp=sharing +[retro-kickoff-call]: https://gitlab.zoom.us/j/918821206 + ## Copy & paste responses ### Improperly formatted issue diff --git a/app/assets/javascripts/commit/pipelines/pipelines_table.js b/app/assets/javascripts/commit/pipelines/pipelines_table.js index e704be8b53e..ad9c600b499 100644 --- a/app/assets/javascripts/commit/pipelines/pipelines_table.js +++ b/app/assets/javascripts/commit/pipelines/pipelines_table.js @@ -46,6 +46,7 @@ export default Vue.component('pipelines-table', { isLoading: false, hasError: false, isMakingRequest: false, + updateGraphDropdown: false, }; }, @@ -130,15 +131,21 @@ export default Vue.component('pipelines-table', { const pipelines = response.pipelines || response; this.store.storePipelines(pipelines); this.isLoading = false; + this.updateGraphDropdown = true; }, errorCallback() { this.hasError = true; this.isLoading = false; + this.updateGraphDropdown = false; }, setIsMakingRequest(isMakingRequest) { this.isMakingRequest = isMakingRequest; + + if (isMakingRequest) { + this.updateGraphDropdown = false; + } }, }, @@ -163,7 +170,9 @@ export default Vue.component('pipelines-table', { v-if="shouldRenderTable"> <pipelines-table-component :pipelines="state.pipelines" - :service="service" /> + :service="service" + :update-graph-dropdown="updateGraphDropdown" + /> </div> </div> `, diff --git a/app/assets/javascripts/create_merge_request_dropdown.js b/app/assets/javascripts/create_merge_request_dropdown.js new file mode 100644 index 00000000000..ff2f2c81971 --- /dev/null +++ b/app/assets/javascripts/create_merge_request_dropdown.js @@ -0,0 +1,193 @@ +/* eslint-disable no-new */ +/* global Flash */ +import DropLab from './droplab/drop_lab'; +import ISetter from './droplab/plugins/input_setter'; + +// Todo: Remove this when fixing issue in input_setter plugin +const InputSetter = Object.assign({}, ISetter); + +const CREATE_MERGE_REQUEST = 'create-mr'; +const CREATE_BRANCH = 'create-branch'; + +export default class CreateMergeRequestDropdown { + constructor(wrapperEl) { + this.wrapperEl = wrapperEl; + this.createMergeRequestButton = this.wrapperEl.querySelector('.js-create-merge-request'); + this.dropdownToggle = this.wrapperEl.querySelector('.js-dropdown-toggle'); + this.dropdownList = this.wrapperEl.querySelector('.dropdown-menu'); + this.availableButton = this.wrapperEl.querySelector('.available'); + this.unavailableButton = this.wrapperEl.querySelector('.unavailable'); + this.unavailableButtonArrow = this.unavailableButton.querySelector('.fa'); + this.unavailableButtonText = this.unavailableButton.querySelector('.text'); + + this.createBranchPath = this.wrapperEl.dataset.createBranchPath; + this.canCreatePath = this.wrapperEl.dataset.canCreatePath; + this.createMrPath = this.wrapperEl.dataset.createMrPath; + this.droplabInitialized = false; + this.isCreatingMergeRequest = false; + this.mergeRequestCreated = false; + this.isCreatingBranch = false; + this.branchCreated = false; + + this.init(); + } + + init() { + this.checkAbilityToCreateBranch(); + } + + available() { + this.availableButton.classList.remove('hide'); + this.unavailableButton.classList.add('hide'); + } + + unavailable() { + this.availableButton.classList.add('hide'); + this.unavailableButton.classList.remove('hide'); + } + + enable() { + this.createMergeRequestButton.classList.remove('disabled'); + this.createMergeRequestButton.removeAttribute('disabled'); + + this.dropdownToggle.classList.remove('disabled'); + this.dropdownToggle.removeAttribute('disabled'); + } + + disable() { + this.createMergeRequestButton.classList.add('disabled'); + this.createMergeRequestButton.setAttribute('disabled', 'disabled'); + + this.dropdownToggle.classList.add('disabled'); + this.dropdownToggle.setAttribute('disabled', 'disabled'); + } + + hide() { + this.wrapperEl.classList.add('hide'); + } + + setUnavailableButtonState(isLoading = true) { + if (isLoading) { + this.unavailableButtonArrow.classList.add('fa-spinner', 'fa-spin'); + this.unavailableButtonArrow.classList.remove('fa-exclamation-triangle'); + this.unavailableButtonText.textContent = 'Checking branch availability…'; + } else { + this.unavailableButtonArrow.classList.remove('fa-spinner', 'fa-spin'); + this.unavailableButtonArrow.classList.add('fa-exclamation-triangle'); + this.unavailableButtonText.textContent = 'New branch unavailable'; + } + } + + checkAbilityToCreateBranch() { + return $.ajax({ + type: 'GET', + dataType: 'json', + url: this.canCreatePath, + beforeSend: () => this.setUnavailableButtonState(), + }) + .done((data) => { + this.setUnavailableButtonState(false); + + if (data.can_create_branch) { + this.available(); + this.enable(); + + if (!this.droplabInitialized) { + this.droplabInitialized = true; + this.initDroplab(); + this.bindEvents(); + } + } else if (data.has_related_branch) { + this.hide(); + } + }).fail(() => { + this.unavailable(); + this.disable(); + new Flash('Failed to check if a new branch can be created.'); + }); + } + + initDroplab() { + this.droplab = new DropLab(); + + this.droplab.init(this.dropdownToggle, this.dropdownList, [InputSetter], + this.getDroplabConfig()); + } + + getDroplabConfig() { + return { + InputSetter: [{ + input: this.createMergeRequestButton, + valueAttribute: 'data-value', + inputAttribute: 'data-action', + }, { + input: this.createMergeRequestButton, + valueAttribute: 'data-text', + }], + }; + } + + bindEvents() { + this.createMergeRequestButton + .addEventListener('click', this.onClickCreateMergeRequestButton.bind(this)); + } + + isBusy() { + return this.isCreatingMergeRequest || + this.mergeRequestCreated || + this.isCreatingBranch || + this.branchCreated; + } + + onClickCreateMergeRequestButton(e) { + let xhr = null; + e.preventDefault(); + + if (this.isBusy()) { + return; + } + + if (e.target.dataset.action === CREATE_MERGE_REQUEST) { + xhr = this.createMergeRequest(); + } else if (e.target.dataset.action === CREATE_BRANCH) { + xhr = this.createBranch(); + } + + xhr.fail(() => { + this.isCreatingMergeRequest = false; + this.isCreatingBranch = false; + }); + + xhr.always(() => this.enable()); + + this.disable(); + } + + createMergeRequest() { + return $.ajax({ + method: 'POST', + dataType: 'json', + url: this.createMrPath, + beforeSend: () => (this.isCreatingMergeRequest = true), + }) + .done((data) => { + this.mergeRequestCreated = true; + window.location.href = data.url; + }) + .fail(() => new Flash('Failed to create Merge Request. Please try again.')); + } + + createBranch() { + return $.ajax({ + method: 'POST', + dataType: 'json', + url: this.createBranchPath, + beforeSend: () => (this.isCreatingBranch = true), + }) + .done((data) => { + this.branchCreated = true; + window.location.href = data.url; + }) + .fail(() => new Flash('Failed to create a branch for this issue. Please try again.')); + } +} diff --git a/app/assets/javascripts/dropzone_input.js b/app/assets/javascripts/dropzone_input.js index b70d242269d..b3a76fbb43e 100644 --- a/app/assets/javascripts/dropzone_input.js +++ b/app/assets/javascripts/dropzone_input.js @@ -5,7 +5,7 @@ require('./preview_markdown'); window.DropzoneInput = (function() { function DropzoneInput(form) { - var $mdArea, alertAttr, alertClass, appendToTextArea, btnAlert, child, closeAlertMessage, closeSpinner, divAlert, divHover, divSpinner, dropzone, form_dropzone, form_textarea, getFilename, handlePaste, iconPaperclip, iconSpinner, insertToTextArea, isImage, max_file_size, pasteText, project_uploads_path, showError, showSpinner, uploadFile, uploadProgress; + var $mdArea, alertAttr, alertClass, appendToTextArea, btnAlert, child, closeAlertMessage, closeSpinner, divAlert, divHover, divSpinner, dropzone, form_dropzone, form_textarea, getFilename, handlePaste, iconPaperclip, iconSpinner, insertToTextArea, isImage, max_file_size, pasteText, uploads_path, showError, showSpinner, uploadFile, uploadProgress; Dropzone.autoDiscover = false; alertClass = "alert alert-danger alert-dismissable div-dropzone-alert"; alertAttr = "class=\"close\" data-dismiss=\"alert\"" + "aria-hidden=\"true\""; @@ -16,7 +16,7 @@ window.DropzoneInput = (function() { iconSpinner = "<i class=\"fa fa-spinner fa-spin div-dropzone-icon\"></i>"; uploadProgress = $("<div class=\"div-dropzone-progress\"></div>"); btnAlert = "<button type=\"button\"" + alertAttr + ">×</button>"; - project_uploads_path = window.project_uploads_path || null; + uploads_path = window.uploads_path || null; max_file_size = gon.max_file_size || 10; form_textarea = $(form).find(".js-gfm-input"); form_textarea.wrap("<div class=\"div-dropzone\"></div>"); @@ -39,10 +39,10 @@ window.DropzoneInput = (function() { "display": "none" }); - if (!project_uploads_path) return; + if (!uploads_path) return; dropzone = form_dropzone.dropzone({ - url: project_uploads_path, + url: uploads_path, dictDefaultMessage: "", clickable: true, paramName: "file", @@ -159,7 +159,7 @@ window.DropzoneInput = (function() { formData = new FormData(); formData.append("file", item, filename); return $.ajax({ - url: project_uploads_path, + url: uploads_path, type: "POST", data: formData, dataType: "json", diff --git a/app/assets/javascripts/environments/components/environment.vue b/app/assets/javascripts/environments/components/environment.vue index f319d6ca0c8..e0088d496eb 100644 --- a/app/assets/javascripts/environments/components/environment.vue +++ b/app/assets/javascripts/environments/components/environment.vue @@ -1,6 +1,4 @@ <script> - -/* eslint-disable no-new */ /* global Flash */ import EnvironmentsService from '../services/environments_service'; import EnvironmentTable from './environments_table.vue'; @@ -71,11 +69,13 @@ export default { eventHub.$on('refreshEnvironments', this.fetchEnvironments); eventHub.$on('toggleFolder', this.toggleFolder); + eventHub.$on('postAction', this.postAction); }, beforeDestroyed() { eventHub.$off('refreshEnvironments'); eventHub.$off('toggleFolder'); + eventHub.$off('postAction'); }, methods: { @@ -122,6 +122,7 @@ export default { }) .catch(() => { this.isLoading = false; + // eslint-disable-next-line no-new new Flash('An error occurred while fetching the environments.'); }); }, @@ -137,9 +138,16 @@ export default { }) .catch(() => { this.isLoadingFolderContent = false; + // eslint-disable-next-line no-new new Flash('An error occurred while fetching the environments.'); }); }, + + postAction(endpoint) { + this.service.postAction(endpoint) + .then(() => this.fetchEnvironments()) + .catch(() => new Flash('An error occured while making the request.')); + }, }, }; </script> @@ -217,7 +225,6 @@ export default { :environments="state.environments" :can-create-deployment="canCreateDeploymentParsed" :can-read-environment="canReadEnvironmentParsed" - :service="service" :is-loading-folder-content="isLoadingFolderContent" /> </div> diff --git a/app/assets/javascripts/environments/components/environment_actions.vue b/app/assets/javascripts/environments/components/environment_actions.vue index e81c97260d7..63bffe8a998 100644 --- a/app/assets/javascripts/environments/components/environment_actions.vue +++ b/app/assets/javascripts/environments/components/environment_actions.vue @@ -1,7 +1,4 @@ <script> -/* global Flash */ -/* eslint-disable no-new */ - import playIconSvg from 'icons/_icon_play.svg'; import eventHub from '../event_hub'; @@ -12,11 +9,6 @@ export default { required: false, default: () => [], }, - - service: { - type: Object, - required: true, - }, }, data() { @@ -38,15 +30,7 @@ export default { $(this.$refs.tooltip).tooltip('destroy'); - this.service.postAction(endpoint) - .then(() => { - this.isLoading = false; - eventHub.$emit('refreshEnvironments'); - }) - .catch(() => { - this.isLoading = false; - new Flash('An error occured while making the request.'); - }); + eventHub.$emit('postAction', endpoint); }, isActionDisabled(action) { diff --git a/app/assets/javascripts/environments/components/environment_item.vue b/app/assets/javascripts/environments/components/environment_item.vue index 73679de6039..0ffe9ea17fa 100644 --- a/app/assets/javascripts/environments/components/environment_item.vue +++ b/app/assets/javascripts/environments/components/environment_item.vue @@ -46,11 +46,6 @@ export default { required: false, default: false, }, - - service: { - type: Object, - required: true, - }, }, computed: { @@ -543,31 +538,34 @@ export default { <actions-component v-if="hasManualActions && canCreateDeployment" - :service="service" - :actions="manualActions"/> + :actions="manualActions" + /> <external-url-component v-if="externalURL && canReadEnvironment" - :external-url="externalURL"/> + :external-url="externalURL" + /> <monitoring-button-component v-if="monitoringUrl && canReadEnvironment" - :monitoring-url="monitoringUrl"/> + :monitoring-url="monitoringUrl" + /> <terminal-button-component v-if="model && model.terminal_path" - :terminal-path="model.terminal_path"/> + :terminal-path="model.terminal_path" + /> <stop-component v-if="hasStopAction && canCreateDeployment" :stop-url="model.stop_path" - :service="service"/> + /> <rollback-component v-if="canRetry && canCreateDeployment" :is-last-deployment="isLastDeployment" :retry-url="retryUrl" - :service="service"/> + /> </div> </td> </tr> diff --git a/app/assets/javascripts/environments/components/environment_rollback.vue b/app/assets/javascripts/environments/components/environment_rollback.vue index f139f24036f..44b8730fd09 100644 --- a/app/assets/javascripts/environments/components/environment_rollback.vue +++ b/app/assets/javascripts/environments/components/environment_rollback.vue @@ -1,6 +1,4 @@ <script> -/* global Flash */ -/* eslint-disable no-new */ /** * Renders Rollback or Re deploy button in environments table depending * of the provided property `isLastDeployment`. @@ -20,11 +18,6 @@ export default { type: Boolean, default: true, }, - - service: { - type: Object, - required: true, - }, }, data() { @@ -37,17 +30,7 @@ export default { onClick() { this.isLoading = true; - $(this.$el).tooltip('destroy'); - - this.service.postAction(this.retryUrl) - .then(() => { - this.isLoading = false; - eventHub.$emit('refreshEnvironments'); - }) - .catch(() => { - this.isLoading = false; - new Flash('An error occured while making the request.'); - }); + eventHub.$emit('postAction', this.retryUrl); }, }, }; diff --git a/app/assets/javascripts/environments/components/environment_stop.vue b/app/assets/javascripts/environments/components/environment_stop.vue index 11e9aff7b92..f483ea7e937 100644 --- a/app/assets/javascripts/environments/components/environment_stop.vue +++ b/app/assets/javascripts/environments/components/environment_stop.vue @@ -1,6 +1,4 @@ <script> -/* global Flash */ -/* eslint-disable no-new, no-alert */ /** * Renders the stop "button" that allows stop an environment. * Used in environments table. @@ -13,11 +11,6 @@ export default { type: String, default: '', }, - - service: { - type: Object, - required: true, - }, }, data() { @@ -34,20 +27,13 @@ export default { methods: { onClick() { + // eslint-disable-next-line no-alert if (confirm('Are you sure you want to stop this environment?')) { this.isLoading = true; $(this.$el).tooltip('destroy'); - this.service.postAction(this.retryUrl) - .then(() => { - this.isLoading = false; - eventHub.$emit('refreshEnvironments'); - }) - .catch(() => { - this.isLoading = false; - new Flash('An error occured while making the request.', 'alert'); - }); + eventHub.$emit('postAction', this.stopUrl); } }, }, diff --git a/app/assets/javascripts/environments/components/environments_table.vue b/app/assets/javascripts/environments/components/environments_table.vue index 87f7cb4a536..15eedaf76e1 100644 --- a/app/assets/javascripts/environments/components/environments_table.vue +++ b/app/assets/javascripts/environments/components/environments_table.vue @@ -28,11 +28,6 @@ export default { default: false, }, - service: { - type: Object, - required: true, - }, - isLoadingFolderContent: { type: Boolean, required: false, @@ -78,7 +73,7 @@ export default { :model="model" :can-create-deployment="canCreateDeployment" :can-read-environment="canReadEnvironment" - :service="service" /> + /> <template v-if="model.isFolder && model.isOpen && model.children && model.children.length > 0"> <tr v-if="isLoadingFolderContent"> @@ -96,7 +91,7 @@ export default { :model="children" :can-create-deployment="canCreateDeployment" :can-read-environment="canReadEnvironment" - :service="service" /> + /> <tr> <td diff --git a/app/assets/javascripts/environments/folder/environments_folder_view.vue b/app/assets/javascripts/environments/folder/environments_folder_view.vue index d27b2acfcdf..f4a0c390c91 100644 --- a/app/assets/javascripts/environments/folder/environments_folder_view.vue +++ b/app/assets/javascripts/environments/folder/environments_folder_view.vue @@ -1,5 +1,4 @@ <script> -/* eslint-disable no-new */ /* global Flash */ import EnvironmentsService from '../services/environments_service'; import EnvironmentTable from '../components/environments_table.vue'; @@ -99,6 +98,7 @@ export default { }) .catch(() => { this.isLoading = false; + // eslint-disable-next-line no-new new Flash('An error occurred while fetching the environments.', 'alert'); }); }, @@ -169,7 +169,7 @@ export default { :environments="state.environments" :can-create-deployment="canCreateDeploymentParsed" :can-read-environment="canReadEnvironmentParsed" - :service="service"/> + /> <table-pagination v-if="state.paginationInformation && state.paginationInformation.totalPages > 1" diff --git a/app/assets/javascripts/gl_form.js b/app/assets/javascripts/gl_form.js index ff10f19a4fe..ff06092e4d6 100644 --- a/app/assets/javascripts/gl_form.js +++ b/app/assets/javascripts/gl_form.js @@ -34,9 +34,9 @@ GLForm.prototype.setupForm = function() { gl.GfmAutoComplete.setup(this.form.find('.js-gfm-input')); new DropzoneInput(this.form); autosize(this.textarea); - // form and textarea event listeners - this.addEventListeners(); } + // form and textarea event listeners + this.addEventListeners(); gl.text.init(this.form); // hide discard button this.form.find('.js-note-discard').hide(); diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js index 011043e992f..694c6177a07 100644 --- a/app/assets/javascripts/issue.js +++ b/app/assets/javascripts/issue.js @@ -1,5 +1,6 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, one-var, no-underscore-dangle, one-var-declaration-per-line, object-shorthand, no-unused-vars, no-new, comma-dangle, consistent-return, quotes, dot-notation, quote-props, prefer-arrow-callback, max-len */ -/* global Flash */ + /* global Flash */ +import CreateMergeRequestDropdown from './create_merge_request_dropdown'; require('./flash'); require('~/lib/utils/text_utility'); @@ -18,48 +19,49 @@ class Issue { document.querySelector('#task_status_short').innerText = result.task_status_short; } }); - Issue.initIssueBtnEventListeners(); + this.initIssueBtnEventListeners(); } Issue.$btnNewBranch = $('#new-branch'); + Issue.createMrDropdownWrap = document.querySelector('.create-mr-dropdown-wrap'); Issue.initMergeRequests(); Issue.initRelatedBranches(); - Issue.initCanCreateBranch(); + + if (Issue.createMrDropdownWrap) { + this.createMergeRequestDropdown = new CreateMergeRequestDropdown(Issue.createMrDropdownWrap); + } } - static initIssueBtnEventListeners() { + initIssueBtnEventListeners() { const issueFailMessage = 'Unable to update this issue at this time.'; - const closeButtons = $('a.btn-close'); const isClosedBadge = $('div.status-box-closed'); const isOpenBadge = $('div.status-box-open'); const projectIssuesCounter = $('.issue_counter'); const reopenButtons = $('a.btn-reopen'); - return closeButtons.add(reopenButtons).on('click', function(e) { - var $this, shouldSubmit, url; + return closeButtons.add(reopenButtons).on('click', (e) => { + var $button, shouldSubmit, url; e.preventDefault(); e.stopImmediatePropagation(); - $this = $(this); - shouldSubmit = $this.hasClass('btn-comment'); + $button = $(e.currentTarget); + shouldSubmit = $button.hasClass('btn-comment'); if (shouldSubmit) { - Issue.submitNoteForm($this.closest('form')); + Issue.submitNoteForm($button.closest('form')); } - $this.prop('disabled', true); - Issue.setNewBranchButtonState(true, null); - url = $this.attr('href'); + $button.prop('disabled', true); + url = $button.attr('href'); return $.ajax({ type: 'PUT', url: url - }).fail(function(jqXHR, textStatus, errorThrown) { - new Flash(issueFailMessage); - Issue.initCanCreateBranch(); - }).done(function(data, textStatus, jqXHR) { + }) + .fail(() => new Flash(issueFailMessage)) + .done((data) => { if ('id' in data) { $(document).trigger('issuable:change'); - const isClosed = $this.hasClass('btn-close'); + const isClosed = $button.hasClass('btn-close'); closeButtons.toggleClass('hidden', isClosed); reopenButtons.toggleClass('hidden', !isClosed); isClosedBadge.toggleClass('hidden', !isClosed); @@ -68,12 +70,21 @@ class Issue { let numProjectIssues = Number(projectIssuesCounter.text().replace(/[^\d]/, '')); numProjectIssues = isClosed ? numProjectIssues - 1 : numProjectIssues + 1; projectIssuesCounter.text(gl.text.addDelimiter(numProjectIssues)); + + if (this.createMergeRequestDropdown) { + if (isClosed) { + this.createMergeRequestDropdown.unavailable(); + this.createMergeRequestDropdown.disable(); + } else { + // We should check in case a branch was created in another tab + this.createMergeRequestDropdown.checkAbilityToCreateBranch(); + } + } } else { new Flash(issueFailMessage); } - $this.prop('disabled', false); - Issue.initCanCreateBranch(); + $button.prop('disabled', false); }); }); } @@ -109,29 +120,6 @@ class Issue { } }); } - - static initCanCreateBranch() { - // If the user doesn't have the required permissions the container isn't - // rendered at all. - if (Issue.$btnNewBranch.length === 0) { - return; - } - return $.getJSON(Issue.$btnNewBranch.data('path')).fail(function() { - Issue.setNewBranchButtonState(false, false); - new Flash('Failed to check if a new branch can be created.'); - }).done(function(data) { - Issue.setNewBranchButtonState(false, data.can_create_branch); - }); - } - - static setNewBranchButtonState(isPending, canCreate) { - if (Issue.$btnNewBranch.length === 0) { - return; - } - - Issue.$btnNewBranch.find('.available').toggle(!isPending && canCreate); - Issue.$btnNewBranch.find('.unavailable').toggle(!isPending && !canCreate); - } } export default Issue; diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 974fb0d83da..87f03a40eba 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -4,6 +4,7 @@ /* global ResolveService */ /* global mrRefreshWidgetUrl */ +import $ from 'jquery'; import Cookies from 'js-cookie'; import CommentTypeToggle from './comment_type_toggle'; @@ -16,6 +17,10 @@ require('vendor/jquery.caret'); // required by jquery.atwho require('vendor/jquery.atwho'); require('./task_list'); +const normalizeNewlines = function(str) { + return str.replace(/\r\n/g, '\n'); +}; + (function() { var bind = function(fn, me) { return function() { return fn.apply(me, arguments); }; }; @@ -42,13 +47,17 @@ require('./task_list'); this.refresh = bind(this.refresh, this); this.keydownNoteText = bind(this.keydownNoteText, this); this.toggleCommitList = bind(this.toggleCommitList, this); + this.notes_url = notes_url; this.note_ids = note_ids; + // Used to keep track of updated notes while people are editing things + this.updatedNotesTrackingMap = {}; this.last_fetched_at = last_fetched_at; this.noteable_url = document.URL; this.notesCountBadge || (this.notesCountBadge = $(".issuable-details").find(".notes-tab .badge")); this.basePollingInterval = 15000; this.maxPollingSteps = 4; + this.cleanBinding(); this.addBinding(); this.setPollingInterval(); @@ -128,7 +137,7 @@ require('./task_list'); $(document).off("click", ".js-discussion-reply-button"); $(document).off("click", ".js-add-diff-note-button"); $(document).off("visibilitychange"); - $(document).off("keyup", ".js-note-text"); + $(document).off("keyup input", ".js-note-text"); $(document).off("click", ".js-note-target-reopen"); $(document).off("click", ".js-note-target-close"); $(document).off("click", ".js-note-discard"); @@ -267,20 +276,20 @@ require('./task_list'); return this.initRefresh(); }; - Notes.prototype.handleCreateChanges = function(note) { + Notes.prototype.handleCreateChanges = function(noteEntity) { var votesBlock; - if (typeof note === 'undefined') { + if (typeof noteEntity === 'undefined') { return; } - if (note.commands_changes) { - if ('merge' in note.commands_changes) { + if (noteEntity.commands_changes) { + if ('merge' in noteEntity.commands_changes) { $.get(mrRefreshWidgetUrl); } - if ('emoji_award' in note.commands_changes) { + if ('emoji_award' in noteEntity.commands_changes) { votesBlock = $('.js-awards-block').eq(0); - gl.awardsHandler.addAwardToEmojiBar(votesBlock, note.commands_changes.emoji_award); + gl.awardsHandler.addAwardToEmojiBar(votesBlock, noteEntity.commands_changes.emoji_award); return gl.awardsHandler.scrollToAwards(); } } @@ -292,41 +301,76 @@ require('./task_list'); Note: for rendering inline notes use renderDiscussionNote */ - Notes.prototype.renderNote = function(note, $form) { - var $notesList; - if (note.discussion_html != null) { - return this.renderDiscussionNote(note, $form); + Notes.prototype.renderNote = function(noteEntity, $form, $notesList = $('.main-notes-list')) { + if (noteEntity.discussion_html != null) { + return this.renderDiscussionNote(noteEntity, $form); } - if (!note.valid) { - if (note.errors.commands_only) { - new Flash(note.errors.commands_only, 'notice', this.parentTimeline); + if (!noteEntity.valid) { + if (noteEntity.errors.commands_only) { + new Flash(noteEntity.errors.commands_only, 'notice', this.parentTimeline); this.refresh(); } return; } - if (this.isNewNote(note)) { - this.note_ids.push(note.id); + const $note = $notesList.find(`#note_${noteEntity.id}`); + if (this.isNewNote(noteEntity)) { + this.note_ids.push(noteEntity.id); - $notesList = window.$('ul.main-notes-list'); - Notes.animateAppendNote(note.html, $notesList); + const $newNote = Notes.animateAppendNote(noteEntity.html, $notesList); // Update datetime format on the recent note - gl.utils.localTimeAgo($notesList.find("#note_" + note.id + " .js-timeago"), false); + gl.utils.localTimeAgo($newNote.find('.js-timeago'), false); this.collapseLongCommitList(); this.taskList.init(); this.refresh(); return this.updateNotesCount(1); } + // The server can send the same update multiple times so we need to make sure to only update once per actual update. + else if (this.isUpdatedNote(noteEntity, $note)) { + const isEditing = $note.hasClass('is-editing'); + const initialContent = normalizeNewlines( + $note.find('.original-note-content').text().trim() + ); + const $textarea = $note.find('.js-note-text'); + const currentContent = $textarea.val(); + // There can be CRLF vs LF mismatches if we don't sanitize and compare the same way + const sanitizedNoteNote = normalizeNewlines(noteEntity.note); + const isTextareaUntouched = currentContent === initialContent || currentContent === sanitizedNoteNote; + + if (isEditing && isTextareaUntouched) { + $textarea.val(noteEntity.note); + this.updatedNotesTrackingMap[noteEntity.id] = noteEntity; + } + else if (isEditing && !isTextareaUntouched) { + this.putConflictEditWarningInPlace(noteEntity, $note); + this.updatedNotesTrackingMap[noteEntity.id] = noteEntity; + } + else { + const $updatedNote = Notes.animateUpdateNote(noteEntity.html, $note); + + // Update datetime format on the recent note + gl.utils.localTimeAgo($updatedNote.find('.js-timeago'), false); + } + } }; /* Check if note does not exists on page */ - Notes.prototype.isNewNote = function(note) { - return $.inArray(note.id, this.note_ids) === -1; + Notes.prototype.isNewNote = function(noteEntity) { + return $.inArray(noteEntity.id, this.note_ids) === -1; + }; + + Notes.prototype.isUpdatedNote = function(noteEntity, $note) { + // There can be CRLF vs LF mismatches if we don't sanitize and compare the same way + const sanitizedNoteNote = normalizeNewlines(noteEntity.note); + const currentNoteText = normalizeNewlines( + $note.find('.original-note-content').text().trim() + ); + return sanitizedNoteNote !== currentNoteText; }; Notes.prototype.isParallelView = function() { @@ -339,31 +383,31 @@ require('./task_list'); Note: for rendering inline notes use renderDiscussionNote */ - Notes.prototype.renderDiscussionNote = function(note, $form) { + Notes.prototype.renderDiscussionNote = function(noteEntity, $form) { var discussionContainer, form, row, lineType, diffAvatarContainer; - if (!this.isNewNote(note)) { + if (!this.isNewNote(noteEntity)) { return; } - this.note_ids.push(note.id); - form = $form || $(".js-discussion-note-form[data-discussion-id='" + note.discussion_id + "']"); + this.note_ids.push(noteEntity.id); + form = $form || $(".js-discussion-note-form[data-discussion-id='" + noteEntity.discussion_id + "']"); row = form.closest("tr"); lineType = this.isParallelView() ? form.find('#line_type').val() : 'old'; diffAvatarContainer = row.prevAll('.line_holder').first().find('.js-avatar-container.' + lineType + '_line'); // is this the first note of discussion? - discussionContainer = window.$(`.notes[data-discussion-id="${note.discussion_id}"]`); + discussionContainer = $(`.notes[data-discussion-id="${noteEntity.discussion_id}"]`); if (!discussionContainer.length) { discussionContainer = form.closest('.discussion').find('.notes'); } if (discussionContainer.length === 0) { - if (note.diff_discussion_html) { - var $discussion = $(note.diff_discussion_html).renderGFM(); + if (noteEntity.diff_discussion_html) { + var $discussion = $(noteEntity.diff_discussion_html).renderGFM(); if (!this.isParallelView() || row.hasClass('js-temp-notes-holder')) { // insert the note and the reply button after the temp row row.after($discussion); } else { // Merge new discussion HTML in - var $notes = $discussion.find('.notes[data-discussion-id="' + note.discussion_id + '"]'); + var $notes = $discussion.find('.notes[data-discussion-id="' + noteEntity.discussion_id + '"]'); var contentContainerClass = '.' + $notes.closest('.notes_content') .attr('class') .split(' ') @@ -373,17 +417,18 @@ require('./task_list'); } } // Init discussion on 'Discussion' page if it is merge request page - if (window.$('body').attr('data-page').indexOf('projects:merge_request') === 0 || !note.diff_discussion_html) { - Notes.animateAppendNote(note.discussion_html, window.$('ul.main-notes-list')); + const page = $('body').attr('data-page'); + if ((page && page.indexOf('projects:merge_request') === 0) || !noteEntity.diff_discussion_html) { + Notes.animateAppendNote(noteEntity.discussion_html, $('.main-notes-list')); } } else { // append new note to all matching discussions - Notes.animateAppendNote(note.html, discussionContainer); + Notes.animateAppendNote(noteEntity.html, discussionContainer); } - if (typeof gl.diffNotesCompileComponents !== 'undefined' && note.discussion_resolvable) { + if (typeof gl.diffNotesCompileComponents !== 'undefined' && noteEntity.discussion_resolvable) { gl.diffNotesCompileComponents(); - this.renderDiscussionAvatar(diffAvatarContainer, note); + this.renderDiscussionAvatar(diffAvatarContainer, noteEntity); } gl.utils.localTimeAgo($('.js-timeago'), false); @@ -397,13 +442,13 @@ require('./task_list'); .get(0); }; - Notes.prototype.renderDiscussionAvatar = function(diffAvatarContainer, note) { + Notes.prototype.renderDiscussionAvatar = function(diffAvatarContainer, noteEntity) { var commentButton = diffAvatarContainer.find('.js-add-diff-note-button'); var avatarHolder = diffAvatarContainer.find('.diff-comment-avatar-holders'); if (!avatarHolder.length) { avatarHolder = document.createElement('diff-note-avatars'); - avatarHolder.setAttribute('discussion-id', note.discussion_id); + avatarHolder.setAttribute('discussion-id', noteEntity.discussion_id); diffAvatarContainer.append(avatarHolder); @@ -550,16 +595,16 @@ require('./task_list'); Updates the current note field. */ - Notes.prototype.updateNote = function(_xhr, note, _status) { + Notes.prototype.updateNote = function(_xhr, noteEntity, _status) { var $html, $note_li; // Convert returned HTML to a jQuery object so we can modify it further - $html = $(note.html); + $html = $(noteEntity.html); this.revertNoteEditForm(); gl.utils.localTimeAgo($('.js-timeago', $html)); $html.renderGFM(); $html.find('.js-task-list-container').taskList('enable'); // Find the note's `li` element by ID and replace it with the updated HTML - $note_li = $('.note-row-' + note.id); + $note_li = $('.note-row-' + noteEntity.id); $note_li.replaceWith($html); @@ -570,7 +615,7 @@ require('./task_list'); Notes.prototype.checkContentToAllowEditing = function($el) { var initialContent = $el.find('.original-note-content').text().trim(); - var currentContent = $el.find('.note-textarea').val(); + var currentContent = $el.find('.js-note-text').val(); var isAllowed = true; if (currentContent === initialContent) { @@ -584,7 +629,7 @@ require('./task_list'); gl.utils.scrollToElement($el); } - $el.find('.js-edit-warning').show(); + $el.find('.js-finish-edit-warning').show(); isAllowed = false; } @@ -603,7 +648,7 @@ require('./task_list'); var $target = $(e.target); var $editForm = $(this.getEditFormSelector($target)); var $note = $target.closest('.note'); - var $currentlyEditing = $('.note.is-editting:visible'); + var $currentlyEditing = $('.note.is-editing:visible'); if ($currentlyEditing.length) { var isEditAllowed = this.checkContentToAllowEditing($currentlyEditing); @@ -615,7 +660,7 @@ require('./task_list'); $note.find('.js-note-attachment-delete').show(); $editForm.addClass('current-note-edit-form'); - $note.addClass('is-editting'); + $note.addClass('is-editing'); this.putEditFormInPlace($target); }; @@ -627,21 +672,34 @@ require('./task_list'); Notes.prototype.cancelEdit = function(e) { e.preventDefault(); - var $target = $(e.target); - var note = $target.closest('.note'); - note.find('.js-edit-warning').hide(); + const $target = $(e.target); + const $note = $target.closest('.note'); + const noteId = $note.attr('data-note-id'); + this.revertNoteEditForm($target); - return this.removeNoteEditForm(note); + + if (this.updatedNotesTrackingMap[noteId]) { + const $newNote = $(this.updatedNotesTrackingMap[noteId].html); + $note.replaceWith($newNote); + this.updatedNotesTrackingMap[noteId] = null; + + // Update datetime format on the recent note + gl.utils.localTimeAgo($newNote.find('.js-timeago'), false); + } + else { + $note.find('.js-finish-edit-warning').hide(); + this.removeNoteEditForm($note); + } }; Notes.prototype.revertNoteEditForm = function($target) { - $target = $target || $('.note.is-editting:visible'); + $target = $target || $('.note.is-editing:visible'); var selector = this.getEditFormSelector($target); var $editForm = $(selector); $editForm.insertBefore('.notes-form'); $editForm.find('.js-comment-button').enable(); - $editForm.find('.js-edit-warning').hide(); + $editForm.find('.js-finish-edit-warning').hide(); }; Notes.prototype.getEditFormSelector = function($el) { @@ -654,11 +712,11 @@ require('./task_list'); return selector; }; - Notes.prototype.removeNoteEditForm = function(note) { - var form = note.find('.current-note-edit-form'); - note.removeClass('is-editting'); + Notes.prototype.removeNoteEditForm = function($note) { + var form = $note.find('.current-note-edit-form'); + $note.removeClass('is-editing'); form.removeClass('current-note-edit-form'); - form.find('.js-edit-warning').hide(); + form.find('.js-finish-edit-warning').hide(); // Replace markdown textarea text with original note text. return form.find('.js-note-text').val(form.find('form.edit-note').data('original-note')); }; @@ -683,9 +741,9 @@ require('./task_list'); // to remove all. Using $(".note[id='noteId']") ensure we get all the notes, // where $("#noteId") would return only one. return function(i, el) { - var note, notes; - note = $(el); - notes = note.closest(".discussion-notes"); + var $note, $notes; + $note = $(el); + $notes = $note.closest(".discussion-notes"); if (typeof gl.diffNotesCompileComponents !== 'undefined') { if (gl.diffNoteApps[noteElId]) { @@ -693,18 +751,18 @@ require('./task_list'); } } - note.remove(); + $note.remove(); // check if this is the last note for this line - if (notes.find(".note").length === 0) { - var notesTr = notes.closest("tr"); + if ($notes.find(".note").length === 0) { + var notesTr = $notes.closest("tr"); // "Discussions" tab - notes.closest(".timeline-entry").remove(); + $notes.closest(".timeline-entry").remove(); // The notes tr can contain multiple lists of notes, like on the parallel diff if (notesTr.find('.discussion-notes').length > 1) { - notes.remove(); + $notes.remove(); } else { notesTr.remove(); } @@ -723,12 +781,11 @@ require('./task_list'); */ Notes.prototype.removeAttachment = function() { - var note; - note = $(this).closest(".note"); - note.find(".note-attachment").remove(); - note.find(".note-body > .note-text").show(); - note.find(".note-header").show(); - return note.find(".current-note-edit-form").remove(); + const $note = $(this).closest(".note"); + $note.find(".note-attachment").remove(); + $note.find(".note-body > .note-text").show(); + $note.find(".note-header").show(); + return $note.find(".current-note-edit-form").remove(); }; /* @@ -1004,6 +1061,19 @@ require('./task_list'); $editForm.find('.referenced-users').hide(); }; + Notes.prototype.putConflictEditWarningInPlace = function(noteEntity, $note) { + if ($note.find('.js-conflict-edit-warning').length === 0) { + const $alert = $(`<div class="js-conflict-edit-warning alert alert-danger"> + This comment has changed since you started editing, please review the + <a href="#note_${noteEntity.id}" target="_blank" rel="noopener noreferrer"> + updated comment + </a> + to ensure information is not lost + </div>`); + $alert.insertAfter($note.find('.note-text')); + } + }; + Notes.prototype.updateNotesCount = function(updateCount) { return this.notesCountBadge.text(parseInt(this.notesCountBadge.text(), 10) + updateCount); }; @@ -1064,11 +1134,20 @@ require('./task_list'); return $form; }; - Notes.animateAppendNote = function(noteHTML, $notesList) { - const $note = window.$(noteHTML); + Notes.animateAppendNote = function(noteHtml, $notesList) { + const $note = $(noteHtml); $note.addClass('fade-in').renderGFM(); $notesList.append($note); + return $note; + }; + + Notes.animateUpdateNote = function(noteHtml, $note) { + const $updatedNote = $(noteHtml); + + $updatedNote.addClass('fade-in').renderGFM(); + $note.replaceWith($updatedNote); + return $updatedNote; }; return Notes; diff --git a/app/assets/javascripts/pipelines/components/stage.js b/app/assets/javascripts/pipelines/components/stage.js deleted file mode 100644 index 203485f2990..00000000000 --- a/app/assets/javascripts/pipelines/components/stage.js +++ /dev/null @@ -1,115 +0,0 @@ -/* global Flash */ -import StatusIconEntityMap from '../../ci_status_icons'; - -export default { - props: { - stage: { - type: Object, - required: true, - }, - }, - - data() { - return { - builds: '', - spinner: '<span class="fa fa-spinner fa-spin"></span>', - }; - }, - - updated() { - if (this.builds) { - this.stopDropdownClickPropagation(); - } - }, - - methods: { - fetchBuilds(e) { - const ariaExpanded = e.currentTarget.attributes['aria-expanded']; - - if (ariaExpanded && (ariaExpanded.textContent === 'true')) return null; - - return this.$http.get(this.stage.dropdown_path) - .then((response) => { - this.builds = JSON.parse(response.body).html; - }) - .catch(() => { - // If dropdown is opened we'll close it. - if (this.$el.classList.contains('open')) { - $(this.$refs.dropdown).dropdown('toggle'); - } - - const flash = new Flash('Something went wrong on our end.'); - return flash; - }); - }, - - /** - * When the user right clicks or cmd/ctrl + click in the job name - * the dropdown should not be closed and the link should open in another tab, - * so we stop propagation of the click event inside the dropdown. - * - * Since this component is rendered multiple times per page we need to guarantee we only - * target the click event of this component. - */ - stopDropdownClickPropagation() { - $(this.$el.querySelectorAll('.js-builds-dropdown-list a.mini-pipeline-graph-dropdown-item')) - .on('click', (e) => { - e.stopPropagation(); - }); - }, - }, - computed: { - buildsOrSpinner() { - return this.builds ? this.builds : this.spinner; - }, - dropdownClass() { - if (this.builds) return 'js-builds-dropdown-container'; - return 'js-builds-dropdown-loading builds-dropdown-loading'; - }, - buildStatus() { - return `Build: ${this.stage.status.label}`; - }, - tooltip() { - return `has-tooltip ci-status-icon ci-status-icon-${this.stage.status.group}`; - }, - triggerButtonClass() { - return `mini-pipeline-graph-dropdown-toggle has-tooltip js-builds-dropdown-button ci-status-icon-${this.stage.status.group}`; - }, - svgHTML() { - return StatusIconEntityMap[this.stage.status.icon]; - }, - }, - template: ` - <div> - <button - @click="fetchBuilds($event)" - :class="triggerButtonClass" - :title="stage.title" - data-placement="top" - data-toggle="dropdown" - type="button" - :aria-label="stage.title" - ref="dropdown"> - <span - v-html="svgHTML" - aria-hidden="true"> - </span> - <i - class="fa fa-caret-down" - aria-hidden="true" /> - </button> - <ul - ref="dropdown-content" - class="dropdown-menu mini-pipeline-graph-dropdown-menu js-builds-dropdown-container"> - <div - class="arrow-up" - aria-hidden="true"></div> - <div - :class="dropdownClass" - class="js-builds-dropdown-list scrollable-menu" - v-html="buildsOrSpinner"> - </div> - </ul> - </div> - `, -}; diff --git a/app/assets/javascripts/pipelines/components/stage.vue b/app/assets/javascripts/pipelines/components/stage.vue new file mode 100644 index 00000000000..2e485f951a1 --- /dev/null +++ b/app/assets/javascripts/pipelines/components/stage.vue @@ -0,0 +1,173 @@ +<script> + +/** + * Renders each stage of the pipeline mini graph. + * + * Given the provided endpoint will make a request to + * fetch the dropdown data when the stage is clicked. + * + * Request is made inside this component to make it reusable between: + * 1. Pipelines main table + * 2. Pipelines table in commit and Merge request views + * 3. Merge request widget + * 4. Commit widget + */ + +/* global Flash */ +import StatusIconEntityMap from '../../ci_status_icons'; + +export default { + props: { + stage: { + type: Object, + required: true, + }, + + updateDropdown: { + type: Boolean, + required: false, + default: false, + }, + }, + + data() { + return { + isLoading: false, + dropdownContent: '', + endpoint: this.stage.dropdown_path, + }; + }, + + updated() { + if (this.dropdownContent.length > 0) { + this.stopDropdownClickPropagation(); + } + }, + + watch: { + updateDropdown() { + if (this.updateDropdown && + this.isDropdownOpen() && + !this.isLoading) { + this.fetchJobs(); + } + }, + }, + + methods: { + onClickStage() { + if (!this.isDropdownOpen()) { + this.isLoading = true; + this.fetchJobs(); + } + }, + + fetchJobs() { + this.$http.get(this.endpoint) + .then((response) => { + this.dropdownContent = response.json().html; + this.isLoading = false; + }) + .catch(() => { + this.closeDropdown(); + this.isLoading = false; + + const flash = new Flash('Something went wrong on our end.'); + return flash; + }); + }, + + /** + * When the user right clicks or cmd/ctrl + click in the job name + * the dropdown should not be closed and the link should open in another tab, + * so we stop propagation of the click event inside the dropdown. + * + * Since this component is rendered multiple times per page we need to guarantee we only + * target the click event of this component. + */ + stopDropdownClickPropagation() { + $(this.$el.querySelectorAll('.js-builds-dropdown-list a.mini-pipeline-graph-dropdown-item')) + .on('click', (e) => { + e.stopPropagation(); + }); + }, + + closeDropdown() { + if (this.isDropdownOpen()) { + $(this.$refs.dropdown).dropdown('toggle'); + } + }, + + isDropdownOpen() { + return this.$el.classList.contains('open'); + }, + }, + + computed: { + dropdownClass() { + return this.dropdownContent.length > 0 ? 'js-builds-dropdown-container' : 'js-builds-dropdown-loading'; + }, + + triggerButtonClass() { + return `ci-status-icon-${this.stage.status.group}`; + }, + + svgIcon() { + return StatusIconEntityMap[this.stage.status.icon]; + }, + }, +}; +</script> + +<template> + <div class="dropdown"> + <button + :class="triggerButtonClass" + @click="onClickStage" + class="mini-pipeline-graph-dropdown-toggle has-tooltip js-builds-dropdown-button" + :title="stage.title" + data-placement="top" + data-toggle="dropdown" + type="button" + id="stageDropdown" + aria-haspopup="true" + aria-expanded="false"> + + <span + v-html="svgIcon" + aria-hidden="true" + :aria-label="stage.title"> + </span> + + <i + class="fa fa-caret-down" + aria-hidden="true"> + </i> + </button> + + <ul + class="dropdown-menu mini-pipeline-graph-dropdown-menu js-builds-dropdown-container" + aria-labelledby="stageDropdown"> + + <li + :class="dropdownClass" + class="js-builds-dropdown-list scrollable-menu"> + + <div + class="text-center" + v-if="isLoading"> + <i + class="fa fa-spin fa-spinner" + aria-hidden="true" + aria-label="Loading"> + </i> + </div> + + <ul + v-else + v-html="dropdownContent"> + </ul> + </li> + </ul> + </div> +</script> diff --git a/app/assets/javascripts/pipelines/pipelines.js b/app/assets/javascripts/pipelines/pipelines.js index 93d4818231f..934bd7deb31 100644 --- a/app/assets/javascripts/pipelines/pipelines.js +++ b/app/assets/javascripts/pipelines/pipelines.js @@ -49,6 +49,7 @@ export default { isLoading: false, hasError: false, isMakingRequest: false, + updateGraphDropdown: false, }; }, @@ -198,15 +199,21 @@ export default { this.store.storePagination(response.headers); this.isLoading = false; + this.updateGraphDropdown = true; }, errorCallback() { this.hasError = true; this.isLoading = false; + this.updateGraphDropdown = false; }, setIsMakingRequest(isMakingRequest) { this.isMakingRequest = isMakingRequest; + + if (isMakingRequest) { + this.updateGraphDropdown = false; + } }, }, @@ -263,7 +270,9 @@ export default { <pipelines-table-component :pipelines="state.pipelines" - :service="service"/> + :service="service" + :update-graph-dropdown="updateGraphDropdown" + /> </div> <gl-pagination diff --git a/app/assets/javascripts/vue_shared/components/pipelines_table.js b/app/assets/javascripts/vue_shared/components/pipelines_table.js index afd8d7acf6b..48a39f18112 100644 --- a/app/assets/javascripts/vue_shared/components/pipelines_table.js +++ b/app/assets/javascripts/vue_shared/components/pipelines_table.js @@ -10,13 +10,18 @@ export default { pipelines: { type: Array, required: true, - default: () => ([]), }, service: { type: Object, required: true, }, + + updateGraphDropdown: { + type: Boolean, + required: false, + default: false, + }, }, components: { @@ -40,7 +45,9 @@ export default { v-bind:model="model"> <tr is="pipelines-table-row-component" :pipeline="model" - :service="service"></tr> + :service="service" + :update-graph-dropdown="updateGraphDropdown" + /> </template> </tbody> </table> diff --git a/app/assets/javascripts/vue_shared/components/pipelines_table_row.js b/app/assets/javascripts/vue_shared/components/pipelines_table_row.js index 79806bc7204..fbae85c85f6 100644 --- a/app/assets/javascripts/vue_shared/components/pipelines_table_row.js +++ b/app/assets/javascripts/vue_shared/components/pipelines_table_row.js @@ -3,7 +3,7 @@ import AsyncButtonComponent from '../../pipelines/components/async_button.vue'; import PipelinesActionsComponent from '../../pipelines/components/pipelines_actions'; import PipelinesArtifactsComponent from '../../pipelines/components/pipelines_artifacts'; import PipelinesStatusComponent from '../../pipelines/components/status'; -import PipelinesStageComponent from '../../pipelines/components/stage'; +import PipelinesStageComponent from '../../pipelines/components/stage.vue'; import PipelinesUrlComponent from '../../pipelines/components/pipeline_url'; import PipelinesTimeagoComponent from '../../pipelines/components/time_ago'; import CommitComponent from './commit'; @@ -24,6 +24,12 @@ export default { type: Object, required: true, }, + + updateGraphDropdown: { + type: Boolean, + required: false, + default: false, + }, }, components: { @@ -213,7 +219,10 @@ export default { <div class="stage-container dropdown js-mini-pipeline-graph" v-if="pipeline.details.stages.length > 0" v-for="stage in pipeline.details.stages"> - <dropdown-stage :stage="stage"/> + + <dropdown-stage + :stage="stage" + :update-dropdown="updateGraphDropdown"/> </div> </td> diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 1b4694377b3..feefaad8a15 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -425,12 +425,6 @@ float: right; } -.diffs { - .content-block { - border-bottom: none; - } -} - .files-changed { border-bottom: none; } diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index 2aa52986e0a..b18bbc329c3 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -161,3 +161,86 @@ ul.related-merge-requests > li { .recaptcha { margin-bottom: 30px; } + +.new-branch-col { + padding-top: 10px; +} + +.create-mr-dropdown-wrap { + .btn-group:not(.hide) { + display: flex; + } + + .js-create-merge-request { + flex-grow: 1; + flex-shrink: 0; + } + + .dropdown-menu { + width: 300px; + opacity: 1; + visibility: visible; + transform: translateY(0); + display: none; + } + + .dropdown-toggle { + .fa-caret-down { + pointer-events: none; + margin-left: 0; + color: inherit; + margin-left: 0; + } + } + + li:not(.divider) { + padding: 6px; + cursor: pointer; + + &:hover, + &:focus { + background-color: $dropdown-hover-color; + color: $white-light; + } + + &.droplab-item-selected { + .icon-container { + i { + visibility: visible; + } + } + } + + .icon-container { + float: left; + padding-left: 6px; + + i { + visibility: hidden; + } + } + + .description { + padding-left: 30px; + font-size: 13px; + + strong { + display: block; + font-weight: 600; + } + } + } +} + +@media (min-width: $screen-sm-min) { + .new-branch-col { + padding-top: 0; + text-align: right; + } + + .create-mr-dropdown-wrap { + .btn-group:not(.hide) { + display: inline-block; + } + } +} diff --git a/app/assets/stylesheets/pages/members.scss b/app/assets/stylesheets/pages/members.scss index be7193bae04..8dbac76e30a 100644 --- a/app/assets/stylesheets/pages/members.scss +++ b/app/assets/stylesheets/pages/members.scss @@ -133,3 +133,55 @@ right: 160px; } } + +.flex-project-members-panel { + display: flex; + flex-direction: row; + align-items: center; + justify-content: center; + + @media (max-width: $screen-sm-min) { + display: block; + + .flex-project-title { + vertical-align: top; + display: inline-block; + max-width: 90%; + } + } + + .flex-project-title { + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; + } + + .badge { + height: 17px; + line-height: 16px; + margin-right: 5px; + padding-top: 1px; + padding-bottom: 1px; + } + + .flex-project-members-form { + flex-wrap: nowrap; + white-space: nowrap; + margin-left: auto; + } +} + +.panel { + .panel-heading { + .badge { + margin-top: 0; + } + + @media (max-width: $screen-sm-min) { + .badge { + margin-right: 0; + margin-left: 0; + } + } + } +}
\ No newline at end of file diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 6a419384a34..bca62b7fc31 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -511,7 +511,6 @@ .mr-version-controls { background: $gray-light; - border-bottom: 1px solid $border-color; color: $gl-text-color; .mr-version-menus-container { diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 7cf74502a3a..f89150ebead 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -67,7 +67,7 @@ ul.notes { } } - &.is-editting { + &.is-editing { .note-header, .note-text, .edited-text { diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index a4fe652b52f..9115d26c779 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -781,16 +781,11 @@ } .scrollable-menu { + padding: 0; max-height: 245px; overflow: auto; } - // Loading icon - .builds-dropdown-loading { - margin: 0 auto; - width: 20px; - } - // Action icon on the right a.ci-action-icon-wrapper { color: $action-icon-color; @@ -893,30 +888,29 @@ * Top arrow in the dropdown in the mini pipeline graph */ .mini-pipeline-graph-dropdown-menu { - .arrow-up { - &::before, - &::after { - content: ''; - display: inline-block; - position: absolute; - width: 0; - height: 0; - border-color: transparent; - border-style: solid; - top: -6px; - left: 2px; - border-width: 0 5px 6px; - } - &::before { - border-width: 0 5px 5px; - border-bottom-color: $border-color; - } + &::before, + &::after { + content: ''; + display: inline-block; + position: absolute; + width: 0; + height: 0; + border-color: transparent; + border-style: solid; + top: -6px; + left: 2px; + border-width: 0 5px 6px; + } - &::after { - margin-top: 1px; - border-bottom-color: $white-light; - } + &::before { + border-width: 0 5px 5px; + border-bottom-color: $border-color; + } + + &::after { + margin-top: 1px; + border-bottom-color: $white-light; } } diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb new file mode 100644 index 00000000000..dec2e27335a --- /dev/null +++ b/app/controllers/concerns/uploads_actions.rb @@ -0,0 +1,27 @@ +module UploadsActions + def create + link_to_file = UploadService.new(model, params[:file], uploader_class).execute + + respond_to do |format| + if link_to_file + format.json do + render json: { link: link_to_file } + end + else + format.json do + render json: 'Invalid file.', status: :unprocessable_entity + end + end + end + end + + def show + return render_404 unless uploader.exists? + + disposition = uploader.image_or_video? ? 'inline' : 'attachment' + + expires_in 0.seconds, must_revalidate: true, private: true + + send_file uploader.file.path, disposition: disposition + end +end diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index e2f81b09adc..89f1128ec36 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -89,4 +89,8 @@ class Projects::ApplicationController < ApplicationController def builds_enabled return render_404 unless @project.feature_available?(:builds, current_user) end + + def require_pages_enabled! + not_found unless Gitlab.config.pages.enabled + end end diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 59222637961..a13588b4218 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -16,7 +16,8 @@ class Projects::ArtifactsController < Projects::ApplicationController end def browse - directory = params[:path] ? "#{params[:path]}/" : '' + @path = params[:path] + directory = @path ? "#{@path}/" : '' @entry = build.artifacts_metadata_entry(directory) render_404 unless @entry.exists? @@ -60,7 +61,10 @@ class Projects::ArtifactsController < Projects::ApplicationController end def build - @build ||= build_from_id || build_from_ref + @build ||= begin + build = build_from_id || build_from_ref + build&.present(current_user: current_user) + end end def build_from_id diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index 840405f38cb..f0f031303d8 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -46,20 +46,28 @@ class Projects::BranchesController < Projects::ApplicationController SystemNoteService.new_issue_branch(issue, @project, current_user, branch_name) if issue end - if result[:status] == :success - @branch = result[:branch] - - if redirect_to_autodeploy - redirect_to( - url_to_autodeploy_setup(project, branch_name), - notice: view_context.autodeploy_flash_notice(branch_name)) - else - redirect_to namespace_project_tree_path(@project.namespace, @project, - @branch.name) + respond_to do |format| + format.html do + if result[:status] == :success + if redirect_to_autodeploy + redirect_to url_to_autodeploy_setup(project, branch_name), + notice: view_context.autodeploy_flash_notice(branch_name) + else + redirect_to namespace_project_tree_path(@project.namespace, @project, branch_name) + end + else + @error = result[:message] + render action: 'new' + end + end + + format.json do + if result[:status] == :success + render json: { name: branch_name, url: namespace_project_tree_url(@project.namespace, @project, branch_name) } + else + render json: result[:messsage], status: :unprocessable_entity + end end - else - @error = result[:message] - render action: 'new' end end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index cbf67137261..af9157bfbb5 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -11,7 +11,7 @@ class Projects::IssuesController < Projects::ApplicationController before_action :redirect_to_external_issue_tracker, only: [:index, :new] before_action :module_enabled before_action :issue, only: [:edit, :update, :show, :referenced_merge_requests, - :related_branches, :can_create_branch, :rendered_title] + :related_branches, :can_create_branch, :rendered_title, :create_merge_request] # Allow read any issue before_action :authorize_read_issue!, only: [:show, :rendered_title] @@ -22,6 +22,9 @@ class Projects::IssuesController < Projects::ApplicationController # Allow modify issue before_action :authorize_update_issue!, only: [:edit, :update] + # Allow create a new branch and empty WIP merge request from current issue + before_action :authorize_create_merge_request!, only: [:create_merge_request] + respond_to :html def index @@ -191,7 +194,7 @@ class Projects::IssuesController < Projects::ApplicationController respond_to do |format| format.json do - render json: { can_create_branch: can_create } + render json: { can_create_branch: can_create, has_related_branch: @issue.has_related_branch? } end end end @@ -201,6 +204,16 @@ class Projects::IssuesController < Projects::ApplicationController render json: { title: view_context.markdown_field(@issue, :title) } end + def create_merge_request + result = MergeRequests::CreateFromIssueService.new(project, current_user, issue_iid: issue.iid).execute + + if result[:status] == :success + render json: MergeRequestCreateSerializer.new.represent(result[:merge_request]) + else + render json: result[:messsage], status: :unprocessable_entity + end + end + protected def issue @@ -224,6 +237,10 @@ class Projects::IssuesController < Projects::ApplicationController return render_404 unless can?(current_user, :admin_issue, @project) end + def authorize_create_merge_request! + return render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user) + end + def module_enabled return render_404 unless @project.feature_available?(:issues, current_user) && @project.default_issues_tracker? end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 09dc8b38229..a63b7ff0bed 100755 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -120,7 +120,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController define_diff_comment_vars else build_merge_request - @diffs = @merge_request.diffs(diff_options) + @compare = @merge_request + @diffs = @compare.diffs(diff_options) @diff_notes_disabled = true end @@ -584,12 +585,14 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end - @diffs = + @compare = if @start_sha - @merge_request_diff.compare_with(@start_sha).diffs(diff_options) + @merge_request_diff.compare_with(@start_sha) else - @merge_request_diff.diffs(diff_options) + @merge_request_diff end + + @diffs = @compare.diffs(diff_options) end def define_diff_comment_vars @@ -598,11 +601,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController noteable_id: @merge_request.id } - @diff_notes_disabled = !@merge_request_diff.latest? || @start_sha + @diff_notes_disabled = false @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs? - @grouped_diff_discussions = @merge_request.grouped_diff_discussions(@merge_request_diff.diff_refs) + @grouped_diff_discussions = @merge_request.grouped_diff_discussions(@compare.diff_refs) @notes = prepare_notes_for_rendering(@grouped_diff_discussions.values.flatten.flat_map(&:notes)) end diff --git a/app/controllers/projects/pages_controller.rb b/app/controllers/projects/pages_controller.rb index fbd18b68141..93b2c180810 100644 --- a/app/controllers/projects/pages_controller.rb +++ b/app/controllers/projects/pages_controller.rb @@ -1,6 +1,7 @@ class Projects::PagesController < Projects::ApplicationController layout 'project_settings' + before_action :require_pages_enabled! before_action :authorize_read_pages!, only: [:show] before_action :authorize_update_pages!, except: [:show] diff --git a/app/controllers/projects/pages_domains_controller.rb b/app/controllers/projects/pages_domains_controller.rb index b8c253f6ae3..3a93977fd27 100644 --- a/app/controllers/projects/pages_domains_controller.rb +++ b/app/controllers/projects/pages_domains_controller.rb @@ -1,6 +1,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController layout 'project_settings' + before_action :require_pages_enabled! before_action :authorize_update_pages!, except: [:show] before_action :domain, only: [:show, :destroy] diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 1780cc0233c..454b8ee17af 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -9,19 +9,19 @@ class Projects::PipelinesController < Projects::ApplicationController def index @scope = params[:scope] @pipelines = PipelinesFinder - .new(project) - .execute(scope: @scope) + .new(project, scope: @scope) + .execute .page(params[:page]) .per(30) @running_count = PipelinesFinder - .new(project).execute(scope: 'running').count + .new(project, scope: 'running').execute.count @pending_count = PipelinesFinder - .new(project).execute(scope: 'pending').count + .new(project, scope: 'pending').execute.count @finished_count = PipelinesFinder - .new(project).execute(scope: 'finished').count + .new(project, scope: 'finished').execute.count @pipelines_count = PipelinesFinder .new(project).execute.count diff --git a/app/controllers/projects/raw_controller.rb b/app/controllers/projects/raw_controller.rb index a0b08ad130f..a02cc477e08 100644 --- a/app/controllers/projects/raw_controller.rb +++ b/app/controllers/projects/raw_controller.rb @@ -15,7 +15,7 @@ class Projects::RawController < Projects::ApplicationController return if cached_blob? - if @blob.valid_lfs_pointer? + if @blob.stored_externally? send_lfs_object else send_git_blob @repository, @blob diff --git a/app/controllers/projects/uploads_controller.rb b/app/controllers/projects/uploads_controller.rb index 61686499bd3..6966a7c5fee 100644 --- a/app/controllers/projects/uploads_controller.rb +++ b/app/controllers/projects/uploads_controller.rb @@ -1,33 +1,11 @@ class Projects::UploadsController < Projects::ApplicationController + include UploadsActions + skip_before_action :project, :repository, if: -> { action_name == 'show' && image_or_video? } before_action :authorize_upload_file!, only: [:create] - def create - link_to_file = ::Projects::UploadService.new(project, params[:file]). - execute - - respond_to do |format| - if link_to_file - format.json do - render json: { link: link_to_file } - end - else - format.json do - render json: 'Invalid file.', status: :unprocessable_entity - end - end - end - end - - def show - return render_404 if uploader.nil? || !uploader.file.exists? - - disposition = uploader.image_or_video? ? 'inline' : 'attachment' - send_file uploader.file.path, disposition: disposition - end - private def uploader @@ -52,4 +30,10 @@ class Projects::UploadsController < Projects::ApplicationController def image_or_video? uploader && uploader.file.exists? && uploader.image_or_video? end + + def uploader_class + FileUploader + end + + alias_method :model, :project end diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index f1bfd574f04..21a964fb391 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -1,50 +1,43 @@ class UploadsController < ApplicationController - skip_before_action :authenticate_user! - before_action :find_model, :authorize_access! - - def show - uploader = @model.send(upload_mount) - - unless uploader.file_storage? - return redirect_to uploader.url - end + include UploadsActions - unless uploader.file && uploader.file.exists? - return render_404 - end - - disposition = uploader.image? ? 'inline' : 'attachment' - - expires_in 0.seconds, must_revalidate: true, private: true - send_file uploader.file.path, disposition: disposition - end + skip_before_action :authenticate_user! + before_action :find_model + before_action :authorize_access!, only: [:show] + before_action :authorize_create_access!, only: [:create] private def find_model - unless upload_model && upload_mount - return render_404 - end + return render_404 unless upload_model && upload_mount @model = upload_model.find(params[:id]) end def authorize_access! authorized = - case @model - when Project - can?(current_user, :read_project, @model) - when Group - can?(current_user, :read_group, @model) + case model when Note - can?(current_user, :read_project, @model.project) - else - # No authentication required for user avatars. + can?(current_user, :read_project, model.project) + when User true + else + permission = "read_#{model.class.to_s.underscore}".to_sym + + can?(current_user, permission, model) end - return if authorized + render_unauthorized unless authorized + end + + def authorize_create_access! + # for now we support only personal snippets comments + authorized = can?(current_user, :comment_personal_snippet, model) + render_unauthorized unless authorized + end + + def render_unauthorized if current_user render_404 else @@ -58,17 +51,44 @@ class UploadsController < ApplicationController "project" => Project, "note" => Note, "group" => Group, - "appearance" => Appearance + "appearance" => Appearance, + "personal_snippet" => PersonalSnippet } upload_models[params[:model]] end def upload_mount + return true unless params[:mounted_as] + upload_mounts = %w(avatar attachment file logo header_logo) if upload_mounts.include?(params[:mounted_as]) params[:mounted_as] end end + + def uploader + return @uploader if defined?(@uploader) + + if model.is_a?(PersonalSnippet) + @uploader = PersonalFileUploader.new(model, params[:secret]) + + @uploader.retrieve_from_store!(params[:filename]) + else + @uploader = @model.send(upload_mount) + + redirect_to @uploader.url unless @uploader.file_storage? + end + + @uploader + end + + def uploader_class + PersonalFileUploader + end + + def model + @model ||= find_model + end end diff --git a/app/finders/pipelines_finder.rb b/app/finders/pipelines_finder.rb index a9172f6767f..f187a3b61fe 100644 --- a/app/finders/pipelines_finder.rb +++ b/app/finders/pipelines_finder.rb @@ -1,29 +1,23 @@ class PipelinesFinder - attr_reader :project, :pipelines + attr_reader :project, :pipelines, :params - def initialize(project) + ALLOWED_INDEXED_COLUMNS = %w[id status ref user_id].freeze + + def initialize(project, params = {}) @project = project @pipelines = project.pipelines + @params = params end - def execute(scope: nil) - scoped_pipelines = - case scope - when 'running' - pipelines.running - when 'pending' - pipelines.pending - when 'finished' - pipelines.finished - when 'branches' - from_ids(ids_for_ref(branches)) - when 'tags' - from_ids(ids_for_ref(tags)) - else - pipelines - end - - scoped_pipelines.order(id: :desc) + def execute + items = pipelines + items = by_scope(items) + items = by_status(items) + items = by_ref(items) + items = by_name(items) + items = by_username(items) + items = by_yaml_errors(items) + sort_items(items) end private @@ -43,4 +37,78 @@ class PipelinesFinder def tags project.repository.tag_names end + + def by_scope(items) + case params[:scope] + when 'running' + items.running + when 'pending' + items.pending + when 'finished' + items.finished + when 'branches' + from_ids(ids_for_ref(branches)) + when 'tags' + from_ids(ids_for_ref(tags)) + else + items + end + end + + def by_status(items) + return items unless HasStatus::AVAILABLE_STATUSES.include?(params[:status]) + + items.where(status: params[:status]) + end + + def by_ref(items) + if params[:ref].present? + items.where(ref: params[:ref]) + else + items + end + end + + def by_name(items) + if params[:name].present? + items.joins(:user).where(users: { name: params[:name] }) + else + items + end + end + + def by_username(items) + if params[:username].present? + items.joins(:user).where(users: { username: params[:username] }) + else + items + end + end + + def by_yaml_errors(items) + case Gitlab::Utils.to_boolean(params[:yaml_errors]) + when true + items.where("yaml_errors IS NOT NULL") + when false + items.where("yaml_errors IS NULL") + else + items + end + end + + def sort_items(items) + order_by = if ALLOWED_INDEXED_COLUMNS.include?(params[:order_by]) + params[:order_by] + else + :id + end + + sort = if params[:sort] =~ /\A(ASC|DESC)\z/i + params[:sort] + else + :desc + end + + items.order(order_by => sort) + end end diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 377b080b3c6..37b6f4ad5cc 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -52,7 +52,7 @@ module BlobHelper if !on_top_of_branch?(project, ref) button_tag label, class: "#{common_classes} disabled has-tooltip", title: "You can only #{action} files when you are on a branch", data: { container: 'body' } - elsif blob.valid_lfs_pointer? + elsif blob.stored_externally? button_tag label, class: "#{common_classes} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' } elsif can_modify_blob?(blob, project, ref) button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal' @@ -95,7 +95,7 @@ module BlobHelper end def can_modify_blob?(blob, project = @project, ref = @ref) - !blob.valid_lfs_pointer? && can_edit_tree?(project, ref) + !blob.stored_externally? && can_edit_tree?(project, ref) end def leave_edit_message @@ -223,7 +223,9 @@ module BlobHelper end def open_raw_blob_button(blob) - if blob.raw_binary? + return if blob.empty? + + if blob.raw_binary? || blob.stored_externally? icon = icon('download') title = 'Download' else @@ -244,19 +246,27 @@ module BlobHelper viewer.max_size end "it is larger than #{number_to_human_size(max_size)}" - when :server_side_but_stored_in_lfs - "it is stored in LFS" + when :server_side_but_stored_externally + case viewer.blob.external_storage + when :lfs + 'it is stored in LFS' + else + 'it is stored externally' + end end end def blob_render_error_options(viewer) + error = viewer.render_error options = [] - if viewer.render_error == :too_large && viewer.can_override_max_size? + if error == :too_large && viewer.can_override_max_size? options << link_to('load it anyway', url_for(params.merge(viewer: viewer.type, override_max_size: true, format: nil))) end - if viewer.rich? && viewer.blob.rendered_as_text? + # If the error is `:server_side_but_stored_externally`, the simple viewer will show the same error, + # so don't bother switching. + if viewer.rich? && viewer.blob.rendered_as_text? && error != :server_side_but_stored_externally options << link_to('view the source', '#', class: 'js-blob-viewer-switch-btn', data: { viewer: 'simple' }) end diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index eab0738a368..08180883eb9 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -60,20 +60,16 @@ module NotesHelper note.project.team.human_max_access(note.author_id) end - def discussion_diff_path(discussion) - if discussion.for_merge_request? && discussion.diff_discussion? - if discussion.active? - # Without a diff ID, the link always points to the latest diff version - diff_id = nil - elsif merge_request_diff = discussion.latest_merge_request_diff - diff_id = merge_request_diff.id - else - # If the discussion is not active, and we cannot find the latest - # merge request diff for this discussion, we return no path at all. - return - end - - diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, diff_id: diff_id, anchor: discussion.line_code) + def discussion_path(discussion) + if discussion.for_merge_request? + return unless discussion.diff_discussion? + + version_params = discussion.merge_request_version_params + return unless version_params + + path_params = version_params.merge(anchor: discussion.line_code) + + diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, path_params) elsif discussion.for_commit? anchor = discussion.line_code if discussion.diff_discussion? diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb index 2fda98cae90..4882d9b71d2 100644 --- a/app/helpers/sorting_helper.rb +++ b/app/helpers/sorting_helper.rb @@ -70,6 +70,14 @@ module SortingHelper } end + def tags_sort_options_hash + { + sort_value_name => sort_title_name, + sort_value_recently_updated => sort_title_recently_updated, + sort_value_oldest_updated => sort_title_oldest_updated + } + end + def sort_title_priority 'Priority' end diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index f7b5a5f4dfc..a91e3da309c 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -76,7 +76,7 @@ module TreeHelper "A new branch will be created in your fork and a new merge request will be started." end - def tree_breadcrumbs(tree, max_links = 2) + def path_breadcrumbs(max_links = 6) if @path.present? part_path = "" parts = @path.split('/') @@ -88,7 +88,7 @@ module TreeHelper part_path = part if part_path.empty? next if parts.count > max_links && !parts.last(2).include?(part) - yield(part, tree_join(@ref, part_path)) + yield(part, part_path) end end end diff --git a/app/models/blob.rb b/app/models/blob.rb index 1cdb8811cff..a4fae22a0c4 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -28,7 +28,7 @@ class Blob < SimpleDelegator BlobViewer::Sketch, BlobViewer::Video, - + BlobViewer::PDF, BlobViewer::BinarySTL, @@ -75,19 +75,37 @@ class Blob < SimpleDelegator end def no_highlighting? - size && size > MAXIMUM_TEXT_HIGHLIGHT_SIZE + raw_size && raw_size > MAXIMUM_TEXT_HIGHLIGHT_SIZE + end + + def empty? + raw_size == 0 end def too_large? size && truncated? end + def external_storage_error? + if external_storage == :lfs + !project&.lfs_enabled? + else + false + end + end + + def stored_externally? + return @stored_externally if defined?(@stored_externally) + + @stored_externally = external_storage && !external_storage_error? + end + # Returns the size of the file that this blob represents. If this blob is an # LFS pointer, this is the size of the file stored in LFS. Otherwise, this is # the size of the blob itself. def raw_size - if valid_lfs_pointer? - lfs_size + if stored_externally? + external_size else size end @@ -98,9 +116,13 @@ class Blob < SimpleDelegator # text-based rich blob viewer matched on the file's extension. Otherwise, this # depends on the type of the blob itself. def raw_binary? - if valid_lfs_pointer? + if stored_externally? if rich_viewer rich_viewer.binary? + elsif Linguist::Language.find_by_filename(name).any? + false + elsif _mime_type + _mime_type.binary? else true end @@ -118,15 +140,7 @@ class Blob < SimpleDelegator end def readable_text? - text? && !valid_lfs_pointer? && !too_large? - end - - def valid_lfs_pointer? - lfs_pointer? && project&.lfs_enabled? - end - - def invalid_lfs_pointer? - lfs_pointer? && !project&.lfs_enabled? + text? && !stored_externally? && !too_large? end def simple_viewer @@ -165,10 +179,10 @@ class Blob < SimpleDelegator end def rich_viewer_class - return if invalid_lfs_pointer? || empty? + return if empty? || external_storage_error? classes = - if valid_lfs_pointer? + if stored_externally? BINARY_VIEWERS + TEXT_VIEWERS elsif binary? BINARY_VIEWERS diff --git a/app/models/blob_viewer/base.rb b/app/models/blob_viewer/base.rb index f944b00c9d3..a8b91d8d6bc 100644 --- a/app/models/blob_viewer/base.rb +++ b/app/models/blob_viewer/base.rb @@ -70,12 +70,13 @@ module BlobViewer return @render_error if defined?(@render_error) @render_error = - if server_side_but_stored_in_lfs? - # Files stored in LFS can only be rendered using a client-side viewer, + if server_side_but_stored_externally? + # Files that are not stored in the repository, like LFS files and + # build artifacts, can only be rendered using a client-side viewer, # since we do not want to read large amounts of data into memory on the # server side. Client-side viewers use JS and can fetch the file from # `blob_raw_url` using AJAX. - :server_side_but_stored_in_lfs + :server_side_but_stored_externally elsif override_max_size ? absolutely_too_large? : too_large? :too_large end @@ -89,8 +90,8 @@ module BlobViewer private - def server_side_but_stored_in_lfs? - server_side? && blob.valid_lfs_pointer? + def server_side_but_stored_externally? + server_side? && blob.stored_externally? end end end diff --git a/app/models/commit.rb b/app/models/commit.rb index bb4cb8efd15..88a015cdb77 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -236,8 +236,8 @@ class Commit project.pipelines.where(sha: sha) end - def latest_pipeline - pipelines.last + def last_pipeline + @last_pipeline ||= pipelines.last end def status(ref = nil) diff --git a/app/models/concerns/blob_like.rb b/app/models/concerns/blob_like.rb new file mode 100644 index 00000000000..adb81561000 --- /dev/null +++ b/app/models/concerns/blob_like.rb @@ -0,0 +1,48 @@ +module BlobLike + extend ActiveSupport::Concern + include Linguist::BlobHelper + + def id + raise NotImplementedError + end + + def name + raise NotImplementedError + end + + def path + raise NotImplementedError + end + + def size + 0 + end + + def data + nil + end + + def mode + nil + end + + def binary? + false + end + + def load_all_data!(repository) + # No-op + end + + def truncated? + false + end + + def external_storage + nil + end + + def external_size + nil + end +end diff --git a/app/models/concerns/discussion_on_diff.rb b/app/models/concerns/discussion_on_diff.rb index 8ee42875670..a7bdf5587b2 100644 --- a/app/models/concerns/discussion_on_diff.rb +++ b/app/models/concerns/discussion_on_diff.rb @@ -11,6 +11,7 @@ module DiscussionOnDiff :diff_line, :for_line?, :active?, + :created_at_diff?, to: :first_note diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb index 6c27dd5aa5c..6359f7596b1 100644 --- a/app/models/concerns/note_on_diff.rb +++ b/app/models/concerns/note_on_diff.rb @@ -30,6 +30,10 @@ module NoteOnDiff raise NotImplementedError end + def created_at_diff?(diff_refs) + false + end + private def noteable_diff_refs diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index 6a6466b493b..d627fbe327f 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -10,7 +10,6 @@ class DiffDiscussion < Discussion delegate :position, :original_position, - :latest_merge_request_diff, to: :first_note @@ -18,6 +17,25 @@ class DiffDiscussion < Discussion false end + def merge_request_version_params + return unless for_merge_request? + + if active? + {} + else + diff_refs = position.diff_refs + + if diff = noteable.merge_request_diff_for(diff_refs) + { diff_id: diff.id } + elsif diff = noteable.merge_request_diff_for(diff_refs.head_sha) + { + diff_id: diff.id, + start_sha: diff_refs.start_sha + } + end + end + end + def reply_attributes super.merge( original_position: original_position.to_json, diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index abe4518d62a..76c59199afd 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -65,10 +65,11 @@ class DiffNote < Note self.position.diff_refs == diff_refs end - def latest_merge_request_diff - return unless for_merge_request? + def created_at_diff?(diff_refs) + return false unless supported? + return true if for_commit? - self.noteable.merge_request_diff_for(self.position.diff_refs) + self.original_position.diff_refs == diff_refs end private diff --git a/app/models/issue.rb b/app/models/issue.rb index 305fc01f041..78bde6820da 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -143,6 +143,14 @@ class Issue < ActiveRecord::Base branches_with_iid - branches_with_merge_request end + # Returns boolean if a related branch exists for the current issue + # ignores merge requests branchs + def has_related_branch? + project.repository.branch_names.any? do |branch| + /\A#{iid}-(?!\d+-stable)/i =~ branch + end + end + # To allow polymorphism with MergeRequest. def source_project project diff --git a/app/models/legacy_diff_discussion.rb b/app/models/legacy_diff_discussion.rb index e617ce36f56..3c1d34db5fa 100644 --- a/app/models/legacy_diff_discussion.rb +++ b/app/models/legacy_diff_discussion.rb @@ -9,14 +9,14 @@ class LegacyDiffDiscussion < Discussion memoized_values << :active - def legacy_diff_discussion? - true - end - def self.note_class LegacyDiffNote end + def legacy_diff_discussion? + true + end + def active?(*args) return @active if @active.present? @@ -27,6 +27,16 @@ class LegacyDiffDiscussion < Discussion !active? end + def merge_request_version_params + return unless for_merge_request? + + if active? + {} + else + nil + end + end + def reply_attributes super.merge(line_code: line_code) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 365fa4f1e70..12c5481cd6d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -374,12 +374,18 @@ class MergeRequest < ActiveRecord::Base merge_request_diff(true) end - def merge_request_diff_for(diff_refs) - @merge_request_diffs_by_diff_refs ||= Hash.new do |h, diff_refs| - h[diff_refs] = merge_request_diffs.viewable.select_without_diff.find_by_diff_refs(diff_refs) - end - - @merge_request_diffs_by_diff_refs[diff_refs] + def merge_request_diff_for(diff_refs_or_sha) + @merge_request_diffs_by_diff_refs_or_sha ||= Hash.new do |h, diff_refs_or_sha| + diffs = merge_request_diffs.viewable.select_without_diff + h[diff_refs_or_sha] = + if diff_refs_or_sha.is_a?(Gitlab::Diff::DiffRefs) + diffs.find_by_diff_refs(diff_refs_or_sha) + else + diffs.find_by(head_commit_sha: diff_refs_or_sha) + end + end + + @merge_request_diffs_by_diff_refs_or_sha[diff_refs_or_sha] end def reload_diff_if_branch_changed diff --git a/app/models/note.rb b/app/models/note.rb index e720bfba030..b06985b4a6f 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -115,11 +115,19 @@ class Note < ActiveRecord::Base end def grouped_diff_discussions(diff_refs = nil) - diff_notes. - fresh. - discussions. - select { |n| n.active?(diff_refs) }. - group_by(&:line_code) + groups = {} + + diff_notes.fresh.discussions.each do |discussion| + if discussion.active?(diff_refs) + discussions = groups[discussion.line_code] ||= [] + elsif diff_refs && discussion.created_at_diff?(diff_refs) + discussions = groups[discussion.original_line_code] ||= [] + end + + discussions << discussion if discussions + end + + groups end def count_for_collection(ids, type) @@ -141,10 +149,6 @@ class Note < ActiveRecord::Base true end - def latest_merge_request_diff - nil - end - def max_attachment_size current_application_settings.max_attachment_size.megabytes.to_i end diff --git a/app/models/project_services/chat_message/base_message.rb b/app/models/project_services/chat_message/base_message.rb index 7621a5fa2d8..e2ad586aea7 100644 --- a/app/models/project_services/chat_message/base_message.rb +++ b/app/models/project_services/chat_message/base_message.rb @@ -50,5 +50,16 @@ module ChatMessage def link(text, url) "[#{text}](#{url})" end + + def pretty_duration(seconds) + parse_string = + if duration < 1.hour + '%M:%S' + else + '%H:%M:%S' + end + + Time.at(seconds).utc.strftime(parse_string) + end end end diff --git a/app/models/project_services/chat_message/pipeline_message.rb b/app/models/project_services/chat_message/pipeline_message.rb index 4628d9b1a7b..47b68f00cff 100644 --- a/app/models/project_services/chat_message/pipeline_message.rb +++ b/app/models/project_services/chat_message/pipeline_message.rb @@ -15,7 +15,7 @@ module ChatMessage @ref_type = pipeline_attributes[:tag] ? 'tag' : 'branch' @ref = pipeline_attributes[:ref] @status = pipeline_attributes[:status] - @duration = pipeline_attributes[:duration] + @duration = pipeline_attributes[:duration].to_i @pipeline_id = pipeline_attributes[:id] end @@ -37,7 +37,7 @@ module ChatMessage { title: "Pipeline #{pipeline_link} of #{branch_link} #{ref_type} by #{user_name} #{humanized_status}", subtitle: "in #{project_link}", - text: "in #{duration} #{time_measure}", + text: "in #{pretty_duration(duration)}", image: user_avatar || '' } end @@ -45,7 +45,7 @@ module ChatMessage private def message - "#{project_link}: Pipeline #{pipeline_link} of #{branch_link} #{ref_type} by #{user_name} #{humanized_status} in #{duration} #{time_measure}" + "#{project_link}: Pipeline #{pipeline_link} of #{branch_link} #{ref_type} by #{user_name} #{humanized_status} in #{pretty_duration(duration)}" end def humanized_status @@ -84,9 +84,5 @@ module ChatMessage def pipeline_link "[##{pipeline_id}](#{pipeline_url})" end - - def time_measure - 'second'.pluralize(duration) - end end end diff --git a/app/models/repository.rb b/app/models/repository.rb index ba34d570dbd..0c797dd5814 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -789,7 +789,7 @@ class Repository } options.merge!(get_committer_and_author(user, email: author_email, name: author_name)) - Rugged::Commit.create(rugged, options) + create_commit(options) end end # rubocop:enable Metrics/ParameterLists @@ -836,7 +836,7 @@ class Repository tree: merge_index.write_tree(rugged), ) - commit_id = Rugged::Commit.create(rugged, actual_options) + commit_id = create_commit(actual_options) merge_request.update(in_progress_merge_commit_sha: commit_id) commit_id end @@ -859,12 +859,11 @@ class Repository committer = user_to_committer(user) - Rugged::Commit.create(rugged, - message: commit.revert_message(user), - author: committer, - committer: committer, - tree: revert_tree_id, - parents: [start_commit.sha]) + create_commit(message: commit.revert_message(user), + author: committer, + committer: committer, + tree: revert_tree_id, + parents: [start_commit.sha]) end end @@ -883,16 +882,15 @@ class Repository committer = user_to_committer(user) - Rugged::Commit.create(rugged, - message: commit.message, - author: { - email: commit.author_email, - name: commit.author_name, - time: commit.authored_date - }, - committer: committer, - tree: cherry_pick_tree_id, - parents: [start_commit.sha]) + create_commit(message: commit.message, + author: { + email: commit.author_email, + name: commit.author_name, + time: commit.authored_date + }, + committer: committer, + tree: cherry_pick_tree_id, + parents: [start_commit.sha]) end end @@ -900,7 +898,7 @@ class Repository GitOperationService.new(user, self).with_branch(branch_name) do committer = user_to_committer(user) - Rugged::Commit.create(rugged, params.merge(author: committer, committer: committer)) + create_commit(params.merge(author: committer, committer: committer)) end end @@ -1142,6 +1140,12 @@ class Repository Gitlab::Metrics.add_event(event, { path: path_with_namespace }.merge(tags)) end + def create_commit(params = {}) + params[:message].delete!("\r") + + Rugged::Commit.create(rugged, params) + end + def repository_storage_path @project.repository_storage_path end diff --git a/app/models/snippet_blob.rb b/app/models/snippet_blob.rb index d6cab74eb1a..fa5fa151607 100644 --- a/app/models/snippet_blob.rb +++ b/app/models/snippet_blob.rb @@ -1,5 +1,5 @@ class SnippetBlob - include Linguist::BlobHelper + include BlobLike attr_reader :snippet @@ -28,32 +28,4 @@ class SnippetBlob Banzai.render_field(snippet, :content) end - - def mode - nil - end - - def binary? - false - end - - def load_all_data!(repository) - # No-op - end - - def lfs_pointer? - false - end - - def lfs_oid - nil - end - - def lfs_size - nil - end - - def truncated? - false - end end diff --git a/app/policies/personal_snippet_policy.rb b/app/policies/personal_snippet_policy.rb index d3913986cd8..e1e5336da8c 100644 --- a/app/policies/personal_snippet_policy.rb +++ b/app/policies/personal_snippet_policy.rb @@ -3,11 +3,16 @@ class PersonalSnippetPolicy < BasePolicy can! :read_personal_snippet if @subject.public? return unless @user + if @subject.public? + can! :comment_personal_snippet + end + if @subject.author == @user can! :read_personal_snippet can! :update_personal_snippet can! :destroy_personal_snippet can! :admin_personal_snippet + can! :comment_personal_snippet end unless @user.external? @@ -16,6 +21,7 @@ class PersonalSnippetPolicy < BasePolicy if @subject.internal? && !@user.external? can! :read_personal_snippet + can! :comment_personal_snippet end end end diff --git a/app/serializers/merge_request_create_entity.rb b/app/serializers/merge_request_create_entity.rb new file mode 100644 index 00000000000..11234313293 --- /dev/null +++ b/app/serializers/merge_request_create_entity.rb @@ -0,0 +1,7 @@ +class MergeRequestCreateEntity < Grape::Entity + expose :iid + + expose :url do |merge_request| + Gitlab::UrlBuilder.build(merge_request) + end +end diff --git a/app/serializers/merge_request_create_serializer.rb b/app/serializers/merge_request_create_serializer.rb new file mode 100644 index 00000000000..08daf473319 --- /dev/null +++ b/app/serializers/merge_request_create_serializer.rb @@ -0,0 +1,3 @@ +class MergeRequestCreateSerializer < BaseSerializer + entity MergeRequestCreateEntity +end diff --git a/app/services/merge_requests/create_from_issue_service.rb b/app/services/merge_requests/create_from_issue_service.rb new file mode 100644 index 00000000000..738cedbaed7 --- /dev/null +++ b/app/services/merge_requests/create_from_issue_service.rb @@ -0,0 +1,54 @@ +module MergeRequests + class CreateFromIssueService < MergeRequests::CreateService + def execute + return error('Invalid issue iid') unless issue_iid.present? && issue.present? + + result = CreateBranchService.new(project, current_user).execute(branch_name, ref) + return result if result[:status] == :error + + SystemNoteService.new_issue_branch(issue, project, current_user, branch_name) + + new_merge_request = create(merge_request) + + if new_merge_request.valid? + success(new_merge_request) + else + error(new_merge_request.errors) + end + end + + private + + def issue_iid + @isssue_iid ||= params.delete(:issue_iid) + end + + def issue + @issue ||= IssuesFinder.new(current_user, project_id: project.id).find_by(iid: issue_iid) + end + + def branch_name + @branch_name ||= issue.to_branch_name + end + + def ref + project.default_branch || 'master' + end + + def merge_request + MergeRequests::BuildService.new(project, current_user, merge_request_params).execute + end + + def merge_request_params + { + source_project_id: project.id, + source_branch: branch_name, + target_project_id: project.id + } + end + + def success(merge_request) + super().merge(merge_request: merge_request) + end + end +end diff --git a/app/services/projects/upload_service.rb b/app/services/projects/upload_service.rb deleted file mode 100644 index be34d4fa9b8..00000000000 --- a/app/services/projects/upload_service.rb +++ /dev/null @@ -1,22 +0,0 @@ -module Projects - class UploadService < BaseService - def initialize(project, file) - @project, @file = project, file - end - - def execute - return nil unless @file && @file.size <= max_attachment_size - - uploader = FileUploader.new(@project) - uploader.store!(@file) - - uploader.to_h - end - - private - - def max_attachment_size - current_application_settings.max_attachment_size.megabytes.to_i - end - end -end diff --git a/app/services/upload_service.rb b/app/services/upload_service.rb new file mode 100644 index 00000000000..6c5b2baff41 --- /dev/null +++ b/app/services/upload_service.rb @@ -0,0 +1,20 @@ +class UploadService + def initialize(model, file, uploader_class = FileUploader) + @model, @file, @uploader_class = model, file, uploader_class + end + + def execute + return nil unless @file && @file.size <= max_attachment_size + + uploader = @uploader_class.new(@model) + uploader.store!(@file) + + uploader.to_h + end + + private + + def max_attachment_size + current_application_settings.max_attachment_size.megabytes.to_i + end +end diff --git a/app/uploaders/artifact_uploader.rb b/app/uploaders/artifact_uploader.rb index e84944ed411..3e36ec91205 100644 --- a/app/uploaders/artifact_uploader.rb +++ b/app/uploaders/artifact_uploader.rb @@ -30,8 +30,4 @@ class ArtifactUploader < GitlabUploader def filename file.try(:filename) end - - def exists? - file.try(:exists?) - end end diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index d2783ce5b2f..7e94218c23d 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -26,11 +26,11 @@ class FileUploader < GitlabUploader File.join(CarrierWave.root, base_dir, model.path_with_namespace) end - attr_accessor :project + attr_accessor :model attr_reader :secret - def initialize(project, secret = nil) - @project = project + def initialize(model, secret = nil) + @model = model @secret = secret || generate_secret end @@ -38,10 +38,6 @@ class FileUploader < GitlabUploader File.join(dynamic_path_segment, @secret) end - def model - project - end - def relative_path self.file.path.sub("#{dynamic_path_segment}/", '') end diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb index d662ba6820c..e0a6c9b4067 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -33,4 +33,8 @@ class GitlabUploader < CarrierWave::Uploader::Base def relative_path self.file.path.sub("#{root}/", '') end + + def exists? + file.try(:exists?) + end end diff --git a/app/uploaders/lfs_object_uploader.rb b/app/uploaders/lfs_object_uploader.rb index faab539b8e0..95a891111e1 100644 --- a/app/uploaders/lfs_object_uploader.rb +++ b/app/uploaders/lfs_object_uploader.rb @@ -9,10 +9,6 @@ class LfsObjectUploader < GitlabUploader "#{Gitlab.config.lfs.storage_path}/tmp/cache" end - def exists? - file.try(:exists?) - end - def filename model.oid[4..-1] end diff --git a/app/uploaders/personal_file_uploader.rb b/app/uploaders/personal_file_uploader.rb new file mode 100644 index 00000000000..969b0a20d38 --- /dev/null +++ b/app/uploaders/personal_file_uploader.rb @@ -0,0 +1,15 @@ +class PersonalFileUploader < FileUploader + def self.dynamic_path_segment(model) + File.join(CarrierWave.root, model_path(model)) + end + + private + + def secure_url + File.join(self.class.model_path(model), secret, file.filename) + end + + def self.model_path(model) + File.join("/#{base_dir}", model.class.to_s.underscore, model.id.to_s) + end +end diff --git a/app/views/discussions/_diff_with_notes.html.haml b/app/views/discussions/_diff_with_notes.html.haml index 549364761e6..78c5b0c1dda 100644 --- a/app/views/discussions/_diff_with_notes.html.haml +++ b/app/views/discussions/_diff_with_notes.html.haml @@ -3,7 +3,7 @@ .diff-file.file-holder .js-file-title.file-title - = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_file.content_commit, project: discussion.project, url: discussion_diff_path(discussion) + = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_file.content_commit, project: discussion.project, url: discussion_path(discussion) .diff-content.code.js-syntax-highlight %table diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml index 8440fb3d785..38e85168f40 100644 --- a/app/views/discussions/_discussion.html.haml +++ b/app/views/discussions/_discussion.html.haml @@ -20,7 +20,7 @@ = discussion.author.to_reference started a discussion - - url = discussion_diff_path(discussion) + - url = discussion_path(discussion) - if discussion.for_commit? && @noteable != discussion.noteable on - commit = discussion.noteable diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 8ab9747efc5..cdcac7e4264 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -38,7 +38,7 @@ %span.badge.count.merge_counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.count) - if project_nav_tab? :pipelines - = nav_link(controller: [:pipelines, :builds, :environments]) do + = nav_link(controller: [:pipelines, :builds, :environments, :artifacts]) do = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-pipelines' do %span Pipelines diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml index f5e7ea7710d..e9e06e5c8e3 100644 --- a/app/views/layouts/project.html.haml +++ b/app/views/layouts/project.html.haml @@ -11,7 +11,7 @@ - preview_markdown_path = preview_markdown_namespace_project_path(project.namespace, project) - if current_user :javascript - window.project_uploads_path = "#{namespace_project_uploads_path project.namespace,project}"; + window.uploads_path = "#{namespace_project_uploads_path project.namespace,project}"; window.preview_markdown_path = "#{preview_markdown_path}"; - content_for :header_content do diff --git a/app/views/projects/artifacts/_tree_directory.html.haml b/app/views/projects/artifacts/_tree_directory.html.haml index 9e49c93388a..34d5c3b7285 100644 --- a/app/views/projects/artifacts/_tree_directory.html.haml +++ b/app/views/projects/artifacts/_tree_directory.html.haml @@ -3,6 +3,6 @@ %tr.tree-item{ 'data-link' => path_to_directory } %td.tree-item-file-name = tree_icon('folder', '755', directory.name) - %span.str-truncated - = link_to directory.name, path_to_directory + = link_to path_to_directory do + %span.str-truncated= directory.name %td diff --git a/app/views/projects/artifacts/browse.html.haml b/app/views/projects/artifacts/browse.html.haml index de8c173f26f..9fbb30f7c7c 100644 --- a/app/views/projects/artifacts/browse.html.haml +++ b/app/views/projects/artifacts/browse.html.haml @@ -1,13 +1,23 @@ -- page_title 'Artifacts', "#{@build.name} (##{@build.id})", 'Jobs' +- page_title @path.presence, 'Artifacts', "#{@build.name} (##{@build.id})", 'Jobs' += render "projects/pipelines/head" -.top-block.row-content-block.clearfix - .pull-right - = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build), - rel: 'nofollow', download: '', class: 'btn btn-default download' do - = icon('download') - Download artifacts archive += render "projects/builds/header", show_controls: false .tree-holder + .nav-block + .tree-controls + = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build), + rel: 'nofollow', download: '', class: 'btn btn-default download' do + = icon('download') + Download artifacts archive + + %ul.breadcrumb.repo-breadcrumb + %li + = link_to 'Artifacts', browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build) + - path_breadcrumbs do |title, path| + %li + = link_to truncate(title, length: 40), browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build, path) + .tree-content-holder %table.table.tree-table %thead diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml index 3f12d64d044..f04df441ccb 100644 --- a/app/views/projects/blob/_blob.html.haml +++ b/app/views/projects/blob/_blob.html.haml @@ -6,17 +6,14 @@ %li = link_to namespace_project_tree_path(@project.namespace, @project, @ref) do = @project.path - - tree_breadcrumbs(@tree, 6) do |title, path| + - path_breadcrumbs do |title, path| + - title = truncate(title, length: 40) %li - - if path - - if path.end_with?(@path) - = link_to namespace_project_blob_path(@project.namespace, @project, path) do - %strong - = truncate(title, length: 40) - - else - = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, path) + - if path == @path + = link_to namespace_project_blob_path(@project.namespace, @project, tree_join(@ref, path)) do + %strong= title - else - = link_to title, '#' + = link_to title, namespace_project_tree_path(@project.namespace, @project, tree_join(@ref, path)) %ul.blob-commit-info.hidden-xs - blob_commit = @repository.last_commit_for_path(@commit.id, blob.path) @@ -25,5 +22,4 @@ #blob-content-holder.blob-content-holder %article.file-holder = render "projects/blob/header", blob: blob - = render 'projects/blob/content', blob: blob diff --git a/app/views/projects/builds/_header.html.haml b/app/views/projects/builds/_header.html.haml index 104db85809c..a0f8f105d9a 100644 --- a/app/views/projects/builds/_header.html.haml +++ b/app/views/projects/builds/_header.html.haml @@ -1,10 +1,13 @@ +- show_controls = local_assigns.fetch(:show_controls, true) - pipeline = @build.pipeline .content-block.build-header.top-area .header-content = render 'ci/status/badge', status: @build.detailed_status(current_user), link: false, title: @build.status_title - Job - %strong.js-build-id ##{@build.id} + %strong + Job + = link_to namespace_project_build_path(@project.namespace, @project, @build), class: 'js-build-id' do + \##{@build.id} in pipeline = link_to pipeline_path(pipeline) do %strong ##{pipeline.id} @@ -15,13 +18,16 @@ = link_to namespace_project_commits_path(@project.namespace, @project, @build.ref) do %code = @build.ref - - if @build.user - = render "user" + + = render "projects/builds/user" if @build.user + = time_ago_with_tooltip(@build.created_at) - .nav-controls - - if can?(current_user, :create_issue, @project) && @build.failed? - = link_to "New issue", new_namespace_project_issue_path(@project.namespace, @project, issue: build_failed_issue_options), class: 'btn btn-new btn-inverted' - - if can?(current_user, :update_build, @build) && @build.retryable? - = link_to "Retry job", retry_namespace_project_build_path(@project.namespace, @project, @build), class: 'btn btn-inverted-secondary', method: :post - %button.btn.btn-default.pull-right.visible-xs-block.visible-sm-block.build-gutter-toggle.js-sidebar-build-toggle{ role: "button", type: "button" } - = icon('angle-double-left') + + - if show_controls + .nav-controls + - if can?(current_user, :create_issue, @project) && @build.failed? + = link_to "New issue", new_namespace_project_issue_path(@project.namespace, @project, issue: build_failed_issue_options), class: 'btn btn-new btn-inverted' + - if can?(current_user, :update_build, @build) && @build.retryable? + = link_to "Retry job", retry_namespace_project_build_path(@project.namespace, @project, @build), class: 'btn btn-inverted-secondary', method: :post + %button.btn.btn-default.pull-right.visible-xs-block.visible-sm-block.build-gutter-toggle.js-sidebar-build-toggle{ role: "button", type: "button" } + = icon('angle-double-left') diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index f604d6e5fbb..64adb70cb81 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -61,19 +61,20 @@ %span.commit-info.branches %i.fa.fa-spinner.fa-spin - - if @commit.status + - if @commit.last_pipeline + - last_pipeline = @commit.last_pipeline .well-segment.pipeline-info .status-icon-container{ class: "ci-status-icon-#{@commit.status}" } - = link_to namespace_project_pipeline_path(@project.namespace, @project, @commit.latest_pipeline.id) do - = ci_icon_for_status(@commit.status) + = link_to namespace_project_pipeline_path(@project.namespace, @project, last_pipeline.id) do + = ci_icon_for_status(last_pipeline.status) Pipeline - = link_to "##{@commit.latest_pipeline.id}", namespace_project_pipeline_path(@project.namespace, @project, @commit.latest_pipeline.id), class: "monospace" - = ci_label_for_status(@commit.status) - - if @commit.latest_pipeline.stages.any? + = link_to "##{last_pipeline.id}", namespace_project_pipeline_path(@project.namespace, @project, last_pipeline.id), class: "monospace" + = ci_label_for_status(last_pipeline.status) + - if last_pipeline.stages.any? .mr-widget-pipeline-graph - = render 'shared/mini_pipeline_graph', pipeline: @commit.latest_pipeline, klass: 'js-commit-pipeline-graph' + = render 'shared/mini_pipeline_graph', pipeline: last_pipeline, klass: 'js-commit-pipeline-graph' in - = time_interval_in_words @commit.pipelines.total_duration + = time_interval_in_words last_pipeline.duration :javascript $(".commit-info.branches").load("#{branches_namespace_project_commit_path(@project.namespace, @project, @commit.id)}"); diff --git a/app/views/projects/diffs/_line.html.haml b/app/views/projects/diffs/_line.html.haml index 3e426ee9e7d..7439b8a66f7 100644 --- a/app/views/projects/diffs/_line.html.haml +++ b/app/views/projects/diffs/_line.html.haml @@ -35,6 +35,6 @@ - else = diff_line_content(line.text) -- if line_discussions +- if line_discussions&.any? - discussion_expanded = local_assigns.fetch(:discussion_expanded, line_discussions.any?(&:expanded?)) = render "discussions/diff_discussion", discussions: line_discussions, expanded: discussion_expanded diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml index 13e2150f997..6bc6bf76e18 100644 --- a/app/views/projects/issues/_new_branch.html.haml +++ b/app/views/projects/issues/_new_branch.html.haml @@ -1,9 +1,29 @@ - if can?(current_user, :push_code, @project) - .pull-right - #new-branch.new-branch{ 'data-path' => can_create_branch_namespace_project_issue_path(@project.namespace, @project, @issue) } - = link_to namespace_project_branches_path(@project.namespace, @project, branch_name: @issue.to_branch_name, issue_iid: @issue.iid), - method: :post, class: 'btn btn-new btn-inverted btn-grouped has-tooltip available hide', title: @issue.to_branch_name do - New branch - = link_to '#', class: 'unavailable btn btn-grouped hide', disabled: 'disabled' do - = icon('exclamation-triangle') - New branch unavailable + .create-mr-dropdown-wrap{ data: { can_create_path: can_create_branch_namespace_project_issue_path(@project.namespace, @project, @issue), create_mr_path: create_merge_request_namespace_project_issue_path(@project.namespace, @project, @issue), create_branch_path: namespace_project_branches_path(@project.namespace, @project, branch_name: @issue.to_branch_name, issue_iid: @issue.iid) } } + .btn-group.unavailable + %button.btn.btn-grouped{ type: 'button', disabled: 'disabled' } + = icon('spinner', class: 'fa-spin') + %span.text + Checking branch availability… + .btn-group.available.hide + %input.btn.js-create-merge-request.btn-inverted.btn-success{ type: 'button', value: 'Create a merge request', data: { action: 'create-mr' } } + %button.btn.btn-inverted.dropdown-toggle.btn-inverted.btn-success.js-dropdown-toggle{ type: 'button', data: { 'dropdown-trigger' => '#create-merge-request-dropdown' } } + = icon('caret-down') + %ul#create-merge-request-dropdown.dropdown-menu.dropdown-menu-align-right{ data: { dropdown: true } } + %li.droplab-item-selected{ role: 'button', data: { value: 'create-mr', 'text' => 'Create a merge request' } } + .menu-item + .icon-container + = icon('check') + .description + %strong Create a merge request + %span + Creates a branch named after this issue and a merge request. The source branch is '#{@project.default_branch}' by default. + %li.divider.droplab-item-ignore + %li{ role: 'button', data: { value: 'create-branch', 'text' => 'Create a branch' } } + .menu-item + .icon-container + = icon('check') + .description + %strong Create a branch + %span + Creates a branch named after this issue. The source branch is '#{@project.default_branch}' by default. diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 2a871966aa8..1418ad73553 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -70,8 +70,11 @@ // This element is filled in using JavaScript. .content-block.content-block-small - = render 'new_branch' unless @issue.confidential? - = render 'award_emoji/awards_block', awardable: @issue, inline: true + .row + .col-sm-6 + = render 'award_emoji/awards_block', awardable: @issue, inline: true + .col-sm-6.new-branch-col + = render 'new_branch' unless @issue.confidential? %section.issuable-discussion = render 'projects/issues/discussion' diff --git a/app/views/projects/merge_requests/show/_versions.html.haml b/app/views/projects/merge_requests/show/_versions.html.haml index 547be78992e..11b0c55be0b 100644 --- a/app/views/projects/merge_requests/show/_versions.html.haml +++ b/app/views/projects/merge_requests/show/_versions.html.haml @@ -35,7 +35,7 @@ %span.dropdown.inline.mr-version-compare-dropdown %a.btn.btn-default.dropdown-toggle{ data: {toggle: :dropdown} } %span - - if @start_sha + - if @start_version version #{version_index(@start_version)} - else #{@merge_request.target_branch} @@ -59,7 +59,7 @@ %small = time_ago_with_tooltip(merge_request_diff.created_at) %li - = link_to merge_request_version_path(@project, @merge_request, @merge_request_diff), class: ('is-active' unless @start_sha) do + = link_to merge_request_version_path(@project, @merge_request, @merge_request_diff), class: ('is-active' unless @start_version) do %strong #{@merge_request.target_branch} (base) .monospace= short_sha(@merge_request_diff.base_commit_sha) @@ -75,13 +75,15 @@ = succeed '.' do %code= @merge_request.target_branch - - if @diff_notes_disabled + - if @start_version || !@merge_request_diff.latest? .comments-disabled-notif.content-block = icon('info-circle') - - if @start_sha - Comments are disabled because you're comparing two versions of this merge request. + Not all comments are displayed because you're + - if @start_version + comparing two versions - else - Discussions on this version of the merge request are displayed but comment creation is disabled. + viewing an old version + of this merge request. .pull-right = link_to 'Show latest version', diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-sm' diff --git a/app/views/projects/notes/_edit_form.html.haml b/app/views/projects/notes/_edit_form.html.haml index 8b4e5928e0d..a1efc0b051a 100644 --- a/app/views/projects/notes/_edit_form.html.haml +++ b/app/views/projects/notes/_edit_form.html.haml @@ -7,7 +7,7 @@ = render 'projects/notes/hints' .note-form-actions.clearfix - .settings-message.note-edit-warning.js-edit-warning + .settings-message.note-edit-warning.js-finish-edit-warning Finish editing this message first! = submit_tag 'Save comment', class: 'btn btn-nr btn-save js-comment-button' %button.btn.btn-nr.btn-cancel.note-edit-cancel{ type: 'button' } diff --git a/app/views/projects/pages/_disabled.html.haml b/app/views/projects/pages/_disabled.html.haml deleted file mode 100644 index ad51fbc6cab..00000000000 --- a/app/views/projects/pages/_disabled.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -.panel.panel-default - .nothing-here-block - GitLab Pages are disabled. - Ask your system's administrator to enable it. diff --git a/app/views/projects/pages/show.html.haml b/app/views/projects/pages/show.html.haml index 259d5bd63d6..b22a54d75c8 100644 --- a/app/views/projects/pages/show.html.haml +++ b/app/views/projects/pages/show.html.haml @@ -16,13 +16,10 @@ %hr.clearfix -- if Gitlab.config.pages.enabled - = render 'access' - = render 'use' - - if Gitlab.config.pages.external_http || Gitlab.config.pages.external_https - = render 'list' - - else - = render 'no_domains' - = render 'destroy' += render 'access' += render 'use' +- if Gitlab.config.pages.external_http || Gitlab.config.pages.external_https + = render 'list' - else - = render 'disabled' + = render 'no_domains' += render 'destroy' diff --git a/app/views/projects/pipelines/_head.html.haml b/app/views/projects/pipelines/_head.html.haml index bc57f7f1c46..b0dac9de1c6 100644 --- a/app/views/projects/pipelines/_head.html.haml +++ b/app/views/projects/pipelines/_head.html.haml @@ -4,13 +4,13 @@ .nav-links.sub-nav.scrolling-tabs %ul{ class: (container_class) } - if project_nav_tab? :pipelines - = nav_link(path: 'pipelines#index', controller: :pipelines) do + = nav_link(path: ['pipelines#index', 'pipelines#show']) do = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-pipelines' do %span Pipelines - if project_nav_tab? :builds - = nav_link(controller: :builds) do + = nav_link(controller: [:builds, :artifacts]) do = link_to project_builds_path(@project), title: 'Jobs', class: 'shortcuts-builds' do %span Jobs diff --git a/app/views/projects/project_members/_index.html.haml b/app/views/projects/project_members/_index.html.haml index ab0771b5751..f83521052ed 100644 --- a/app/views/projects/project_members/_index.html.haml +++ b/app/views/projects/project_members/_index.html.haml @@ -6,6 +6,12 @@ %p Add a new member to %strong= @project.name + - else + %p + Members can be added by project + %i Masters + or + %i Owners .col-lg-9 .light.prepend-top-default - if can?(current_user, :admin_project_member, @project) diff --git a/app/views/projects/project_members/_team.html.haml b/app/views/projects/project_members/_team.html.haml index 81d57c77edf..7b1a26043e1 100644 --- a/app/views/projects/project_members/_team.html.haml +++ b/app/views/projects/project_members/_team.html.haml @@ -1,9 +1,11 @@ .panel.panel-default - .panel-heading - Members with access to - %strong= @project.name + .panel-heading.flex-project-members-panel + %span.flex-project-title + Members of + %strong + #{@project.name} %span.badge= @project_members.total_count - = form_tag namespace_project_settings_members_path(@project.namespace, @project), method: :get, class: 'form-inline member-search-form' do + = form_tag namespace_project_settings_members_path(@project.namespace, @project), method: :get, class: 'form-inline member-search-form flex-project-members-form' do .form-group = search_field_tag :search, params[:search], { placeholder: 'Find existing members by name', class: 'form-control', spellcheck: false } %button.member-search-btn{ type: "submit", "aria-label" => "Submit search" } diff --git a/app/views/projects/protected_tags/_create_protected_tag.html.haml b/app/views/projects/protected_tags/_create_protected_tag.html.haml index 6e187b54a59..af9a080f0a2 100644 --- a/app/views/projects/protected_tags/_create_protected_tag.html.haml +++ b/app/views/projects/protected_tags/_create_protected_tag.html.haml @@ -9,7 +9,7 @@ .form-group = f.label :name, class: 'col-md-2 text-right' do Tag: - .col-md-10 + .col-md-10.protected-tags-dropdown = render partial: "projects/protected_tags/dropdown", locals: { f: f } .help-block = link_to 'Wildcards', help_page_path('user/project/protected_tags', anchor: 'wildcard-protected-tags') diff --git a/app/views/projects/settings/_head.html.haml b/app/views/projects/settings/_head.html.haml index 5a5ade03624..8c7f9e0191e 100644 --- a/app/views/projects/settings/_head.html.haml +++ b/app/views/projects/settings/_head.html.haml @@ -27,7 +27,8 @@ = link_to namespace_project_settings_ci_cd_path(@project.namespace, @project), title: 'CI/CD Pipelines' do %span CI/CD Pipelines - = nav_link(controller: :pages) do - = link_to namespace_project_pages_path(@project.namespace, @project), title: 'Pages' do - %span - Pages + - if Gitlab.config.pages.enabled + = nav_link(controller: :pages) do + = link_to namespace_project_pages_path(@project.namespace, @project), title: 'Pages' do + %span + Pages diff --git a/app/views/projects/tags/index.html.haml b/app/views/projects/tags/index.html.haml index 7f9a44e565f..56656ea3d86 100644 --- a/app/views/projects/tags/index.html.haml +++ b/app/views/projects/tags/index.html.haml @@ -1,4 +1,5 @@ - @no_container = true +- @sort ||= sort_value_recently_updated - page_title "Tags" = render "projects/commits/head" @@ -14,16 +15,14 @@ .dropdown %button.dropdown-toggle{ type: 'button', data: { toggle: 'dropdown'} } %span.light - = projects_sort_options_hash[@sort] + = tags_sort_options_hash[@sort] = icon('chevron-down') - %ul.dropdown-menu.dropdown-menu-align-right - %li - = link_to filter_tags_path(sort: sort_value_name) do - = sort_title_name - = link_to filter_tags_path(sort: sort_value_recently_updated) do - = sort_title_recently_updated - = link_to filter_tags_path(sort: sort_value_oldest_updated) do - = sort_title_oldest_updated + %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-selectable + %li.dropdown-header + Sort by + - tags_sort_options_hash.each do |value, title| + %li + = link_to title, filter_tags_path(sort: value), class: ("is-active" if @sort == value) - if can?(current_user, :push_code, @project) = link_to new_namespace_project_tag_path(@project.namespace, @project), class: 'btn btn-create new-tag-btn' do New tag diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml index e7b3fe3ffda..396d1ecd77b 100644 --- a/app/views/projects/tree/_tree_header.html.haml +++ b/app/views/projects/tree/_tree_header.html.haml @@ -9,12 +9,9 @@ %li = link_to namespace_project_tree_path(@project.namespace, @project, @ref) do = @project.path - - tree_breadcrumbs(tree, 6) do |title, path| + - path_breadcrumbs do |title, path| %li - - if path - = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, path) - - else - = link_to title, '#' + = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, tree_join(@ref, path)) - if current_user %li diff --git a/app/views/shared/_mini_pipeline_graph.html.haml b/app/views/shared/_mini_pipeline_graph.html.haml index b0778653d4e..07970ad9cba 100644 --- a/app/views/shared/_mini_pipeline_graph.html.haml +++ b/app/views/shared/_mini_pipeline_graph.html.haml @@ -11,8 +11,8 @@ = icon('caret-down') %ul.dropdown-menu.mini-pipeline-graph-dropdown-menu.js-builds-dropdown-container - .arrow-up - .js-builds-dropdown-list.scrollable-menu + %li.js-builds-dropdown-list.scrollable-menu - .js-builds-dropdown-loading.builds-dropdown-loading.hidden - %span.fa.fa-spinner.fa-spin + %li.js-builds-dropdown-loading.hidden + .text-center + %i.fa.fa-spinner.fa-spin{ 'aria-hidden': 'true', 'aria-label': 'Loading' } diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index 731270d4127..9657b4eea82 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -2,7 +2,11 @@ - return if note.cross_reference_not_visible_for?(current_user) - note_editable = note_editable?(note) -%li.timeline-entry{ id: dom_id(note), class: ["note", "note-row-#{note.id}", ('system-note' if note.system)], data: {author_id: note.author.id, editable: note_editable, note_id: note.id} } +%li.timeline-entry{ id: dom_id(note), + class: ["note", "note-row-#{note.id}", ('system-note' if note.system)], + data: { author_id: note.author.id, + editable: note_editable, + note_id: note.id } } .timeline-entry-inner .timeline-icon - if note.system diff --git a/changelogs/unreleased/12910-uploader-pers-snippet.yml b/changelogs/unreleased/12910-uploader-pers-snippet.yml new file mode 100644 index 00000000000..1c163632fc6 --- /dev/null +++ b/changelogs/unreleased/12910-uploader-pers-snippet.yml @@ -0,0 +1,4 @@ +--- +title: Support uploaders for personal snippets comments +merge_request: +author: diff --git a/changelogs/unreleased/2247-emails-forwarded-to-service-desk-email-don-t-come.yml b/changelogs/unreleased/2247-emails-forwarded-to-service-desk-email-don-t-come.yml new file mode 100644 index 00000000000..f062143960e --- /dev/null +++ b/changelogs/unreleased/2247-emails-forwarded-to-service-desk-email-don-t-come.yml @@ -0,0 +1,4 @@ +--- +title: Handle incoming emails from aliases correctly +merge_request: +author: diff --git a/changelogs/unreleased/26883-members-page-layout-looks-broken.yml b/changelogs/unreleased/26883-members-page-layout-looks-broken.yml new file mode 100644 index 00000000000..e0e3a529c3e --- /dev/null +++ b/changelogs/unreleased/26883-members-page-layout-looks-broken.yml @@ -0,0 +1,4 @@ +--- +title: Improved UX on project members settings view +merge_request: +author: diff --git a/changelogs/unreleased/28408-feature-proposal-include-search-options-to-pipelines-api.yml b/changelogs/unreleased/28408-feature-proposal-include-search-options-to-pipelines-api.yml new file mode 100644 index 00000000000..9b9f0032810 --- /dev/null +++ b/changelogs/unreleased/28408-feature-proposal-include-search-options-to-pipelines-api.yml @@ -0,0 +1,4 @@ +--- +title: 'API: Add parameters to allow filtering project pipelines' +merge_request: 9367 +author: dosuken123 diff --git a/changelogs/unreleased/28558-create-new-branch-from-issue-page.yml b/changelogs/unreleased/28558-create-new-branch-from-issue-page.yml new file mode 100644 index 00000000000..e43b043d6c5 --- /dev/null +++ b/changelogs/unreleased/28558-create-new-branch-from-issue-page.yml @@ -0,0 +1,4 @@ +--- +title: Allow to create new branch and empty WIP merge request from issue page +merge_request: +author: diff --git a/changelogs/unreleased/30458-real-time-note-edits.yml b/changelogs/unreleased/30458-real-time-note-edits.yml new file mode 100644 index 00000000000..f67348c5302 --- /dev/null +++ b/changelogs/unreleased/30458-real-time-note-edits.yml @@ -0,0 +1,4 @@ +--- +title: Update note edits in real-time +merge_request: +author: diff --git a/changelogs/unreleased/30529-remove-pages-tab-if-pages-isn-t-enabled.yml b/changelogs/unreleased/30529-remove-pages-tab-if-pages-isn-t-enabled.yml new file mode 100644 index 00000000000..16938f05326 --- /dev/null +++ b/changelogs/unreleased/30529-remove-pages-tab-if-pages-isn-t-enabled.yml @@ -0,0 +1,4 @@ +--- +title: Disable navigation to Project-level pages configuration when Pages disabled +merge_request: 11008 +author: diff --git a/changelogs/unreleased/30973-network-graph-sorted-by-date-and-topo.yml b/changelogs/unreleased/30973-network-graph-sorted-by-date-and-topo.yml new file mode 100644 index 00000000000..42426c1865e --- /dev/null +++ b/changelogs/unreleased/30973-network-graph-sorted-by-date-and-topo.yml @@ -0,0 +1,4 @@ +--- +title: Sort the network graph both by commit date and topographically +merge_request: 11057 +author: diff --git a/changelogs/unreleased/31156-environments-vue-service.yml b/changelogs/unreleased/31156-environments-vue-service.yml new file mode 100644 index 00000000000..8b899ed9861 --- /dev/null +++ b/changelogs/unreleased/31156-environments-vue-service.yml @@ -0,0 +1,4 @@ +--- +title: Fix environments vue architecture to match documentation +merge_request: +author: diff --git a/changelogs/unreleased/31544-size-of-project-from-api.yml b/changelogs/unreleased/31544-size-of-project-from-api.yml new file mode 100644 index 00000000000..a707d49aecd --- /dev/null +++ b/changelogs/unreleased/31544-size-of-project-from-api.yml @@ -0,0 +1,4 @@ +--- +title: Expose project statistics on single requests via the API +merge_request: +author: diff --git a/changelogs/unreleased/31558-job-dropdown.yml b/changelogs/unreleased/31558-job-dropdown.yml new file mode 100644 index 00000000000..acd7b2addb6 --- /dev/null +++ b/changelogs/unreleased/31558-job-dropdown.yml @@ -0,0 +1,4 @@ +--- +title: Job dropdown of pipeline mini graph updates in realtime when its opened +merge_request: +author: diff --git a/changelogs/unreleased/31647-fix-snippet-content_html.yml b/changelogs/unreleased/31647-fix-snippet-content_html.yml new file mode 100644 index 00000000000..db6d45926fd --- /dev/null +++ b/changelogs/unreleased/31647-fix-snippet-content_html.yml @@ -0,0 +1,4 @@ +--- +title: Fix caching large snippet HTML content on MySQL databases +merge_request: 11024 +author: diff --git a/changelogs/unreleased/31671-merge-request-message-contains-carriage-returns.yml b/changelogs/unreleased/31671-merge-request-message-contains-carriage-returns.yml new file mode 100644 index 00000000000..c33fa944a83 --- /dev/null +++ b/changelogs/unreleased/31671-merge-request-message-contains-carriage-returns.yml @@ -0,0 +1,4 @@ +--- +title: Remove carriage returns from commit messages +merge_request: 11077 +author: diff --git a/changelogs/unreleased/always-show-latest-pipeline-in-commit-box.yml b/changelogs/unreleased/always-show-latest-pipeline-in-commit-box.yml new file mode 100644 index 00000000000..6aa0c89f6f7 --- /dev/null +++ b/changelogs/unreleased/always-show-latest-pipeline-in-commit-box.yml @@ -0,0 +1,4 @@ +--- +title: Always show the latest pipeline information in the commit box +merge_request: 11038 +author: diff --git a/changelogs/unreleased/dm-artifact-browser-header.yml b/changelogs/unreleased/dm-artifact-browser-header.yml new file mode 100644 index 00000000000..b88ab2ac7e5 --- /dev/null +++ b/changelogs/unreleased/dm-artifact-browser-header.yml @@ -0,0 +1,4 @@ +--- +title: Add breadcrumb, build header and pipelines submenu to artifacts browser +merge_request: +author: diff --git a/changelogs/unreleased/dm-comment-on-diff-versions.yml b/changelogs/unreleased/dm-comment-on-diff-versions.yml new file mode 100644 index 00000000000..af299713ad3 --- /dev/null +++ b/changelogs/unreleased/dm-comment-on-diff-versions.yml @@ -0,0 +1,4 @@ +--- +title: Allow commenting on older versions of the diff and comparisons between diff versions +merge_request: +author: diff --git a/changelogs/unreleased/tags-sort-default.yml b/changelogs/unreleased/tags-sort-default.yml new file mode 100644 index 00000000000..265b765d540 --- /dev/null +++ b/changelogs/unreleased/tags-sort-default.yml @@ -0,0 +1,4 @@ +--- +title: Fixed tags sort from defaulting to empty +merge_request: +author: diff --git a/changelogs/unreleased/zj-chat-message-pretty-time.yml b/changelogs/unreleased/zj-chat-message-pretty-time.yml new file mode 100644 index 00000000000..68bc647bab2 --- /dev/null +++ b/changelogs/unreleased/zj-chat-message-pretty-time.yml @@ -0,0 +1,4 @@ +--- +title: Pipeline chat notifications convert seconds to minutes and hours +merge_request: +author: diff --git a/config/routes/project.rb b/config/routes/project.rb index a15e365cc2f..956afe4faa3 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -234,6 +234,7 @@ constraints(ProjectUrlConstrainer.new) do get :related_branches get :can_create_branch get :rendered_title + post :create_merge_request end collection do post :bulk_update diff --git a/config/routes/uploads.rb b/config/routes/uploads.rb index 2b22148a134..b315186b178 100644 --- a/config/routes/uploads.rb +++ b/config/routes/uploads.rb @@ -4,6 +4,11 @@ scope path: :uploads do to: "uploads#show", constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ } + # show uploads for models, snippets (notes) available for now + get ':model/:id/:secret/:filename', + to: 'uploads#show', + constraints: { model: /personal_snippet/, id: /\d+/, filename: /[^\/]+/ } + # Appearance get ":model/:mounted_as/:id/:filename", to: "uploads#show", @@ -13,6 +18,12 @@ scope path: :uploads do get ":namespace_id/:project_id/:secret/:filename", to: "projects/uploads#show", constraints: { namespace_id: /[a-zA-Z.0-9_\-]+/, project_id: /[a-zA-Z.0-9_\-]+/, filename: /[^\/]+/ } + + # create uploads for models, snippets (notes) available for now + post ':model/:id/', + to: 'uploads#create', + constraints: { model: /personal_snippet/, id: /\d+/ }, + as: 'upload' end # Redirect old note attachments path to new uploads path. diff --git a/db/migrate/20170502091007_markdown_cache_limits_to_mysql.rb b/db/migrate/20170502091007_markdown_cache_limits_to_mysql.rb new file mode 100644 index 00000000000..008a94d8334 --- /dev/null +++ b/db/migrate/20170502091007_markdown_cache_limits_to_mysql.rb @@ -0,0 +1,2 @@ +# rubocop:disable all +require_relative 'markdown_cache_limits_to_mysql' diff --git a/db/migrate/markdown_cache_limits_to_mysql.rb b/db/migrate/markdown_cache_limits_to_mysql.rb new file mode 100644 index 00000000000..f6686db3dc0 --- /dev/null +++ b/db/migrate/markdown_cache_limits_to_mysql.rb @@ -0,0 +1,13 @@ +class MarkdownCacheLimitsToMysql < ActiveRecord::Migration + DOWNTIME = false + + def up + return unless Gitlab::Database.mysql? + + change_column :snippets, :content_html, :text, limit: 2147483647 + end + + def down + # no-op + end +end diff --git a/db/schema.rb b/db/schema.rb index be6684f3a6b..01c0f00c924 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170426181740) do +ActiveRecord::Schema.define(version: 20170502091007) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/doc/api/pipelines.md b/doc/api/pipelines.md index 732ad8da4ac..890945cfc7e 100644 --- a/doc/api/pipelines.md +++ b/doc/api/pipelines.md @@ -11,6 +11,14 @@ GET /projects/:id/pipelines | Attribute | Type | Required | Description | |-----------|---------|----------|---------------------| | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `scope` | string | no | The scope of pipelines, one of: `running`, `pending`, `finished`, `branches`, `tags` | +| `status` | string | no | The status of pipelines, one of: `running`, `pending`, `success`, `failed`, `canceled`, `skipped` | +| `ref` | string | no | The ref of pipelines | +| `yaml_errors`| boolean | no | Returns pipelines with invalid configurations | +| `name`| string | no | The name of the user who triggered pipelines | +| `username`| string | no | The username of the user who triggered pipelines | +| `order_by`| string | no | Order pipelines by `id`, `status`, `ref`, or `user_id` (default: `id`) | +| `sort` | string | no | Sort pipelines in `asc` or `desc` order (default: `desc`) | ``` curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/pipelines" diff --git a/doc/api/projects.md b/doc/api/projects.md index 51de4fef7ff..188fbe7447d 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -40,6 +40,7 @@ Parameters: | `owned` | boolean | no | Limit by projects owned by the current user | | `membership` | boolean | no | Limit by projects that the current user is a member of | | `starred` | boolean | no | Limit by projects starred by the current user | +| `statistics` | boolean | no | Include project statistics | ```json [ @@ -91,7 +92,14 @@ Parameters: "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, - "request_access_enabled": false + "request_access_enabled": false, + "statistics": { + "commit_count": 37, + "storage_size": 1038090, + "repository_size": 1038090, + "lfs_objects_size": 0, + "job_artifacts_size": 0 + } }, { "id": 6, @@ -151,7 +159,14 @@ Parameters: "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, - "request_access_enabled": false + "request_access_enabled": false, + "statistics": { + "commit_count": 12, + "storage_size": 2066080, + "repository_size": 2066080, + "lfs_objects_size": 0, + "job_artifacts_size": 0 + } } ] ``` @@ -170,6 +185,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `statistics` | boolean | no | Include project statistics | ```json { @@ -241,7 +257,14 @@ Parameters: ], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, - "request_access_enabled": false + "request_access_enabled": false, + "statistics": { + "commit_count": 37, + "storage_size": 1038090, + "repository_size": 1038090, + "lfs_objects_size": 0, + "job_artifacts_size": 0 + } } ``` diff --git a/doc/articles/how_to_configure_ldap_gitlab_ce/img/gitlab_ou.png b/doc/articles/how_to_configure_ldap_gitlab_ce/img/gitlab_ou.png Binary files differnew file mode 100644 index 00000000000..11ce324f938 --- /dev/null +++ b/doc/articles/how_to_configure_ldap_gitlab_ce/img/gitlab_ou.png diff --git a/doc/articles/how_to_configure_ldap_gitlab_ce/img/ldap_ou.gif b/doc/articles/how_to_configure_ldap_gitlab_ce/img/ldap_ou.gif Binary files differnew file mode 100644 index 00000000000..a6727a3d85f --- /dev/null +++ b/doc/articles/how_to_configure_ldap_gitlab_ce/img/ldap_ou.gif diff --git a/doc/articles/how_to_configure_ldap_gitlab_ce/img/user_auth.gif b/doc/articles/how_to_configure_ldap_gitlab_ce/img/user_auth.gif Binary files differnew file mode 100644 index 00000000000..36e6085259f --- /dev/null +++ b/doc/articles/how_to_configure_ldap_gitlab_ce/img/user_auth.gif diff --git a/doc/articles/how_to_configure_ldap_gitlab_ce/index.md b/doc/articles/how_to_configure_ldap_gitlab_ce/index.md new file mode 100644 index 00000000000..1702c2184f2 --- /dev/null +++ b/doc/articles/how_to_configure_ldap_gitlab_ce/index.md @@ -0,0 +1,266 @@ +# How to configure LDAP with GitLab CE + +> **Type:** admin guide || +> **Level:** intermediary || +> **Author:** [Chris Wilson](https://gitlab.com/MrChrisW) || +> **Publication date:** 2017/05/03 + +## Introduction + +Managing a large number of users in GitLab can become a burden for system administrators. As an organization grows so do user accounts. Keeping these user accounts in sync across multiple enterprise applications often becomes a time consuming task. + +In this guide we will focus on configuring GitLab with Active Directory. [Active Directory](https://en.wikipedia.org/wiki/Active_Directory) is a popular LDAP compatible directory service provided by Microsoft, included in all modern Windows Server operating systems. + +GitLab has supported LDAP integration since [version 2.2](https://about.gitlab.com/2012/02/22/gitlab-version-2-2/). With GitLab LDAP [group syncing](#group-syncing-ee) being added to GitLab Enterprise Edition in [version 6.0](https://about.gitlab.com/2013/08/20/gitlab-6-dot-0-released/). LDAP integration has become one of the most popular features in GitLab. + +## Getting started + +### Choosing an LDAP Server + +The main reason organizations choose to utilize a LDAP server is to keep the entire organization's user base consolidated into a central repository. Users can access multiple applications and systems across the IT environment using a single login. Because LDAP is an open, vendor-neutral, industry standard application protocol, the number of applications using LDAP authentication continues to increase. + +There are many commercial and open source [directory servers](https://en.wikipedia.org/wiki/Directory_service#LDAP_implementations) that support the LDAP protocol. Deciding on the right directory server highly depends on the existing IT environment in which the server will be integrated with. + +For example, [Active Directory](https://technet.microsoft.com/en-us/library/hh831484(v=ws.11).aspx) is generally favored in a primarily Windows environment, as this allows quick integration with existing services. Other popular directory services include: + +- [Oracle Internet Directory](http://www.oracle.com/technetwork/middleware/id-mgmt/overview/index-082035.html) +- [OpenLDAP](http://www.openldap.org/) +- [389 Directory](http://directory.fedoraproject.org/) +- [OpenDJ](https://forgerock.org/opendj/) +- [ApacheDS](https://directory.apache.org/) + +> GitLab uses the [Net::LDAP](https://rubygems.org/gems/net-ldap) library under the hood. This means it supports all [IETF](https://tools.ietf.org/html/rfc2251) compliant LDAPv3 servers. + +### Active Directory (AD) + +We won't cover the installation and configuration of Windows Server or Active Directory Domain Services in this tutorial. There are a number of resources online to guide you through this process: + +- Install Windows Server 2012 - (_technet.microsoft.com_) - [Installing Windows Server 2012 ](https://technet.microsoft.com/en-us/library/jj134246(v=ws.11).aspx) + +- Install Active Directory Domain Services (AD DS) (_technet.microsoft.com_)- [Install Active Directory Domain Services](https://technet.microsoft.com/windows-server-docs/identity/ad-ds/deploy/install-active-directory-domain-services--level-100-#BKMK_PS) + +> **Shortcut:** You can quickly install AD DS via PowerShell using +`Install-WindowsFeature AD-Domain-Services -IncludeManagementTools` + +### Creating an AD **OU** structure + +Configuring organizational units (**OU**s) is an important part of setting up Active Directory. **OU**s form the base for an entire organizational structure. Using GitLab as an example we have designed the **OU** structure below using the geographic **OU** model. In the Geographic Model we separate **OU**s for different geographic regions. + +| GitLab **OU** Design | GitLab AD Structure | +| :----------------------------: | :------------------------------: | +| ![GitLab OU Design][gitlab_ou] | ![GitLab AD Structure][ldap_ou] | + +[gitlab_ou]: img/gitlab_ou.png +[ldap_ou]: img/ldap_ou.gif + +Using PowerShell you can output the **OU** structure as a table (_all names are examples only_): + +```ps +Get-ADObject -LDAPFilter "(objectClass=*)" -SearchBase 'OU=GitLab INT,DC=GitLab,DC=org' -Properties CanonicalName | Format-Table Name,CanonicalName -A +``` + +``` +OU CanonicalName +---- ------------- +GitLab INT GitLab.org/GitLab INT +United States GitLab.org/GitLab INT/United States +Developers GitLab.org/GitLab INT/United States/Developers +Gary Johnson GitLab.org/GitLab INT/United States/Developers/Gary Johnson +Ellis Matthews GitLab.org/GitLab INT/United States/Developers/Ellis Matthews +William Collins GitLab.org/GitLab INT/United States/Developers/William Collins +People Ops GitLab.org/GitLab INT/United States/People Ops +Margaret Baker GitLab.org/GitLab INT/United States/People Ops/Margaret Baker +Libby Hartzler GitLab.org/GitLab INT/United States/People Ops/Libby Hartzler +Victoria Ryles GitLab.org/GitLab INT/United States/People Ops/Victoria Ryles +The Netherlands GitLab.org/GitLab INT/The Netherlands +Developers GitLab.org/GitLab INT/The Netherlands/Developers +John Doe GitLab.org/GitLab INT/The Netherlands/Developers/John Doe +Jon Mealy GitLab.org/GitLab INT/The Netherlands/Developers/Jon Mealy +Jane Weingarten GitLab.org/GitLab INT/The Netherlands/Developers/Jane Weingarten +Production GitLab.org/GitLab INT/The Netherlands/Production +Sarah Konopka GitLab.org/GitLab INT/The Netherlands/Production/Sarah Konopka +Cynthia Bruno GitLab.org/GitLab INT/The Netherlands/Production/Cynthia Bruno +David George GitLab.org/GitLab INT/The Netherlands/Production/David George +United Kingdom GitLab.org/GitLab INT/United Kingdom +Developers GitLab.org/GitLab INT/United Kingdom/Developers +Leroy Fox GitLab.org/GitLab INT/United Kingdom/Developers/Leroy Fox +Christopher Alley GitLab.org/GitLab INT/United Kingdom/Developers/Christopher Alley +Norris Morita GitLab.org/GitLab INT/United Kingdom/Developers/Norris Morita +Support GitLab.org/GitLab INT/United Kingdom/Support +Laura Stanley GitLab.org/GitLab INT/United Kingdom/Support/Laura Stanley +Nikki Schuman GitLab.org/GitLab INT/United Kingdom/Support/Nikki Schuman +Harriet Butcher GitLab.org/GitLab INT/United Kingdom/Support/Harriet Butcher +Global Groups GitLab.org/GitLab INT/Global Groups +DevelopersNL GitLab.org/GitLab INT/Global Groups/DevelopersNL +DevelopersUK GitLab.org/GitLab INT/Global Groups/DevelopersUK +DevelopersUS GitLab.org/GitLab INT/Global Groups/DevelopersUS +ProductionNL GitLab.org/GitLab INT/Global Groups/ProductionNL +SupportUK GitLab.org/GitLab INT/Global Groups/SupportUK +People Ops US GitLab.org/GitLab INT/Global Groups/People Ops US +Global Admins GitLab.org/GitLab INT/Global Groups/Global Admins +``` + +> See [more information](https://technet.microsoft.com/en-us/library/ff730967.aspx) on searching Active Directory with Windows PowerShell from [The Scripting Guys](https://technet.microsoft.com/en-us/scriptcenter/dd901334.aspx) + +## GitLab LDAP configuration + +The initial configuration of LDAP in GitLab requires changes to the `gitlab.rb` configuration file. Below is an example of a complete configuration using an Active Directory. + +The two Active Directory specific values are `active_directory: true` and `uid: 'sAMAccountName'`. `sAMAccountName` is an attribute returned by Active Directory used for GitLab usernames. See the example output from `ldapsearch` for a full list of attributes a "person" object (user) has in **AD** - [`ldapsearch` example](#using-ldapsearch-unix) + +> Both group_base and admin_group configuration options are only available in GitLab Enterprise Edition. See [GitLab EE - LDAP Features](#gitlab-enterprise-edition---ldap-features) + +### Example `gitlab.rb` LDAP + +``` +gitlab_rails['ldap_enabled'] = true +gitlab_rails['ldap_servers'] = { +'main' => { + 'label' => 'GitLab AD', + 'host' => 'ad.example.org', + 'port' => 636, + 'uid' => 'sAMAccountName', + 'method' => 'ssl', + 'bind_dn' => 'CN=GitLabSRV,CN=Users,DC=GitLab,DC=org', + 'password' => 'Password1', + 'active_directory' => true, + 'base' => 'OU=GitLab INT,DC=GitLab,DC=org', + 'group_base' => 'OU=Global Groups,OU=GitLab INT,DC=GitLab,DC=org', + 'admin_group' => 'Global Admins' + } +} +``` + +> **Note:** Remember to run `gitlab-ctl reconfigure` after modifying `gitlab.rb` + +## Security improvements (LDAPS) + +Security is an important aspect when deploying an LDAP server. By default, LDAP traffic is transmitted unsecured. LDAP can be secured using SSL/TLS called LDAPS, or commonly "LDAP over SSL". + +Securing LDAP (enabling LDAPS) on Windows Server 2012 involves installing a valid SSL certificate. For full details see Microsoft's guide [How to enable LDAP over SSL with a third-party certification authority](https://support.microsoft.com/en-us/help/321051/how-to-enable-ldap-over-ssl-with-a-third-party-certification-authority) + +> By default a LDAP service listens for connections on TCP and UDP port 389. LDAPS (LDAP over SSL) listens on port 636 + +### Testing you AD server + +#### Using **AdFind** (Windows) + +You can use the [`AdFind`](https://social.technet.microsoft.com/wiki/contents/articles/7535.adfind-command-examples.aspx) utility (on Windows based systems) to test that your LDAP server is accessible and authentication is working correctly. This is a freeware utility built by [Joe Richards](http://www.joeware.net/freetools/tools/adfind/index.htm). + +**Return all objects** + +You can use the filter `objectclass=*` to return all directory objects. + +```sh +adfind -h ad.example.org:636 -ssl -u "CN=GitLabSRV,CN=Users,DC=GitLab,DC=org" -up Password1 -b "OU=GitLab INT,DC=GitLab,DC=org" -f (objectClass=*) +``` + +**Return single object using filter** + +You can also retrieve a single object by **specifying** the object name or full **DN**. In this example we specify the object name only `CN=Leroy Fox`. + +```sh +adfind -h ad.example.org:636 -ssl -u "CN=GitLabSRV,CN=Users,DC=GitLab,DC=org" -up Password1 -b "OU=GitLab INT,DC=GitLab,DC=org" -f (&(objectcategory=person)(CN=Leroy Fox))” +``` + +#### Using **ldapsearch** (Unix) + +You can use the `ldapsearch` utility (on Unix based systems) to test that your LDAP server is accessible and authentication is working correctly. This utility is included in the [`ldap-utils`](https://wiki.debian.org/LDAP/LDAPUtils) package. + +**Return all objects** + +You can use the filter `objectclass=*` to return all directory objects. + +```sh +ldapsearch -D "CN=GitLabSRV,CN=Users,DC=GitLab,DC=org" \ +-w Password1 -p 636 -h ad.example.org \ +-b "OU=GitLab INT,DC=GitLab,DC=org" -Z \ +-s sub "(objectclass=*)" +``` + +**Return single object using filter** + +You can also retrieve a single object by **specifying** the object name or full **DN**. In this example we specify the object name only `CN=Leroy Fox`. + +```sh +ldapsearch -D "CN=GitLabSRV,CN=Users,DC=GitLab,DC=org" -w Password1 -p 389 -h ad.example.org -b "OU=GitLab INT,DC=GitLab,DC=org" -Z -s sub "CN=Leroy Fox" +``` + +**Full output of `ldapsearch` command:** - Filtering for _CN=Leroy Fox_ + +``` +# LDAPv3 +# base <OU=GitLab INT,DC=GitLab,DC=org> with scope subtree +# filter: CN=Leroy Fox +# requesting: ALL +# + +# Leroy Fox, Developers, United Kingdom, GitLab INT, GitLab.org +dn: CN=Leroy Fox,OU=Developers,OU=United Kingdom,OU=GitLab INT,DC=GitLab,DC=or + g +objectClass: top +objectClass: person +objectClass: organizationalPerson +objectClass: user +cn: Leroy Fox +sn: Fox +givenName: Leroy +distinguishedName: CN=Leroy Fox,OU=Developers,OU=United Kingdom,OU=GitLab INT, + DC=GitLab,DC=org +instanceType: 4 +whenCreated: 20170210030500.0Z +whenChanged: 20170213050128.0Z +displayName: Leroy Fox +uSNCreated: 16790 +memberOf: CN=DevelopersUK,OU=Global Groups,OU=GitLab INT,DC=GitLab,DC=org +uSNChanged: 20812 +name: Leroy Fox +objectGUID:: rBCAo6NR6E6vfSKgzcUILg== +userAccountControl: 512 +badPwdCount: 0 +codePage: 0 +countryCode: 0 +badPasswordTime: 0 +lastLogoff: 0 +lastLogon: 0 +pwdLastSet: 131311695009850084 +primaryGroupID: 513 +objectSid:: AQUAAAAAAAUVAAAA9GMAb7tdJZvsATf7ZwQAAA== +accountExpires: 9223372036854775807 +logonCount: 0 +sAMAccountName: Leroyf +sAMAccountType: 805306368 +userPrincipalName: Leroyf@GitLab.org +objectCategory: CN=Person,CN=Schema,CN=Configuration,DC=GitLab,DC=org +dSCorePropagationData: 16010101000000.0Z +lastLogonTimestamp: 131314356887754250 + +# search result +search: 2 +result: 0 Success + +# numResponses: 2 +# numEntries: 1 +``` + +## Basic user authentication + +After configuring LDAP, basic authentication will be available. Users can then login using their directory credentials. An extra tab is added to the GitLab login screen for the configured LDAP server (e.g "**GitLab AD**"). + + + +Users that are removed from the LDAP base group (e.g `OU=GitLab INT,DC=GitLab,DC=org`) will be **blocked** in GitLab. [More information](../../administration/auth/ldap.md#security) on LDAP security. + +If `allow_username_or_email_login` is enabled in the LDAP configuration, GitLab will ignore everything after the first '@' in the LDAP username used on login. Example: The username `jon.doe@example.com` is converted to `jon.doe` when authenticating with the LDAP server. Disable this setting if you use `userPrincipalName` as the `uid`. + +## LDAP extended features on GitLab EE + +With [GitLab Enterprise Edition (EE)](https://about.gitlab.com/giltab-ee/), besides everything we just described, you'll +have extended functionalities with LDAP, such as: + +- Group sync +- Group permissions +- Updating user permissions +- Multiple LDAP servers + +Read through the article on [LDAP for GitLab EE](https://docs.gitlab.com/ee/articles/how_to_configure_ldap_gitlab_ee/) for an overview. diff --git a/doc/articles/index.md b/doc/articles/index.md index 67eab36bf2c..49db64134f5 100644 --- a/doc/articles/index.md +++ b/doc/articles/index.md @@ -7,6 +7,11 @@ to provide the community with guidance on specific processes to achieve certain They are written by members of the GitLab Team and by [Community Writers](https://about.gitlab.com/handbook/product/technical-writing/community-writers/). +## Authentication + +- **LDAP** + - [How to configure LDAP with GitLab CE](how_to_configure_ldap_gitlab_ce/index.md) + ## GitLab Pages - **GitLab Pages from A to Z** diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 819578404b6..be3dd1e2cc6 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -1,5 +1,25 @@ # Code Review Guidelines +## Getting your merge request reviewed, approved, and merged + +There are a few rules to get your merge request accepted: + +1. Your merge request should only be **merged by a [maintainer][team]**. + 1. If your merge request includes only backend changes [^1], it must be + **approved by a [backend maintainer][team]**. + 1. If your merge request includes only frontend changes [^1], it must be + **approved by a [frontend maintainer][team]**. + 1. If your merge request includes frontend and backend changes [^1], it must + be **approved by a [frontend and a backend maintainer][team]**. +1. To lower the amount of merge requests maintainers need to review, you can + ask or assign any [reviewers][team] for a first review. + 1. If you need some guidance (e.g. it's your first merge request), feel free + to ask one of the [Merge request coaches][team]. + 1. The reviewer will assign the merge request to a maintainer once the + reviewer is satisfied with the state of the merge request. + +## Best practices + This guide contains advice and best practices for performing code review, and having your code reviewed. @@ -12,7 +32,7 @@ of colleagues and contributors. However, the final decision to accept a merge request is up to one the project's maintainers, denoted on the [team page](https://about.gitlab.com/team). -## Everyone +### Everyone - Accept that many programming decisions are opinions. Discuss tradeoffs, which you prefer, and reach a resolution quickly. @@ -31,8 +51,11 @@ request is up to one the project's maintainers, denoted on the - Consider one-on-one chats or video calls if there are too many "I didn't understand" or "Alternative solution:" comments. Post a follow-up comment summarizing one-on-one discussion. +- If you ask a question to a specific person, always start the comment by + mentioning them; this will ensure they see it if their notification level is + set to "mentioned" and other people will understand they don't have to respond. -## Having your code reviewed +### Having your code reviewed Please keep in mind that code review is a process that can take multiple iterations, and reviewers may spot things later that they may not have seen the @@ -50,11 +73,12 @@ first time. - Extract unrelated changes and refactorings into future merge requests/issues. - Seek to understand the reviewer's perspective. - Try to respond to every comment. +- Let the reviewer select the "Resolve discussion" buttons. - Push commits based on earlier rounds of feedback as isolated commits to the branch. Do not squash until the branch is ready to merge. Reviewers should be able to read individual updates based on their earlier feedback. -## Reviewing code +### Reviewing code Understand why the change is necessary (fixes a bug, improves the user experience, refactors the existing code). Then: @@ -69,12 +93,19 @@ experience, refactors the existing code). Then: someone else would be confused by it as well. - After a round of line notes, it can be helpful to post a summary note such as "LGTM :thumbsup:", or "Just a couple things to address." +- Assign the merge request to the author if changes are required following your + review. +- Set the milestone before merging a merge request. - Avoid accepting a merge request before the job succeeds. Of course, "Merge When Pipeline Succeeds" (MWPS) is fine. - If you set the MR to "Merge When Pipeline Succeeds", you should take over subsequent revisions for anything that would be spotted after that. +- Consider using the [Squash and + merge][squash-and-merge] feature when the merge request has a lot of commits. + +[squash-and-merge]: https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html#squash-and-merge -## The right balance +### The right balance One of the most difficult things during code review is finding the right balance in how deep the reviewer can interfere with the code created by a @@ -100,7 +131,7 @@ reviewee. tomorrow. When you are not able to find the right balance, ask other people about their opinion. -## Credits +### Credits Largely based on the [thoughtbot code review guide]. diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index ec9e4dcc59d..fdaaa65fa28 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -31,16 +31,26 @@ files it can find, also the ones in `/tmp` To run a single test file you can use: -- `bundle exec rspec spec/controllers/commit_controller_spec.rb` for a rspec test -- `bundle exec spinach features/project/issues/milestones.feature` for a spinach test +- `bin/rspec spec/controllers/commit_controller_spec.rb` for a rspec test +- `bin/spinach features/project/issues/milestones.feature` for a spinach test To run several tests inside one directory: -- `bundle exec rspec spec/requests/api/` for the rspec tests if you want to test API only -- `bundle exec spinach features/profile/` for the spinach tests if you want to test only profile pages +- `bin/rspec spec/requests/api/` for the rspec tests if you want to test API only +- `bin/spinach features/profile/` for the spinach tests if you want to test only profile pages -If you want to use [Spring](https://github.com/rails/spring) set -`ENABLE_SPRING=1` in your environment. +### Speed-up tests, rake tasks, and migrations + +[Spring](https://github.com/rails/spring) is a Rails application preloader. It +speeds up development by keeping your application running in the background so +you don't need to boot it every time you run a test, rake task or migration. + +If you want to use it, you'll need to export the `ENABLE_SPRING` environment +variable to `1`: + +``` +export ENABLE_SPRING=1 +``` ## Compile Frontend Assets diff --git a/doc/topics/authentication/index.md b/doc/topics/authentication/index.md index eafd2fd9d04..3e756d96ed2 100644 --- a/doc/topics/authentication/index.md +++ b/doc/topics/authentication/index.md @@ -18,6 +18,8 @@ This page gathers all the resources for the topic **Authentication** within GitL - [LDAP (Enterprise Edition)](https://docs.gitlab.com/ee/administration/auth/ldap-ee.html) - [Enforce Two-factor Authentication (2FA)](../../security/two_factor_authentication.md#enforce-two-factor-authentication-2fa) - **Articles:** + - [How to Configure LDAP with GitLab CE](../../articles/how_to_configure_ldap_gitlab_ce/index.md) + - [How to Configure LDAP with GitLab EE](https://docs.gitlab.com/articles/how_to_configure_ldap_gitlab_ee/) - [Feature Highlight: LDAP Integration](https://about.gitlab.com/2014/07/10/feature-highlight-ldap-sync/) - [Debugging LDAP](https://about.gitlab.com/handbook/support/workflows/ldap/debugging_ldap.html) - **Integrations:** diff --git a/doc/update/9.0-to-9.1.md b/doc/update/9.0-to-9.1.md index 2d597894517..2b582d4eefd 100644 --- a/doc/update/9.0-to-9.1.md +++ b/doc/update/9.0-to-9.1.md @@ -104,6 +104,7 @@ cd /home/git/gitlab-shell sudo -u git -H git fetch --all --tags sudo -u git -H git checkout v$(</home/git/gitlab/GITLAB_SHELL_VERSION) +sudo -u git -H bin/compile ``` ### 7. Update gitlab-workhorse diff --git a/doc/update/patch_versions.md b/doc/update/patch_versions.md index f69d567eeb7..ac1bcb8f241 100644 --- a/doc/update/patch_versions.md +++ b/doc/update/patch_versions.md @@ -75,6 +75,7 @@ cd /home/git/gitlab-shell sudo -u git -H git fetch --all --tags sudo -u git -H git checkout v`cat /home/git/gitlab/GITLAB_SHELL_VERSION` -b v`cat /home/git/gitlab/GITLAB_SHELL_VERSION` +sudo -u git -H sh -c 'if [ -x bin/compile ]; then bin/compile; fi' ``` ### 6. Start application diff --git a/doc/update/upgrader.md b/doc/update/upgrader.md index 5fa39ef1b0a..eb7f14a96d5 100644 --- a/doc/update/upgrader.md +++ b/doc/update/upgrader.md @@ -60,6 +60,7 @@ GitLab Shell might be outdated, running the commands below ensures you're using cd /home/git/gitlab-shell sudo -u git -H git fetch sudo -u git -H git checkout v`cat /home/git/gitlab/GITLAB_SHELL_VERSION` +sudo -u git -H sh -c 'if [ -x bin/compile ] ; then bin/compile ; fi' ``` ## One line upgrade command @@ -78,6 +79,7 @@ cd /home/git/gitlab; \ cd /home/git/gitlab-shell; \ sudo -u git -H git fetch; \ sudo -u git -H git checkout v`cat /home/git/gitlab/GITLAB_SHELL_VERSION`; \ + sudo -u git -H sh -c 'if [ -x bin/compile ] ; then bin/compile ; fi'; \ cd /home/git/gitlab; \ sudo service gitlab start; \ sudo service nginx restart; \ diff --git a/doc/workflow/img/todos_icon.png b/doc/workflow/img/todos_icon.png Binary files differindex 1ed16b09669..9fee4337a75 100644 --- a/doc/workflow/img/todos_icon.png +++ b/doc/workflow/img/todos_icon.png diff --git a/doc/workflow/img/todos_index.png b/doc/workflow/img/todos_index.png Binary files differindex 902a5aa6bd3..99c1575d157 100644 --- a/doc/workflow/img/todos_index.png +++ b/doc/workflow/img/todos_index.png diff --git a/features/project/active_tab.feature b/features/project/active_tab.feature index 0d6f7350181..34201cd8486 100644 --- a/features/project/active_tab.feature +++ b/features/project/active_tab.feature @@ -63,13 +63,6 @@ Feature: Project Active Tab And no other sub tabs should be active And the active main tab should be Settings - Scenario: On Project Settings/Pages - Given I visit my project's settings page - And I click the "Pages" tab - Then the active sub tab should be Pages - And no other sub tabs should be active - And the active main tab should be Settings - Scenario: On Project Members Given I visit my project's members page Then the active sub tab should be Members diff --git a/features/project/builds/artifacts.feature b/features/project/builds/artifacts.feature index 52dc15f2eb6..09094d638c9 100644 --- a/features/project/builds/artifacts.feature +++ b/features/project/builds/artifacts.feature @@ -17,6 +17,7 @@ Feature: Project Builds Artifacts When I visit recent build details page And I click artifacts browse button Then I should see content of artifacts archive + And I should see the build header Scenario: I browse subdirectory of build artifacts Given recent build has artifacts available @@ -25,6 +26,7 @@ Feature: Project Builds Artifacts And I click artifacts browse button And I click link to subdirectory within build artifacts Then I should see content of subdirectory within artifacts archive + And I should see the directory name in the breadcrumb Scenario: I browse directory with UTF-8 characters in name Given recent build has artifacts available diff --git a/features/project/pages.feature b/features/project/pages.feature index 87d88348d09..56e47287b5c 100644 --- a/features/project/pages.feature +++ b/features/project/pages.feature @@ -3,10 +3,15 @@ Feature: Project Pages Given I sign in as a user And I own a project - Scenario: Pages are disabled + Scenario: I cannot navigate to Pages settings if pages enabled Given pages are disabled - When I visit the Project Pages - Then I should see that GitLab Pages are disabled + And I visit my project's settings page + Then I should not see the "Pages" tab + + Scenario: I can navigate to Pages settings if pages enabled + Given pages are enabled + And I visit my project's settings page + Then I should see the "Pages" tab Scenario: I can see the pages usage if not deployed Given pages are enabled diff --git a/features/steps/project/active_tab.rb b/features/steps/project/active_tab.rb index 4befd49ac81..5cd9bd38c9d 100644 --- a/features/steps/project/active_tab.rb +++ b/features/steps/project/active_tab.rb @@ -39,12 +39,6 @@ class Spinach::Features::ProjectActiveTab < Spinach::FeatureSteps end end - step 'I click the "Pages" tab' do - page.within '.sub-nav' do - click_link('Pages') - end - end - step 'I click the "Activity" tab' do page.within '.sub-nav' do click_link('Activity') diff --git a/features/steps/project/builds/artifacts.rb b/features/steps/project/builds/artifacts.rb index be0f6eee55a..3ec5c8e2f4f 100644 --- a/features/steps/project/builds/artifacts.rb +++ b/features/steps/project/builds/artifacts.rb @@ -22,6 +22,12 @@ class Spinach::Features::ProjectBuildsArtifacts < Spinach::FeatureSteps end end + step 'I should see the build header' do + page.within('.build-header') do + expect(page).to have_content "Job ##{@build.id} in pipeline ##{@pipeline.id} for commit #{@pipeline.short_sha}" + end + end + step 'I click link to subdirectory within build artifacts' do page.within('.tree-table') { click_link 'other_artifacts_0.1.2' } end @@ -34,6 +40,12 @@ class Spinach::Features::ProjectBuildsArtifacts < Spinach::FeatureSteps end end + step 'I should see the directory name in the breadcrumb' do + page.within('.repo-breadcrumb') do + expect(page).to have_content 'other_artifacts_0.1.2' + end + end + step 'recent build artifacts contain directory with UTF-8 characters' do # metadata fixture contains relevant directory end diff --git a/features/steps/project/pages.rb b/features/steps/project/pages.rb index 4045955a8b9..fea82d9fb57 100644 --- a/features/steps/project/pages.rb +++ b/features/steps/project/pages.rb @@ -18,14 +18,22 @@ class Spinach::Features::ProjectPages < Spinach::FeatureSteps visit namespace_project_pages_path(@project.namespace, @project) end - step 'I should see that GitLab Pages are disabled' do - expect(page).to have_content('GitLab Pages are disabled') - end - step 'I should see the usage of GitLab Pages' do expect(page).to have_content('Configure pages') end + step 'I should see the "Pages" tab' do + page.within '.sub-nav' do + expect(page).to have_link('Pages') + end + end + + step 'I should not see the "Pages" tab' do + page.within '.sub-nav' do + expect(page).not_to have_link('Pages') + end + end + step 'pages are deployed' do pipeline = @project.ensure_pipeline('HEAD', @project.commit('HEAD').sha) build = build(:ci_build, diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb index 754c3d85a04..9117704aa46 100644 --- a/lib/api/pipelines.rb +++ b/lib/api/pipelines.rb @@ -14,13 +14,23 @@ module API end params do use :pagination - optional :scope, type: String, values: %w(running branches tags), - desc: 'Either running, branches, or tags' + optional :scope, type: String, values: %w[running pending finished branches tags], + desc: 'The scope of pipelines' + optional :status, type: String, values: HasStatus::AVAILABLE_STATUSES, + desc: 'The status of pipelines' + optional :ref, type: String, desc: 'The ref of pipelines' + optional :yaml_errors, type: Boolean, desc: 'Returns pipelines with invalid configurations' + optional :name, type: String, desc: 'The name of the user who triggered pipelines' + optional :username, type: String, desc: 'The username of the user who triggered pipelines' + optional :order_by, type: String, values: PipelinesFinder::ALLOWED_INDEXED_COLUMNS, default: 'id', + desc: 'Order pipelines' + optional :sort, type: String, values: %w[asc desc], default: 'desc', + desc: 'Sort pipelines' end get ':id/pipelines' do authorize! :read_pipeline, user_project - pipelines = PipelinesFinder.new(user_project).execute(scope: params[:scope]) + pipelines = PipelinesFinder.new(user_project, params).execute present paginate(pipelines), with: Entities::PipelineBasic end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index db4b31b55bc..9a6cb43abf7 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -26,6 +26,10 @@ module API params :optional_params do use :optional_params_ce end + + params :statistics_params do + optional :statistics, type: Boolean, default: false, desc: 'Include project statistics' + end end resource :projects do @@ -56,10 +60,6 @@ module API optional :membership, type: Boolean, default: false, desc: 'Limit by projects that the current user is a member of' end - params :statistics_params do - optional :statistics, type: Boolean, default: false, desc: 'Include project statistics' - end - params :create_params do optional :namespace_id, type: Integer, desc: 'Namespace ID for the new project. Default to the user namespace.' optional :import_url, type: String, desc: 'URL from which the project is imported' @@ -85,6 +85,7 @@ module API end params do use :collection_params + use :statistics_params end get do entity = current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails @@ -151,10 +152,13 @@ module API desc 'Get a single project' do success Entities::ProjectWithAccess end + params do + use :statistics_params + end get ":id" do entity = current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails present user_project, with: entity, current_user: current_user, - user_can_admin_project: can?(current_user, :admin_project, user_project) + user_can_admin_project: can?(current_user, :admin_project, user_project), statistics: params[:statistics] end desc 'Get events for a single project' do @@ -381,7 +385,7 @@ module API requires :file, type: File, desc: 'The file to be uploaded' end post ":id/uploads" do - ::Projects::UploadService.new(user_project, params[:file]).execute + UploadService.new(user_project, params[:file]).execute end desc 'Get the users list of a project' do diff --git a/lib/api/v3/pipelines.rb b/lib/api/v3/pipelines.rb index 82827249244..c48cbd2b765 100644 --- a/lib/api/v3/pipelines.rb +++ b/lib/api/v3/pipelines.rb @@ -21,7 +21,7 @@ module API get ':id/pipelines' do authorize! :read_pipeline, user_project - pipelines = PipelinesFinder.new(user_project).execute(scope: params[:scope]) + pipelines = PipelinesFinder.new(user_project, scope: params[:scope]).execute present paginate(pipelines), with: ::API::Entities::Pipeline end end diff --git a/lib/api/v3/projects.rb b/lib/api/v3/projects.rb index ba9748ada59..06cc704afc6 100644 --- a/lib/api/v3/projects.rb +++ b/lib/api/v3/projects.rb @@ -452,7 +452,7 @@ module API requires :file, type: File, desc: 'The file to be uploaded' end post ":id/uploads" do - ::Projects::UploadService.new(user_project, params[:file]).execute + UploadService.new(user_project, params[:file]).execute end desc 'Get the users list of a project' do diff --git a/lib/gitlab/email/attachment_uploader.rb b/lib/gitlab/email/attachment_uploader.rb index 32cece8316b..83440ae227d 100644 --- a/lib/gitlab/email/attachment_uploader.rb +++ b/lib/gitlab/email/attachment_uploader.rb @@ -21,7 +21,7 @@ module Gitlab content_type: attachment.content_type } - link = ::Projects::UploadService.new(project, file).execute + link = UploadService.new(project, file).execute attachments << link if link ensure tmp.close! diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index c270c0ea9ff..0d6b08b5d29 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -57,9 +57,8 @@ module Gitlab end def key_from_additional_headers(mail) - references = ensure_references_array(mail.references) - - find_key_from_references(references) + find_key_from_references(mail) || + find_key_from_delivered_to_header(mail) end def ensure_references_array(references) @@ -75,12 +74,19 @@ module Gitlab end end - def find_key_from_references(references) - references.find do |mail_id| + def find_key_from_references(mail) + ensure_references_array(mail.references).find do |mail_id| key = Gitlab::IncomingEmail.key_from_fallback_message_id(mail_id) break key if key end end + + def find_key_from_delivered_to_header(mail) + Array(mail[:delivered_to]).find do |header| + key = Gitlab::IncomingEmail.key_from_address(header.value) + break key if key + end + end end end end diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index e8bb9e1f805..12458f9f410 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -128,6 +128,10 @@ module Gitlab encode! @name end + def truncated? + size && (size > loaded_size) + end + # Valid LFS object pointer is a text file consisting of # version # oid @@ -155,10 +159,14 @@ module Gitlab nil end - def truncated? - size && (size > loaded_size) + def external_storage + return unless lfs_pointer? + + :lfs end + alias_method :external_size, :lfs_size + private def has_lfs_version_key? diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index acd0037ee4f..6a0f12b7e50 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -499,8 +499,9 @@ module Gitlab # :contains is the commit contained by the refs from which to begin (SHA1 or name) # :max_count is the maximum number of commits to fetch # :skip is the number of commits to skip - # :order is the commits order and allowed value is :none (default), :date, or :topo - # commit ordering types are documented here: + # :order is the commits order and allowed value is :none (default), :date, + # :topo, or any combination of them (in an array). Commit ordering types + # are documented here: # http://www.rubydoc.info/github/libgit2/rugged/Rugged#SORT_NONE-constant) # def find_commits(options = {}) @@ -876,27 +877,6 @@ module Gitlab rugged.remotes[remote_name].push(refspecs) end - # Merge the +source_name+ branch into the +target_name+ branch. This is - # equivalent to `git merge --no_ff +source_name+`, since a merge commit - # is always created. - def merge(source_name, target_name, options = {}) - our_commit = rugged.branches[target_name].target - their_commit = rugged.branches[source_name].target - - raise "Invalid merge target" if our_commit.nil? - raise "Invalid merge source" if their_commit.nil? - - merge_index = rugged.merge_commits(our_commit, their_commit) - return false if merge_index.conflicts? - - actual_options = options.merge( - parents: [our_commit, their_commit], - tree: merge_index.write_tree(rugged), - update_ref: "refs/heads/#{target_name}" - ) - Rugged::Commit.create(rugged, actual_options) - end - AUTOCRLF_VALUES = { "true" => true, "false" => false, @@ -1290,16 +1270,18 @@ module Gitlab raise CommandError.new(e) end - # Returns the `Rugged` sorting type constant for a given - # sort type key. Valid keys are `:none`, `:topo`, and `:date` - def rugged_sort_type(key) + # Returns the `Rugged` sorting type constant for one or more given + # sort types. Valid keys are `:none`, `:topo`, and `:date`, or an array + # containing more than one of them. `:date` uses a combination of date and + # topological sorting to closer mimic git's native ordering. + def rugged_sort_type(sort_type) @rugged_sort_types ||= { none: Rugged::SORT_NONE, topo: Rugged::SORT_TOPO, - date: Rugged::SORT_DATE + date: Rugged::SORT_DATE | Rugged::SORT_TOPO } - @rugged_sort_types.fetch(key, Rugged::SORT_NONE) + @rugged_sort_types.fetch(sort_type, Rugged::SORT_NONE) end end end diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index b95cea371b9..3aac731e844 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -84,6 +84,7 @@ excluded_attributes: - :import_jid - :id - :star_count + - :last_activity_at snippets: - :expired_at merge_request_diff: diff --git a/lib/tasks/gitlab/db.rake b/lib/tasks/gitlab/db.rake index 5476438b8fa..139ab70e125 100644 --- a/lib/tasks/gitlab/db.rake +++ b/lib/tasks/gitlab/db.rake @@ -65,6 +65,7 @@ namespace :gitlab do migrations = `git diff #{ref}.. --diff-filter=A --name-only -- db/migrate`.lines .map { |file| Rails.root.join(file.strip).to_s } .select { |file| File.file?(file) } + .select { |file| /\A[0-9]+.*\.rb\z/ =~ File.basename(file) } Gitlab::DowntimeCheck.new.check_and_print(migrations) end diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index 95687066819..ee2cdcdea1b 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -41,8 +41,14 @@ namespace :gitlab do # Generate config.yml based on existing gitlab settings File.open("config.yml", "w+") {|f| f.puts config.to_yaml} - # Launch installation process - system(*%w(bin/install) + repository_storage_paths_args) + [ + %w(bin/install) + repository_storage_paths_args, + %w(bin/compile) + ].each do |cmd| + unless Kernel.system(*cmd) + raise "command failed: #{cmd.join(' ')}" + end + end end # (Re)create hooks diff --git a/lib/tasks/migrate/add_limits_mysql.rake b/lib/tasks/migrate/add_limits_mysql.rake index 6ded519aff2..761f275d42a 100644 --- a/lib/tasks/migrate/add_limits_mysql.rake +++ b/lib/tasks/migrate/add_limits_mysql.rake @@ -1,7 +1,9 @@ require Rails.root.join('db/migrate/limits_to_mysql') +require Rails.root.join('db/migrate/markdown_cache_limits_to_mysql') desc "GitLab | Add limits to strings in mysql database" task add_limits_mysql: :environment do puts "Adding limits to schema.rb for mysql" LimitsToMysql.new.up + MarkdownCacheLimitsToMysql.new.up end diff --git a/scripts/static-analysis b/scripts/static-analysis index 192d9d4c3ba..1bd6b339830 100755 --- a/scripts/static-analysis +++ b/scripts/static-analysis @@ -9,7 +9,6 @@ tasks = [ %w[bundle exec rake scss_lint], %w[bundle exec rake brakeman], %w[bundle exec license_finder], - %w[scripts/lint-doc.sh], %w[yarn run eslint], %w[bundle exec rubocop --require rubocop-rspec] ] diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index d20e7368086..8f915d9d210 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -14,7 +14,7 @@ describe Projects::BranchesController do controller.instance_variable_set(:@project, project) end - describe "POST create" do + describe "POST create with HTML format" do render_views context "on creation of a new branch" do @@ -152,6 +152,42 @@ describe Projects::BranchesController do end end + describe 'POST create with JSON format' do + before do + sign_in(user) + end + + context 'with valid params' do + it 'returns a successful 200 response' do + create_branch name: 'my-branch', ref: 'master' + + expect(response).to have_http_status(200) + end + + it 'returns the created branch' do + create_branch name: 'my-branch', ref: 'master' + + expect(response).to match_response_schema('branch') + end + end + + context 'with invalid params' do + it 'returns an unprocessable entity 422 response' do + create_branch name: "<script>alert('merge');</script>", ref: "<script>alert('ref');</script>" + + expect(response).to have_http_status(422) + end + end + + def create_branch(name:, ref:) + post :create, namespace_id: project.namespace.to_param, + project_id: project.to_param, + branch_name: name, + ref: ref, + format: :json + end + end + describe "POST destroy with HTML format" do render_views diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 79034b8d24d..5f1f892821a 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -756,4 +756,28 @@ describe Projects::IssuesController do expect(response).to have_http_status(200) end end + + describe 'POST create_merge_request' do + before do + project.add_developer(user) + sign_in(user) + end + + it 'creates a new merge request' do + expect { create_merge_request }.to change(project.merge_requests, :count).by(1) + end + + it 'render merge request as json' do + create_merge_request + + expect(response).to match_response_schema('merge_request') + end + + def create_merge_request + post :create_merge_request, namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: issue.to_param, + format: :json + end + end end diff --git a/spec/controllers/projects/pages_controller_spec.rb b/spec/controllers/projects/pages_controller_spec.rb new file mode 100644 index 00000000000..df35d8e86b9 --- /dev/null +++ b/spec/controllers/projects/pages_controller_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe Projects::PagesController do + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public, :access_requestable) } + + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project + } + end + + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + sign_in(user) + project.add_master(user) + end + + describe 'GET show' do + it 'returns 200 status' do + get :show, request_params + + expect(response).to have_http_status(200) + end + end + + describe 'DELETE destroy' do + it 'returns 302 status' do + delete :destroy, request_params + + expect(response).to have_http_status(302) + end + end + + context 'pages disabled' do + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(false) + end + + describe 'GET show' do + it 'returns 404 status' do + get :show, request_params + + expect(response).to have_http_status(404) + end + end + + describe 'DELETE destroy' do + it 'returns 404 status' do + delete :destroy, request_params + + expect(response).to have_http_status(404) + end + end + end +end diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb index 2362df895a8..33853c4b9d0 100644 --- a/spec/controllers/projects/pages_domains_controller_spec.rb +++ b/spec/controllers/projects/pages_domains_controller_spec.rb @@ -1,8 +1,9 @@ require 'spec_helper' describe Projects::PagesDomainsController do - let(:user) { create(:user) } - let(:project) { create(:project) } + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let!(:pages_domain) { create(:pages_domain, project: project) } let(:request_params) do { @@ -11,14 +12,17 @@ describe Projects::PagesDomainsController do } end + let(:pages_domain_params) do + build(:pages_domain, :with_certificate, :with_key, domain: 'my.otherdomain.com').slice(:key, :certificate, :domain) + end + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) sign_in(user) - project.team << [user, :master] + project.add_master(user) end describe 'GET show' do - let!(:pages_domain) { create(:pages_domain, project: project) } - it "displays the 'show' page" do get(:show, request_params.merge(id: pages_domain.domain)) @@ -37,10 +41,6 @@ describe Projects::PagesDomainsController do end describe 'POST create' do - let(:pages_domain_params) do - build(:pages_domain, :with_certificate, :with_key).slice(:key, :certificate, :domain) - end - it "creates a new pages domain" do expect do post(:create, request_params.merge(pages_domain: pages_domain_params)) @@ -51,8 +51,6 @@ describe Projects::PagesDomainsController do end describe 'DELETE destroy' do - let!(:pages_domain) { create(:pages_domain, project: project) } - it "deletes the pages domain" do expect do delete(:destroy, request_params.merge(id: pages_domain.domain)) @@ -61,4 +59,42 @@ describe Projects::PagesDomainsController do expect(response).to redirect_to(namespace_project_pages_path(project.namespace, project)) end end + + context 'pages disabled' do + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(false) + end + + describe 'GET show' do + it 'returns 404 status' do + get(:show, request_params.merge(id: pages_domain.domain)) + + expect(response).to have_http_status(404) + end + end + + describe 'GET new' do + it 'returns 404 status' do + get :new, request_params + + expect(response).to have_http_status(404) + end + end + + describe 'POST create' do + it "returns 404 status" do + post(:create, request_params.merge(pages_domain: pages_domain_params)) + + expect(response).to have_http_status(404) + end + end + + describe 'DELETE destroy' do + it "deletes the pages domain" do + delete(:destroy, request_params.merge(id: pages_domain.domain)) + + expect(response).to have_http_status(404) + end + end + end end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index f67d26da0ac..7dedfe160a6 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -8,6 +8,93 @@ end describe UploadsController do let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } + describe 'POST create' do + let(:model) { 'personal_snippet' } + let(:snippet) { create(:personal_snippet, :public) } + let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') } + let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') } + + context 'when a user does not have permissions to upload a file' do + it "returns 401 when the user is not logged in" do + post :create, model: model, id: snippet.id, format: :json + + expect(response).to have_http_status(401) + end + + it "returns 404 when user can't comment on a snippet" do + private_snippet = create(:personal_snippet, :private) + + sign_in(user) + post :create, model: model, id: private_snippet.id, format: :json + + expect(response).to have_http_status(404) + end + end + + context 'when a user is logged in' do + before do + sign_in(user) + end + + it "returns an error without file" do + post :create, model: model, id: snippet.id, format: :json + + expect(response).to have_http_status(422) + end + + it "returns an error with invalid model" do + expect { post :create, model: 'invalid', id: snippet.id, format: :json } + .to raise_error(ActionController::UrlGenerationError) + end + + it "returns 404 status when object not found" do + post :create, model: model, id: 9999, format: :json + + expect(response).to have_http_status(404) + end + + context 'with valid image' do + before do + post :create, model: 'personal_snippet', id: snippet.id, file: jpg, format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + expect(response.body).to match '\"alt\":\"rails_sample\"' + expect(response.body).to match "\"url\":\"/uploads" + end + + it 'creates a corresponding Upload record' do + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq snippet + end + end + end + + context 'with valid non-image file' do + before do + post :create, model: 'personal_snippet', id: snippet.id, file: txt, format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + expect(response.body).to match '\"alt\":\"doc_sample.txt\"' + expect(response.body).to match "\"url\":\"/uploads" + end + + it 'creates a corresponding Upload record' do + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq snippet + end + end + end + end + end + describe "GET show" do context 'Content-Disposition security measures' do let(:project) { create(:empty_project, :public) } diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/gitlab_flavored_markdown_spec.rb index 01b1aee4fd3..f5b54463df8 100644 --- a/spec/features/gitlab_flavored_markdown_spec.rb +++ b/spec/features/gitlab_flavored_markdown_spec.rb @@ -62,6 +62,8 @@ describe "GitLab Flavored Markdown", feature: true do project: project, title: "fix #{@other_issue.to_reference}", description: "ask #{fred.to_reference} for details") + + @note = create(:note_on_issue, noteable: @issue, project: @issue.project, note: "Hello world") end it "renders subject in issues#index" do @@ -81,14 +83,6 @@ describe "GitLab Flavored Markdown", feature: true do expect(page).to have_link(fred.to_reference) end - - it "renders updated subject once edited somewhere else in issues#show" do - visit namespace_project_issue_path(project.namespace, project, @issue) - @issue.update(title: "fix #{@other_issue.to_reference} and update") - - wait_for_vue_resource - expect(page).to have_text("fix #{@other_issue.to_reference} and update") - end end describe "for merge requests" do diff --git a/spec/features/issues/create_branch_merge_request_spec.rb b/spec/features/issues/create_branch_merge_request_spec.rb new file mode 100644 index 00000000000..44c19275ae5 --- /dev/null +++ b/spec/features/issues/create_branch_merge_request_spec.rb @@ -0,0 +1,91 @@ +require 'rails_helper' + +feature 'Create Branch/Merge Request Dropdown on issue page', feature: true, js: true do + let(:user) { create(:user) } + let!(:project) { create(:project) } + let(:issue) { create(:issue, project: project, title: 'Cherry-Coloured Funk') } + + context 'for team members' do + before do + project.team << [user, :developer] + login_as(user) + end + + it 'allows creating a merge request from the issue page' do + visit namespace_project_issue_path(project.namespace, project, issue) + + select_dropdown_option('create-mr') + + wait_for_ajax + + expect(page).to have_content("created branch 1-cherry-coloured-funk") + expect(page).to have_content("mentioned in merge request !1") + + visit namespace_project_merge_request_path(project.namespace, project, MergeRequest.first) + + expect(page).to have_content('WIP: Resolve "Cherry-Coloured Funk"') + expect(current_path).to eq(namespace_project_merge_request_path(project.namespace, project, MergeRequest.first)) + end + + it 'allows creating a branch from the issue page' do + visit namespace_project_issue_path(project.namespace, project, issue) + + select_dropdown_option('create-branch') + + wait_for_ajax + + expect(page).to have_selector('.dropdown-toggle-text ', text: '1-cherry-coloured-funk') + expect(current_path).to eq namespace_project_tree_path(project.namespace, project, '1-cherry-coloured-funk') + end + + context "when there is a referenced merge request" do + let!(:note) do + create(:note, :on_issue, :system, project: project, noteable: issue, + note: "mentioned in #{referenced_mr.to_reference}") + end + + let(:referenced_mr) do + create(:merge_request, :simple, source_project: project, target_project: project, + description: "Fixes #{issue.to_reference}", author: user) + end + + before do + referenced_mr.cache_merge_request_closes_issues!(user) + + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it 'disables the create branch button' do + expect(page).to have_css('.create-mr-dropdown-wrap .unavailable:not(.hide)') + expect(page).to have_css('.create-mr-dropdown-wrap .available.hide', visible: false) + expect(page).to have_content /1 Related Merge Request/ + end + end + + context 'when issue is confidential' do + it 'disables the create branch button' do + issue = create(:issue, :confidential, project: project) + + visit namespace_project_issue_path(project.namespace, project, issue) + + expect(page).not_to have_css('.create-mr-dropdown-wrap') + end + end + end + + context 'for visitors' do + before do + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it 'shows no buttons' do + expect(page).not_to have_selector('.create-mr-dropdown-wrap') + end + end + + def select_dropdown_option(option) + find('.create-mr-dropdown-wrap .dropdown-toggle').click + find("li[data-value='#{option}']").click + find('.js-create-merge-request').click + end +end diff --git a/spec/features/issues/new_branch_button_spec.rb b/spec/features/issues/new_branch_button_spec.rb deleted file mode 100644 index c0ab42c6822..00000000000 --- a/spec/features/issues/new_branch_button_spec.rb +++ /dev/null @@ -1,62 +0,0 @@ -require 'rails_helper' - -feature 'Start new branch from an issue', feature: true, js: true do - let!(:project) { create(:project) } - let!(:issue) { create(:issue, project: project) } - let!(:user) { create(:user)} - - context "for team members" do - before do - project.team << [user, :master] - login_as(user) - end - - it 'shows the new branch button' do - visit namespace_project_issue_path(project.namespace, project, issue) - - expect(page).to have_css('#new-branch .available') - end - - context "when there is a referenced merge request" do - let!(:note) do - create(:note, :on_issue, :system, project: project, noteable: issue, - note: "mentioned in #{referenced_mr.to_reference}") - end - - let(:referenced_mr) do - create(:merge_request, :simple, source_project: project, target_project: project, - description: "Fixes #{issue.to_reference}", author: user) - end - - before do - referenced_mr.cache_merge_request_closes_issues!(user) - - visit namespace_project_issue_path(project.namespace, project, issue) - end - - it "hides the new branch button" do - expect(page).to have_css('#new-branch .unavailable') - expect(page).not_to have_css('#new-branch .available') - expect(page).to have_content /1 Related Merge Request/ - end - end - - context 'when issue is confidential' do - it 'hides the new branch button' do - issue = create(:issue, :confidential, project: project) - - visit namespace_project_issue_path(project.namespace, project, issue) - - expect(page).not_to have_css('#new-branch') - end - end - end - - context 'for visitors' do - it 'shows no buttons' do - visit namespace_project_issue_path(project.namespace, project, issue) - - expect(page).not_to have_css('#new-branch') - end - end -end diff --git a/spec/features/issues/note_polling_spec.rb b/spec/features/issues/note_polling_spec.rb index 378f6de1a78..58b3215f14c 100644 --- a/spec/features/issues/note_polling_spec.rb +++ b/spec/features/issues/note_polling_spec.rb @@ -4,14 +4,77 @@ feature 'Issue notes polling', :feature, :js do let(:project) { create(:empty_project, :public) } let(:issue) { create(:issue, project: project) } - before do - visit namespace_project_issue_path(project.namespace, project, issue) + describe 'creates' do + before do + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it 'displays the new comment' do + note = create(:note, noteable: issue, project: project, note: 'Looks good!') + page.execute_script('notes.refresh();') + + expect(page).to have_selector("#note_#{note.id}", text: 'Looks good!') + end end - it 'should display the new comment' do - note = create(:note, noteable: issue, project: project, note: 'Looks good!') - page.execute_script('notes.refresh();') + describe 'updates' do + let(:user) { create(:user) } + let(:note_text) { "Hello World" } + let(:updated_text) { "Bye World" } + let!(:existing_note) { create(:note, noteable: issue, project: project, author: user, note: note_text) } + + before do + login_as(user) + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it 'displays the updated content' do + expect(page).to have_selector("#note_#{existing_note.id}", text: note_text) + + update_note(existing_note, updated_text) + + expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text) + end + + it 'when editing but have not changed anything, and an update comes in, show the updated content in the textarea' do + find("#note_#{existing_note.id} .js-note-edit").click + + expect(page).to have_field("note[note]", with: note_text) + + update_note(existing_note, updated_text) + + expect(page).to have_field("note[note]", with: updated_text) + end + + it 'when editing but you changed some things, and an update comes in, show a warning' do + find("#note_#{existing_note.id} .js-note-edit").click - expect(page).to have_selector("#note_#{note.id}", text: 'Looks good!') + expect(page).to have_field("note[note]", with: note_text) + + find("#note_#{existing_note.id} .js-note-text").set('something random') + + update_note(existing_note, updated_text) + + expect(page).to have_selector(".alert") + end + + it 'when editing but you changed some things, an update comes in, and you press cancel, show the updated content' do + find("#note_#{existing_note.id} .js-note-edit").click + + expect(page).to have_field("note[note]", with: note_text) + + find("#note_#{existing_note.id} .js-note-text").set('something random') + + update_note(existing_note, updated_text) + + find("#note_#{existing_note.id} .note-edit-cancel").click + + expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text) + end + end + + def update_note(note, new_text) + note.update(note: new_text) + page.execute_script('notes.refresh();') end end diff --git a/spec/features/merge_requests/versions_spec.rb b/spec/features/merge_requests/versions_spec.rb index 7a2da623c58..2b5b803946c 100644 --- a/spec/features/merge_requests/versions_spec.rb +++ b/spec/features/merge_requests/versions_spec.rb @@ -24,7 +24,12 @@ feature 'Merge Request versions', js: true, feature: true do before do page.within '.mr-version-dropdown' do find('.btn-default').click - find(:link, 'version 1').trigger('click') + click_link 'version 1' + end + + # Wait for the page to load + page.within '.mr-version-dropdown' do + expect(page).to have_content 'version 1' end end @@ -36,8 +41,8 @@ feature 'Merge Request versions', js: true, feature: true do expect(page).to have_content '5 changed files' end - it 'show the message about disabled comment creation' do - expect(page).to have_content 'comment creation is disabled' + it 'show the message about comments' do + expect(page).to have_content 'Not all comments are displayed' end it 'shows comments that were last relevant at that version' do @@ -52,15 +57,41 @@ feature 'Merge Request versions', js: true, feature: true do outdated_diff_note.position = outdated_diff_note.original_position outdated_diff_note.save! + visit current_url + expect(page).to have_css(".diffs .notes[data-discussion-id='#{outdated_diff_note.discussion_id}']") end + + it 'allows commenting' do + diff_file_selector = ".diff-file[id='7445606fbf8f3683cd42bdc54b05d7a0bc2dfc44']" + line_code = '7445606fbf8f3683cd42bdc54b05d7a0bc2dfc44_2_2' + + page.within(diff_file_selector) do + find(".line_holder[id='#{line_code}'] td:nth-of-type(1)").trigger 'mouseover' + find(".line_holder[id='#{line_code}'] button").trigger 'click' + + page.within("form[data-line-code='#{line_code}']") do + fill_in "note[note]", with: "Typo, please fix" + find(".js-comment-button").click + end + + wait_for_ajax + + expect(page).to have_content("Typo, please fix") + end + end end describe 'compare with older version' do before do page.within '.mr-version-compare-dropdown' do find('.btn-default').click - find(:link, 'version 1').trigger('click') + click_link 'version 1' + end + + # Wait for the page to load + page.within '.mr-version-compare-dropdown' do + expect(page).to have_content 'version 1' end end @@ -80,8 +111,43 @@ feature 'Merge Request versions', js: true, feature: true do end end - it 'show the message about disabled comments' do - expect(page).to have_content 'Comments are disabled' + it 'show the message about comments' do + expect(page).to have_content 'Not all comments are displayed' + end + + it 'shows comments that were last relevant at that version' do + position = Gitlab::Diff::Position.new( + old_path: ".gitmodules", + new_path: ".gitmodules", + old_line: 4, + new_line: 4, + diff_refs: merge_request_diff3.compare_with(merge_request_diff1.head_commit_sha).diff_refs + ) + outdated_diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) + + visit current_url + wait_for_ajax + + expect(page).to have_css(".diffs .notes[data-discussion-id='#{outdated_diff_note.discussion_id}']") + end + + it 'allows commenting' do + diff_file_selector = ".diff-file[id='7445606fbf8f3683cd42bdc54b05d7a0bc2dfc44']" + line_code = '7445606fbf8f3683cd42bdc54b05d7a0bc2dfc44_4_4' + + page.within(diff_file_selector) do + find(".line_holder[id='#{line_code}'] td:nth-of-type(1)").trigger 'mouseover' + find(".line_holder[id='#{line_code}'] button").trigger 'click' + + page.within("form[data-line-code='#{line_code}']") do + fill_in "note[note]", with: "Typo, please fix" + find(".js-comment-button").click + end + + wait_for_ajax + + expect(page).to have_content("Typo, please fix") + end end it 'show diff between new and old version' do diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 8dba2ccbafa..5955623f565 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -159,7 +159,7 @@ feature 'File blob', :js, feature: true do expect(page).to have_selector('.blob-viewer[data-type="rich"]') # shows an error message - expect(page).to have_content('The rendered file could not be displayed because it is stored in LFS. You can view the source or download it instead.') + expect(page).to have_content('The rendered file could not be displayed because it is stored in LFS. You can download it instead.') # shows a viewer switcher expect(page).to have_selector('.js-blob-viewer-switcher') @@ -167,8 +167,8 @@ feature 'File blob', :js, feature: true do # does not show a copy button expect(page).not_to have_selector('.js-copy-blob-source-btn') - # shows a raw button - expect(page).to have_link('Open raw') + # shows a download button + expect(page).to have_link('Download') end end @@ -332,4 +332,41 @@ feature 'File blob', :js, feature: true do end end end + + context 'empty file' do + before do + project.add_master(project.creator) + + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add empty file", + file_path: 'files/empty.md', + file_content: '' + ).execute + + visit_blob('files/empty.md') + + wait_for_ajax + end + + it 'displays an error' do + aggregate_failures do + # shows an error message + expect(page).to have_content('Empty file') + + # does not show a viewer switcher + expect(page).not_to have_selector('.js-blob-viewer-switcher') + + # does not show a copy button + expect(page).not_to have_selector('.js-copy-blob-source-btn') + + # does not show a download or raw button + expect(page).not_to have_link('Download') + expect(page).not_to have_link('Open raw') + end + end + end end diff --git a/spec/features/protected_tags/access_control_ce_spec.rb b/spec/features/protected_tags/access_control_ce_spec.rb index 5b24ac0292b..a04fbcdd15f 100644 --- a/spec/features/protected_tags/access_control_ce_spec.rb +++ b/spec/features/protected_tags/access_control_ce_spec.rb @@ -10,8 +10,8 @@ RSpec.shared_examples "protected tags > access control > CE" do unless allowed_to_create_button.text == access_type_name allowed_to_create_button.click - find('.dropdown.open .dropdown-menu li', match: :first) - within(".dropdown.open .dropdown-menu") { click_on access_type_name } + find('.create_access_levels-container .dropdown-menu li', match: :first) + within('.create_access_levels-container .dropdown-menu') { click_on access_type_name } end end diff --git a/spec/features/protected_tags_spec.rb b/spec/features/protected_tags_spec.rb index e3aa87ded28..e68448467b0 100644 --- a/spec/features/protected_tags_spec.rb +++ b/spec/features/protected_tags_spec.rb @@ -11,6 +11,7 @@ feature 'Projected Tags', feature: true, js: true do find(".js-protected-tag-select").click find(".dropdown-input-field").set(tag_name) click_on("Create wildcard #{tag_name}") + find('.protected-tags-dropdown .dropdown-menu', visible: false) end describe "explicit protected tags" do diff --git a/spec/finders/pipelines_finder_spec.rb b/spec/finders/pipelines_finder_spec.rb index 6bada7b3eb9..f2aeda241c1 100644 --- a/spec/finders/pipelines_finder_spec.rb +++ b/spec/finders/pipelines_finder_spec.rb @@ -3,50 +3,205 @@ require 'spec_helper' describe PipelinesFinder do let(:project) { create(:project, :repository) } - let!(:tag_pipeline) { create(:ci_pipeline, project: project, ref: 'v1.0.0') } - let!(:branch_pipeline) { create(:ci_pipeline, project: project) } - - subject { described_class.new(project).execute(params) } + subject { described_class.new(project, params).execute } describe "#execute" do - context 'when a scope is passed' do - context 'when scope is nil' do - let(:params) { { scope: nil } } + context 'when params is empty' do + let(:params) { {} } + let!(:pipelines) { create_list(:ci_pipeline, 2, project: project) } + + it 'returns all pipelines' do + is_expected.to match_array(pipelines) + end + end + + %w[running pending].each do |target| + context "when scope is #{target}" do + let(:params) { { scope: target } } + let!(:pipeline) { create(:ci_pipeline, project: project, status: target) } - it 'selects all pipelines' do - expect(subject.count).to be 2 - expect(subject).to include tag_pipeline - expect(subject).to include branch_pipeline + it 'returns matched pipelines' do + is_expected.to eq([pipeline]) end end + end + + context 'when scope is finished' do + let(:params) { { scope: 'finished' } } + let!(:pipelines) do + [create(:ci_pipeline, project: project, status: 'success'), + create(:ci_pipeline, project: project, status: 'failed'), + create(:ci_pipeline, project: project, status: 'canceled')] + end - context 'when selecting branches' do + it 'returns matched pipelines' do + is_expected.to match_array(pipelines) + end + end + + context 'when scope is branches or tags' do + let!(:pipeline_branch) { create(:ci_pipeline, project: project) } + let!(:pipeline_tag) { create(:ci_pipeline, project: project, ref: 'v1.0.0', tag: true) } + + context 'when scope is branches' do let(:params) { { scope: 'branches' } } - it 'excludes tags' do - expect(subject).not_to include tag_pipeline - expect(subject).to include branch_pipeline + it 'returns matched pipelines' do + is_expected.to eq([pipeline_branch]) end end - context 'when selecting tags' do + context 'when scope is tags' do let(:params) { { scope: 'tags' } } - it 'excludes branches' do - expect(subject).to include tag_pipeline - expect(subject).not_to include branch_pipeline + it 'returns matched pipelines' do + is_expected.to eq([pipeline_tag]) + end + end + end + + HasStatus::AVAILABLE_STATUSES.each do |target| + context "when status is #{target}" do + let(:params) { { status: target } } + let!(:pipeline) { create(:ci_pipeline, project: project, status: target) } + + before do + exception_status = HasStatus::AVAILABLE_STATUSES - [target] + create(:ci_pipeline, project: project, status: exception_status.first) + end + + it 'returns matched pipelines' do + is_expected.to eq([pipeline]) end end end - # Scoping to pending will speed up the test as it doesn't hit the FS - let(:params) { { scope: 'pending' } } + context 'when ref is specified' do + let!(:pipeline) { create(:ci_pipeline, project: project) } + + context 'when ref exists' do + let(:params) { { ref: 'master' } } + + it 'returns matched pipelines' do + is_expected.to eq([pipeline]) + end + end + + context 'when ref does not exist' do + let(:params) { { ref: 'invalid-ref' } } + + it 'returns empty' do + is_expected.to be_empty + end + end + end + + context 'when name is specified' do + let(:user) { create(:user) } + let!(:pipeline) { create(:ci_pipeline, project: project, user: user) } + + context 'when name exists' do + let(:params) { { name: user.name } } + + it 'returns matched pipelines' do + is_expected.to eq([pipeline]) + end + end + + context 'when name does not exist' do + let(:params) { { name: 'invalid-name' } } + + it 'returns empty' do + is_expected.to be_empty + end + end + end - it 'orders in descending order on ID' do - feature_pipeline = create(:ci_pipeline, project: project, ref: 'feature') + context 'when username is specified' do + let(:user) { create(:user) } + let!(:pipeline) { create(:ci_pipeline, project: project, user: user) } - expected_ids = [feature_pipeline.id, branch_pipeline.id, tag_pipeline.id].sort.reverse - expect(subject.map(&:id)).to eq expected_ids + context 'when username exists' do + let(:params) { { username: user.username } } + + it 'returns matched pipelines' do + is_expected.to eq([pipeline]) + end + end + + context 'when username does not exist' do + let(:params) { { username: 'invalid-username' } } + + it 'returns empty' do + is_expected.to be_empty + end + end + end + + context 'when yaml_errors is specified' do + let!(:pipeline1) { create(:ci_pipeline, project: project, yaml_errors: 'Syntax error') } + let!(:pipeline2) { create(:ci_pipeline, project: project) } + + context 'when yaml_errors is true' do + let(:params) { { yaml_errors: true } } + + it 'returns matched pipelines' do + is_expected.to eq([pipeline1]) + end + end + + context 'when yaml_errors is false' do + let(:params) { { yaml_errors: false } } + + it 'returns matched pipelines' do + is_expected.to eq([pipeline2]) + end + end + + context 'when yaml_errors is invalid' do + let(:params) { { yaml_errors: "invalid-yaml_errors" } } + + it 'returns all pipelines' do + is_expected.to match_array([pipeline1, pipeline2]) + end + end + end + + context 'when order_by and sort are specified' do + context 'when order_by user_id' do + let(:params) { { order_by: 'user_id', sort: 'asc' } } + let!(:pipelines) { create_list(:ci_pipeline, 2, project: project, user: create(:user)) } + + it 'sorts as user_id: :asc' do + is_expected.to match_array(pipelines) + end + + context 'when sort is invalid' do + let(:params) { { order_by: 'user_id', sort: 'invalid_sort' } } + + it 'sorts as user_id: :desc' do + is_expected.to eq(pipelines.sort_by { |p| -p.user.id }) + end + end + end + + context 'when order_by is invalid' do + let(:params) { { order_by: 'invalid_column', sort: 'asc' } } + let!(:pipelines) { create_list(:ci_pipeline, 2, project: project) } + + it 'sorts as id: :asc' do + is_expected.to eq(pipelines.sort_by { |p| p.id }) + end + end + + context 'when both are nil' do + let(:params) { { order_by: nil, sort: nil } } + let!(:pipelines) { create_list(:ci_pipeline, 2, project: project) } + + it 'sorts as id: :desc' do + is_expected.to eq(pipelines.sort_by { |p| -p.id }) + end + end end end end diff --git a/spec/fixtures/api/schemas/branch.json b/spec/fixtures/api/schemas/branch.json new file mode 100644 index 00000000000..0bb74577010 --- /dev/null +++ b/spec/fixtures/api/schemas/branch.json @@ -0,0 +1,12 @@ +{ + "type": "object", + "required" : [ + "name", + "url" + ], + "properties" : { + "name": { "type": "string" }, + "url": { "type": "uri" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/merge_request.json b/spec/fixtures/api/schemas/merge_request.json new file mode 100644 index 00000000000..36962660cd9 --- /dev/null +++ b/spec/fixtures/api/schemas/merge_request.json @@ -0,0 +1,12 @@ +{ + "type": "object", + "required" : [ + "iid", + "url" + ], + "properties" : { + "iid": { "type": "integer" }, + "url": { "type": "uri" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/emails/forwarded_new_issue.eml b/spec/fixtures/emails/forwarded_new_issue.eml new file mode 100644 index 00000000000..258106bb897 --- /dev/null +++ b/spec/fixtures/emails/forwarded_new_issue.eml @@ -0,0 +1,25 @@ +Delivered-To: incoming+gitlabhq/gitlabhq+auth_token@appmail.adventuretime.ooo +Return-Path: <jake@adventuretime.ooo> +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog <jake@adventuretime.ooo> +Delivered-To: support@adventuretime.ooo +To: support@adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: New Issue by email +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 + +The reply by email functionality should be extended to allow creating a new issue by email. + +* Allow an admin to specify which project the issue should be created under by checking the sender domain. +* Possibly allow the use of regular expression matches within the subject/body to specify which project the issue should be created under. diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index 075f1887d91..1b4393e6167 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -145,7 +145,7 @@ describe BlobHelper do end end - context 'for error :server_side_but_stored_in_lfs' do + context 'for error :server_side_but_stored_externally' do let(:blob) { fake_blob(lfs: true) } it 'returns an error message' do @@ -183,40 +183,56 @@ describe BlobHelper do expect(helper.blob_render_error_options(viewer)).not_to include(/load it anyway/) end end - end - context 'when the viewer is rich' do - context 'the blob is rendered as text' do - let(:blob) { fake_blob(path: 'file.md', lfs: true) } + context 'when the viewer is rich' do + context 'the blob is rendered as text' do + let(:blob) { fake_blob(path: 'file.md', size: 2.megabytes) } + + it 'includes a "view the source" link' do + expect(helper.blob_render_error_options(viewer)).to include(/view the source/) + end + end + + context 'the blob is not rendered as text' do + let(:blob) { fake_blob(path: 'file.pdf', binary: true, size: 2.megabytes) } - it 'includes a "view the source" link' do - expect(helper.blob_render_error_options(viewer)).to include(/view the source/) + it 'does not include a "view the source" link' do + expect(helper.blob_render_error_options(viewer)).not_to include(/view the source/) + end end end - context 'the blob is not rendered as text' do - let(:blob) { fake_blob(path: 'file.pdf', binary: true, lfs: true) } + context 'when the viewer is not rich' do + before do + viewer_class.type = :simple + end + + let(:blob) { fake_blob(path: 'file.md', size: 2.megabytes) } it 'does not include a "view the source" link' do expect(helper.blob_render_error_options(viewer)).not_to include(/view the source/) end end - end - context 'when the viewer is not rich' do - before do - viewer_class.type = :simple + it 'includes a "download it" link' do + expect(helper.blob_render_error_options(viewer)).to include(/download it/) end + end + context 'for error :server_side_but_stored_externally' do let(:blob) { fake_blob(path: 'file.md', lfs: true) } + it 'does not include a "load it anyway" link' do + expect(helper.blob_render_error_options(viewer)).not_to include(/load it anyway/) + end + it 'does not include a "view the source" link' do expect(helper.blob_render_error_options(viewer)).not_to include(/view the source/) end - end - it 'includes a "download it" link' do - expect(helper.blob_render_error_options(viewer)).to include(/download it/) + it 'includes a "download it" link' do + expect(helper.blob_render_error_options(viewer)).to include(/download it/) + end end end end diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb index a427de32c4c..6c990f94175 100644 --- a/spec/helpers/notes_helper_spec.rb +++ b/spec/helpers/notes_helper_spec.rb @@ -1,6 +1,8 @@ require "spec_helper" describe NotesHelper do + include RepoHelpers + let(:owner) { create(:owner) } let(:group) { create(:group) } let(:project) { create(:empty_project, namespace: group) } @@ -36,4 +38,141 @@ describe NotesHelper do expect(helper.note_max_access_for_user(other_note)).to eq('Reporter') end end + + describe '#discussion_path' do + let(:project) { create(:project) } + + context 'for a merge request discusion' do + let(:merge_request) { create(:merge_request, source_project: project, target_project: project, importing: true) } + let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff2) { merge_request.merge_request_diffs.create(head_commit_sha: nil) } + let!(:merge_request_diff3) { merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + + context 'for a diff discussion' do + context 'when the discussion is active' do + let(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion } + + it 'returns the diff path with the line code' do + expect(helper.discussion_path(discussion)).to eq(diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, anchor: discussion.line_code)) + end + end + + context 'when the discussion is on an older merge request version' do + let(:position) do + Gitlab::Diff::Position.new( + old_path: ".gitmodules", + new_path: ".gitmodules", + old_line: nil, + new_line: 4, + diff_refs: merge_request_diff1.diff_refs + ) + end + + let(:diff_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, position: position) } + let(:discussion) { diff_note.to_discussion } + + before do + diff_note.position = diff_note.original_position + diff_note.save! + end + + it 'returns the diff version path with the line code' do + expect(helper.discussion_path(discussion)).to eq(diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, diff_id: merge_request_diff1, anchor: discussion.line_code)) + end + end + + context 'when the discussion is on a comparison between merge request versions' do + let(:position) do + Gitlab::Diff::Position.new( + old_path: ".gitmodules", + new_path: ".gitmodules", + old_line: 4, + new_line: 4, + diff_refs: merge_request_diff3.compare_with(merge_request_diff1.head_commit_sha).diff_refs + ) + end + + let(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, position: position).to_discussion } + + it 'returns the diff version comparison path with the line code' do + expect(helper.discussion_path(discussion)).to eq(diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, diff_id: merge_request_diff3, start_sha: merge_request_diff1.head_commit_sha, anchor: discussion.line_code)) + end + end + + context 'when the discussion does not have a merge request version' do + let(:outdated_diff_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, diff_refs: project.commit(sample_commit.id).diff_refs) } + let(:discussion) { outdated_diff_note.to_discussion } + + before do + outdated_diff_note.position = outdated_diff_note.original_position + outdated_diff_note.save! + end + + it 'returns nil' do + expect(helper.discussion_path(discussion)).to be_nil + end + end + end + + context 'for a legacy diff discussion' do + let(:discussion) { create(:legacy_diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion } + + context 'when the discussion is active' do + before do + allow(discussion).to receive(:active?).and_return(true) + end + + it 'returns the diff path with the line code' do + expect(helper.discussion_path(discussion)).to eq(diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, anchor: discussion.line_code)) + end + end + + context 'when the discussion is outdated' do + before do + allow(discussion).to receive(:active?).and_return(false) + end + + it 'returns nil' do + expect(helper.discussion_path(discussion)).to be_nil + end + end + end + + context 'for a non-diff discussion' do + let(:discussion) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project).to_discussion } + + it 'returns nil' do + expect(helper.discussion_path(discussion)).to be_nil + end + end + end + + context 'for a commit discussion' do + let(:commit) { discussion.noteable } + + context 'for a diff discussion' do + let(:discussion) { create(:diff_note_on_commit, project: project).to_discussion } + + it 'returns the commit path with the line code' do + expect(helper.discussion_path(discussion)).to eq(namespace_project_commit_path(project.namespace, project, commit, anchor: discussion.line_code)) + end + end + + context 'for a legacy diff discussion' do + let(:discussion) { create(:legacy_diff_note_on_commit, project: project).to_discussion } + + it 'returns the commit path with the line code' do + expect(helper.discussion_path(discussion)).to eq(namespace_project_commit_path(project.namespace, project, commit, anchor: discussion.line_code)) + end + end + + context 'for a non-diff discussion' do + let(:discussion) { create(:discussion_note_on_commit, project: project).to_discussion } + + it 'returns the commit path' do + expect(helper.discussion_path(discussion)).to eq(namespace_project_commit_path(project.namespace, project, commit)) + end + end + end + end end diff --git a/spec/javascripts/environments/environment_actions_spec.js b/spec/javascripts/environments/environment_actions_spec.js index 676bf61cfd9..596d812c724 100644 --- a/spec/javascripts/environments/environment_actions_spec.js +++ b/spec/javascripts/environments/environment_actions_spec.js @@ -4,7 +4,6 @@ import actionsComp from '~/environments/components/environment_actions.vue'; describe('Actions Component', () => { let ActionsComponent; let actionsMock; - let spy; let component; beforeEach(() => { @@ -26,13 +25,9 @@ describe('Actions Component', () => { }, ]; - spy = jasmine.createSpy('spy').and.returnValue(Promise.resolve()); component = new ActionsComponent({ propsData: { actions: actionsMock, - service: { - postAction: spy, - }, }, }).$mount(); }); @@ -48,13 +43,6 @@ describe('Actions Component', () => { ).toEqual(actionsMock.length); }); - it('should call the service when an action is clicked', () => { - component.$el.querySelector('.dropdown').click(); - component.$el.querySelector('.js-manual-action-link').click(); - - expect(spy).toHaveBeenCalledWith(actionsMock[0].play_path); - }); - it('should render a disabled action when it\'s not playable', () => { expect( component.$el.querySelector('.dropdown-menu li:last-child button').getAttribute('disabled'), diff --git a/spec/javascripts/environments/environment_rollback_spec.js b/spec/javascripts/environments/environment_rollback_spec.js index 25397714a76..eb8e49d81fe 100644 --- a/spec/javascripts/environments/environment_rollback_spec.js +++ b/spec/javascripts/environments/environment_rollback_spec.js @@ -4,11 +4,9 @@ import rollbackComp from '~/environments/components/environment_rollback.vue'; describe('Rollback Component', () => { const retryURL = 'https://gitlab.com/retry'; let RollbackComponent; - let spy; beforeEach(() => { RollbackComponent = Vue.extend(rollbackComp); - spy = jasmine.createSpy('spy').and.returnValue(Promise.resolve()); }); it('Should render Re-deploy label when isLastDeployment is true', () => { @@ -17,9 +15,6 @@ describe('Rollback Component', () => { propsData: { retryUrl: retryURL, isLastDeployment: true, - service: { - postAction: spy, - }, }, }).$mount(); @@ -32,28 +27,9 @@ describe('Rollback Component', () => { propsData: { retryUrl: retryURL, isLastDeployment: false, - service: { - postAction: spy, - }, }, }).$mount(); expect(component.$el.querySelector('span').textContent).toContain('Rollback'); }); - - it('should call the service when the button is clicked', () => { - const component = new RollbackComponent({ - propsData: { - retryUrl: retryURL, - isLastDeployment: false, - service: { - postAction: spy, - }, - }, - }).$mount(); - - component.$el.click(); - - expect(spy).toHaveBeenCalledWith(retryURL); - }); }); diff --git a/spec/javascripts/environments/environment_stop_spec.js b/spec/javascripts/environments/environment_stop_spec.js index 942e4aaabd4..8131f1e5b11 100644 --- a/spec/javascripts/environments/environment_stop_spec.js +++ b/spec/javascripts/environments/environment_stop_spec.js @@ -4,20 +4,15 @@ import stopComp from '~/environments/components/environment_stop.vue'; describe('Stop Component', () => { let StopComponent; let component; - let spy; const stopURL = '/stop'; beforeEach(() => { StopComponent = Vue.extend(stopComp); - spy = jasmine.createSpy('spy').and.returnValue(Promise.resolve()); spyOn(window, 'confirm').and.returnValue(true); component = new StopComponent({ propsData: { stopUrl: stopURL, - service: { - postAction: spy, - }, }, }).$mount(); }); @@ -26,9 +21,4 @@ describe('Stop Component', () => { expect(component.$el.tagName).toEqual('BUTTON'); expect(component.$el.getAttribute('title')).toEqual('Stop'); }); - - it('should call the service when an action is clicked', () => { - component.$el.click(); - expect(spy).toHaveBeenCalled(); - }); }); diff --git a/spec/javascripts/fixtures/mini_dropdown_graph.html.haml b/spec/javascripts/fixtures/mini_dropdown_graph.html.haml index 29370b974af..b532b48a95b 100644 --- a/spec/javascripts/fixtures/mini_dropdown_graph.html.haml +++ b/spec/javascripts/fixtures/mini_dropdown_graph.html.haml @@ -3,7 +3,7 @@ Dropdown %ul.dropdown-menu.mini-pipeline-graph-dropdown-menu.js-builds-dropdown-container - .js-builds-dropdown-list.scrollable-menu + %li.js-builds-dropdown-list.scrollable-menu - .js-builds-dropdown-loading.builds-dropdown-loading.hidden - %span.fa.fa-spinner.fa-spin + %li.js-builds-dropdown-loading.hidden + %span.fa.fa-spinner diff --git a/spec/javascripts/issue_spec.js b/spec/javascripts/issue_spec.js index 9a2570ef7e9..0fd573eae3f 100644 --- a/spec/javascripts/issue_spec.js +++ b/spec/javascripts/issue_spec.js @@ -108,8 +108,8 @@ describe('Issue', function() { expect(this.$triggeredButton).toHaveProp('disabled', true); expectNewBranchButtonState(true, false); return this.issueStateDeferred; - } else if (req.url === Issue.$btnNewBranch.data('path')) { - expect(req.type).toBe('get'); + } else if (req.url === Issue.createMrDropdownWrap.dataset.canCreatePath) { + expect(req.type).toBe('GET'); expectNewBranchButtonState(true, false); return this.canCreateBranchDeferred; } diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index ca8ee04d955..cdc5c4510ff 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -1,10 +1,12 @@ /* eslint-disable space-before-function-paren, no-unused-expressions, no-var, object-shorthand, comma-dangle, max-len */ /* global Notes */ -require('~/notes'); -require('vendor/autosize'); -require('~/gl_form'); -require('~/lib/utils/text_utility'); +import 'vendor/autosize'; +import '~/gl_form'; +import '~/lib/utils/text_utility'; +import '~/render_gfm'; +import '~/render_math'; +import '~/notes'; (function() { window.gon || (window.gon = {}); @@ -80,35 +82,78 @@ require('~/lib/utils/text_utility'); beforeEach(() => { note = { + id: 1, discussion_html: null, valid: true, - html: '<div></div>', + note: 'heya', + html: '<div>heya</div>', }; - $notesList = jasmine.createSpyObj('$notesList', ['find']); + $notesList = jasmine.createSpyObj('$notesList', [ + 'find', + 'append', + ]); notes = jasmine.createSpyObj('notes', [ 'refresh', 'isNewNote', + 'isUpdatedNote', 'collapseLongCommitList', 'updateNotesCount', + 'putConflictEditWarningInPlace' ]); notes.taskList = jasmine.createSpyObj('tasklist', ['init']); notes.note_ids = []; + notes.updatedNotesTrackingMap = {}; - spyOn(window, '$').and.returnValue($notesList); spyOn(gl.utils, 'localTimeAgo'); - spyOn(Notes, 'animateAppendNote'); - notes.isNewNote.and.returnValue(true); - - Notes.prototype.renderNote.call(notes, note); + spyOn(Notes, 'animateAppendNote').and.callThrough(); + spyOn(Notes, 'animateUpdateNote').and.callThrough(); }); - it('should query for the notes list', () => { - expect(window.$).toHaveBeenCalledWith('ul.main-notes-list'); + describe('when adding note', () => { + it('should call .animateAppendNote', () => { + notes.isNewNote.and.returnValue(true); + Notes.prototype.renderNote.call(notes, note, null, $notesList); + + expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, $notesList); + }); }); - it('should call .animateAppendNote', () => { - expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, $notesList); + describe('when note was edited', () => { + it('should call .animateUpdateNote', () => { + notes.isUpdatedNote.and.returnValue(true); + const $note = $('<div>'); + $notesList.find.and.returnValue($note); + Notes.prototype.renderNote.call(notes, note, null, $notesList); + + expect(Notes.animateUpdateNote).toHaveBeenCalledWith(note.html, $note); + }); + + describe('while editing', () => { + it('should update textarea if nothing has been touched', () => { + notes.isUpdatedNote.and.returnValue(true); + const $note = $(`<div class="is-editing"> + <div class="original-note-content">initial</div> + <textarea class="js-note-text">initial</textarea> + </div>`); + $notesList.find.and.returnValue($note); + Notes.prototype.renderNote.call(notes, note, null, $notesList); + + expect($note.find('.js-note-text').val()).toEqual(note.note); + }); + + it('should call .putConflictEditWarningInPlace', () => { + notes.isUpdatedNote.and.returnValue(true); + const $note = $(`<div class="is-editing"> + <div class="original-note-content">initial</div> + <textarea class="js-note-text">different</textarea> + </div>`); + $notesList.find.and.returnValue($note); + Notes.prototype.renderNote.call(notes, note, null, $notesList); + + expect(notes.putConflictEditWarningInPlace).toHaveBeenCalledWith(note, $note); + }); + }); }); }); @@ -147,14 +192,12 @@ require('~/lib/utils/text_utility'); }); describe('Discussion root note', () => { - let $notesList; let body; beforeEach(() => { body = jasmine.createSpyObj('body', ['attr']); discussionContainer = { length: 0 }; - spyOn(window, '$').and.returnValues(discussionContainer, body, $notesList); $form.closest.and.returnValues(row, $form); $form.find.and.returnValues(discussionContainer); body.attr.and.returnValue(''); @@ -162,12 +205,8 @@ require('~/lib/utils/text_utility'); Notes.prototype.renderDiscussionNote.call(notes, note, $form); }); - it('should query for the notes list', () => { - expect(window.$.calls.argsFor(2)).toEqual(['ul.main-notes-list']); - }); - it('should call Notes.animateAppendNote', () => { - expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.discussion_html, $notesList); + expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.discussion_html, $('.main-notes-list')); }); }); @@ -175,16 +214,12 @@ require('~/lib/utils/text_utility'); beforeEach(() => { discussionContainer = { length: 1 }; - spyOn(window, '$').and.returnValues(discussionContainer); - $form.closest.and.returnValues(row); + $form.closest.and.returnValues(row, $form); + $form.find.and.returnValues(discussionContainer); Notes.prototype.renderDiscussionNote.call(notes, note, $form); }); - it('should query foor the discussion container', () => { - expect(window.$).toHaveBeenCalledWith(`.notes[data-discussion-id="${note.discussion_id}"]`); - }); - it('should call Notes.animateAppendNote', () => { expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, discussionContainer); }); @@ -193,35 +228,45 @@ require('~/lib/utils/text_utility'); describe('animateAppendNote', () => { let noteHTML; - let $note; let $notesList; + let $resultantNote; beforeEach(() => { noteHTML = '<div></div>'; - $note = jasmine.createSpyObj('$note', ['addClass', 'renderGFM', 'removeClass']); $notesList = jasmine.createSpyObj('$notesList', ['append']); - spyOn(window, '$').and.returnValue($note); - spyOn(window, 'setTimeout').and.callThrough(); - $note.addClass.and.returnValue($note); - $note.renderGFM.and.returnValue($note); + $resultantNote = Notes.animateAppendNote(noteHTML, $notesList); + }); - Notes.animateAppendNote(noteHTML, $notesList); + it('should have `fade-in` class', () => { + expect($resultantNote.hasClass('fade-in')).toEqual(true); }); - it('should init the note jquery object', () => { - expect(window.$).toHaveBeenCalledWith(noteHTML); + it('should append note to the notes list', () => { + expect($notesList.append).toHaveBeenCalledWith($resultantNote); }); + }); + + describe('animateUpdateNote', () => { + let noteHTML; + let $note; + let $updatedNote; - it('should call addClass', () => { - expect($note.addClass).toHaveBeenCalledWith('fade-in'); + beforeEach(() => { + noteHTML = '<div></div>'; + $note = jasmine.createSpyObj('$note', [ + 'replaceWith' + ]); + + $updatedNote = Notes.animateUpdateNote(noteHTML, $note); }); - it('should call renderGFM', () => { - expect($note.renderGFM).toHaveBeenCalledWith(); + + it('should have `fade-in` class', () => { + expect($updatedNote.hasClass('fade-in')).toEqual(true); }); - it('should append note to the notes list', () => { - expect($notesList.append).toHaveBeenCalledWith($note); + it('should call replaceWith on $note', () => { + expect($note.replaceWith).toHaveBeenCalledWith($updatedNote); }); }); }); diff --git a/spec/javascripts/pipelines/stage_spec.js b/spec/javascripts/pipelines/stage_spec.js index 2f1154bd999..a4f32a1faed 100644 --- a/spec/javascripts/pipelines/stage_spec.js +++ b/spec/javascripts/pipelines/stage_spec.js @@ -1,81 +1,86 @@ import Vue from 'vue'; -import { SUCCESS_SVG } from '~/ci_status_icons'; -import Stage from '~/pipelines/components/stage'; +import stage from '~/pipelines/components/stage.vue'; + +describe('Pipelines stage component', () => { + let StageComponent; + let component; + + beforeEach(() => { + StageComponent = Vue.extend(stage); + + component = new StageComponent({ + propsData: { + stage: { + status: { + group: 'success', + icon: 'icon_status_success', + title: 'success', + }, + dropdown_path: 'foo', + }, + updateDropdown: false, + }, + }).$mount(); + }); -function minify(string) { - return string.replace(/\s/g, ''); -} + it('should render a dropdown with the status icon', () => { + expect(component.$el.getAttribute('class')).toEqual('dropdown'); + expect(component.$el.querySelector('svg')).toBeDefined(); + expect(component.$el.querySelector('button').getAttribute('data-toggle')).toEqual('dropdown'); + }); -describe('Pipelines Stage', () => { - describe('data', () => { - let stageReturnValue; + describe('with successfull request', () => { + const interceptor = (request, next) => { + next(request.respondWith(JSON.stringify({ html: 'foo' }), { + status: 200, + })); + }; beforeEach(() => { - stageReturnValue = Stage.data(); + Vue.http.interceptors.push(interceptor); }); - it('should return object with .builds and .spinner', () => { - expect(stageReturnValue).toEqual({ - builds: '', - spinner: '<span class="fa fa-spinner fa-spin"></span>', - }); + afterEach(() => { + Vue.http.interceptors = _.without( + Vue.http.interceptors, interceptor, + ); }); - }); - describe('computed', () => { - describe('svgHTML', function () { - let stage; - let svgHTML; + it('should render the received data', (done) => { + component.$el.querySelector('button').click(); - beforeEach(() => { - stage = { stage: { status: { icon: 'icon_status_success' } } }; - - svgHTML = Stage.computed.svgHTML.call(stage); - }); - - it("should return the correct icon for the stage's status", () => { - expect(svgHTML).toBe(SUCCESS_SVG); - }); + setTimeout(() => { + expect( + component.$el.querySelector('.js-builds-dropdown-container ul').textContent.trim(), + ).toEqual('foo'); + done(); + }, 0); }); }); - describe('when mounted', () => { - let StageComponent; - let renderedComponent; - let stage; + describe('when request fails', () => { + const interceptor = (request, next) => { + next(request.respondWith(JSON.stringify({}), { + status: 500, + })); + }; beforeEach(() => { - stage = { status: { icon: 'icon_status_success' } }; - - StageComponent = Vue.extend(Stage); - - renderedComponent = new StageComponent({ - propsData: { - stage, - }, - }).$mount(); + Vue.http.interceptors.push(interceptor); }); - it('should render the correct status svg', () => { - const minifiedComponent = minify(renderedComponent.$el.outerHTML); - const expectedSVG = minify(SUCCESS_SVG); - - expect(minifiedComponent).toContain(expectedSVG); + afterEach(() => { + Vue.http.interceptors = _.without( + Vue.http.interceptors, interceptor, + ); }); - }); - - describe('when request fails', () => { - it('closes dropdown', () => { - spyOn($, 'ajax').and.callFake(options => options.error()); - const StageComponent = Vue.extend(Stage); - const component = new StageComponent({ - propsData: { stage: { status: { icon: 'foo' } } }, - }).$mount(); + it('should close the dropdown', () => { + component.$el.click(); - expect( - component.$el.classList.contains('open'), - ).toEqual(false); + setTimeout(() => { + expect(component.$el.classList.contains('open')).toEqual(false); + }, 0); }); }); }); diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index f127e45ae6a..c6e3524f743 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/spec/lib/gitlab/email/receiver_spec.rb @@ -4,6 +4,24 @@ require_relative 'email_shared_blocks' describe Gitlab::Email::Receiver, lib: true do include_context :email_shared_context + context "when the email contains a valid email address in a Delivered-To header" do + let(:email_raw) { fixture_file('emails/forwarded_new_issue.eml') } + let(:handler) { double(:handler) } + + before do + stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.adventuretime.ooo") + + allow(handler).to receive(:execute) + allow(handler).to receive(:metrics_params) + end + + it "finds the mail key" do + expect(Gitlab::Email::Handler).to receive(:for).with(an_instance_of(Mail::Message), 'gitlabhq/gitlabhq+auth_token').and_return(handler) + + receiver.execute + end + end + context "when we cannot find a capable handler" do let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "!!!") } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index ddedb7c3443..fea186fd4f4 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1062,7 +1062,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end it "allows ordering by date" do - expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_DATE) + expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_DATE | Rugged::SORT_TOPO) repository.find_commits(order: :date) end diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index 6e145947104..1035428b2e7 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:user) { create(:user) } - let(:project) { setup_project } + let!(:project) { setup_project } before do project.team << [user, :master] @@ -219,7 +219,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do releases: [release], group: group ) - project.update(description_html: 'description') + project.update_column(:description_html, 'description') project_label = create(:label, project: project) group_label = create(:group_label, group: group) create(:label_link, label: project_label, target: issue) diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 7e8a1c8add7..f84c6b48173 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -35,8 +35,68 @@ describe Blob do end end + describe '#external_storage_error?' do + context 'if the blob is stored in LFS' do + let(:blob) { fake_blob(path: 'file.pdf', lfs: true) } + + context 'when the project has LFS enabled' do + it 'returns false' do + expect(blob.external_storage_error?).to be_falsey + end + end + + context 'when the project does not have LFS enabled' do + before do + project.lfs_enabled = false + end + + it 'returns true' do + expect(blob.external_storage_error?).to be_truthy + end + end + end + + context 'if the blob is not stored in LFS' do + let(:blob) { fake_blob(path: 'file.md') } + + it 'returns false' do + expect(blob.external_storage_error?).to be_falsey + end + end + end + + describe '#stored_externally?' do + context 'if the blob is stored in LFS' do + let(:blob) { fake_blob(path: 'file.pdf', lfs: true) } + + context 'when the project has LFS enabled' do + it 'returns true' do + expect(blob.stored_externally?).to be_truthy + end + end + + context 'when the project does not have LFS enabled' do + before do + project.lfs_enabled = false + end + + it 'returns false' do + expect(blob.stored_externally?).to be_falsey + end + end + end + + context 'if the blob is not stored in LFS' do + let(:blob) { fake_blob(path: 'file.md') } + + it 'returns false' do + expect(blob.stored_externally?).to be_falsey + end + end + end + describe '#raw_binary?' do - context 'if the blob is a valid LFS pointer' do + context 'if the blob is stored externally' do context 'if the extension has a rich viewer' do context 'if the viewer is binary' do it 'returns true' do @@ -56,15 +116,63 @@ describe Blob do end context "if the extension doesn't have a rich viewer" do - it 'returns true' do - blob = fake_blob(path: 'file.exe', lfs: true) + context 'if the extension has a text mime type' do + context 'if the extension is for a programming language' do + it 'returns false' do + blob = fake_blob(path: 'file.txt', lfs: true) - expect(blob.raw_binary?).to be_truthy + expect(blob.raw_binary?).to be_falsey + end + end + + context 'if the extension is not for a programming language' do + it 'returns false' do + blob = fake_blob(path: 'file.ics', lfs: true) + + expect(blob.raw_binary?).to be_falsey + end + end + end + + context 'if the extension has a binary mime type' do + context 'if the extension is for a programming language' do + it 'returns false' do + blob = fake_blob(path: 'file.rb', lfs: true) + + expect(blob.raw_binary?).to be_falsey + end + end + + context 'if the extension is not for a programming language' do + it 'returns true' do + blob = fake_blob(path: 'file.exe', lfs: true) + + expect(blob.raw_binary?).to be_truthy + end + end + end + + context 'if the extension has an unknown mime type' do + context 'if the extension is for a programming language' do + it 'returns false' do + blob = fake_blob(path: 'file.ini', lfs: true) + + expect(blob.raw_binary?).to be_falsey + end + end + + context 'if the extension is not for a programming language' do + it 'returns true' do + blob = fake_blob(path: 'file.wtf', lfs: true) + + expect(blob.raw_binary?).to be_truthy + end + end end end end - context 'if the blob is not an LFS pointer' do + context 'if the blob is not stored externally' do context 'if the blob is binary' do it 'returns true' do blob = fake_blob(path: 'file.pdf', binary: true) @@ -94,7 +202,7 @@ describe Blob do describe '#simple_viewer' do context 'when the blob is empty' do it 'returns an empty viewer' do - blob = fake_blob(data: '') + blob = fake_blob(data: '', size: 0) expect(blob.simple_viewer).to be_a(BlobViewer::Empty) end @@ -118,7 +226,7 @@ describe Blob do end describe '#rich_viewer' do - context 'when the blob is an invalid LFS pointer' do + context 'when the blob has an external storage error' do before do project.lfs_enabled = false end @@ -138,7 +246,7 @@ describe Blob do end end - context 'when the blob is a valid LFS pointer' do + context 'when the blob is stored externally' do it 'returns a matching viewer' do blob = fake_blob(path: 'file.pdf', lfs: true) diff --git a/spec/models/blob_viewer/base_spec.rb b/spec/models/blob_viewer/base_spec.rb index a3e598de56d..740ad9d275e 100644 --- a/spec/models/blob_viewer/base_spec.rb +++ b/spec/models/blob_viewer/base_spec.rb @@ -139,7 +139,7 @@ describe BlobViewer::Base, model: true do end end - context 'when the viewer is server side but the blob is stored in LFS' do + context 'when the viewer is server side but the blob is stored externally' do let(:project) { build(:empty_project, lfs_enabled: true) } let(:blob) { fake_blob(path: 'file.pdf', lfs: true) } @@ -148,8 +148,8 @@ describe BlobViewer::Base, model: true do allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) end - it 'return :server_side_but_stored_in_lfs' do - expect(viewer.render_error).to eq(:server_side_but_stored_in_lfs) + it 'return :server_side_but_stored_externally' do + expect(viewer.render_error).to eq(:server_side_but_stored_externally) end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ce31c8ed94c..08b2169fea7 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -212,7 +212,7 @@ eos end end - describe '#latest_pipeline' do + describe '#last_pipeline' do let!(:first_pipeline) do create(:ci_empty_pipeline, project: project, @@ -226,8 +226,8 @@ eos status: 'success') end - it 'returns latest pipeline' do - expect(commit.latest_pipeline).to eq second_pipeline + it 'returns last pipeline' do + expect(commit.last_pipeline).to eq second_pipeline end end diff --git a/spec/models/diff_discussion_spec.rb b/spec/models/diff_discussion_spec.rb index 48e7c0a822c..81f338745b1 100644 --- a/spec/models/diff_discussion_spec.rb +++ b/spec/models/diff_discussion_spec.rb @@ -1,19 +1,86 @@ require 'spec_helper' describe DiffDiscussion, model: true do - subject { described_class.new([first_note, second_note, third_note]) } + include RepoHelpers - let(:first_note) { create(:diff_note_on_merge_request) } - let(:merge_request) { first_note.noteable } - let(:project) { first_note.project } - let(:second_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: first_note) } - let(:third_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: first_note) } + subject { described_class.new([diff_note]) } + + let(:project) { create(:project) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:diff_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } describe '#reply_attributes' do it 'includes position and original_position' do attributes = subject.reply_attributes - expect(attributes[:position]).to eq(first_note.position.to_json) - expect(attributes[:original_position]).to eq(first_note.original_position.to_json) + expect(attributes[:position]).to eq(diff_note.position.to_json) + expect(attributes[:original_position]).to eq(diff_note.original_position.to_json) + end + end + + describe '#merge_request_version_params' do + let(:merge_request) { create(:merge_request, source_project: project, target_project: project, importing: true) } + let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff2) { merge_request.merge_request_diffs.create(head_commit_sha: nil) } + let!(:merge_request_diff3) { merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + + context 'when the discussion is active' do + it 'returns an empty hash, which will end up showing the latest version' do + expect(subject.merge_request_version_params).to eq({}) + end + end + + context 'when the discussion is on an older merge request version' do + let(:position) do + Gitlab::Diff::Position.new( + old_path: ".gitmodules", + new_path: ".gitmodules", + old_line: nil, + new_line: 4, + diff_refs: merge_request_diff1.diff_refs + ) + end + + let(:diff_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, position: position) } + + before do + diff_note.position = diff_note.original_position + diff_note.save! + end + + it 'returns the diff ID for the version to show' do + expect(diff_id: merge_request_diff1.id) + end + end + + context 'when the discussion is on a comparison between merge request versions' do + let(:position) do + Gitlab::Diff::Position.new( + old_path: ".gitmodules", + new_path: ".gitmodules", + old_line: 4, + new_line: 4, + diff_refs: merge_request_diff3.compare_with(merge_request_diff1.head_commit_sha).diff_refs + ) + end + + let(:diff_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, position: position) } + + it 'returns the diff ID and start sha of the versions to compare' do + expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff3.id, start_sha: merge_request_diff1.head_commit_sha) + end + end + + context 'when the discussion does not have a merge request version' do + let(:diff_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, diff_refs: project.commit(sample_commit.id).diff_refs) } + + before do + diff_note.position = diff_note.original_position + diff_note.save! + end + + it 'returns nil' do + expect(subject.merge_request_version_params).to be_nil + end end end end diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index f32b6b99b3d..ab4c51a87b0 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -155,23 +155,6 @@ describe DiffNote, models: true do end end - describe '#latest_merge_request_diff' do - context 'when active' do - it 'returns the current merge request diff' do - expect(subject.latest_merge_request_diff).to eq(merge_request.merge_request_diff) - end - end - - context 'when outdated' do - let!(:old_merge_request_diff) { merge_request.merge_request_diff } - let!(:new_merge_request_diff) { merge_request.merge_request_diffs.create(diff_refs: commit.diff_refs) } - - it 'returns the latest merge request diff that this diff note applied to' do - expect(subject.latest_merge_request_diff).to eq(old_merge_request_diff) - end - end - end - describe "creation" do describe "updating of position" do context "when noteable is a commit" do @@ -256,4 +239,39 @@ describe DiffNote, models: true do end end end + + describe '#created_at_diff?' do + let(:diff_refs) { project.commit(sample_commit.id).diff_refs } + let(:position) do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: diff_refs + ) + end + + context "when noteable is a commit" do + subject { build(:diff_note_on_commit, project: project, position: position) } + + it "returns true" do + expect(subject.created_at_diff?(diff_refs)).to be true + end + end + + context "when noteable is a merge request" do + context "when the diff refs match the original one of the diff note" do + it "returns true" do + expect(subject.created_at_diff?(diff_refs)).to be true + end + end + + context "when the diff refs don't match the original one of the diff note" do + it "returns false" do + expect(subject.created_at_diff?(merge_request.diff_refs)).to be false + end + end + end + end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 11befd4edfe..8748b98a4e3 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -291,6 +291,27 @@ describe Issue, models: true do end end + describe '#has_related_branch?' do + let(:issue) { create(:issue, title: "Blue Bell Knoll") } + subject { issue.has_related_branch? } + + context 'branch found' do + before do + allow(issue.project.repository).to receive(:branch_names).and_return(["iceblink-luck", issue.to_branch_name]) + end + + it { is_expected.to eq true } + end + + context 'branch not found' do + before do + allow(issue.project.repository).to receive(:branch_names).and_return(["lazy-calm"]) + end + + it { is_expected.to eq false } + end + end + it_behaves_like 'an editable mentionable' do subject { create(:issue, project: create(:project, :repository)) } diff --git a/spec/models/legacy_diff_discussion_spec.rb b/spec/models/legacy_diff_discussion_spec.rb index 153e757a0ef..6eb4a2aaf39 100644 --- a/spec/models/legacy_diff_discussion_spec.rb +++ b/spec/models/legacy_diff_discussion_spec.rb @@ -8,4 +8,26 @@ describe LegacyDiffDiscussion, models: true do expect(subject.reply_attributes[:line_code]).to eq(subject.line_code) end end + + describe '#merge_request_version_params' do + context 'when the discussion is active' do + before do + allow(subject).to receive(:active?).and_return(true) + end + + it 'returns an empty hash, which will end up showing the latest version' do + expect(subject.merge_request_version_params).to eq({}) + end + end + + context 'when the discussion is outdated' do + before do + allow(subject).to receive(:active?).and_return(false) + end + + it 'returns nil' do + expect(subject.merge_request_version_params).to be_nil + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index be08b96641a..8b72125dd5d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1554,4 +1554,23 @@ describe MergeRequest, models: true do expect(subject.has_no_commits?).to be_truthy end end + + describe '#merge_request_diff_for' do + subject { create(:merge_request, importing: true) } + let!(:merge_request_diff1) { subject.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff2) { subject.merge_request_diffs.create(head_commit_sha: nil) } + let!(:merge_request_diff3) { subject.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + + context 'with diff refs' do + it 'returns the diffs' do + expect(subject.merge_request_diff_for(merge_request_diff1.diff_refs)).to eq(merge_request_diff1) + end + end + + context 'with a commit SHA' do + it 'returns the diffs' do + expect(subject.merge_request_diff_for(merge_request_diff3.head_commit_sha)).to eq(merge_request_diff3) + end + end + end end diff --git a/spec/models/network/graph_spec.rb b/spec/models/network/graph_spec.rb index 46b36e11c23..0fe8a591a45 100644 --- a/spec/models/network/graph_spec.rb +++ b/spec/models/network/graph_spec.rb @@ -10,17 +10,17 @@ describe Network::Graph, models: true do expect(graph.notes).to eq( { note_on_commit.commit_id => 1 } ) end - describe "#commits" do + describe '#commits' do let(:graph) { described_class.new(project, 'refs/heads/master', project.repository.commit, nil) } - it "returns a list of commits" do + it 'returns a list of commits' do commits = graph.commits expect(commits).not_to be_empty expect(commits).to all( be_kind_of(Network::Commit) ) end - it "sorts the commits by commit date (descending)" do + it 'it the commits by commit date (descending)' do # Remove duplicate timestamps because they make it harder to # assert that the commits are sorted as expected. commits = graph.commits.uniq(&:date) @@ -29,5 +29,20 @@ describe Network::Graph, models: true do expect(commits).not_to be_empty expect(commits.map(&:id)).to eq(sorted_commits.map(&:id)) end + + it 'sorts children before parents for commits with the same timestamp' do + commits_by_time = graph.commits.group_by(&:date) + + commits_by_time.each do |time, commits| + commit_ids = commits.map(&:id) + + commits.each_with_index do |commit, index| + parent_indexes = commit.parent_ids.map { |parent_id| commit_ids.find_index(parent_id) }.compact + + # All parents of the current commit should appear after it + expect(parent_indexes).to all( be > index ) + end + end + end end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 557ea97b008..7a01cef9b4b 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -272,9 +272,9 @@ describe Note, models: true do Gitlab::Diff::Position.new( old_path: "files/ruby/popen.rb", new_path: "files/ruby/popen.rb", - old_line: 16, - new_line: 22, - diff_refs: merge_request.diff_refs + old_line: nil, + new_line: 13, + diff_refs: project.commit(sample_commit.id).diff_refs ) end @@ -288,26 +288,78 @@ describe Note, models: true do ) end - subject { merge_request.notes.grouped_diff_discussions } + context 'active diff discussions' do + subject { merge_request.notes.grouped_diff_discussions } - it "includes active discussions" do - discussions = subject.values.flatten + it "includes active discussions" do + discussions = subject.values.flatten - expect(discussions.count).to eq(2) - expect(discussions.map(&:id)).to eq([active_diff_note1.discussion_id, active_diff_note3.discussion_id]) - expect(discussions.all?(&:active?)).to be true + expect(discussions.count).to eq(2) + expect(discussions.map(&:id)).to eq([active_diff_note1.discussion_id, active_diff_note3.discussion_id]) + expect(discussions.all?(&:active?)).to be true - expect(discussions.first.notes).to eq([active_diff_note1, active_diff_note2]) - expect(discussions.last.notes).to eq([active_diff_note3]) - end + expect(discussions.first.notes).to eq([active_diff_note1, active_diff_note2]) + expect(discussions.last.notes).to eq([active_diff_note3]) + end - it "doesn't include outdated discussions" do - expect(subject.values.flatten.map(&:id)).not_to include(outdated_diff_note1.discussion_id) + it "doesn't include outdated discussions" do + expect(subject.values.flatten.map(&:id)).not_to include(outdated_diff_note1.discussion_id) + end + + it "groups the discussions by line code" do + expect(subject[active_diff_note1.line_code].first.id).to eq(active_diff_note1.discussion_id) + expect(subject[active_diff_note3.line_code].first.id).to eq(active_diff_note3.discussion_id) + end end - it "groups the discussions by line code" do - expect(subject[active_diff_note1.line_code].first.id).to eq(active_diff_note1.discussion_id) - expect(subject[active_diff_note3.line_code].first.id).to eq(active_diff_note3.discussion_id) + context 'diff discussions for older diff refs' do + subject { merge_request.notes.grouped_diff_discussions(diff_refs) } + + context 'for diff refs a discussion was created at' do + let(:diff_refs) { active_position2.diff_refs } + + it "includes discussions that were created then" do + discussions = subject.values.flatten + + expect(discussions.count).to eq(1) + + discussion = discussions.first + + expect(discussion.id).to eq(active_diff_note3.discussion_id) + expect(discussion.active?).to be true + expect(discussion.active?(diff_refs)).to be false + expect(discussion.created_at_diff?(diff_refs)).to be true + + expect(discussion.notes).to eq([active_diff_note3]) + end + + it "groups the discussions by original line code" do + expect(subject[active_diff_note3.original_line_code].first.id).to eq(active_diff_note3.discussion_id) + end + end + + context 'for diff refs a discussion was last active at' do + let(:diff_refs) { outdated_position.diff_refs } + + it "includes discussions that were last active" do + discussions = subject.values.flatten + + expect(discussions.count).to eq(1) + + discussion = discussions.first + + expect(discussion.id).to eq(outdated_diff_note1.discussion_id) + expect(discussion.active?).to be false + expect(discussion.active?(diff_refs)).to be true + expect(discussion.created_at_diff?(diff_refs)).to be true + + expect(discussion.notes).to eq([outdated_diff_note1, outdated_diff_note2]) + end + + it "groups the discussions by line code" do + expect(subject[outdated_diff_note1.line_code].first.id).to eq(outdated_diff_note1.discussion_id) + end + end end end diff --git a/spec/models/project_services/chat_message/pipeline_message_spec.rb b/spec/models/project_services/chat_message/pipeline_message_spec.rb index ec5c6c5e0ed..e005be42b0d 100644 --- a/spec/models/project_services/chat_message/pipeline_message_spec.rb +++ b/spec/models/project_services/chat_message/pipeline_message_spec.rb @@ -4,6 +4,7 @@ describe ChatMessage::PipelineMessage do subject { described_class.new(args) } let(:user) { { name: 'hacker' } } + let(:duration) { 7210 } let(:args) do { object_attributes: { @@ -26,7 +27,6 @@ describe ChatMessage::PipelineMessage do context 'pipeline succeeded' do let(:status) { 'success' } let(:color) { 'good' } - let(:duration) { 10 } let(:message) { build_message('passed') } it 'returns a message with information about succeeded build' do @@ -39,7 +39,6 @@ describe ChatMessage::PipelineMessage do context 'pipeline failed' do let(:status) { 'failed' } let(:color) { 'danger' } - let(:duration) { 10 } let(:message) { build_message } it 'returns a message with information about failed build' do @@ -64,7 +63,7 @@ describe ChatMessage::PipelineMessage do "<http://example.gitlab.com|project_name>:" \ " Pipeline <http://example.gitlab.com/pipelines/123|#123>" \ " of <http://example.gitlab.com/commits/develop|develop> branch" \ - " by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" + " by #{name} #{status_text} in 02:00:10" end end @@ -76,7 +75,6 @@ describe ChatMessage::PipelineMessage do context 'pipeline succeeded' do let(:status) { 'success' } let(:color) { 'good' } - let(:duration) { 10 } let(:message) { build_markdown_message('passed') } it 'returns a message with information about succeeded build' do @@ -85,7 +83,7 @@ describe ChatMessage::PipelineMessage do expect(subject.activity).to eq({ title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of [develop](http://example.gitlab.com/commits/develop) branch by hacker passed', subtitle: 'in [project_name](http://example.gitlab.com)', - text: 'in 10 seconds', + text: 'in 02:00:10', image: '' }) end @@ -94,7 +92,6 @@ describe ChatMessage::PipelineMessage do context 'pipeline failed' do let(:status) { 'failed' } let(:color) { 'danger' } - let(:duration) { 10 } let(:message) { build_markdown_message } it 'returns a message with information about failed build' do @@ -103,7 +100,7 @@ describe ChatMessage::PipelineMessage do expect(subject.activity).to eq({ title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of [develop](http://example.gitlab.com/commits/develop) branch by hacker failed', subtitle: 'in [project_name](http://example.gitlab.com)', - text: 'in 10 seconds', + text: 'in 02:00:10', image: '' }) end @@ -118,7 +115,7 @@ describe ChatMessage::PipelineMessage do expect(subject.activity).to eq({ title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of [develop](http://example.gitlab.com/commits/develop) branch by API failed', subtitle: 'in [project_name](http://example.gitlab.com)', - text: 'in 10 seconds', + text: 'in 02:00:10', image: '' }) end @@ -129,7 +126,7 @@ describe ChatMessage::PipelineMessage do "[project_name](http://example.gitlab.com):" \ " Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ " of [develop](http://example.gitlab.com/commits/develop)" \ - " branch by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" + " branch by #{name} #{status_text} in 02:00:10" end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 5216764a82d..dd6514b3b50 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1098,21 +1098,33 @@ describe Repository, models: true do end describe '#merge' do - it 'merges the code and return the commit id' do + let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project) } + + let(:commit_options) do + author = repository.user_to_committer(user) + { message: 'Test \r\n\r\n message', committer: author, author: author } + end + + it 'merges the code and returns the commit id' do expect(merge_commit).to be_present expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present end it 'sets the `in_progress_merge_commit_sha` flag for the given merge request' do - merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project) - - merge_commit_id = repository.merge(user, - merge_request.diff_head_sha, - merge_request, - commit_options) + merge_commit_id = merge(repository, user, merge_request, commit_options) expect(merge_request.in_progress_merge_commit_sha).to eq(merge_commit_id) end + + it 'removes carriage returns from commit message' do + merge_commit_id = merge(repository, user, merge_request, commit_options) + + expect(repository.commit(merge_commit_id).message).to eq(commit_options[:message].delete("\r")) + end + + def merge(repository, user, merge_request, options = {}) + repository.merge(user, merge_request.diff_head_sha, merge_request, options) + end end describe '#revert' do diff --git a/spec/policies/personal_snippet_policy_spec.rb b/spec/policies/personal_snippet_policy_spec.rb new file mode 100644 index 00000000000..58aa1145c9e --- /dev/null +++ b/spec/policies/personal_snippet_policy_spec.rb @@ -0,0 +1,141 @@ +require 'spec_helper' + +describe PersonalSnippetPolicy, models: true do + let(:regular_user) { create(:user) } + let(:external_user) { create(:user, :external) } + let(:admin_user) { create(:user, :admin) } + + let(:author_permissions) do + [ + :update_personal_snippet, + :admin_personal_snippet, + :destroy_personal_snippet + ] + end + + def permissions(user) + described_class.abilities(user, snippet).to_set + end + + context 'public snippet' do + let(:snippet) { create(:personal_snippet, :public) } + + context 'no user' do + subject { permissions(nil) } + + it do + is_expected.to include(:read_personal_snippet) + is_expected.not_to include(:comment_personal_snippet) + is_expected.not_to include(*author_permissions) + end + end + + context 'regular user' do + subject { permissions(regular_user) } + + it do + is_expected.to include(:read_personal_snippet) + is_expected.to include(:comment_personal_snippet) + is_expected.not_to include(*author_permissions) + end + end + + context 'author' do + subject { permissions(snippet.author) } + + it do + is_expected.to include(:read_personal_snippet) + is_expected.to include(:comment_personal_snippet) + is_expected.to include(*author_permissions) + end + end + end + + context 'internal snippet' do + let(:snippet) { create(:personal_snippet, :internal) } + + context 'no user' do + subject { permissions(nil) } + + it do + is_expected.not_to include(:read_personal_snippet) + is_expected.not_to include(:comment_personal_snippet) + is_expected.not_to include(*author_permissions) + end + end + + context 'regular user' do + subject { permissions(regular_user) } + + it do + is_expected.to include(:read_personal_snippet) + is_expected.to include(:comment_personal_snippet) + is_expected.not_to include(*author_permissions) + end + end + + context 'external user' do + subject { permissions(external_user) } + + it do + is_expected.not_to include(:read_personal_snippet) + is_expected.not_to include(:comment_personal_snippet) + is_expected.not_to include(*author_permissions) + end + end + + context 'snippet author' do + subject { permissions(snippet.author) } + + it do + is_expected.to include(:read_personal_snippet) + is_expected.to include(:comment_personal_snippet) + is_expected.to include(*author_permissions) + end + end + end + + context 'private snippet' do + let(:snippet) { create(:project_snippet, :private) } + + context 'no user' do + subject { permissions(nil) } + + it do + is_expected.not_to include(:read_personal_snippet) + is_expected.not_to include(:comment_personal_snippet) + is_expected.not_to include(*author_permissions) + end + end + + context 'regular user' do + subject { permissions(regular_user) } + + it do + is_expected.not_to include(:read_personal_snippet) + is_expected.not_to include(:comment_personal_snippet) + is_expected.not_to include(*author_permissions) + end + end + + context 'external user' do + subject { permissions(external_user) } + + it do + is_expected.not_to include(:read_personal_snippet) + is_expected.not_to include(:comment_personal_snippet) + is_expected.not_to include(*author_permissions) + end + end + + context 'snippet author' do + subject { permissions(snippet.author) } + + it do + is_expected.to include(:read_personal_snippet) + is_expected.to include(:comment_personal_snippet) + is_expected.to include(*author_permissions) + end + end + end +end diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb index 762345cd41c..f9e5316b3de 100644 --- a/spec/requests/api/pipelines_spec.rb +++ b/spec/requests/api/pipelines_spec.rb @@ -24,6 +24,245 @@ describe API::Pipelines do expect(json_response.first['id']).to eq pipeline.id expect(json_response.first.keys).to contain_exactly(*%w[id sha ref status]) end + + context 'when parameter is passed' do + %w[running pending].each do |target| + context "when scope is #{target}" do + before do + create(:ci_pipeline, project: project, status: target) + end + + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), scope: target + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).not_to be_empty + json_response.each { |r| expect(r['status']).to eq(target) } + end + end + end + + context 'when scope is finished' do + before do + create(:ci_pipeline, project: project, status: 'success') + create(:ci_pipeline, project: project, status: 'failed') + create(:ci_pipeline, project: project, status: 'canceled') + end + + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), scope: 'finished' + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).not_to be_empty + json_response.each { |r| expect(r['status']).to be_in(%w[success failed canceled]) } + end + end + + context 'when scope is branches or tags' do + let!(:pipeline_branch) { create(:ci_pipeline, project: project) } + let!(:pipeline_tag) { create(:ci_pipeline, project: project, ref: 'v1.0.0', tag: true) } + + context 'when scope is branches' do + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), scope: 'branches' + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).not_to be_empty + expect(json_response.last['id']).to eq(pipeline_branch.id) + end + end + + context 'when scope is tags' do + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), scope: 'tags' + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).not_to be_empty + expect(json_response.last['id']).to eq(pipeline_tag.id) + end + end + end + + context 'when scope is invalid' do + it 'returns bad_request' do + get api("/projects/#{project.id}/pipelines", user), scope: 'invalid-scope' + + expect(response).to have_http_status(:bad_request) + end + end + + HasStatus::AVAILABLE_STATUSES.each do |target| + context "when status is #{target}" do + before do + create(:ci_pipeline, project: project, status: target) + exception_status = HasStatus::AVAILABLE_STATUSES - [target] + create(:ci_pipeline, project: project, status: exception_status.sample) + end + + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), status: target + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).not_to be_empty + json_response.each { |r| expect(r['status']).to eq(target) } + end + end + end + + context 'when status is invalid' do + it 'returns bad_request' do + get api("/projects/#{project.id}/pipelines", user), status: 'invalid-status' + + expect(response).to have_http_status(:bad_request) + end + end + + context 'when ref is specified' do + before do + create(:ci_pipeline, project: project) + end + + context 'when ref exists' do + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), ref: 'master' + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).not_to be_empty + json_response.each { |r| expect(r['ref']).to eq('master') } + end + end + + context 'when ref does not exist' do + it 'returns empty' do + get api("/projects/#{project.id}/pipelines", user), ref: 'invalid-ref' + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_empty + end + end + end + + context 'when name is specified' do + let!(:pipeline) { create(:ci_pipeline, project: project, user: user) } + + context 'when name exists' do + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), name: user.name + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.first['id']).to eq(pipeline.id) + end + end + + context 'when name does not exist' do + it 'returns empty' do + get api("/projects/#{project.id}/pipelines", user), name: 'invalid-name' + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_empty + end + end + end + + context 'when username is specified' do + let!(:pipeline) { create(:ci_pipeline, project: project, user: user) } + + context 'when username exists' do + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), username: user.username + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.first['id']).to eq(pipeline.id) + end + end + + context 'when username does not exist' do + it 'returns empty' do + get api("/projects/#{project.id}/pipelines", user), username: 'invalid-username' + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_empty + end + end + end + + context 'when yaml_errors is specified' do + let!(:pipeline1) { create(:ci_pipeline, project: project, yaml_errors: 'Syntax error') } + let!(:pipeline2) { create(:ci_pipeline, project: project) } + + context 'when yaml_errors is true' do + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), yaml_errors: true + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.first['id']).to eq(pipeline1.id) + end + end + + context 'when yaml_errors is false' do + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), yaml_errors: false + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.first['id']).to eq(pipeline2.id) + end + end + + context 'when yaml_errors is invalid' do + it 'returns bad_request' do + get api("/projects/#{project.id}/pipelines", user), yaml_errors: 'invalid-yaml_errors' + + expect(response).to have_http_status(:bad_request) + end + end + end + + context 'when order_by and sort are specified' do + context 'when order_by user_id' do + let!(:pipeline) { create_list(:ci_pipeline, 2, project: project, user: create(:user)) } + + it 'sorts as user_id: :asc' do + get api("/projects/#{project.id}/pipelines", user), order_by: 'user_id', sort: 'asc' + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).not_to be_empty + pipeline.sort_by { |p| p.user.id }.tap do |sorted_pipeline| + json_response.each_with_index { |r, i| expect(r['id']).to eq(sorted_pipeline[i].id) } + end + end + + context 'when sort is invalid' do + it 'returns bad_request' do + get api("/projects/#{project.id}/pipelines", user), order_by: 'user_id', sort: 'invalid_sort' + + expect(response).to have_http_status(:bad_request) + end + end + end + + context 'when order_by is invalid' do + it 'returns bad_request' do + get api("/projects/#{project.id}/pipelines", user), order_by: 'lock_version', sort: 'asc' + + expect(response).to have_http_status(:bad_request) + end + end + end + end end context 'unauthorized user' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index cc03d7a933b..ab70ce5cd2f 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -665,6 +665,20 @@ describe API::Projects do }) end + it "does not include statistics by default" do + get api("/projects/#{project.id}", user) + + expect(response).to have_http_status(200) + expect(json_response).not_to include 'statistics' + end + + it "includes statistics if requested" do + get api("/projects/#{project.id}", user), statistics: true + + expect(response).to have_http_status(200) + expect(json_response).to include 'statistics' + end + describe 'permissions' do context 'all projects' do before { project.team << [user, :master] } diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb new file mode 100644 index 00000000000..1588d30c394 --- /dev/null +++ b/spec/services/merge_requests/create_from_issue_service_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe MergeRequests::CreateFromIssueService, services: true do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } + + subject(:service) { described_class.new(project, user, issue_iid: issue.iid) } + + before do + project.add_developer(user) + end + + describe '#execute' do + it 'returns an error with invalid issue iid' do + result = described_class.new(project, user, issue_iid: -1).execute + + expect(result[:status]).to eq :error + expect(result[:message]).to eq 'Invalid issue iid' + end + + it 'delegates issue search to IssuesFinder' do + expect_any_instance_of(IssuesFinder).to receive(:execute).once.and_call_original + + described_class.new(project, user, issue_iid: -1).execute + end + + it 'delegates the branch creation to CreateBranchService' do + expect_any_instance_of(CreateBranchService).to receive(:execute).once.and_call_original + + service.execute + end + + it 'creates a branch based on issue title' do + service.execute + + expect(project.repository.branch_exists?(issue.to_branch_name)).to be_truthy + end + + it 'creates a system note' do + expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, issue.to_branch_name) + + service.execute + end + + it 'creates a merge request' do + expect { service.execute }.to change(project.merge_requests, :count).by(1) + end + + it 'sets the merge request title to: "WIP: Resolves "$issue-title"' do + result = service.execute + + expect(result[:merge_request].title).to eq("WIP: Resolve \"#{issue.title}\"") + end + + it 'sets the merge request author to current user' do + result = service.execute + + expect(result[:merge_request].author).to eq user + end + + it 'sets the merge request source branch to the new issue branch' do + result = service.execute + + expect(result[:merge_request].source_branch).to eq issue.to_branch_name + end + + it 'sets the merge request target branch to the project default branch' do + result = service.execute + + expect(result[:merge_request].target_branch).to eq project.default_branch + end + end +end diff --git a/spec/services/projects/upload_service_spec.rb b/spec/services/upload_service_spec.rb index d2cefa46bfa..95ba28dbecd 100644 --- a/spec/services/projects/upload_service_spec.rb +++ b/spec/services/upload_service_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Projects::UploadService, services: true do +describe UploadService, services: true do describe 'File service' do before do @user = create(:user) @@ -68,6 +68,6 @@ describe Projects::UploadService, services: true do end def upload_file(project, file) - Projects::UploadService.new(project, file).execute + described_class.new(project, file, FileUploader).execute end end diff --git a/spec/support/helpers/fake_blob_helpers.rb b/spec/support/helpers/fake_blob_helpers.rb index b29af732ad3..bc9686ed9cf 100644 --- a/spec/support/helpers/fake_blob_helpers.rb +++ b/spec/support/helpers/fake_blob_helpers.rb @@ -1,6 +1,6 @@ module FakeBlobHelpers class FakeBlob - include Linguist::BlobHelper + include BlobLike attr_reader :path, :size, :data, :lfs_oid, :lfs_size @@ -19,10 +19,6 @@ module FakeBlobHelpers alias_method :name, :path - def mode - nil - end - def id 0 end @@ -31,17 +27,11 @@ module FakeBlobHelpers @binary end - def load_all_data!(repository) - # No-op + def external_storage + :lfs if @lfs_pointer end - def lfs_pointer? - @lfs_pointer - end - - def truncated? - false - end + alias_method :external_size, :lfs_size end def fake_blob(**kwargs) diff --git a/spec/tasks/gitlab/shell_rake_spec.rb b/spec/tasks/gitlab/shell_rake_spec.rb index 226d34fe2c9..ee3614c50f6 100644 --- a/spec/tasks/gitlab/shell_rake_spec.rb +++ b/spec/tasks/gitlab/shell_rake_spec.rb @@ -11,6 +11,10 @@ describe 'gitlab:shell rake tasks' do it 'invokes create_hooks task' do expect(Rake::Task['gitlab:shell:create_hooks']).to receive(:invoke) + storages = Gitlab.config.repositories.storages.values.map { |rs| rs['path'] } + expect(Kernel).to receive(:system).with('bin/install', *storages).and_call_original + expect(Kernel).to receive(:system).with('bin/compile').and_call_original + run_rake_task('gitlab:shell:install') end end diff --git a/spec/uploaders/personal_file_uploader_spec.rb b/spec/uploaders/personal_file_uploader_spec.rb new file mode 100644 index 00000000000..fb92f2ae3ab --- /dev/null +++ b/spec/uploaders/personal_file_uploader_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe PersonalFileUploader do + let(:uploader) { described_class.new(build_stubbed(:empty_project)) } + let(:snippet) { create(:personal_snippet) } + + describe '.absolute_path' do + it 'returns the correct absolute path by building it dynamically' do + upload = double(model: snippet, path: 'secret/foo.jpg') + + dynamic_segment = "personal_snippet/#{snippet.id}" + + expect(described_class.absolute_path(upload)).to end_with("#{dynamic_segment}/secret/foo.jpg") + end + end + + describe '#to_h' do + it 'returns the hass' do + uploader = described_class.new(snippet, 'secret') + + allow(uploader).to receive(:file).and_return(double(extension: 'txt', filename: 'file_name')) + expected_url = "/uploads/personal_snippet/#{snippet.id}/secret/file_name" + + expect(uploader.to_h).to eq( + alt: 'file_name', + url: expected_url, + markdown: "[file_name](#{expected_url})" + ) + end + end +end diff --git a/spec/views/projects/tags/index.html.haml_spec.rb b/spec/views/projects/tags/index.html.haml_spec.rb new file mode 100644 index 00000000000..33122365e9a --- /dev/null +++ b/spec/views/projects/tags/index.html.haml_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe 'projects/tags/index', :view do + let(:project) { create(:project) } + + before do + assign(:project, project) + assign(:repository, project.repository) + assign(:tags, []) + + allow(view).to receive(:current_ref).and_return('master') + allow(view).to receive(:can?).and_return(false) + end + + it 'defaults sort dropdown toggle to last updated' do + render + + expect(rendered).to have_button('Last updated') + end +end |
