diff options
114 files changed, 2156 insertions, 1280 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f506b1b375e..24627b5faca 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -333,6 +333,7 @@ migration paths: - bundle install --without postgres production --jobs $(nproc) $FLAGS --retry=3 - bundle exec rake db:drop db:create db:schema:load db:seed_fu - git checkout $CI_COMMIT_SHA + - bundle install --without postgres production --jobs $(nproc) $FLAGS --retry=3 - source scripts/prepare_build.sh - bundle exec rake db:migrate diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a285e8ab74f..275c0cd1777 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -82,7 +82,7 @@ If a contributor is no longer actively working on a submitted merge request we can decide that the merge request will be finished by one of our [Merge request coaches][team] or close the merge request. We make this decision based on how important the change is for our product vision. If a Merge request -coach is going to finish the merge request we assign the +coach is going to finish the merge request we assign the ~"coach will finish" label. ## Helping others @@ -479,8 +479,7 @@ merge request: 1. [Rails](https://github.com/bbatsov/rails-style-guide) 1. [Newlines styleguide][newlines-styleguide] 1. [Testing](doc/development/testing.md) -1. [JavaScript (ES6)](https://github.com/airbnb/javascript) -1. [JavaScript (ES5)](https://github.com/airbnb/javascript/tree/es5-deprecated/es5) +1. [JavaScript styleguide][js-styleguide] 1. [SCSS styleguide][scss-styleguide] 1. [Shell commands](doc/development/shell_commands.md) created by GitLab contributors to enhance security @@ -549,7 +548,8 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor [rss-naming]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#naming [changelog]: doc/development/changelog.md "Generate a changelog entry" [doc-styleguide]: doc/development/doc_styleguide.md "Documentation styleguide" -[scss-styleguide]: doc/development/scss_styleguide.md "SCSS styleguide" +[js-styleguide]: doc/development/fe_guide/style_guide_js.md "JavaScript styleguide" +[scss-styleguide]: doc/development/fe_guide/style_guide_scss.md "SCSS styleguide" [newlines-styleguide]: doc/development/newlines_styleguide.md "Newlines styleguide" [UX Guide for GitLab]: http://docs.gitlab.com/ce/development/ux_guide/ [license-finder-doc]: doc/development/licensing.md @@ -327,6 +327,7 @@ group :test do gem 'test_after_commit', '~> 1.1' gem 'sham_rack', '~> 1.3.6' gem 'timecop', '~> 0.8.0' + gem 'concurrent-ruby', '~> 1.0.5' end gem 'octokit', '~> 4.6.2' diff --git a/Gemfile.lock b/Gemfile.lock index 043ca4f8800..07be5d7aded 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -128,7 +128,7 @@ GEM execjs coffee-script-source (1.10.0) colorize (0.7.7) - concurrent-ruby (1.0.4) + concurrent-ruby (1.0.5) connection_pool (2.2.1) crack (0.4.3) safe_yaml (~> 1.0.0) @@ -868,6 +868,7 @@ DEPENDENCIES chronic (~> 0.10.2) chronic_duration (~> 0.10.6) coffee-rails (~> 4.1.0) + concurrent-ruby (~> 1.0.5) connection_pool (~> 2.0) creole (~> 0.5.0) d3_rails (~> 3.5.0) diff --git a/app/assets/javascripts/activities.js b/app/assets/javascripts/activities.js index aebda7780e1..d816df831eb 100644 --- a/app/assets/javascripts/activities.js +++ b/app/assets/javascripts/activities.js @@ -1,6 +1,7 @@ /* eslint-disable no-param-reassign, class-methods-use-this */ /* global Pager */ -/* global Cookies */ + +import Cookies from 'js-cookie'; class Activities { constructor() { diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index 9349918f7a0..c743dd551d7 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -1,4 +1,4 @@ -/* global Cookies */ +import Cookies from 'js-cookie'; import emojiMap from 'emojis/digests.json'; import emojiAliases from 'emojis/aliases.json'; diff --git a/app/assets/javascripts/behaviors/toggler_behavior.js b/app/assets/javascripts/behaviors/toggler_behavior.js index 92f3bb3ff52..86927314dd4 100644 --- a/app/assets/javascripts/behaviors/toggler_behavior.js +++ b/app/assets/javascripts/behaviors/toggler_behavior.js @@ -24,7 +24,7 @@ $('body').on('click', '.js-toggle-button', function(e) { toggleContainer($(this).closest('.js-toggle-container')); - const targetTag = e.target.tagName.toLowerCase(); + const targetTag = e.currentTarget.tagName.toLowerCase(); if (targetTag === 'a' || targetTag === 'button') { e.preventDefault(); } diff --git a/app/assets/javascripts/boards/components/board_blank_state.js b/app/assets/javascripts/boards/components/board_blank_state.js index 52893d4642b..3fc68457961 100644 --- a/app/assets/javascripts/boards/components/board_blank_state.js +++ b/app/assets/javascripts/boards/components/board_blank_state.js @@ -1,5 +1,7 @@ /* global ListLabel */ -/* global Cookies */ + +import Cookies from 'js-cookie'; + const Store = gl.issueBoards.BoardsStore; export default { diff --git a/app/assets/javascripts/boards/components/board_list.js b/app/assets/javascripts/boards/components/board_list.js index 9248d036497..86e6c26e570 100644 --- a/app/assets/javascripts/boards/components/board_list.js +++ b/app/assets/javascripts/boards/components/board_list.js @@ -48,7 +48,7 @@ import boardCard from './board_card'; this.list.getIssues(false); } - if (this.scrollHeight() > this.listHeight()) { + if (this.scrollHeight() > Math.ceil(this.listHeight())) { this.showCount = true; } else { this.showCount = false; diff --git a/app/assets/javascripts/boards/components/modal/filters.js b/app/assets/javascripts/boards/components/modal/filters.js index bd394a2318c..2e22b1eca47 100644 --- a/app/assets/javascripts/boards/components/modal/filters.js +++ b/app/assets/javascripts/boards/components/modal/filters.js @@ -14,6 +14,8 @@ export default { this.filteredSearch = new FilteredSearchBoards(this.store); this.filteredSearch.removeTokens(); + this.filteredSearch.handleInputPlaceholder(); + this.filteredSearch.toggleClearSearchButton(); }, beforeDestroy() { this.filteredSearch.cleanup(); diff --git a/app/assets/javascripts/boards/filtered_search_boards.js b/app/assets/javascripts/boards/filtered_search_boards.js index 101732309ea..1264280284c 100644 --- a/app/assets/javascripts/boards/filtered_search_boards.js +++ b/app/assets/javascripts/boards/filtered_search_boards.js @@ -28,6 +28,8 @@ export default class FilteredSearchBoards extends gl.FilteredSearchManager { [].forEach.call(tokens, (el) => { el.parentNode.removeChild(el); }); + + this.filteredSearchInput.value = ''; } updateTokens() { diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index 28ecb322df7..8912f234aa6 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -1,7 +1,8 @@ /* eslint-disable comma-dangle, space-before-function-paren, one-var, no-shadow, dot-notation, max-len */ -/* global Cookies */ /* global List */ +import Cookies from 'js-cookie'; + (() => { window.gl = window.gl || {}; window.gl.issueBoards = window.gl.issueBoards || {}; diff --git a/app/assets/javascripts/commit/pipelines/pipelines_table.js b/app/assets/javascripts/commit/pipelines/pipelines_table.js index 832c4b1bd2a..360fb39dc9c 100644 --- a/app/assets/javascripts/commit/pipelines/pipelines_table.js +++ b/app/assets/javascripts/commit/pipelines/pipelines_table.js @@ -87,7 +87,7 @@ export default Vue.component('pipelines-table', { }, template: ` - <div class="pipelines"> + <div class="content-list pipelines"> <div class="realtime-loading" v-if="isLoading"> <i class="fa fa-spinner fa-spin"></i> </div> @@ -99,7 +99,7 @@ export default Vue.component('pipelines-table', { </h2> </div> - <div class="table-holder pipelines" + <div class="table-holder" v-if="!isLoading && state.pipelines.length > 0"> <pipelines-table-component :pipelines="state.pipelines" diff --git a/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js b/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js index 18b682fd5cb..ae17d05e679 100644 --- a/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js +++ b/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js @@ -1,9 +1,8 @@ -/* global Cookies */ /* global Flash */ import Vue from 'vue'; +import Cookies from 'js-cookie'; -window.Cookies = require('js-cookie'); require('./components/stage_code_component'); require('./components/stage_issue_component'); require('./components/stage_plan_component'); diff --git a/app/assets/javascripts/diff_notes/components/diff_note_avatars.js b/app/assets/javascripts/diff_notes/components/diff_note_avatars.js index dd7081aefb7..0297add94d5 100644 --- a/app/assets/javascripts/diff_notes/components/diff_note_avatars.js +++ b/app/assets/javascripts/diff_notes/components/diff_note_avatars.js @@ -1,4 +1,6 @@ -/* global CommentsStore Cookies notes */ +/* global CommentsStore */ +/* global notes */ + import Vue from 'vue'; import collapseIcon from '../icons/collapse_icon.svg'; diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 3557f6f617e..d1a662459e1 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -41,9 +41,9 @@ import GroupsList from './groups_list'; import ProjectsList from './projects_list'; import MiniPipelineGraph from './mini_pipeline_graph_dropdown'; import BlobLinePermalinkUpdater from './blob/blob_line_permalink_updater'; +import UserCallout from './user_callout'; const ShortcutsBlob = require('./shortcuts_blob'); -const UserCallout = require('./user_callout'); (function() { var Dispatcher; diff --git a/app/assets/javascripts/filtered_search/filtered_search_manager.js b/app/assets/javascripts/filtered_search/filtered_search_manager.js index c6bb7fda8f2..22352950452 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_manager.js @@ -384,7 +384,7 @@ import FilteredSearchContainer from './container'; paths.push(`search=${sanitized}`); } - const parameterizedUrl = `?scope=all&utf8=✓&${paths.join('&')}`; + const parameterizedUrl = `?scope=all&utf8=%E2%9C%93&${paths.join('&')}`; if (this.updateObject) { this.updateObject(parameterizedUrl); diff --git a/app/assets/javascripts/issuable_context.js b/app/assets/javascripts/issuable_context.js index 115312d4b83..834b98e8601 100644 --- a/app/assets/javascripts/issuable_context.js +++ b/app/assets/javascripts/issuable_context.js @@ -1,8 +1,9 @@ /* eslint-disable func-names, space-before-function-paren, wrap-iife, no-new, comma-dangle, quotes, prefer-arrow-callback, consistent-return, one-var, no-var, one-var-declaration-per-line, no-underscore-dangle, max-len */ /* global UsersSelect */ -/* global Cookies */ /* global bp */ +import Cookies from 'js-cookie'; + (function() { this.IssuableContext = (function() { function IssuableContext(currentUser) { diff --git a/app/assets/javascripts/lib/utils/poll.js b/app/assets/javascripts/lib/utils/poll.js new file mode 100644 index 00000000000..c30a1fcb5da --- /dev/null +++ b/app/assets/javascripts/lib/utils/poll.js @@ -0,0 +1,73 @@ +import httpStatusCodes from './http_status'; + +/** + * Polling utility for handling realtime updates. + * Service for vue resouce and method need to be provided as props + * + * @example + * new poll({ + * resource: resource, + * method: 'name', + * data: {page: 1, scope: 'all'}, + * successCallback: () => {}, + * errorCallback: () => {}, + * }).makeRequest(); + * + * this.service = new BoardsService(endpoint); + * new poll({ + * resource: this.service, + * method: 'get', + * data: {page: 1, scope: 'all'}, + * successCallback: () => {}, + * errorCallback: () => {}, + * }).makeRequest(); + * + * + * 1. Checks for response and headers before start polling + * 2. Interval is provided by `Poll-Interval` header. + * 3. If `Poll-Interval` is -1, we stop polling + * 4. If HTTP response is 200, we poll. + * 5. If HTTP response is different from 200, we stop polling. + * + */ +export default class Poll { + constructor(options = {}) { + this.options = options; + this.options.data = options.data || {}; + + this.intervalHeader = 'POLL-INTERVAL'; + this.timeoutID = null; + this.canPoll = true; + } + + checkConditions(response) { + const headers = gl.utils.normalizeHeaders(response.headers); + const pollInterval = headers[this.intervalHeader]; + + if (pollInterval > 0 && response.status === httpStatusCodes.OK && this.canPoll) { + this.timeoutID = setTimeout(() => { + this.makeRequest(); + }, pollInterval); + } + + this.options.successCallback(response); + } + + makeRequest() { + const { resource, method, data, errorCallback } = this.options; + + return resource[method](data) + .then(response => this.checkConditions(response)) + .catch(error => errorCallback(error)); + } + + /** + * Stops the polling recursive chain + * and guarantees if the timeout is already running it won't make another request by + * cancelling the previously established timeout. + */ + stop() { + this.canPoll = false; + clearTimeout(this.timeoutID); + } +} diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 81d5748191d..b1ca0dc091d 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -1,6 +1,5 @@ /* eslint-disable func-names, space-before-function-paren, no-var, quotes, consistent-return, prefer-arrow-callback, comma-dangle, object-shorthand, no-new, max-len, no-multi-spaces, import/newline-after-import, import/first */ /* global bp */ -/* global Cookies */ /* global Flash */ /* global ConfirmDangerModal */ /* global Aside */ @@ -24,7 +23,6 @@ import './extensions/array'; window.jQuery = jQuery; window.$ = jQuery; window._ = _; -window.Cookies = Cookies; window.Pikaday = Pikaday; window.Dropzone = Dropzone; window.Sortable = Sortable; diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js index 07632d6225c..c4e379a4a0b 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js @@ -1,7 +1,7 @@ /* eslint-disable comma-dangle, object-shorthand, no-param-reassign, camelcase, no-nested-ternary, no-continue, max-len */ -/* global Cookies */ import Vue from 'vue'; +import Cookies from 'js-cookie'; ((global) => { global.mergeConflicts = global.mergeConflicts || {}; diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 190336dbd20..d9692269c38 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -1,10 +1,10 @@ /* eslint-disable no-new, class-methods-use-this */ /* global Breakpoints */ -/* global Cookies */ /* global Flash */ +import Cookies from 'js-cookie'; + require('./breakpoints'); -window.Cookies = require('js-cookie'); require('./flash'); /* eslint-disable max-len */ diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 47cc34e7a20..1d563c63f39 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1,14 +1,14 @@ /* eslint-disable no-restricted-properties, func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, no-use-before-define, camelcase, no-unused-expressions, quotes, max-len, one-var, one-var-declaration-per-line, default-case, prefer-template, consistent-return, no-alert, no-return-assign, no-param-reassign, prefer-arrow-callback, no-else-return, comma-dangle, no-new, brace-style, no-lonely-if, vars-on-top, no-unused-vars, no-sequences, no-shadow, newline-per-chained-call, no-useless-escape */ /* global Flash */ /* global Autosave */ -/* global Cookies */ /* global ResolveService */ /* global mrRefreshWidgetUrl */ +import Cookies from 'js-cookie'; + require('./autosave'); window.autosize = require('vendor/autosize'); window.Dropzone = require('dropzone'); -window.Cookies = require('js-cookie'); require('./dropzone_input'); require('./gfm_auto_complete'); require('vendor/jquery.caret'); // required by jquery.atwho diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index db7ceaa2421..f944fcc5a58 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -1,7 +1,8 @@ /* eslint-disable func-names, space-before-function-paren, wrap-iife, no-var, quotes, consistent-return, no-new, prefer-arrow-callback, no-return-assign, one-var, one-var-declaration-per-line, object-shorthand, comma-dangle, no-else-return, newline-per-chained-call, no-shadow, vars-on-top, prefer-template, max-len */ -/* global Cookies */ /* global ProjectSelect */ +import Cookies from 'js-cookie'; + (function() { this.Project = (function() { function Project() { diff --git a/app/assets/javascripts/right_sidebar.js b/app/assets/javascripts/right_sidebar.js index 903862cac6b..7298a7d5347 100644 --- a/app/assets/javascripts/right_sidebar.js +++ b/app/assets/javascripts/right_sidebar.js @@ -1,5 +1,6 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, no-unused-vars, consistent-return, one-var, one-var-declaration-per-line, quotes, prefer-template, object-shorthand, comma-dangle, no-else-return, no-param-reassign, max-len */ -/* global Cookies */ + +import Cookies from 'js-cookie'; (function() { var bind = function(fn, me) { return function() { return fn.apply(me, arguments); }; }; diff --git a/app/assets/javascripts/shortcuts.js b/app/assets/javascripts/shortcuts.js index 81766f4bd55..fd5097696ad 100644 --- a/app/assets/javascripts/shortcuts.js +++ b/app/assets/javascripts/shortcuts.js @@ -33,6 +33,10 @@ }; Shortcuts.prototype.toggleMarkdownPreview = function(e) { + // Check if short-cut was triggered while in Write Mode + if ($(e.target).hasClass('js-note-text')) { + $('.js-md-preview-button').focus(); + } return $(document).triggerHandler('markdown-preview:toggle', [e]); }; diff --git a/app/assets/javascripts/user.js b/app/assets/javascripts/user.js index 059e6c628b3..19c9efe7fbd 100644 --- a/app/assets/javascripts/user.js +++ b/app/assets/javascripts/user.js @@ -1,5 +1,6 @@ /* eslint-disable class-methods-use-this, comma-dangle, arrow-parens, no-param-reassign */ -/* global Cookies */ + +import Cookies from 'js-cookie'; ((global) => { global.User = class { diff --git a/app/assets/javascripts/user_callout.js b/app/assets/javascripts/user_callout.js index 99419e85b20..b27d252a3ef 100644 --- a/app/assets/javascripts/user_callout.js +++ b/app/assets/javascripts/user_callout.js @@ -1,4 +1,4 @@ -/* global Cookies */ +import Cookies from 'js-cookie'; const userCalloutElementName = '.user-callout'; const closeButton = '.close-user-callout'; @@ -27,7 +27,7 @@ const USER_CALLOUT_TEMPLATE = ` </div> </div>`; -class UserCallout { +export default class UserCallout { constructor() { this.isCalloutDismissed = Cookies.get(USER_CALLOUT_COOKIE); this.userCalloutBody = $(userCalloutElementName); @@ -56,5 +56,3 @@ class UserCallout { } } } - -module.exports = UserCallout; diff --git a/app/assets/javascripts/user_tabs.js b/app/assets/javascripts/user_tabs.js index 465618e3d53..5db0d936ad8 100644 --- a/app/assets/javascripts/user_tabs.js +++ b/app/assets/javascripts/user_tabs.js @@ -1,4 +1,4 @@ -/* eslint-disable max-len, space-before-function-paren, no-underscore-dangle, consistent-return, comma-dangle, no-unused-vars, dot-notation, no-new, no-return-assign, camelcase, no-param-reassign */ +/* eslint-disable max-len, space-before-function-paren, no-underscore-dangle, consistent-return, comma-dangle, no-unused-vars, dot-notation, no-new, no-return-assign, camelcase, no-param-reassign, class-methods-use-this */ /* UserTabs @@ -82,8 +82,19 @@ content on the Users#show page. } bindEvents() { - return this.$parentEl.off('shown.bs.tab', '.nav-links a[data-toggle="tab"]') + this.changeProjectsPageWrapper = this.changeProjectsPage.bind(this); + + this.$parentEl.off('shown.bs.tab', '.nav-links a[data-toggle="tab"]') .on('shown.bs.tab', '.nav-links a[data-toggle="tab"]', event => this.tabShown(event)); + + this.$parentEl.on('click', '.gl-pagination a', this.changeProjectsPageWrapper); + } + + changeProjectsPage(e) { + e.preventDefault(); + + $('.tab-pane.active').empty(); + this.loadTab($(e.target).attr('href'), this.getCurrentAction()); } tabShown(event) { @@ -119,7 +130,7 @@ content on the Users#show page. complete: () => this.toggleLoading(false), dataType: 'json', type: 'GET', - url: `${source}.json`, + url: source, success: (data) => { const tabSelector = `div#${action}`; this.$parentEl.find(tabSelector).html(data.html); @@ -153,6 +164,10 @@ content on the Users#show page. }, document.title, new_state); return new_state; } + + getCurrentAction() { + return this.$parentEl.find('.nav-links .active a').data('action'); + } } global.UserTabs = UserTabs; })(window.gl || (window.gl = {})); diff --git a/app/assets/stylesheets/framework/forms.scss b/app/assets/stylesheets/framework/forms.scss index 25d6fbe465a..2890fcd088b 100644 --- a/app/assets/stylesheets/framework/forms.scss +++ b/app/assets/stylesheets/framework/forms.scss @@ -211,3 +211,11 @@ label { color: $gl-text-color; } } + +@media(max-width: $screen-xs-max) { + .remember-me { + .remember-me-checkbox { + margin-top: 0; + } + } +} diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 6841adb637e..82c9c76c4c0 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -44,7 +44,7 @@ $orange-light: #fc8a51; $orange-normal: darken($orange-light, $darken-normal-factor); $orange-dark: darken($orange-light, $darken-dark-factor); -$red-light: #e52c5a; +$red-light: #eb4d5c; $red-normal: darken($red-light, $darken-normal-factor); $red-dark: darken($red-light, $darken-dark-factor); @@ -84,7 +84,6 @@ $warning-message-border: #f0e2bb; */ $border-color: #e5e5e5; $focus-border-color: #3aabf0; -$sidebar-collapsed-icon-color: #999; $well-expand-item: #e8f2f7; $well-inner-border: #eef0f2; $well-light-border: #f1f1f1; diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index 73a5da715f2..25be7f408d0 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -18,7 +18,10 @@ .environments-container { .table-holder { width: 100%; - overflow: auto; + + @media (max-width: $screen-sm-max) { + overflow: auto; + } } .table.ci-table { diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 4426169ef5a..ddc0e78c7b6 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -498,7 +498,7 @@ svg { width: 16px; height: 16px; - fill: $sidebar-collapsed-icon-color; + fill: $issuable-sidebar-color; } &:hover svg { diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index cb7ebd61504..b27741a928d 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -46,6 +46,10 @@ ul.related-merge-requests > li { .merge-request-id { flex-shrink: 0; } + + .merge-request-info { + margin-left: 5px; + } } .merge-requests-title, @@ -58,10 +62,6 @@ ul.related-merge-requests > li { display: inline-block; } -.merge-request-info { - margin-left: 5px; -} - .merge-request-status { font-size: 13px; padding: 0 5px; diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 33b38ca6923..772f05feb12 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -15,7 +15,10 @@ .table-holder { width: 100%; - overflow: auto; + + @media (max-width: $screen-sm-max) { + overflow: auto; + } } .commit-title { @@ -99,8 +102,6 @@ @media (max-width: $screen-md-max) { .content-list { - &.pipelines, - &.environments-container, &.builds-content-list { width: 100%; overflow: auto; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b7ce081a5cd..6a6e335d314 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -64,8 +64,11 @@ class ApplicationController < ActionController::Base # This filter handles both private tokens and personal access tokens def authenticate_user_from_private_token! - token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence - user = User.find_by_authentication_token(token_string) || User.find_by_personal_access_token(token_string) + token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence + + return unless token.present? + + user = User.find_by_authentication_token(token) || User.find_by_personal_access_token(token) if user && can?(user, :log_in) # Notice we are passing store false, so the user is not diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb index 1502b734f37..d0c44e297e3 100644 --- a/app/controllers/projects/deploy_keys_controller.rb +++ b/app/controllers/projects/deploy_keys_controller.rb @@ -31,8 +31,10 @@ class Projects::DeployKeysController < Projects::ApplicationController end def disable - @project.deploy_keys_projects.find_by(deploy_key_id: params[:id]).destroy + deploy_key_project = @project.deploy_keys_projects.find_by(deploy_key_id: params[:id]) + return render_404 unless deploy_key_project + deploy_key_project.destroy! redirect_to_repository_settings(@project) end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 6e29f1e8a65..2683614d2e8 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -39,7 +39,7 @@ class UsersController < ApplicationController format.html { render 'show' } format.json do render json: { - html: view_to_html_string("shared/projects/_list", projects: @projects, remote: true) + html: view_to_html_string("shared/projects/_list", projects: @projects) } end end @@ -65,7 +65,7 @@ class UsersController < ApplicationController format.html { render 'show' } format.json do render json: { - html: view_to_html_string("snippets/_snippets", collection: @snippets, remote: true) + html: view_to_html_string("snippets/_snippets", collection: @snippets) } end end diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb index 200be99f36b..75834103db5 100644 --- a/app/models/project_services/chat_notification_service.rb +++ b/app/models/project_services/chat_notification_service.rb @@ -6,7 +6,7 @@ class ChatNotificationService < Service default_value_for :category, 'chat' prop_accessor :webhook, :username, :channel - boolean_accessor :notify_only_broken_pipelines + boolean_accessor :notify_only_broken_pipelines, :notify_only_default_branch validates :webhook, presence: true, url: true, if: :activated? @@ -17,6 +17,7 @@ class ChatNotificationService < Service if properties.nil? self.properties = {} self.notify_only_broken_pipelines = true + self.notify_only_default_branch = true end end @@ -29,6 +30,19 @@ class ChatNotificationService < Service pipeline wiki_page] end + def fields + default_fields + build_event_channels + end + + def default_fields + [ + { type: 'text', name: 'webhook', placeholder: "e.g. #{webhook_placeholder}" }, + { type: 'text', name: 'username', placeholder: 'e.g. GitLab' }, + { type: 'checkbox', name: 'notify_only_broken_pipelines' }, + { type: 'checkbox', name: 'notify_only_default_branch' }, + ] + end + def execute(data) return unless supported_events.include?(data[:object_kind]) return unless webhook.present? @@ -123,6 +137,17 @@ class ChatNotificationService < Service end def should_pipeline_be_notified?(data) + notify_for_ref?(data) && notify_for_pipeline?(data) + end + + def notify_for_ref?(data) + return true if data[:object_attributes][:tag] + return true unless notify_only_default_branch + + data[:object_attributes][:ref] == project.default_branch + end + + def notify_for_pipeline?(data) case data[:object_attributes][:status] when 'success' !notify_only_broken_pipelines? diff --git a/app/models/project_services/mattermost_service.rb b/app/models/project_services/mattermost_service.rb index 1156d050622..0362ed172c7 100644 --- a/app/models/project_services/mattermost_service.rb +++ b/app/models/project_services/mattermost_service.rb @@ -22,19 +22,11 @@ class MattermostService < ChatNotificationService </ol>' end - def fields - default_fields + build_event_channels - end - - def default_fields - [ - { type: 'text', name: 'webhook', placeholder: 'e.g. http://mattermost_host/hooks/…' }, - { type: 'text', name: 'username', placeholder: 'e.g. GitLab' }, - { type: 'checkbox', name: 'notify_only_broken_pipelines' }, - ] - end - def default_channel_placeholder "Channel handle (e.g. town-square)" end + + def webhook_placeholder + 'http://mattermost.example.com/hooks/…' + end end diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index b657db6f9ee..71da0af75f6 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -21,19 +21,11 @@ class SlackService < ChatNotificationService </ol>' end - def fields - default_fields + build_event_channels - end - - def default_fields - [ - { type: 'text', name: 'webhook', placeholder: 'e.g. https://hooks.slack.com/services/…' }, - { type: 'text', name: 'username', placeholder: 'e.g. GitLab' }, - { type: 'checkbox', name: 'notify_only_broken_pipelines' }, - ] - end - def default_channel_placeholder "Channel name (e.g. general)" end + + def webhook_placeholder + 'https://hooks.slack.com/services/…' + end end diff --git a/app/models/user.rb b/app/models/user.rb index 8c7ad5d5174..5d19d873f43 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -324,6 +324,8 @@ class User < ActiveRecord::Base end def find_by_personal_access_token(token_string) + return unless token_string + PersonalAccessTokensFinder.new(state: 'active').find_by(token: token_string)&.user end diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb index 83f51947bd4..cb6d30396ec 100644 --- a/app/services/boards/issues/list_service.rb +++ b/app/services/boards/issues/list_service.rb @@ -3,7 +3,7 @@ module Boards class ListService < BaseService def execute issues = IssuesFinder.new(current_user, filter_params).execute - issues = without_board_labels(issues) unless movable_list? + issues = without_board_labels(issues) unless list issues = with_list_label(issues) if movable_list? issues.order_by_position_and_priority end diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index e67ad663720..ebca9beb035 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -43,28 +43,34 @@ %h4 Features %hr - %p - Sign up + - sign_up = "Sign up" + %p{ "aria-label" => "#{sign_up}: status " + (signup_enabled? ? "on" : "off") } + = sign_up %span.light.pull-right = boolean_to_icon signup_enabled? - %p - LDAP + - ldap = "LDAP" + %p{ "aria-label" => "#{ldap}: status " + (Gitlab.config.ldap.enabled ? "on" : "off") } + = ldap %span.light.pull-right = boolean_to_icon Gitlab.config.ldap.enabled - %p - Gravatar + - gravatar = "Gravatar" + %p{ "aria-label" => "#{gravatar}: status " + (gravatar_enabled? ? "on" : "off") } + = gravatar %span.light.pull-right = boolean_to_icon gravatar_enabled? - %p - OmniAuth + - omniauth = "OmniAuth" + %p{ "aria-label" => "#{omniauth}: status " + (Gitlab.config.omniauth.enabled ? "on" : "off") } + = omniauth %span.light.pull-right = boolean_to_icon Gitlab.config.omniauth.enabled - %p - Reply by email + - reply_email = "Reply by email" + %p{ "aria-label" => "#{reply_email}: status " + (Gitlab::IncomingEmail.enabled? ? "on" : "off") } + = reply_email %span.light.pull-right = boolean_to_icon Gitlab::IncomingEmail.enabled? - %p - Container Registry + - container_reg = "Container Registry" + %p{ "aria-label" => "#{container_reg}: status " + (Gitlab.config.registry.enabled ? "on" : "off") } + = container_reg %span.light.pull-right = boolean_to_icon Gitlab.config.registry.enabled diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index d31ced004a0..e31fa5fbe95 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -19,12 +19,13 @@ .nav-controls - if @todos.any?(&:pending?) - = link_to destroy_all_dashboard_todos_path(todos_filter_params), class: 'btn btn-loading js-todos-mark-all', method: :delete, data: { href: destroy_all_dashboard_todos_path(todos_filter_params) } do - Mark all as done - = icon('spinner spin') - = link_to bulk_restore_dashboard_todos_path, class: 'btn btn-loading js-todos-undo-all hidden', method: :patch , data: { href: bulk_restore_dashboard_todos_path(todos_filter_params) } do - Undo mark all as done - = icon('spinner spin') + .append-right-default + = link_to destroy_all_dashboard_todos_path(todos_filter_params), class: 'btn btn-loading js-todos-mark-all', method: :delete, data: { href: destroy_all_dashboard_todos_path(todos_filter_params) } do + Mark all as done + = icon('spinner spin') + = link_to bulk_restore_dashboard_todos_path, class: 'btn btn-loading js-todos-undo-all hidden', method: :patch , data: { href: bulk_restore_dashboard_todos_path(todos_filter_params) } do + Undo mark all as done + = icon('spinner spin') .todos-filters .row-content-block.second-block diff --git a/app/views/devise/sessions/_new_base.html.haml b/app/views/devise/sessions/_new_base.html.haml index 5d359538efe..21c751a23f8 100644 --- a/app/views/devise/sessions/_new_base.html.haml +++ b/app/views/devise/sessions/_new_base.html.haml @@ -8,7 +8,7 @@ - if devise_mapping.rememberable? .remember-me.checkbox %label{ for: "user_remember_me" } - = f.check_box :remember_me + = f.check_box :remember_me, class: 'remember-me-checkbox' %span Remember me .pull-right.forgot-password = link_to "Forgot your password?", new_password_path(resource_name) diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index df0a0212f3d..99690e6b98a 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -6,7 +6,9 @@ %h4.prepend-top-0 Syntax highlighting theme %p - This setting allow you to customize the appearance of the syntax. + This setting allows you to customize the appearance of the syntax. + = succeed '.' do + = link_to 'Learn more', help_page_path('user/profile/preferences', anchor: 'syntax-highlighting-theme'), target: '_blank' .col-lg-9.syntax-theme - Gitlab::ColorSchemes.each do |scheme| = label_tag do @@ -20,6 +22,8 @@ Behavior %p This setting allows you to customize the behavior of the system layout and default views. + = succeed '.' do + = link_to 'Learn more', help_page_path('user/profile/preferences', anchor: 'behavior'), target: '_blank' .col-lg-9 .form-group = f.label :layout, class: 'label-light' do @@ -29,13 +33,11 @@ Choose between fixed (max. 1200px) and fluid (100%) application layout. .form-group = f.label :dashboard, class: 'label-light' do - Default Dashboard - = link_to('(?)', help_page_path('profile/preferences') + '#default-dashboard', target: '_blank') + Default dashboard = f.select :dashboard, dashboard_choices, {}, class: 'form-control' .form-group = f.label :project_view, class: 'label-light' do Project view - = link_to('(?)', help_page_path('profile/preferences') + '#default-project-view', target: '_blank') = f.select :project_view, project_view_choices, {}, class: 'form-control' .help-block Choose what content you want to see on a project's home page. diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 2802a4eca7b..82e0d0025ec 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -31,7 +31,7 @@ = f.select(:default_branch, @project.repository.branch_names, {}, {class: 'select2 select-wide'}) .form-group = f.label :tag_list, "Tags", class: 'label-light' - = f.text_field :tag_list, value: @project.tag_list.to_s, maxlength: 2000, class: "form-control" + = f.text_field :tag_list, value: @project.tag_list.sort.join(', '), maxlength: 2000, class: "form-control" %p.help-block Separate tags with commas. %hr %fieldset diff --git a/app/views/projects/environments/terminal.html.haml b/app/views/projects/environments/terminal.html.haml index ef0dd0eda3c..c8363087d6a 100644 --- a/app/views/projects/environments/terminal.html.haml +++ b/app/views/projects/environments/terminal.html.haml @@ -16,7 +16,7 @@ .col-sm-6 .nav-controls - = link_to @environment.external_url, class: 'btn btn-default' do + = link_to @environment.external_url, class: 'btn btn-default', target: '_blank', rel: 'noopener noreferrer nofollow' do = icon('external-link') = render 'projects/deployments/actions', deployment: @environment.last_deployment diff --git a/app/views/projects/services/_index.html.haml b/app/views/projects/services/_index.html.haml index 964133504e6..86d5a0ec7b8 100644 --- a/app/views/projects/services/_index.html.haml +++ b/app/views/projects/services/_index.html.haml @@ -18,7 +18,7 @@ %th Last edit - @services.sort_by(&:title).each do |service| %tr - %td + %td{ "aria-label" => "#{service.title}: status " + (service.activated? ? "on" : "off") } = boolean_to_icon service.activated? %td = link_to edit_namespace_project_service_path(@project.namespace, @project, service.to_param) do diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index dc9a3b0d0df..601187455b3 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -13,7 +13,7 @@ .cover-block.user-cover-block .cover-controls - if @user == current_user - = link_to profile_path, class: 'btn btn-gray' do + = link_to profile_path, class: 'btn btn-gray has-tooltip', title: 'Edit profile', 'aria-label': 'Edit profile' do = icon('pencil') - elsif current_user - if @user.abuse_report @@ -24,7 +24,7 @@ = link_to new_abuse_report_path(user_id: @user.id, ref_url: request.referrer), class: 'btn btn-gray', title: 'Report abuse', data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do = icon('exclamation-circle') - = link_to user_path(@user, rss_url_options), class: 'btn btn-gray' do + = link_to user_path(@user, rss_url_options), class: 'btn btn-gray has-tooltip', title: 'Subscribe', 'aria-label': 'Subscribe' do = icon('rss') - if current_user && current_user.admin? = link_to [:admin, @user], class: 'btn btn-gray', title: 'View user in admin area', @@ -44,7 +44,7 @@ %span.middle-dot-divider @#{@user.username} %span.middle-dot-divider - Member since #{@user.created_at.to_s(:medium)} + Member since #{@user.created_at.to_date.to_s(:long)} .cover-desc - unless @user.public_email.blank? @@ -97,7 +97,8 @@ Snippets %div{ class: container_class } - .user-callout{ 'callout-svg' => custom_icon('icon_customization') } + - if @user == current_user + .user-callout{ 'callout-svg' => custom_icon('icon_customization') } .tab-content #activity.tab-pane .row-content-block.calender-block.white.second-block.hidden-xs diff --git a/changelogs/unreleased/24215-closed-issues-board.yml b/changelogs/unreleased/24215-closed-issues-board.yml new file mode 100644 index 00000000000..678ec34b274 --- /dev/null +++ b/changelogs/unreleased/24215-closed-issues-board.yml @@ -0,0 +1,4 @@ +--- +title: Display all closed issues in “done” board list +merge_request: +author: diff --git a/changelogs/unreleased/27503-feature-status-aria-labels.yml b/changelogs/unreleased/27503-feature-status-aria-labels.yml new file mode 100644 index 00000000000..f514fd5b631 --- /dev/null +++ b/changelogs/unreleased/27503-feature-status-aria-labels.yml @@ -0,0 +1,4 @@ +--- +title: Add `aria-label` for feature status accessibility +merge_request: 9830 +author: diff --git a/changelogs/unreleased/28713-fe-style-guide.yml b/changelogs/unreleased/28713-fe-style-guide.yml new file mode 100644 index 00000000000..57edb43e27f --- /dev/null +++ b/changelogs/unreleased/28713-fe-style-guide.yml @@ -0,0 +1,4 @@ +--- +title: Adds Frontend Styleguide to documentation +merge_request: 9961 +author: diff --git a/changelogs/unreleased/29550-fix-quick-submit-on-preview.yml b/changelogs/unreleased/29550-fix-quick-submit-on-preview.yml new file mode 100644 index 00000000000..71214971ffd --- /dev/null +++ b/changelogs/unreleased/29550-fix-quick-submit-on-preview.yml @@ -0,0 +1,4 @@ +--- +title: Fix quick submit short-cut on preview tab for comments +merge_request: 10002 +author: diff --git a/changelogs/unreleased/29555-align-all-todo.yml b/changelogs/unreleased/29555-align-all-todo.yml new file mode 100644 index 00000000000..c1555a96a92 --- /dev/null +++ b/changelogs/unreleased/29555-align-all-todo.yml @@ -0,0 +1,4 @@ +--- +title: align Mark all as done with other Done buttons on Todos page +merge_request: +author: diff --git a/changelogs/unreleased/29575-polling.yml b/changelogs/unreleased/29575-polling.yml new file mode 100644 index 00000000000..75016afd455 --- /dev/null +++ b/changelogs/unreleased/29575-polling.yml @@ -0,0 +1,4 @@ +--- +title: Adds polling utility function for vue resource +merge_request: +author: diff --git a/changelogs/unreleased/29930-fix-profile-cover-button-a11y.yml b/changelogs/unreleased/29930-fix-profile-cover-button-a11y.yml new file mode 100644 index 00000000000..754d471c7d7 --- /dev/null +++ b/changelogs/unreleased/29930-fix-profile-cover-button-a11y.yml @@ -0,0 +1,4 @@ +--- +title: Add tooltip and accessibility for profile cover buttons +merge_request: 10182 +author: diff --git a/changelogs/unreleased/filter-bar-fix-ie.yml b/changelogs/unreleased/filter-bar-fix-ie.yml new file mode 100644 index 00000000000..f1fa7d9b177 --- /dev/null +++ b/changelogs/unreleased/filter-bar-fix-ie.yml @@ -0,0 +1,4 @@ +--- +title: Fixed filtered search not working in IE +merge_request: +author: diff --git a/changelogs/unreleased/issue-boards-cant-drag-fix.yml b/changelogs/unreleased/issue-boards-cant-drag-fix.yml new file mode 100644 index 00000000000..ac92573abe8 --- /dev/null +++ b/changelogs/unreleased/issue-boards-cant-drag-fix.yml @@ -0,0 +1,4 @@ +--- +title: Fixed bug in issue boards which stopped cards being able to be dragged +merge_request: +author: diff --git a/changelogs/unreleased/remember-me-missasligned-mobile.yml b/changelogs/unreleased/remember-me-missasligned-mobile.yml new file mode 100644 index 00000000000..7071d32727f --- /dev/null +++ b/changelogs/unreleased/remember-me-missasligned-mobile.yml @@ -0,0 +1,4 @@ +--- +title: Corrected alignment for the remember-me checkbox in the login view +merge_request: +author: diff --git a/changelogs/unreleased/time-tracking-color-not-consistent.yml b/changelogs/unreleased/time-tracking-color-not-consistent.yml new file mode 100644 index 00000000000..50ec9efb1ff --- /dev/null +++ b/changelogs/unreleased/time-tracking-color-not-consistent.yml @@ -0,0 +1,4 @@ +--- +title: Corrected time tracking icon color in the issuable side bar +merge_request: +author: diff --git a/changelogs/unreleased/user-callout-showing-on-all-profiles.yml b/changelogs/unreleased/user-callout-showing-on-all-profiles.yml new file mode 100644 index 00000000000..b8eb5a149b7 --- /dev/null +++ b/changelogs/unreleased/user-callout-showing-on-all-profiles.yml @@ -0,0 +1,4 @@ +--- +title: User callout only shows on current users profile +merge_request: +author: diff --git a/changelogs/unreleased/user-profile-join-date.yml b/changelogs/unreleased/user-profile-join-date.yml new file mode 100644 index 00000000000..f9d78b0dc3e --- /dev/null +++ b/changelogs/unreleased/user-profile-join-date.yml @@ -0,0 +1,4 @@ +--- +title: Removed the hours & minutes from the users start date on their profile +merge_request: +author: diff --git a/changelogs/unreleased/zj-chat-notification-default-branch.yml b/changelogs/unreleased/zj-chat-notification-default-branch.yml new file mode 100644 index 00000000000..fa0052d5034 --- /dev/null +++ b/changelogs/unreleased/zj-chat-notification-default-branch.yml @@ -0,0 +1,4 @@ +--- +title: Only send chat notifications for the default branch +merge_request: +author: diff --git a/config/environments/test.rb b/config/environments/test.rb index fb25d3a8b14..a25c5016a3b 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -1,4 +1,7 @@ Rails.application.configure do + # Make sure the middleware is inserted first in middleware chain + config.middleware.insert_before('ActionDispatch::Static', 'Gitlab::Testing::RequestBlockerMiddleware') + # Settings specified here will take precedence over those in config/application.rb # The test environment is used exclusively to run your application's diff --git a/doc/administration/high_availability/nfs.md b/doc/administration/high_availability/nfs.md index 3893d837006..bf1aa6b9ac5 100644 --- a/doc/administration/high_availability/nfs.md +++ b/doc/administration/high_availability/nfs.md @@ -26,7 +26,7 @@ options: circumstances it could lead to data loss if a failure occurs before data has synced. -## Client mount options +## NFS Client mount options Below is an example of an NFS mount point we use on GitLab.com: diff --git a/doc/api/pipeline_triggers.md b/doc/api/pipeline_triggers.md index fdb41a1d615..50fc19f0e08 100644 --- a/doc/api/pipeline_triggers.md +++ b/doc/api/pipeline_triggers.md @@ -41,10 +41,10 @@ Get details of project's build trigger. GET /projects/:id/triggers/:trigger_id ``` -| Attribute | Type | required | Description | -|-----------|---------|----------|--------------------------| -| `id` | integer | yes | The ID of a project | -| `token` | string | yes | The `token` of a trigger | +| Attribute | Type | required | Description | +|--------------|---------|----------|--------------------------| +| `id` | integer | yes | The ID of a project | +| `trigger_id` | integer | yes | The trigger id | ``` curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers/5" @@ -103,6 +103,7 @@ PUT /projects/:id/triggers/:trigger_id | Attribute | Type | required | Description | |---------------|---------|----------|--------------------------| +| `id` | integer | yes | The ID of a project | | `trigger_id` | integer | yes | The trigger id | | `description` | string | no | The trigger name | @@ -133,6 +134,7 @@ POST /projects/:id/triggers/:trigger_id/take_ownership | Attribute | Type | required | Description | |---------------|---------|----------|--------------------------| +| `id` | integer | yes | The ID of a project | | `trigger_id` | integer | yes | The trigger id | ``` diff --git a/doc/development/README.md b/doc/development/README.md index 265df98fb87..e27e7fc7d19 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -15,7 +15,7 @@ - [SQL Migration Style Guide](migration_style_guide.md) for creating safe SQL migrations - [Testing standards and style guidelines](testing.md) - [UX guide](ux_guide/index.md) for building GitLab with existing CSS styles and elements -- [Frontend guidelines](frontend.md) +- [Frontend guidelines](fe_guide/index.md) - [SQL guidelines](sql.md) for working with SQL queries - [Sidekiq guidelines](sidekiq_style_guide.md) for working with Sidekiq workers - [`Gemfile` guidelines](gemfile.md) diff --git a/doc/development/ci_setup.md b/doc/development/ci_setup.md index b03216fec95..0810b32efd7 100644 --- a/doc/development/ci_setup.md +++ b/doc/development/ci_setup.md @@ -7,7 +7,7 @@ We currently use four CI services to test GitLab: 1. GitLab CI on [GitHost.io](https://gitlab-ce.githost.io/projects/4/) for the [GitLab.com repo](https://gitlab.com/gitlab-org/gitlab-ce) 2. GitLab CI at ci.gitlab.org to test the private GitLab B.V. repo at dev.gitlab.org 3. [Semephore](https://semaphoreapp.com/gitlabhq/gitlabhq/) for [GitHub.com repo](https://github.com/gitlabhq/gitlabhq) -4. [Mock CI Service](user/project/integrations/mock_ci.md) for local development +4. [Mock CI Service](../user/project/integrations/mock_ci.md) for local development | Software @ configuration being tested | GitLab CI (ci.gitlab.org) | GitLab CI (GitHost.io) | Semaphore | |---------------------------------------|---------------------------|---------------------------------------------------------------------------|-----------| diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md new file mode 100644 index 00000000000..366b220cbb2 --- /dev/null +++ b/doc/development/fe_guide/accessibility.md @@ -0,0 +1,13 @@ +# Accessibility + +## Resources + +[Chrome Accessibility Developer Tools][chrome-accessibility-developer-tools] +are useful for testing for potential accessibility problems in GitLab. + +Accessibility best-practices and more in-depth information is available on +[the Audit Rules page][audit-rules] for the Chrome Accessibility Developer Tools. + + +[chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools +[audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules diff --git a/doc/development/fe_guide/architecture.md b/doc/development/fe_guide/architecture.md new file mode 100644 index 00000000000..aebb22caa15 --- /dev/null +++ b/doc/development/fe_guide/architecture.md @@ -0,0 +1,22 @@ +# Architecture + +When you are developing a new feature that requires architectural design, or if +you are changing the fundamental design of an existing feature, make sure it is +discussed with one of the Frontend Architecture Experts. + +A Frontend Architect is an expert who makes high-level Frontend design decisions +and decides on technical standards, including coding standards and frameworks. + +Architectural decisions should be accessible to everyone, so please document +them in the relevant Merge Request discussion or by updating our documentation +when appropriate. + +You can find the Frontend Architecture experts on the [team page][team-page]. + +## Examples + +You can find documentation about the desired architecture for a new feature +built with Vue.js [here][vue-section]. + +[team-page]: https://about.gitlab.com/team +[vue-section]: vue.md#frontend.html#how-to-build-a-new-feature-with-vue-js diff --git a/doc/development/fe_guide/design_patterns.md b/doc/development/fe_guide/design_patterns.md new file mode 100644 index 00000000000..e05887a19af --- /dev/null +++ b/doc/development/fe_guide/design_patterns.md @@ -0,0 +1,78 @@ +# Design Patterns + +## Singletons + +When exactly one object is needed for a given task, prefer to define it as a +`class` rather than as an object literal. Prefer also to explicitly restrict +instantiation, unless flexibility is important (e.g. for testing). + +```javascript +// bad + +const MyThing = { + prop1: 'hello', + method1: () => {} +}; + +export default MyThing; + +// good + +class MyThing { + constructor() { + this.prop1 = 'hello'; + } + method1() {} +} + +export default new MyThing(); + +// best + +export default class MyThing { + constructor() { + if (!this.prototype.singleton) { + this.init(); + this.prototype.singleton = this; + } + return this.prototype.singleton; + } + + init() { + this.prop1 = 'hello'; + } + + method1() {} +} + +``` + +## Manipulating the DOM in a JS Class + +When writing a class that needs to manipulate the DOM guarantee a container option is provided. +This is useful when we need that class to be instantiated more than once in the same page. + +Bad: +```javascript +class Foo { + constructor() { + document.querySelector('.bar'); + } +} +new Foo(); +``` + +Good: +```javascript +class Foo { + constructor(opts) { + document.querySelector(`${opts.container} .bar`); + } +} + +new Foo({ container: '.my-element' }); +``` +You can find an example of the above in this [class][container-class-example]; + + +[container-class-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/mini_pipeline_graph_dropdown.js diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md new file mode 100644 index 00000000000..f963bffde37 --- /dev/null +++ b/doc/development/fe_guide/index.md @@ -0,0 +1,92 @@ +# Frontend Development Guidelines + +This document describes various guidelines to ensure consistency and quality +across GitLab's frontend team. + +## Overview + +GitLab is built on top of [Ruby on Rails][rails] using [Haml][haml] with +[Hamlit][hamlit]. Be wary of [the limitations that come with using +Hamlit][hamlit-limits]. We also use [SCSS][scss] and plain JavaScript with +modern ECMAScript standards supported through [Babel][babel] and ES module +support through [webpack][webpack]. + +We also utilize [webpack][webpack] to handle the bundling, minification, and +compression of our assets. + +Working with our frontend assets requires Node (v4.3 or greater) and Yarn +(v0.17 or greater). You can find information on how to install these on our +[installation guide][install]. + +[jQuery][jquery] is used throughout the application's JavaScript, with +[Vue.js][vue] for particularly advanced, dynamic elements. + +### Browser Support + +For our currently-supported browsers, see our [requirements][requirements]. + +--- + +## [Architecture](architecture.md) +How we go about making fundamental design decisions in GitLab's frontend team +or make changes to our frontend development guidelines. + +--- + +## [Testing](testing.md) +How we write frontend tests, run the GitLab test suite, and debug test related +issues. + +--- + +## [Design Patterns](design_patterns.md) +Common JavaScript design patterns in GitLab's codebase. + +--- + +## [Vue.js Best Practices](vue.md) +Vue specific design patterns and practices. + +--- + +## Style Guides + +### [JavaScript Style Guide](style_guide_js.md) + +We use eslint to enforce our JavaScript style guides. Our guide is based on +the excellent [Airbnb][airbnb-js-style-guide] style guide with a few small +changes. + +### [SCSS Style Guide](style_guide_scss.md) + +Our SCSS conventions which are enforced through [scss-lint][scss-lint]. + +--- + +## [Performance](performance.md) +Best practices for monitoring and maximizing frontend performance. + +--- + +## [Security](security.md) +Frontend security practices. + +--- + +## [Accessibility](accessibility.md) +Our accessibility standards and resources. + + +[rails]: http://rubyonrails.org/ +[haml]: http://haml.info/ +[hamlit]: https://github.com/k0kubun/hamlit +[hamlit-limits]: https://github.com/k0kubun/hamlit/blob/master/REFERENCE.md#limitations +[scss]: http://sass-lang.com/ +[babel]: https://babeljs.io/ +[webpack]: https://webpack.js.org/ +[jquery]: https://jquery.com/ +[vue]: http://vuejs.org/ +[airbnb-js-style-guide]: https://github.com/airbnb/javascript +[scss-lint]: https://github.com/brigade/scss-lint +[install]: ../../install/installation.md#4-node +[requirements]: ../../install/requirements.md#supported-web-browsers diff --git a/doc/development/fe_guide/performance.md b/doc/development/fe_guide/performance.md new file mode 100644 index 00000000000..2d76bb18cff --- /dev/null +++ b/doc/development/fe_guide/performance.md @@ -0,0 +1,95 @@ +# Performance + +## Best Practices + +### Realtime Components + +When writing code for realtime features we have to keep a couple of things in mind: +1. Do not overload the server with requests. +1. It should feel realtime. + +Thus, we must strike a balance between sending requests and the feeling of realtime. +Use the following rules when creating realtime solutions. + +1. The server will tell you how much to poll by sending `Poll-Interval` in the header. +Use that as your polling interval. This way it is easy for system administrators to change the +polling rate. +A `Poll-Interval: -1` means you should disable polling, and this must be implemented. +1. A response with HTTP status `4XX` or `5XX` should disable polling as well. +1. Use a common library for polling. +1. Poll on active tabs only. Use a common library to find out which tab currently has eyes on it. +Please use [Focus](https://gitlab.com/andrewn/focus). Specifically [Eyeballs Detector](https://gitlab.com/andrewn/focus/blob/master/lib/eyeballs-detector.js). +1. Use regular polling intervals, do not use backoff polling, or jitter, as the interval will be +controlled by the server. +1. The backend code will most likely be using etags. You do not and should not check for status +`304 Not Modified`. The browser will transform it for you. + +## Reducing Asset Footprint + +### Page-specific JavaScript + +Certain pages may require the use of a third party library, such as [d3][d3] for +the User Activity Calendar and [Chart.js][chartjs] for the Graphs pages. These +libraries increase the page size significantly, and impact load times due to +bandwidth bottlenecks and the browser needing to parse more JavaScript. + +In cases where libraries are only used on a few specific pages, we use +"page-specific JavaScript" to prevent the main `main.js` file from +becoming unnecessarily large. + +Steps to split page-specific JavaScript from the main `main.js`: + +1. Create a directory for the specific page(s), e.g. `graphs/`. +1. In that directory, create a `namespace_bundle.js` file, e.g. `graphs_bundle.js`. +1. Add the new "bundle" file to the list of entry files in `config/webpack.config.js`. + - For example: `graphs: './graphs/graphs_bundle.js',`. +1. Move code reliant on these libraries into the `graphs` directory. +1. In `graphs_bundle.js` add CommonJS `require('./path_to_some_component.js');` statements to load any other files in this directory. Make sure to use relative urls. +1. In the relevant views, add the scripts to the page with the following: + +```haml +- content_for :page_specific_javascripts do + = page_specific_javascript_bundle_tag('lib_chart') + = page_specific_javascript_bundle_tag('graphs') +``` + +The above loads `chart.js` and `graphs_bundle.js` for this page only. `chart.js` +is separated from the bundle file so it can be cached separately from the bundle +and reused for other pages that also rely on the library. For an example, see +[this Haml file][page-specific-js-example]. + +### Code Splitting + +> *TODO* flesh out this section once webpack is ready for code-splitting + +### Minimizing page size + +A smaller page size means the page loads faster (especially important on mobile +and poor connections), the page is parsed more quickly by the browser, and less +data is used for users with capped data plans. + +General tips: + +- Don't add new fonts. +- Prefer font formats with better compression, e.g. WOFF2 is better than WOFF, which is better than TTF. +- Compress and minify assets wherever possible (For CSS/JS, Sprockets and webpack do this for us). +- If some functionality can reasonably be achieved without adding extra libraries, avoid them. +- Use page-specific JavaScript as described above to dynamically load libraries that are only needed on certain pages. + +------- + +## Additional Resources + +- [WebPage Test][web-page-test] for testing site loading time and size. +- [Google PageSpeed Insights][pagespeed-insights] grades web pages and provides feedback to improve the page. +- [Profiling with Chrome DevTools][google-devtools-profiling] +- [Browser Diet][browser-diet] is a community-built guide that catalogues practical tips for improving web page performance. + + +[web-page-test]: http://www.webpagetest.org/ +[pagespeed-insights]: https://developers.google.com/speed/pagespeed/insights/ +[google-devtools-profiling]: https://developers.google.com/web/tools/chrome-devtools/profile/?hl=en +[browser-diet]: https://browserdiet.com/ +[d3]: https://d3js.org/ +[chartjs]: http://www.chartjs.org/ +[page-specific-js-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/13bb9ed77f405c5f6ee4fdbc964ecf635c9a223f/app/views/projects/graphs/_head.html.haml#L6-8 diff --git a/doc/development/fe_guide/security.md b/doc/development/fe_guide/security.md new file mode 100644 index 00000000000..19e72c1d368 --- /dev/null +++ b/doc/development/fe_guide/security.md @@ -0,0 +1,92 @@ +# Security +### Resources + +[Mozilla’s HTTP Observatory CLI][observatory-cli] and the +[Qualys SSL Labs Server Test][qualys-ssl] are good resources for finding +potential problems and ensuring compliance with security best practices. + +<!-- Uncomment these sections when CSP/SRI are implemented. +### Content Security Policy (CSP) + +Content Security Policy is a web standard that intends to mitigate certain +forms of Cross-Site Scripting (XSS) as well as data injection. + +Content Security Policy rules should be taken into consideration when +implementing new features, especially those that may rely on connection with +external services. + +GitLab's CSP is used for the following: + +- Blocking plugins like Flash and Silverlight from running at all on our pages. +- Blocking the use of scripts and stylesheets downloaded from external sources. +- Upgrading `http` requests to `https` when possible. +- Preventing `iframe` elements from loading in most contexts. + +Some exceptions include: + +- Scripts from Google Analytics and Piwik if either is enabled. +- Connecting with GitHub, Bitbucket, GitLab.com, etc. to allow project importing. +- Connecting with Google, Twitter, GitHub, etc. to allow OAuth authentication. + +We use [the Secure Headers gem][secure_headers] to enable Content +Security Policy headers in the GitLab Rails app. + +Some resources on implementing Content Security Policy: + +- [MDN Article on CSP][mdn-csp] +- [GitHub’s CSP Journey on the GitHub Engineering Blog][github-eng-csp] +- The Dropbox Engineering Blog's series on CSP: [1][dropbox-csp-1], [2][dropbox-csp-2], [3][dropbox-csp-3], [4][dropbox-csp-4] + +### Subresource Integrity (SRI) + +Subresource Integrity prevents malicious assets from being provided by a CDN by +guaranteeing that the asset downloaded is identical to the asset the server +is expecting. + +The Rails app generates a unique hash of the asset, which is used as the +asset's `integrity` attribute. The browser generates the hash of the asset +on-load and will reject the asset if the hashes do not match. + +All CSS and JavaScript assets should use Subresource Integrity. + +Some resources on implementing Subresource Integrity: + +- [MDN Article on SRI][mdn-sri] +- [Subresource Integrity on the GitHub Engineering Blog][github-eng-sri] + +--> + +### Including external resources + +External fonts, CSS, and JavaScript should never be used with the exception of +Google Analytics and Piwik - and only when the instance has enabled it. Assets +should always be hosted and served locally from the GitLab instance. Embedded +resources via `iframes` should never be used except in certain circumstances +such as with ReCaptcha, which cannot be used without an `iframe`. + +### Avoiding inline scripts and styles + +In order to protect users from [XSS vulnerabilities][xss], we will disable +inline scripts in the future using Content Security Policy. + +While inline scripts can be useful, they're also a security concern. If +user-supplied content is unintentionally left un-sanitized, malicious users can +inject scripts into the web app. + +Inline styles should be avoided in almost all cases, they should only be used +when no alternatives can be found. This allows reusability of styles as well as +readability. + + +[observatory-cli]: https://github.com/mozilla/http-observatory-cli +[qualys-ssl]: https://www.ssllabs.com/ssltest/analyze.html +[secure_headers]: https://github.com/twitter/secureheaders +[mdn-csp]: https://developer.mozilla.org/en-US/docs/Web/Security/CSP +[github-eng-csp]: http://githubengineering.com/githubs-csp-journey/ +[dropbox-csp-1]: https://blogs.dropbox.com/tech/2015/09/on-csp-reporting-and-filtering/ +[dropbox-csp-2]: https://blogs.dropbox.com/tech/2015/09/unsafe-inline-and-nonce-deployment/ +[dropbox-csp-3]: https://blogs.dropbox.com/tech/2015/09/csp-the-unexpected-eval/ +[dropbox-csp-4]: https://blogs.dropbox.com/tech/2015/09/csp-third-party-integrations-and-privilege-separation/ +[mdn-sri]: https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity +[github-eng-sri]: http://githubengineering.com/subresource-integrity/ +[xss]: https://en.wikipedia.org/wiki/Cross-site_scripting diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md new file mode 100644 index 00000000000..034cfe73d33 --- /dev/null +++ b/doc/development/fe_guide/style_guide_js.md @@ -0,0 +1,408 @@ +# Style guides and linting +See the relevant style guides for our guidelines and for information on linting: + +## JavaScript +We defer to [Airbnb][airbnb-js-style-guide] on most style-related +conventions and enforce them with eslint. + +See [our current .eslintrc][eslintrc] for specific rules and patterns. + +### Common + +#### ESlint + +- **Never** disable eslint rules unless you have a good reason. You may see a lot of legacy files with `/* eslint-disable some-rule, some-other-rule */` at the top, but legacy files are a special case. Any time you develop a new feature or refactor an existing one, you should abide by the eslint rules. + +- **Never Ever EVER** disable eslint globally for a file + + ```javascript + // bad + /* eslint-disable */ + + // better + /* eslint-disable some-rule, some-other-rule */ + + // best + // nothing :) + ``` + +- If you do need to disable a rule for a single violation, try to do it as locally as possible + + ```javascript + // bad + /* eslint-disable no-new */ + + import Foo from 'foo'; + + new Foo(); + + // better + import Foo from 'foo'; + + // eslint-disable-next-line no-new + new Foo(); + ``` + +- When they are needed _always_ place ESlint directive comment blocks on the first line of a script, followed by any global declarations, then a blank newline prior to any imports or code. + + ```javascript + // bad + /* global Foo */ + /* eslint-disable no-new */ + import Bar from './bar'; + + // good + /* eslint-disable no-new */ + /* global Foo */ + + import Bar from './bar'; + ``` + +- **Never** disable the `no-undef` rule. Declare globals with `/* global Foo */` instead. + +- When declaring multiple globals, always use one `/* global [name] */` line per variable. + + ```javascript + // bad + /* globals Flash, Cookies, jQuery */ + + // good + /* global Flash */ + /* global Cookies */ + /* global jQuery */ + ``` + +#### Modules, Imports, and Exports +- Use ES module syntax to import modules + + ```javascript + // bad + require('foo'); + + // good + import Foo from 'foo'; + + // bad + module.exports = Foo; + + // good + export default Foo; + ``` + +- Relative paths + + Unless you are writing a test, always reference other scripts using relative paths instead of `~` + + In **app/assets/javascripts**: + ```javascript + // bad + import Foo from '~/foo' + + // good + import Foo from '../foo'; + ``` + + In **spec/javascripts**: + ```javascript + // bad + import Foo from '../../app/assets/javascripts/foo' + + // good + import Foo from '~/foo'; + ``` + +- Avoid using IIFE. Although we have a lot of examples of files which wrap their contents in IIFEs (immediately-invoked function expressions), this is no longer necessary after the transition from Sprockets to webpack. Do not use them anymore and feel free to remove them when refactoring legacy code. + +- Avoid adding to the global namespace. + + ```javascript + // bad + window.MyClass = class { /* ... */ }; + + // good + export default class MyClass { /* ... */ } + ``` + +- Side effects are forbidden in any script which contains exports + + ```javascript + // bad + export default class MyClass { /* ... */ } + + document.addEventListener("DOMContentLoaded", function(event) { + new MyClass(); + } + ``` + + +#### Data Mutation and Pure functions +- Strive to write many small pure functions, and minimize where mutations occur. + + ```javascript + // bad + const values = {foo: 1}; + + function impureFunction(items) { + const bar = 1; + + items.foo = items.a * bar + 2; + + return items.a; + } + + const c = impureFunction(values); + + // good + var values = {foo: 1}; + + function pureFunction (foo) { + var bar = 1; + + foo = foo * bar + 2; + + return foo; + } + + var c = pureFunction(values.foo); + ``` + +- Avoid constructors with side-effects + + +#### Parse Strings into Numbers +- `parseInt()` is preferable over `Number()` or `+` + + ```javascript + // bad + +'10' // 10 + + // good + Number('10') // 10 + + // better + parseInt('10', 10); + ``` + + +### Vue.js + + +#### Basic Rules +- Only include one Vue.js component per file. +- Export components as plain objects: + + ```javascript + export default { + template: `<h1>I'm a component</h1> + } + ``` + +#### Naming +- **Extensions**: Use `.vue` extension for Vue components. +- **Reference Naming**: Use PascalCase for Vue components and camelCase for their instances: + + ```javascript + // bad + import cardBoard from 'cardBoard'; + + // good + import CardBoard from 'cardBoard' + + // bad + components: { + CardBoard: CardBoard + }; + + // good + components: { + cardBoard: CardBoard + }; + ``` +- **Props Naming**: Avoid using DOM component prop names. + + ```javascript + // bad + <component class="btn"> + + // good + <component cssClass="btn"> + ``` + +#### Alignment +- Follow these alignment styles for the template method: + + ```javascript + // bad + <component v-if="bar" + param="baz" /> + + // good + <component + v-if="bar" + param="baz" + /> + + // if props fit in one line then keep it on the same line + <component bar="bar" /> + ``` + +#### Quotes +- Always use double quotes `"` inside templates and single quotes `'` for all other JS. + + ```javascript + // bad + template: ` + <button :class='style'>Button</button> + ` + + // good + template: ` + <button :class="style">Button</button> + ` + ``` + +#### Props +- Props should be declared as an object + + ```javascript + // bad + props: ['foo'] + + // good + props: { + foo: { + type: String, + required: false, + default: 'bar' + } + } + ``` + +- Required key should always be provided when declaring a prop + + ```javascript + // bad + props: { + foo: { + type: String, + } + } + + // good + props: { + foo: { + type: String, + required: false, + default: 'bar' + } + } + ``` + +- Default key should always be provided if the prop is not required: + + ```javascript + // bad + props: { + foo: { + type: String, + required: false, + } + } + + // good + props: { + foo: { + type: String, + required: false, + default: 'bar' + } + } + + // good + props: { + foo: { + type: String, + required: true + } + } + ``` + +#### Data +- `data` method should always be a function + + ```javascript + // bad + data: { + foo: 'foo' + } + + // good + data() { + return { + foo: 'foo' + }; + } + ``` + +#### Directives + +- Shorthand `@` is preferable over `v-on` + + ```javascript + // bad + <component v-on:click="eventHandler"/> + + + // good + <component @click="eventHandler"/> + ``` + +- Shorthand `:` is preferable over `v-bind` + + ```javascript + // bad + <component v-bind:class="btn"/> + + + // good + <component :class="btn"/> + ``` + +#### Closing tags +- Prefer self closing component tags + + ```javascript + // bad + <component></component> + + // good + <component /> + ``` + +#### Ordering +- Order for a Vue Component: + 1. `name` + 2. `props` + 3. `data` + 4. `components` + 5. `computedProps` + 6. `methods` + 7. lifecycle methods + 1. `beforeCreate` + 2. `created` + 3. `beforeMount` + 4. `mounted` + 5. `beforeUpdate` + 6. `updated` + 7. `activated` + 8. `deactivated` + 9. `beforeDestroy` + 10. `destroyed` + 8. `template` + + +## SCSS +- [SCSS](style_guide_scss.md) + +[airbnb-js-style-guide]: https://github.com/airbnb/javascript +[eslintrc]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.eslintrc diff --git a/doc/development/scss_styleguide.md b/doc/development/fe_guide/style_guide_scss.md index a79f4073cde..77b308c4a43 100644 --- a/doc/development/scss_styleguide.md +++ b/doc/development/fe_guide/style_guide_scss.md @@ -35,7 +35,7 @@ between the property and its value. ```scss // Bad -.container-item { +.container-item { width: 100px; height: 100px; margin-top: 0; } @@ -63,7 +63,7 @@ between the property and its value. } ``` -Note that there is an exception for single-line rulesets, although these are +Note that there is an exception for single-line rulesets, although these are not typically recommended. ```scss @@ -72,7 +72,7 @@ p { margin: 0; padding: 0; } ### Colors -HEX (hexadecimal) colors should use shorthand where possible, and should use +HEX (hexadecimal) colors should use shorthand where possible, and should use lower case letters to differentiate between letters and numbers, e.g. `#E3E3E3` vs. `#e3e3e3`. @@ -111,7 +111,7 @@ p { ### Semicolons -Always include semicolons after every property. When the stylesheets are +Always include semicolons after every property. When the stylesheets are minified, the semicolons will be removed automatically. ```scss @@ -144,7 +144,7 @@ padding: 10px; ### Zero Units -Omit length units on zero values, they're unnecessary and not including them +Omit length units on zero values, they're unnecessary and not including them is slightly more performant. ```scss @@ -161,36 +161,56 @@ is slightly more performant. ### Selectors with a `js-` Prefix -Do not use any selector prefixed with `js-` for styling purposes. These -selectors are intended for use only with JavaScript to allow for removal or +Do not use any selector prefixed with `js-` for styling purposes. These +selectors are intended for use only with JavaScript to allow for removal or renaming without breaking styling. +### IDs +Don't use ID selectors in CSS. + +```scss +// Bad +#my-element { + padding: 0; +} + +// Good +.my-element { + padding: 0; +} +``` + +### Variables +Before adding a new variable for a color or a size, guarantee: +1. There isn't already one +2. There isn't a similar one we can use instead. + ## Linting -We use [SCSS Lint][scss-lint] to check for style guide conformity. It uses the -ruleset in `.scss-lint.yml`, which is located in the home directory of the +We use [SCSS Lint][scss-lint] to check for style guide conformity. It uses the +ruleset in `.scss-lint.yml`, which is located in the home directory of the project. -To check if any warnings will be produced by your changes, you can run `rake -scss_lint` in the GitLab directory. SCSS Lint will also run in GitLab CI to +To check if any warnings will be produced by your changes, you can run `rake +scss_lint` in the GitLab directory. SCSS Lint will also run in GitLab CI to catch any warnings. -If the Rake task is throwing warnings you don't understand, SCSS Lint's +If the Rake task is throwing warnings you don't understand, SCSS Lint's documentation includes [a full list of their linters][scss-lint-documentation]. ### Fixing issues -If you want to automate changing a large portion of the codebase to conform to +If you want to automate changing a large portion of the codebase to conform to the SCSS style guide, you can use [CSSComb][csscomb]. First install -[Node][node] and [NPM][npm], then run `npm install csscomb -g` to install -CSSComb globally (system-wide). Run it in the GitLab directory with +[Node][node] and [NPM][npm], then run `npm install csscomb -g` to install +CSSComb globally (system-wide). Run it in the GitLab directory with `csscomb app/assets/stylesheets` to automatically fix issues with CSS/SCSS. Note that this won't fix every problem, but it should fix a majority. ### Ignoring issues -If you want a line or set of lines to be ignored by the linter, you can use +If you want a line or set of lines to be ignored by the linter, you can use `// scss-lint:disable RuleName` ([more info][disabling-linters]): ```scss @@ -203,8 +223,8 @@ If you want a line or set of lines to be ignored by the linter, you can use ``` Make sure a comment is added on the line above the `disable` rule, otherwise the -linter will throw a warning. `DisableLinterReason` is enabled to make sure the -style guide isn't being ignored, and to communicate to others why the style +linter will throw a warning. `DisableLinterReason` is enabled to make sure the +style guide isn't being ignored, and to communicate to others why the style guide is ignored in this instance. [csscomb]: https://github.com/csscomb/csscomb.js diff --git a/doc/development/fe_guide/testing.md b/doc/development/fe_guide/testing.md new file mode 100644 index 00000000000..bb6adeacc4c --- /dev/null +++ b/doc/development/fe_guide/testing.md @@ -0,0 +1,129 @@ +# Frontend Testing + +There are two types of tests you'll encounter while developing frontend code +at GitLab. We use Karma and Jasmine for JavaScript unit testing, and RSpec +feature tests with Capybara for integration testing. + +Feature tests need to be written for all new features. Regression tests ought +to be written for all bug fixes to prevent them from recurring in the future. + +See [the Testing Standards and Style Guidelines](/doc/development/testing.md) +for more information on general testing practices at GitLab. + +## Karma test suite + +GitLab uses the [Karma][karma] test runner with [Jasmine][jasmine] as its test +framework for our JavaScript unit tests. For tests that rely on DOM +manipulation we use fixtures which are pre-compiled from HAML source files and +served during testing by the [jasmine-jquery][jasmine-jquery] plugin. + +### Running frontend tests + +`rake karma` runs the frontend-only (JavaScript) tests. +It consists of two subtasks: + +- `rake karma:fixtures` (re-)generates fixtures +- `rake karma:tests` actually executes the tests + +As long as the fixtures don't change, `rake karma:tests` (or `yarn karma`) +is sufficient (and saves you some time). + +### Live testing and focused testing + +While developing locally, it may be helpful to keep karma running so that you +can get instant feedback on as you write tests and modify code. To do this +you can start karma with `npm run karma-start`. It will compile the javascript +assets and run a server at `http://localhost:9876/` where it will automatically +run the tests on any browser which connects to it. You can enter that url on +multiple browsers at once to have it run the tests on each in parallel. + +While karma is running, any changes you make will instantly trigger a recompile +and retest of the entire test suite, so you can see instantly if you've broken +a test with your changes. You can use [jasmine focused][jasmine-focus] or +excluded tests (with `fdescribe` or `xdescribe`) to get karma to run only the +tests you want while you're working on a specific feature, but make sure to +remove these directives when you commit your code. + +## RSpec Feature Integration Tests + +Information on setting up and running RSpec integration tests with +[Capybara][capybara] can be found in the +[general testing guide](/doc/development/testing.md). + +## Gotchas + +### Errors due to use of unsupported JavaScript features + +Similar errors will be thrown if you're using JavaScript features not yet +supported by the PhantomJS test runner which is used for both Karma and RSpec +tests. We polyfill some JavaScript objects for older browsers, but some +features are still unavailable: + +- Array.from +- Array.first +- Async functions +- Generators +- Array destructuring +- For..Of +- Symbol/Symbol.iterator +- Spread + +Until these are polyfilled appropriately, they should not be used. Please +update this list with additional unsupported features. + +### RSpec errors due to JavaScript + +By default RSpec unit tests will not run JavaScript in the headless browser +and will simply rely on inspecting the HTML generated by rails. + +If an integration test depends on JavaScript to run correctly, you need to make +sure the spec is configured to enable JavaScript when the tests are run. If you +don't do this you'll see vague error messages from the spec runner. + +To enable a JavaScript driver in an `rspec` test, add `js: true` to the +individual spec or the context block containing multiple specs that need +JavaScript enabled: + +```ruby + +# For one spec +it 'presents information about abuse report', js: true do + # assertions... +end + +describe "Admin::AbuseReports", js: true do + it 'presents information about abuse report' do + # assertions... + end + it 'shows buttons for adding to abuse report' do + # assertions... + end +end +``` + +### Spinach errors due to missing JavaScript + +> **Note:** Since we are discouraging the use of Spinach when writing new +> feature tests, you shouldn't ever need to use this. This information is kept +> available for legacy purposes only. + +In Spinach, the JavaScript driver is enabled differently. In the `*.feature` +file for the failing spec, add the `@javascript` flag above the Scenario: + +``` +@javascript +Scenario: Developer can approve merge request + Given I am a "Shop" developer + And I visit project "Shop" merge requests page + And merge request 'Bug NS-04' must be approved + And I click link "Bug NS-04" + When I click link "Approve" + Then I should see approved merge request "Bug NS-04" + +``` + +[capybara]: http://teamcapybara.github.io/capybara/ +[jasmine]: https://jasmine.github.io/ +[jasmine-focus]: https://jasmine.github.io/2.5/focused_specs.html +[jasmine-jquery]: https://github.com/velesin/jasmine-jquery +[karma]: http://karma-runner.github.io/ diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md new file mode 100644 index 00000000000..3e3406e7d6a --- /dev/null +++ b/doc/development/fe_guide/vue.md @@ -0,0 +1,104 @@ +# Vue + +For more complex frontend features, we recommend using Vue.js. It shares +some ideas with React.js as well as Angular. + +To get started with Vue, read through [their documentation][vue-docs]. + +## When to use Vue.js + +We recommend using Vue for more complex features. Here are some guidelines for when to use Vue.js: + +- If you are starting a new feature or refactoring an old one that highly interacts with the DOM; +- For real time data updates; +- If you are creating a component that will be reused elsewhere; + +## When not to use Vue.js + +We don't want to refactor all GitLab frontend code into Vue.js, here are some guidelines for +when not to use Vue.js: + +- Adding or changing static information; +- Features that highly depend on jQuery will be hard to work with Vue.js + +As always, the Frontend Architectural Experts are available to help with any Vue or JavaScript questions. + +## How to build a new feature with Vue.js + +**Components, Stores and Services** + +In some features implemented with Vue.js, like the [issue board][issue-boards] +or [environments table][environments-table] +you can find a clear separation of concerns: + +``` +new_feature +├── components +│ └── component.js.es6 +│ └── ... +├── store +│ └── new_feature_store.js.es6 +├── service +│ └── new_feature_service.js.es6 +├── new_feature_bundle.js.es6 +``` +_For consistency purposes, we recommend you to follow the same structure._ + +Let's look into each of them: + +**A `*_bundle.js` file** + +This is the index file of your new feature. This is where the root Vue instance +of the new feature should be. + +The Store and the Service should be imported and initialized in this file and provided as a prop to the main component. + +Don't forget to follow [these steps.][page_specific_javascript] + +**A folder for Components** + +This folder holds all components that are specific of this new feature. +If you need to use or create a component that will probably be used somewhere +else, please refer to `vue_shared/components`. + +A good thumb rule to know when you should create a component is to think if +it will be reusable elsewhere. + +For example, tables are used in a quite amount of places across GitLab, a table +would be a good fit for a component. On the other hand, a table cell used only +in one table would not be a good use of this pattern. + +You can read more about components in Vue.js site, [Component System][component-system] + +**A folder for the Store** + +The Store is a class that allows us to manage the state in a single +source of truth. + +The concept we are trying to follow is better explained by Vue documentation +itself, please read this guide: [State Management][state-management] + +**A folder for the Service** + +The Service is used only to communicate with the server. +It does not store or manipulate any data. +We use [vue-resource][vue-resource-repo] to +communicate with the server. + +The [issue boards service][issue-boards-service] +is a good example of this pattern. + +## Style guide + +Please refer to the Vue section of our [style guide](style_guide_js.md#vuejs) +for best practices while writing your Vue components and templates. + + +[vue-docs]: http://vuejs.org/guide/index.html +[issue-boards]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/boards +[environments-table]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/environments +[page_specific_javascript]: https://docs.gitlab.com/ce/development/frontend.html#page-specific-javascript +[component-system]: https://vuejs.org/v2/guide/#Composing-with-Components +[state-management]: https://vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch +[vue-resource-repo]: https://github.com/pagekit/vue-resource +[issue-boards-service]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/boards/services/board_service.js.es6 diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 50105a486d0..f46cc377f95 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -1,511 +1,4 @@ -# Frontend Development Guidelines - -This document describes various guidelines to ensure consistency and quality -across GitLab's frontend team. - -## Overview - -GitLab is built on top of [Ruby on Rails][rails] using [Haml][haml] with -[Hamlit][hamlit]. Be wary of [the limitations that come with using -Hamlit][hamlit-limits]. We also use [SCSS][scss] and plain JavaScript with -[ES6 by way of Babel][es6]. - -The asset pipeline is [Sprockets][sprockets], which handles the concatenation, -minification, and compression of our assets. - -[jQuery][jquery] is used throughout the application's JavaScript, with -[Vue.js][vue] for particularly advanced, dynamic elements. - -### Architecture - -The Frontend Architect is an expert who makes high-level frontend design choices -and decides on technical standards, including coding standards, and frameworks. - -When you are assigned a new feature that requires architectural design, -make sure it is discussed with one of the Frontend Architecture Experts. - -This rule also applies if you plan to change the architecture of an existing feature. - -These decisions should be accessible to everyone, so please document it on the Merge Request. - -You can find the Frontend Architecture experts on the [team page][team-page]. - -You can find documentation about the desired architecture for a new feature built with Vue.js in [here][vue-section]. - -### Realtime - -When writing code for realtime features we have to keep a couple of things in mind: -1. Do not overload the server with requests. -1. It should feel realtime. - -Thus, we must strike a balance between sending requests and the feeling of realtime. -Use the following rules when creating realtime solutions. - -1. The server will tell you how much to poll by sending `Poll-Interval` in the header. -Use that as your polling interval. This way it is easy for system administrators to change the -polling rate. -A `Poll-Interval: -1` means you should disable polling, and this must be implemented. -1. A response with HTTP status `4XX` or `5XX` should disable polling as well. -1. Use a common library for polling. -1. Poll on active tabs only. Use a common library to find out which tab currently has eyes on it. -Please use [Focus](https://gitlab.com/andrewn/focus). Specifically [Eyeballs Detector](https://gitlab.com/andrewn/focus/blob/master/lib/eyeballs-detector.js). -1. Use regular polling intervals, do not use backoff polling, or jitter, as the interval will be -controlled by the server. -1. The backend code will most likely be using etags. You do not and should not check for status -`304 Not Modified`. The browser will transform it for you. - -### Vue - -For more complex frontend features, we recommend using Vue.js. It shares -some ideas with React.js as well as Angular. - -To get started with Vue, read through [their documentation][vue-docs]. - -#### How to build a new feature with Vue.js -**Components, Stores and Services** - -In some features implemented with Vue.js, like the [issue board][issue-boards] -or [environments table][environments-table] -you can find a clear separation of concerns: - -``` -new_feature -├── components -│ └── component.js.es6 -│ └── ... -├── store -│ └── new_feature_store.js.es6 -├── service -│ └── new_feature_service.js.es6 -├── new_feature_bundle.js.es6 -``` -_For consistency purposes, we recommend you to follow the same structure._ - -Let's look into each of them: - -**A `*_bundle.js` file** - -This is the index file of your new feature. This is where the root Vue instance -of the new feature should be. - -The Store and the Service should be imported and initialized in this file and provided as a prop to the main component. - -Don't forget to follow [these steps.][page_specific_javascript] - -**A folder for Components** - -This folder holds all components that are specific of this new feature. -If you need to use or create a component that will probably be used somewhere -else, please refer to `vue_shared/components`. - -A good thumb rule to know when you should create a component is to think if -it will be reusable elsewhere. - -For example, tables are used in a quite amount of places across GitLab, a table -would be a good fit for a component. -On the other hand, a table cell used only in on table, would not be a good use -of this pattern. - -You can read more about components in Vue.js site, [Component System][component-system] - -**A folder for the Store** - -The Store is a class that allows us to manage the state in a single -source of truth. - -The concept we are trying to follow is better explained by Vue documentation -itself, please read this guide: [State Management][state-management] - -**A folder for the Service** - -The Service is used only to communicate with the server. -It does not store or manipulate any data. -We use [vue-resource][vue-resource-repo] to -communicate with the server. - -The [issue boards service][issue-boards-service] -is a good example of this pattern. - -## Performance - -### Resources - -- [WebPage Test][web-page-test] for testing site loading time and size. -- [Google PageSpeed Insights][pagespeed-insights] grades web pages and provides feedback to improve the page. -- [Profiling with Chrome DevTools][google-devtools-profiling] -- [Browser Diet][browser-diet] is a community-built guide that catalogues practical tips for improving web page performance. - -### Page-specific JavaScript - -Certain pages may require the use of a third party library, such as [d3][d3] for -the User Activity Calendar and [Chart.js][chartjs] for the Graphs pages. These -libraries increase the page size significantly, and impact load times due to -bandwidth bottlenecks and the browser needing to parse more JavaScript. - -In cases where libraries are only used on a few specific pages, we use -"page-specific JavaScript" to prevent the main `application.js` file from -becoming unnecessarily large. - -Steps to split page-specific JavaScript from the main `application.js`: - -1. Create a directory for the specific page(s), e.g. `graphs/`. -1. In that directory, create a `namespace_bundle.js` file, e.g. `graphs_bundle.js`. -1. In `graphs_bundle.js` add the line `//= require_tree .`, this adds all other files in the directory to the bundle. -1. Add any necessary libraries to `app/assets/javascripts/lib/`, all files directly descendant from this directory will be precompiled as separate assets, in this case `chart.js` would be added. -1. Add the new "bundle" file to the list of precompiled assets in -`config/application.rb`. - - For example: `config.assets.precompile << "graphs/graphs_bundle.js"`. -1. Move code reliant on these libraries into the `graphs` directory. -1. In the relevant views, add the scripts to the page with the following: - -```haml -- content_for :page_specific_javascripts do - = page_specific_javascript_tag('lib/chart.js') - = page_specific_javascript_tag('graphs/graphs_bundle.js') -``` - -The above loads `chart.js` and `graphs_bundle.js` for this page only. `chart.js` -is separated from the bundle file so it can be cached separately from the bundle -and reused for other pages that also rely on the library. For an example, see -[this Haml file][page-specific-js-example]. - -### Minimizing page size - -A smaller page size means the page loads faster (especially important on mobile -and poor connections), the page is parsed more quickly by the browser, and less -data is used for users with capped data plans. - -General tips: - -- Don't add new fonts. -- Prefer font formats with better compression, e.g. WOFF2 is better than WOFF, which is better than TTF. -- Compress and minify assets wherever possible (For CSS/JS, Sprockets does this for us). -- If some functionality can reasonably be achieved without adding extra libraries, avoid them. -- Use page-specific JavaScript as described above to dynamically load libraries that are only needed on certain pages. - -## Accessibility - -### Resources - -[Chrome Accessibility Developer Tools][chrome-accessibility-developer-tools] -are useful for testing for potential accessibility problems in GitLab. - -Accessibility best-practices and more in-depth information is available on -[the Audit Rules page][audit-rules] for the Chrome Accessibility Developer Tools. - -## Security - -### Resources - -[Mozilla’s HTTP Observatory CLI][observatory-cli] and the -[Qualys SSL Labs Server Test][qualys-ssl] are good resources for finding -potential problems and ensuring compliance with security best practices. - -<!-- Uncomment these sections when CSP/SRI are implemented. -### Content Security Policy (CSP) - -Content Security Policy is a web standard that intends to mitigate certain -forms of Cross-Site Scripting (XSS) as well as data injection. - -Content Security Policy rules should be taken into consideration when -implementing new features, especially those that may rely on connection with -external services. - -GitLab's CSP is used for the following: - -- Blocking plugins like Flash and Silverlight from running at all on our pages. -- Blocking the use of scripts and stylesheets downloaded from external sources. -- Upgrading `http` requests to `https` when possible. -- Preventing `iframe` elements from loading in most contexts. - -Some exceptions include: - -- Scripts from Google Analytics and Piwik if either is enabled. -- Connecting with GitHub, Bitbucket, GitLab.com, etc. to allow project importing. -- Connecting with Google, Twitter, GitHub, etc. to allow OAuth authentication. - -We use [the Secure Headers gem][secure_headers] to enable Content -Security Policy headers in the GitLab Rails app. - -Some resources on implementing Content Security Policy: - -- [MDN Article on CSP][mdn-csp] -- [GitHub’s CSP Journey on the GitHub Engineering Blog][github-eng-csp] -- The Dropbox Engineering Blog's series on CSP: [1][dropbox-csp-1], [2][dropbox-csp-2], [3][dropbox-csp-3], [4][dropbox-csp-4] - -### Subresource Integrity (SRI) - -Subresource Integrity prevents malicious assets from being provided by a CDN by -guaranteeing that the asset downloaded is identical to the asset the server -is expecting. - -The Rails app generates a unique hash of the asset, which is used as the -asset's `integrity` attribute. The browser generates the hash of the asset -on-load and will reject the asset if the hashes do not match. - -All CSS and JavaScript assets should use Subresource Integrity. For implementation details, -see the documentation for [the Sprockets implementation of SRI][sprockets-sri]. - -Some resources on implementing Subresource Integrity: - -- [MDN Article on SRI][mdn-sri] -- [Subresource Integrity on the GitHub Engineering Blog][github-eng-sri] - ---> - -### Including external resources - -External fonts, CSS, and JavaScript should never be used with the exception of -Google Analytics and Piwik - and only when the instance has enabled it. Assets -should always be hosted and served locally from the GitLab instance. Embedded -resources via `iframes` should never be used except in certain circumstances -such as with ReCaptcha, which cannot be used without an `iframe`. - -### Avoiding inline scripts and styles - -In order to protect users from [XSS vulnerabilities][xss], we will disable inline scripts in the future using Content Security Policy. - -While inline scripts can be useful, they're also a security concern. If -user-supplied content is unintentionally left un-sanitized, malicious users can -inject scripts into the web app. - -Inline styles should be avoided in almost all cases, they should only be used -when no alternatives can be found. This allows reusability of styles as well as -readability. - -## Style guides and linting - -See the relevant style guides for our guidelines and for information on linting: - -- [SCSS][scss-style-guide] -- JavaScript - We defer to [AirBnb][airbnb-js-style-guide] on most style-related -conventions and enforce them with eslint. See [our current .eslintrc][eslintrc] -for specific rules and patterns. - -## Testing - -Feature tests need to be written for all new features. Regression tests -also need to be written for all bug fixes to prevent them from occurring -again in the future. - -See [the Testing Standards and Style Guidelines](testing.md) for more -information. - -### Running frontend tests - -`rake karma` runs the frontend-only (JavaScript) tests. -It consists of two subtasks: - -- `rake karma:fixtures` (re-)generates fixtures -- `rake karma:tests` actually executes the tests - -As long as the fixtures don't change, `rake karma:tests` is sufficient -(and saves you some time). - -Please note: Not all of the frontend fixtures are generated. Some are still static -files. These will not be touched by `rake karma:fixtures`. - -## Design Patterns - -### Singletons - -When exactly one object is needed for a given task, prefer to define it as a -`class` rather than as an object literal. Prefer also to explicitly restrict -instantiation, unless flexibility is important (e.g. for testing). - -```javascript -// bad - -gl.MyThing = { - prop1: 'hello', - method1: () => {} -}; - -// good - -class MyThing { - constructor() { - this.prop1 = 'hello'; - } - method1() {} -} - -gl.MyThing = new MyThing(); - -// best - -let singleton; - -class MyThing { - constructor() { - if (!singleton) { - singleton = this; - singleton.init(); - } - return singleton; - } - - init() { - this.prop1 = 'hello'; - } - - method1() {} -} - -gl.MyThing = MyThing; - -``` - -### Manipulating the DOM in a JS Class - -When writing a class that needs to manipulate the DOM guarantee a container option is provided. -This is useful when we need that class to be instantiated more than once in the same page. - -Bad: -```javascript -class Foo { - constructor() { - document.querySelector('.bar'); - } -} -new Foo(); -``` - -Good: -```javascript -class Foo { - constructor(opts) { - document.querySelector(`${opts.container} .bar`); - } -} - -new Foo({ container: '.my-element' }); -``` -You can find an example of the above in this [class][container-class-example]; - -## Supported browsers - -For our currently-supported browsers, see our [requirements][requirements]. - - -## Gotchas - -### Spec errors due to use of ES6 features in `.js` files - -If you see very generic JavaScript errors (e.g. `jQuery is undefined`) being -thrown in Karma, Spinach, or Rspec tests but can't reproduce them manually, -you may have included `ES6`-style JavaScript in files that don't have the -`.js.es6` file extension. Either use ES5-friendly JavaScript or rename the file -you're working in (`git mv <file.js> <file.js.es6>`). - -### Spec errors due to use of unsupported JavaScript - -Similar errors will be thrown if you're using JavaScript features not yet -supported by our test runner's version of webkit, whether or not you've updated -the file extension. Examples of unsupported JavaScript features are: - -- Array.from -- Array.find -- Array.first -- Object.assign -- Async functions -- Generators -- Array destructuring -- For Of -- Symbol/Symbol.iterator -- Spread - -Until these are polyfilled or transpiled appropriately, they should not be used. -Please update this list with additional unsupported features or when any of -these are made usable. - -### Spec errors due to JavaScript not enabled - -If, as a result of a change you've made, a feature now depends on JavaScript to -run correctly, you need to make sure a JavaScript web driver is enabled when -specs are run. If you don't you'll see vague error messages from the spec -runner, and an explosion of vague console errors in the HTML snapshot. - -To enable a JavaScript driver in an `rspec` test, add `js: true` to the -individual spec or the context block containing multiple specs that need -JavaScript enabled: - -```ruby - -# For one spec -it 'presents information about abuse report', js: true do - # assertions... -end - -describe "Admin::AbuseReports", js: true do - it 'presents information about abuse report' do - # assertions... - end - it 'shows buttons for adding to abuse report' do - # assertions... - end -end -``` - -In Spinach, the JavaScript driver is enabled differently. In the `*.feature` -file for the failing spec, add the `@javascript` flag above the Scenario: - -``` -@javascript -Scenario: Developer can approve merge request - Given I am a "Shop" developer - And I visit project "Shop" merge requests page - And merge request 'Bug NS-04' must be approved - And I click link "Bug NS-04" - When I click link "Approve" - Then I should see approved merge request "Bug NS-04" - -``` +# Frontend Development Guidelines -[rails]: http://rubyonrails.org/ -[haml]: http://haml.info/ -[hamlit]: https://github.com/k0kubun/hamlit -[hamlit-limits]: https://github.com/k0kubun/hamlit/blob/master/REFERENCE.md#limitations -[scss]: http://sass-lang.com/ -[es6]: https://babeljs.io/ -[sprockets]: https://github.com/rails/sprockets -[jquery]: https://jquery.com/ -[vue]: http://vuejs.org/ -[vue-docs]: http://vuejs.org/guide/index.html -[web-page-test]: http://www.webpagetest.org/ -[pagespeed-insights]: https://developers.google.com/speed/pagespeed/insights/ -[google-devtools-profiling]: https://developers.google.com/web/tools/chrome-devtools/profile/?hl=en -[browser-diet]: https://browserdiet.com/ -[d3]: https://d3js.org/ -[chartjs]: http://www.chartjs.org/ -[page-specific-js-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/13bb9ed77f405c5f6ee4fdbc964ecf635c9a223f/app/views/projects/graphs/_head.html.haml#L6-8 -[chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools -[audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules -[observatory-cli]: https://github.com/mozilla/http-observatory-cli -[qualys-ssl]: https://www.ssllabs.com/ssltest/analyze.html -[secure_headers]: https://github.com/twitter/secureheaders -[mdn-csp]: https://developer.mozilla.org/en-US/docs/Web/Security/CSP -[github-eng-csp]: http://githubengineering.com/githubs-csp-journey/ -[dropbox-csp-1]: https://blogs.dropbox.com/tech/2015/09/on-csp-reporting-and-filtering/ -[dropbox-csp-2]: https://blogs.dropbox.com/tech/2015/09/unsafe-inline-and-nonce-deployment/ -[dropbox-csp-3]: https://blogs.dropbox.com/tech/2015/09/csp-the-unexpected-eval/ -[dropbox-csp-4]: https://blogs.dropbox.com/tech/2015/09/csp-third-party-integrations-and-privilege-separation/ -[mdn-sri]: https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity -[github-eng-sri]: http://githubengineering.com/subresource-integrity/ -[sprockets-sri]: https://github.com/rails/sprockets-rails#sri-support -[xss]: https://en.wikipedia.org/wiki/Cross-site_scripting -[scss-style-guide]: scss_styleguide.md -[requirements]: ../install/requirements.md#supported-web-browsers -[issue-boards]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/boards -[environments-table]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/environments -[page_specific_javascript]: https://docs.gitlab.com/ce/development/frontend.html#page-specific-javascript -[component-system]: https://vuejs.org/v2/guide/#Composing-with-Components -[state-management]: https://vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch -[vue-resource-repo]: https://github.com/pagekit/vue-resource -[issue-boards-service]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/boards/services/board_service.js.es6 -[airbnb-js-style-guide]: https://github.com/airbnb/javascript -[eslintrc]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.eslintrc -[team-page]: https://about.gitlab.com/team -[vue-section]: https://docs.gitlab.com/ce/development/frontend.html#how-to-build-a-new-feature-with-vue-js -[container-class-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/mini_pipeline_graph_dropdown.js +This page has moved [here](fe_guide/index.md). diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index dcd978c4cd3..ec9e4dcc59d 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -42,6 +42,20 @@ To run several tests inside one directory: If you want to use [Spring](https://github.com/rails/spring) set `ENABLE_SPRING=1` in your environment. +## Compile Frontend Assets + +You shouldn't ever need to compile frontend assets manually in development, but +if you ever need to test how the assets get compiled in a production +environment you can do so with the following command: + +``` +RAILS_ENV=production NODE_ENV=production bundle exec rake gitlab:assets:compile +``` + +This will compile and minify all JavaScript and CSS assets and copy them along +with all other frontend assets (images, fonts, etc) into `/public/assets` where +they can be easily inspected. + ## Generate API documentation for project services (e.g. Slack) ``` diff --git a/doc/development/testing.md b/doc/development/testing.md index 5ac7b8dadeb..5bc958f5a96 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -34,16 +34,17 @@ GitLab uses [factory_girl] as a test fixture replacement. GitLab uses [Karma] to run its [Jasmine] JavaScript specs. They can be run on the command line via `bundle exec karma`. -- JavaScript tests live in `spec/javascripts/`, matching the folder structure of - `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js` has a corresponding - `spec/javascripts/behaviors/autosize_spec.js` file. +- JavaScript tests live in `spec/javascripts/`, matching the folder structure + of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js` + has a corresponding `spec/javascripts/behaviors/autosize_spec.js` file. - Haml fixtures required for JavaScript tests live in `spec/javascripts/fixtures`. They should contain the bare minimum amount of markup necessary for the test. > **Warning:** Keep in mind that a Rails view may change and invalidate your test, but everything will still pass because your fixture - doesn't reflect the latest view. + doesn't reflect the latest view. Because of this we encourage you to + generate fixtures from actual rails views whenever possible. - Keep in mind that in a CI environment, these tests are run in a headless browser and you will not have access to certain APIs, such as @@ -53,6 +54,8 @@ the command line via `bundle exec karma`. [Karma]: https://github.com/karma-runner/karma [Jasmine]: https://github.com/jasmine/jasmine +For more information, see the [frontend testing guide](fe_guide/testing.md). + ## RSpec ### General Guidelines diff --git a/doc/install/installation.md b/doc/install/installation.md index e79b8d6ce14..a6b10176450 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -109,7 +109,8 @@ Then select 'Internet Site' and press enter to confirm the hostname. ## 2. Ruby -**Note:** The current supported Ruby versions are 2.1.x and 2.3.x. 2.3.x is preferred, and support for 2.1.x will be dropped in the future. +**Note:** The current supported Ruby version is 2.3.x. GitLab 9.0 dropped support +for Ruby 2.1.x. The use of Ruby version managers such as [RVM], [rbenv] or [chruby] with GitLab in production, frequently leads to hard to diagnose problems. For example, diff --git a/doc/profile/README.md b/doc/profile/README.md index 6f8359d87fa..54e44d65959 100644 --- a/doc/profile/README.md +++ b/doc/profile/README.md @@ -1,4 +1,4 @@ -# Profile Settings +# Profile settings -- [Preferences](preferences.md) -- [Two-factor Authentication (2FA)](two_factor_authentication.md) +- [Preferences](../user/profile/preferences.md) +- [Two-factor Authentication (2FA)](../user/profile/account/two_factor_authentication.md) diff --git a/doc/profile/preferences.md b/doc/profile/preferences.md index 4f2b00f3dd1..cc16f3afe41 100644 --- a/doc/profile/preferences.md +++ b/doc/profile/preferences.md @@ -1,36 +1 @@ -# Profile Preferences - -Settings in the **Profile > Preferences** page allow the user to customize -various aspects of the site to their liking. - -## Syntax highlighting theme - -_GitLab uses the [rouge ruby library][rouge] for syntax highlighting. For a -list of supported languages visit the rouge website._ - -Changing this setting allows the user to customize the theme used when viewing -syntax highlighted code on the site. - -The default is **White**. - -## Behavior - -### Default Dashboard - -For users who have access to a large number of projects but only keep up with a -select few, the amount of activity on the default Dashboard page can be -overwhelming. - -Changing this setting allows the user to redefine what their default dashboard -will be. Setting it to **Starred Projects** will make that Dashboard view the -default when signing in or clicking the application logo in the upper left. - -The default is **Your Projects**. - -### Default Project view - -It allows user to choose what content he or she want to see on project page. - -The default is **Readme**. - -[rouge]: http://rouge.jneen.net/ "Rouge website" +This document was moved to [another location](../user/profile/preferences.md). diff --git a/doc/update/8.13-to-8.14.md b/doc/update/8.13-to-8.14.md index 327ecb7cdc2..aa1c659717e 100644 --- a/doc/update/8.13-to-8.14.md +++ b/doc/update/8.13-to-8.14.md @@ -72,7 +72,7 @@ sudo -u git -H git checkout 8-14-stable-ee ```bash cd /home/git/gitlab-shell sudo -u git -H git fetch --all --tags -sudo -u git -H git checkout v4.0.3 +sudo -u git -H git checkout v4.1.1 ``` ### 6. Update gitlab-workhorse diff --git a/doc/update/8.17-to-9.0.md b/doc/update/8.17-to-9.0.md index 140d3684c56..6308317b1f2 100644 --- a/doc/update/8.17-to-9.0.md +++ b/doc/update/8.17-to-9.0.md @@ -25,9 +25,8 @@ sudo -u git -H bundle exec rake gitlab:backup:create RAILS_ENV=production ### 3. Update Ruby -We will continue supporting Ruby < 2.3 for the time being but we recommend you -upgrade to Ruby 2.3 if you're running a source installation, as this is the same -version that ships with our Omnibus package. +NOTE: GitLab 9.0 only supports Ruby 2.3.x and dropped support for Ruby 2.1.x. Be +sure to upgrade your interpreter if necessary. You can check which version you are running with `ruby -v`. @@ -98,62 +97,32 @@ cd /home/git/gitlab sudo -u git -H git checkout 9-0-stable-ee ``` -### 6. Install libs, migrations, etc. +### 6. Update gitlab-shell ```bash -cd /home/git/gitlab - -# MySQL installations (note: the line below states '--without postgres') -sudo -u git -H bundle install --without postgres development test --deployment - -# PostgreSQL installations (note: the line below states '--without mysql') -sudo -u git -H bundle install --without mysql development test --deployment - -# Optional: clean up old gems -sudo -u git -H bundle clean - -# Run database migrations -sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production - -# Update node dependencies and recompile assets -sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile RAILS_ENV=production NODE_ENV=production +cd /home/git/gitlab-shell -# Clean up cache -sudo -u git -H bundle exec rake cache:clear RAILS_ENV=production +sudo -u git -H git fetch --all --tags +sudo -u git -H git checkout v$(</home/git/gitlab/GITLAB_SHELL_VERSION) ``` -**MySQL installations**: Run through the `MySQL strings limits` and `Tables and data conversion to utf8mb4` [tasks](../install/database_mysql.md). - ### 7. Update gitlab-workhorse Install and compile gitlab-workhorse. This requires [Go 1.5](https://golang.org/dl) which should already be on your system from -GitLab 8.1. - -```bash -cd /home/git/gitlab - -sudo -u git -H bundle exec rake "gitlab:workhorse:install[/home/git/gitlab-workhorse]" RAILS_ENV=production -``` - -### 8. Update gitlab-shell +GitLab 8.1. GitLab-Workhorse uses [GNU Make](https://www.gnu.org/software/make/). +If you are not using Linux you may have to run `gmake` instead of +`make` below. ```bash -cd /home/git/gitlab-shell +cd /home/git/gitlab-workhorse sudo -u git -H git fetch --all --tags -sudo -u git -H git checkout v5.0.0 +sudo -u git -H git checkout v$(</home/git/gitlab/GITLAB_WORKHORSE_VERSION) +sudo -u git -H make ``` -### 9. Optional: install Gitaly - -Gitaly is still an optional component of GitLab. If you want to save time -during your 9.0 upgrade **you can skip this step**. - -If you do want to set up Gitaly in GitLab 9.0 then follow [Gitaly section of the installation -guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/9-0-stable/doc/install/installation.md#install-gitaly). - -### 10. Update configuration files +### 8. Update configuration files #### New configuration options for `gitlab.yml` @@ -290,6 +259,40 @@ For Ubuntu 16.04.1 LTS: sudo systemctl daemon-reload ``` +### 9. Install libs, migrations, etc. + +```bash +cd /home/git/gitlab + +# MySQL installations (note: the line below states '--without postgres') +sudo -u git -H bundle install --without postgres development test --deployment + +# PostgreSQL installations (note: the line below states '--without mysql') +sudo -u git -H bundle install --without mysql development test --deployment + +# Optional: clean up old gems +sudo -u git -H bundle clean + +# Run database migrations +sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production + +# Update node dependencies and recompile assets +sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile RAILS_ENV=production NODE_ENV=production + +# Clean up cache +sudo -u git -H bundle exec rake cache:clear RAILS_ENV=production +``` + +**MySQL installations**: Run through the `MySQL strings limits` and `Tables and data conversion to utf8mb4` [tasks](../install/database_mysql.md). + +### 10. Optional: install Gitaly + +Gitaly is still an optional component of GitLab. If you want to save time +during your 9.0 upgrade **you can skip this step**. + +If you do want to set up Gitaly in GitLab 9.0 then follow [Gitaly section of the installation +guide](../install/installation.md#install-gitaly). + ### 11. Start application ```bash diff --git a/doc/user/profile/preferences.md b/doc/user/profile/preferences.md new file mode 100644 index 00000000000..e5038835027 --- /dev/null +++ b/doc/user/profile/preferences.md @@ -0,0 +1,64 @@ +# Profile preferences + +A user's profile preferences page allows the user to customize various aspects +of GitLab to their liking. + +To navigate to your profile's preferences, click your avatar icon in the top +right corner and select **Settings**. From there on, choose the **Preferences** +tab. + +## Syntax highlighting theme + +>**Note:** +GitLab uses the [rouge Ruby library][rouge] for syntax highlighting. For a +list of supported languages visit the rouge website. + +Changing this setting allows you to customize the color theme when viewing any +syntax highlighted code on GitLab. + +The default one is **White**, and you can choose among 5 different colors: + +- White +- Dark +- Solarized light +- Solarized dark +- Monokai + +## Behavior + +The following settings allow you to customize the behavior of GitLab's layout +and default views of your dashboard and the projects' landing pages. + +### Layout width + +GitLab can be set up to use different widths depending on your liking. Choose +between the fixed (max. 1200px) and the fluid (100%) application layout. + +### Default dashboard + +For users who have access to a large number of projects but only keep up with a +select few, the amount of activity on the default Dashboard page can be +overwhelming. Changing this setting allows you to redefine what your default +dashboard will be. + +You have 6 options here that you can use for your default dashboard view: + +- Your projects (default) +- Starred projects +- Your projects' activity +- Starred projects' activity +- Your groups +- Your [Todos] + +### Default project view + +The default project view settings allows you to choose what content you want to +see on a project's landing page. + +You can choose between 2 options: + +- Show the files and the readme (default) +- Show the project's activity + +[rouge]: http://rouge.jneen.net/ "Rouge website" +[todos]: ../../workflow/todos.md diff --git a/lib/gitlab/etag_caching/store.rb b/lib/gitlab/etag_caching/store.rb index 9532e432f78..0039fc01c8f 100644 --- a/lib/gitlab/etag_caching/store.rb +++ b/lib/gitlab/etag_caching/store.rb @@ -1,7 +1,7 @@ module Gitlab module EtagCaching class Store - EXPIRY_TIME = 10.minutes + EXPIRY_TIME = 20.minutes REDIS_NAMESPACE = 'etag:'.freeze def get(key) diff --git a/lib/gitlab/testing/request_blocker_middleware.rb b/lib/gitlab/testing/request_blocker_middleware.rb new file mode 100644 index 00000000000..aa67fa08577 --- /dev/null +++ b/lib/gitlab/testing/request_blocker_middleware.rb @@ -0,0 +1,61 @@ +# rubocop:disable Style/ClassVars + +# This is inspired by http://www.salsify.com/blog/engineering/tearing-capybara-ajax-tests +# Rack middleware that keeps track of the number of active requests and can block new requests. +module Gitlab + module Testing + class RequestBlockerMiddleware + @@num_active_requests = Concurrent::AtomicFixnum.new(0) + @@block_requests = Concurrent::AtomicBoolean.new(false) + + # Returns the number of requests the server is currently processing. + def self.num_active_requests + @@num_active_requests.value + end + + # Prevents the server from accepting new requests. Any new requests will return an HTTP + # 503 status. + def self.block_requests! + @@block_requests.value = true + end + + # Allows the server to accept requests again. + def self.allow_requests! + @@block_requests.value = false + end + + def initialize(app) + @app = app + end + + def call(env) + increment_active_requests + if block_requests? + block_request(env) + else + @app.call(env) + end + ensure + decrement_active_requests + end + + private + + def block_requests? + @@block_requests.true? + end + + def block_request(env) + [503, {}, []] + end + + def increment_active_requests + @@num_active_requests.increment + end + + def decrement_active_requests + @@num_active_requests.decrement + end + end + end +end diff --git a/package.json b/package.json index 91d8752bebb..7b6c4556e2c 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "stats-webpack-plugin": "^0.4.3", "timeago.js": "^2.0.5", "underscore": "^1.8.3", + "visibilityjs": "^1.2.4", "vue": "^2.2.4", "vue-resource": "^0.9.3", "webpack": "^2.2.1", diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 03daab12c8f..5099441dce2 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -34,6 +34,7 @@ feature 'Admin updates settings', feature: true do fill_in 'Username', with: 'test_user' fill_in 'service_push_channel', with: '#test_channel' page.check('Notify only broken pipelines') + page.check('Notify only default branch') check_all_events click_on 'Save' diff --git a/spec/features/boards/add_issues_modal_spec.rb b/spec/features/boards/add_issues_modal_spec.rb index d17a418b8c3..1c0f97d8a1c 100644 --- a/spec/features/boards/add_issues_modal_spec.rb +++ b/spec/features/boards/add_issues_modal_spec.rb @@ -23,6 +23,20 @@ describe 'Issue Boards add issue modal', :feature, :js do wait_for_vue_resource end + it 'resets filtered search state' do + visit namespace_project_board_path(project.namespace, project, board, search: 'testing') + + wait_for_vue_resource + + click_button('Add issues') + + page.within('.add-issues-modal') do + expect(find('.form-control').value).to eq('') + expect(page).to have_selector('.clear-search', visible: false) + expect(find('.form-control')[:placeholder]).to eq('Search or filter results...') + end + end + context 'modal interaction' do it 'opens modal' do click_button('Add issues') diff --git a/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb b/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb index 0ceaf7bc830..79105b1ee46 100644 --- a/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb +++ b/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb @@ -35,6 +35,8 @@ feature 'Merge immediately', :feature, :js do click_link 'Merge Immediately' expect(find('.js-merge-when-pipeline-succeeds-button')).to have_content('Merge in progress') + + wait_for_ajax end end end diff --git a/spec/features/participants_autocomplete_spec.rb b/spec/features/participants_autocomplete_spec.rb index c2545b0c259..decad589c23 100644 --- a/spec/features/participants_autocomplete_spec.rb +++ b/spec/features/participants_autocomplete_spec.rb @@ -1,101 +1,62 @@ require 'spec_helper' -feature 'Member autocomplete', feature: true do - include WaitForAjax - +feature 'Member autocomplete', :js do let(:project) { create(:project, :public) } let(:user) { create(:user) } - let(:participant) { create(:user) } let(:author) { create(:user) } + let(:note) { create(:note, noteable: noteable, project: noteable.project) } before do - allow_any_instance_of(Commit).to receive(:author).and_return(author) - login_as user + note # actually create the note + login_as(user) end - shared_examples "open suggestions" do - it 'displays suggestions' do - expect(page).to have_selector('.atwho-view', visible: true) - end - - it 'suggests author' do - page.within('.atwho-view', visible: true) do - expect(page).to have_content(author.username) + shared_examples "open suggestions when typing @" do + before do + page.within('.new-note') do + find('#note_note').send_keys('@') end end - it 'suggests participant' do + it 'suggests noteable author and note author' do page.within('.atwho-view', visible: true) do - expect(page).to have_content(participant.username) + expect(page).to have_content(author.username) + expect(page).to have_content(note.author.username) end end end - context 'adding a new note on a Issue', js: true do + context 'adding a new note on a Issue' do + let(:noteable) { create(:issue, author: author, project: project) } before do - issue = create(:issue, author: author, project: project) - create(:note, note: 'Ultralight Beam', noteable: issue, - project: project, author: participant) - visit_issue(project, issue) + visit namespace_project_issue_path(project.namespace, project, noteable) end - context 'when typing @' do - include_examples "open suggestions" - before do - open_member_suggestions - end - end + include_examples "open suggestions when typing @" end - context 'adding a new note on a Merge Request ', js: true do + context 'adding a new note on a Merge Request' do + let(:noteable) do + create(:merge_request, source_project: project, + target_project: project, author: author) + end before do - merge = create(:merge_request, source_project: project, target_project: project, author: author) - create(:note, note: 'Ultralight Beam', noteable: merge, - project: project, author: participant) - visit_merge_request(project, merge) + visit namespace_project_merge_request_path(project.namespace, project, noteable) end - context 'when typing @' do - include_examples "open suggestions" - before do - open_member_suggestions - end - end + include_examples "open suggestions when typing @" end - context 'adding a new note on a Commit ', js: true do - let(:commit) { project.commit } + context 'adding a new note on a Commit' do + let(:noteable) { project.commit } + let(:note) { create(:note_on_commit, project: project, commit_id: project.commit.id) } before do - allow(commit).to receive(:author).and_return(author) - create(:note_on_commit, author: participant, project: project, commit_id: project.repository.commit.id, note: 'No More Parties in LA') - visit_commit(project, commit) - end - - context 'when typing @' do - include_examples "open suggestions" - before do - open_member_suggestions - end - end - end + allow_any_instance_of(Commit).to receive(:author).and_return(author) - def open_member_suggestions - page.within('.new-note') do - find('#note_note').send_keys('@') + visit namespace_project_commit_path(project.namespace, project, noteable) end - wait_for_ajax - end - - def visit_issue(project, issue) - visit namespace_project_issue_path(project.namespace, project, issue) - end - - def visit_merge_request(project, merge) - visit namespace_project_merge_request_path(project.namespace, project, merge) - end - def visit_commit(project, commit) - visit namespace_project_commit_path(project.namespace, project, commit) + include_examples "open suggestions when typing @" end end diff --git a/spec/features/projects/deploy_keys_spec.rb b/spec/features/projects/deploy_keys_spec.rb new file mode 100644 index 00000000000..0b997f130ea --- /dev/null +++ b/spec/features/projects/deploy_keys_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe 'Project deploy keys', feature: true do + let(:user) { create(:user) } + let(:project) { create(:project_empty_repo) } + + before do + project.team << [user, :master] + login_as(user) + end + + describe 'removing key' do + before do + create(:deploy_keys_project, project: project) + end + + it 'removes association between project and deploy key' do + visit namespace_project_settings_repository_path(project.namespace, project) + + page.within '.deploy-keys' do + expect { click_on 'Remove' } + .to change { project.deploy_keys.count }.by(-1) + end + end + end +end diff --git a/spec/features/user_callout_spec.rb b/spec/features/user_callout_spec.rb index 336c4092c98..659cd7c7af7 100644 --- a/spec/features/user_callout_spec.rb +++ b/spec/features/user_callout_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe 'User Callouts', js: true do let(:user) { create(:user) } + let(:another_user) { create(:user) } let(:project) { create(:empty_project, path: 'gitlab', name: 'sample') } before do @@ -32,6 +33,11 @@ describe 'User Callouts', js: true do within('.user-callout') do find('.close-user-callout').click end - expect(page).not_to have_selector('#user-callout') + expect(page).not_to have_selector('.user-callout') + end + + it 'does not show callout on another users profile' do + visit user_path(another_user) + expect(page).not_to have_selector('.user-callout') end end diff --git a/spec/features/users/projects_spec.rb b/spec/features/users/projects_spec.rb new file mode 100644 index 00000000000..1d75fe434b0 --- /dev/null +++ b/spec/features/users/projects_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe 'Projects tab on a user profile', :feature, :js do + include WaitForAjax + + let(:user) { create(:user) } + let!(:project) { create(:empty_project, namespace: user.namespace) } + let!(:project2) { create(:empty_project, namespace: user.namespace) } + + before do + allow(Project).to receive(:default_per_page).and_return(1) + + login_as(user) + + visit user_path(user) + + page.within('.user-profile-nav') do + click_link('Personal projects') + end + + wait_for_ajax + end + + it 'paginates results' do + expect(page).to have_content(project2.name) + + click_link('Next') + + expect(page).to have_content(project.name) + end +end diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js index a033ac04da6..e21f4ca2bc0 100644 --- a/spec/javascripts/boards/boards_store_spec.js +++ b/spec/javascripts/boards/boards_store_spec.js @@ -1,12 +1,12 @@ /* eslint-disable comma-dangle, one-var, no-unused-vars */ /* global BoardService */ /* global boardsMockInterceptor */ -/* global Cookies */ /* global listObj */ /* global listObjDuplicate */ /* global ListIssue */ import Vue from 'vue'; +import Cookies from 'js-cookie'; require('~/lib/utils/url_utility'); require('~/boards/models/issue'); diff --git a/spec/javascripts/filtered_search/filtered_search_manager_spec.js b/spec/javascripts/filtered_search/filtered_search_manager_spec.js index 113161c21c6..848c7656a8d 100644 --- a/spec/javascripts/filtered_search/filtered_search_manager_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_manager_spec.js @@ -58,7 +58,7 @@ const FilteredSearchSpecHelper = require('../helpers/filtered_search_spec_helper }); describe('search', () => { - const defaultParams = '?scope=all&utf8=✓&state=opened'; + const defaultParams = '?scope=all&utf8=%E2%9C%93&state=opened'; it('should search with a single word', (done) => { input.value = 'searchTerm'; diff --git a/spec/javascripts/lib/utils/poll_spec.js b/spec/javascripts/lib/utils/poll_spec.js new file mode 100644 index 00000000000..c794a632417 --- /dev/null +++ b/spec/javascripts/lib/utils/poll_spec.js @@ -0,0 +1,163 @@ +import Vue from 'vue'; +import VueResource from 'vue-resource'; +import Poll from '~/lib/utils/poll'; + +Vue.use(VueResource); + +class ServiceMock { + constructor(endpoint) { + this.service = Vue.resource(endpoint); + } + + fetch() { + return this.service.get(); + } +} + +describe('Poll', () => { + let callbacks; + + beforeEach(() => { + callbacks = { + success: () => {}, + error: () => {}, + }; + + spyOn(callbacks, 'success'); + spyOn(callbacks, 'error'); + }); + + it('calls the success callback when no header for interval is provided', (done) => { + const successInterceptor = (request, next) => { + next(request.respondWith(JSON.stringify([]), { status: 200 })); + }; + + Vue.http.interceptors.push(successInterceptor); + + new Poll({ + resource: new ServiceMock('endpoint'), + method: 'fetch', + successCallback: callbacks.success, + errorCallback: callbacks.error, + }).makeRequest(); + + setTimeout(() => { + expect(callbacks.success).toHaveBeenCalled(); + expect(callbacks.error).not.toHaveBeenCalled(); + done(); + }, 0); + + Vue.http.interceptors = _.without(Vue.http.interceptors, successInterceptor); + }); + + it('calls the error callback whe the http request returns an error', (done) => { + const errorInterceptor = (request, next) => { + next(request.respondWith(JSON.stringify([]), { status: 500 })); + }; + + Vue.http.interceptors.push(errorInterceptor); + + new Poll({ + resource: new ServiceMock('endpoint'), + method: 'fetch', + successCallback: callbacks.success, + errorCallback: callbacks.error, + }).makeRequest(); + + setTimeout(() => { + expect(callbacks.success).not.toHaveBeenCalled(); + expect(callbacks.error).toHaveBeenCalled(); + done(); + }, 0); + + Vue.http.interceptors = _.without(Vue.http.interceptors, errorInterceptor); + }); + + it('should call the success callback when the interval header is -1', (done) => { + const intervalInterceptor = (request, next) => { + next(request.respondWith(JSON.stringify([]), { status: 200, headers: { 'poll-interval': -1 } })); + }; + + Vue.http.interceptors.push(intervalInterceptor); + + new Poll({ + resource: new ServiceMock('endpoint'), + method: 'fetch', + successCallback: callbacks.success, + errorCallback: callbacks.error, + }).makeRequest(); + + setTimeout(() => { + expect(callbacks.success).toHaveBeenCalled(); + expect(callbacks.error).not.toHaveBeenCalled(); + done(); + }, 0); + + Vue.http.interceptors = _.without(Vue.http.interceptors, intervalInterceptor); + }); + + it('starts polling when http status is 200 and interval header is provided', (done) => { + const pollInterceptor = (request, next) => { + next(request.respondWith(JSON.stringify([]), { status: 200, headers: { 'poll-interval': 2 } })); + }; + + Vue.http.interceptors.push(pollInterceptor); + + const service = new ServiceMock('endpoint'); + spyOn(service, 'fetch').and.callThrough(); + + new Poll({ + resource: service, + method: 'fetch', + data: { page: 1 }, + successCallback: callbacks.success, + errorCallback: callbacks.error, + }).makeRequest(); + + setTimeout(() => { + expect(service.fetch.calls.count()).toEqual(2); + expect(service.fetch).toHaveBeenCalledWith({ page: 1 }); + expect(callbacks.success).toHaveBeenCalled(); + expect(callbacks.error).not.toHaveBeenCalled(); + done(); + }, 5); + + Vue.http.interceptors = _.without(Vue.http.interceptors, pollInterceptor); + }); + + describe('stop', () => { + it('stops polling when method is called', (done) => { + const pollInterceptor = (request, next) => { + next(request.respondWith(JSON.stringify([]), { status: 200, headers: { 'poll-interval': 2 } })); + }; + + Vue.http.interceptors.push(pollInterceptor); + + const service = new ServiceMock('endpoint'); + spyOn(service, 'fetch').and.callThrough(); + + const Polling = new Poll({ + resource: service, + method: 'fetch', + data: { page: 1 }, + successCallback: () => { + Polling.stop(); + }, + errorCallback: callbacks.error, + }); + + spyOn(Polling, 'stop').and.callThrough(); + + Polling.makeRequest(); + + setTimeout(() => { + expect(service.fetch.calls.count()).toEqual(1); + expect(service.fetch).toHaveBeenCalledWith({ page: 1 }); + expect(Polling.stop).toHaveBeenCalled(); + done(); + }, 100); + + Vue.http.interceptors = _.without(Vue.http.interceptors, pollInterceptor); + }); + }); +}); diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 16df1ad4f28..15465588223 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -8,7 +8,6 @@ jasmine.getJSONFixtures().fixturesPath = 'base/spec/javascripts/fixtures'; require('~/commons/index.js'); window.$ = window.jQuery = require('jquery'); window._ = require('underscore'); -window.Cookies = require('js-cookie'); // stub expected globals window.gl = window.gl || {}; diff --git a/spec/javascripts/user_callout_spec.js b/spec/javascripts/user_callout_spec.js index 205e72af600..2398149d3ad 100644 --- a/spec/javascripts/user_callout_spec.js +++ b/spec/javascripts/user_callout_spec.js @@ -1,7 +1,7 @@ -const UserCallout = require('~/user_callout'); +import Cookies from 'js-cookie'; +import UserCallout from '~/user_callout'; const USER_CALLOUT_COOKIE = 'user_callout_dismissed'; -const Cookie = window.Cookies; describe('UserCallout', function () { const fixtureName = 'static/user_callout.html.raw'; @@ -9,7 +9,7 @@ describe('UserCallout', function () { beforeEach(() => { loadFixtures(fixtureName); - Cookie.remove(USER_CALLOUT_COOKIE); + Cookies.remove(USER_CALLOUT_COOKIE); this.userCallout = new UserCallout(); this.closeButton = $('.close-user-callout'); @@ -18,25 +18,25 @@ describe('UserCallout', function () { }); it('does not show when cookie is set not defined', () => { - expect(Cookie.get(USER_CALLOUT_COOKIE)).toBeUndefined(); + expect(Cookies.get(USER_CALLOUT_COOKIE)).toBeUndefined(); expect(this.userCalloutContainer.is(':visible')).toBe(true); }); it('shows when cookie is set to false', () => { - Cookie.set(USER_CALLOUT_COOKIE, 'false'); + Cookies.set(USER_CALLOUT_COOKIE, 'false'); - expect(Cookie.get(USER_CALLOUT_COOKIE)).toBeDefined(); + expect(Cookies.get(USER_CALLOUT_COOKIE)).toBeDefined(); expect(this.userCalloutContainer.is(':visible')).toBe(true); }); it('hides when user clicks on the dismiss-icon', () => { this.closeButton.click(); - expect(Cookie.get(USER_CALLOUT_COOKIE)).toBe('true'); + expect(Cookies.get(USER_CALLOUT_COOKIE)).toBe('true'); }); it('hides when user clicks on the "check it out" button', () => { this.userCalloutBtn.click(); - expect(Cookie.get(USER_CALLOUT_COOKIE)).toBe('true'); + expect(Cookies.get(USER_CALLOUT_COOKIE)).toBe('true'); }); }); @@ -46,7 +46,7 @@ describe('UserCallout when cookie is present', function () { beforeEach(() => { loadFixtures(fixtureName); - Cookie.set(USER_CALLOUT_COOKIE, 'true'); + Cookies.set(USER_CALLOUT_COOKIE, 'true'); this.userCallout = new UserCallout(); this.userCalloutContainer = $('.user-callout'); }); diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb index e1877d5fde0..5ca936f28f0 100644 --- a/spec/mailers/emails/profile_spec.rb +++ b/spec/mailers/emails/profile_spec.rb @@ -5,6 +5,16 @@ describe Notify do include EmailSpec::Matchers include_context 'gitlab email notification' + shared_examples 'a new user email' do + it 'is sent to the new user with the correct subject and body' do + aggregate_failures do + is_expected.to deliver_to new_user_address + is_expected.to have_subject(/^Account was created for you$/i) + is_expected.to have_body_text(new_user_address) + end + end + end + describe 'profile notifications' do describe 'for new users, the email' do let(:example_site_path) { root_path } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 6ee91576676..4b72eb2eaa3 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -24,14 +24,14 @@ describe Notify do let(:previous_assignee) { create(:user, name: 'Previous Assignee') } shared_examples 'an assignee email' do - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + it 'is sent to the assignee as the author' do + sender = subject.header[:from].addrs.first - it 'is sent to the assignee' do - is_expected.to deliver_to assignee.email + aggregate_failures do + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + expect(subject).to deliver_to(assignee.email) + end end end @@ -49,12 +49,11 @@ describe Notify do it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' - it 'has the correct subject' do - is_expected.to have_referable_subject(issue) - end - - it 'contains a link to the new issue' do - is_expected.to have_body_text namespace_project_issue_path(project.namespace, project, issue) + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue) + is_expected.to have_body_text(namespace_project_issue_path(project.namespace, project, issue)) + end end context 'when enabled email_author_in_body' do @@ -63,7 +62,7 @@ describe Notify do end it 'contains a link to note author' do - is_expected.to have_html_escaped_body_text issue.author_name + is_expected.to have_html_escaped_body_text(issue.author_name) is_expected.to have_body_text 'wrote:' end end @@ -95,20 +94,13 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do - is_expected.to have_referable_subject(issue, reply: true) - end - - it 'contains the name of the previous assignee' do - is_expected.to have_html_escaped_body_text previous_assignee.name - end - - it 'contains the name of the new assignee' do - is_expected.to have_html_escaped_body_text assignee.name - end - - it 'contains a link to the issue' do - is_expected.to have_body_text namespace_project_issue_path(project.namespace, project, issue) + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_html_escaped_body_text(previous_assignee.name) + is_expected.to have_html_escaped_body_text(assignee.name) + is_expected.to have_body_text(namespace_project_issue_path(project.namespace, project, issue)) + end end end @@ -129,16 +121,12 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do - is_expected.to have_referable_subject(issue, reply: true) - end - - it 'contains the names of the added labels' do - is_expected.to have_body_text 'foo, bar, and baz' - end - - it 'contains a link to the issue' do - is_expected.to have_body_text namespace_project_issue_path(project.namespace, project, issue) + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text('foo, bar, and baz') + is_expected.to have_body_text(namespace_project_issue_path(project.namespace, project, issue)) + end end end @@ -158,20 +146,13 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do - is_expected.to have_referable_subject(issue, reply: true) - end - - it 'contains the new status' do - is_expected.to have_body_text status - end - - it 'contains the user name' do - is_expected.to have_html_escaped_body_text current_user.name - end - - it 'contains a link to the issue' do - is_expected.to have_body_text(namespace_project_issue_path project.namespace, project, issue) + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text(status) + is_expected.to have_html_escaped_body_text(current_user.name) + is_expected.to have_body_text(namespace_project_issue_path project.namespace, project, issue) + end end end @@ -189,18 +170,15 @@ describe Notify do is_expected.to have_body_text 'Issue was moved to another project' end - it 'has the correct subject' do - is_expected.to have_referable_subject(issue, reply: true) - end - - it 'contains link to new issue' do + it 'has the correct subject and body' do new_issue_url = namespace_project_issue_path(new_issue.project.namespace, new_issue.project, new_issue) - is_expected.to have_body_text new_issue_url - end - it 'contains a link to the original issue' do - is_expected.to have_body_text namespace_project_issue_path(project.namespace, project, issue) + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text(new_issue_url) + is_expected.to have_body_text(namespace_project_issue_path(project.namespace, project, issue)) + end end end end @@ -220,20 +198,13 @@ describe Notify do it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' - it 'has the correct subject' do - is_expected.to have_referable_subject(merge_request) - end - - it 'contains a link to the new merge request' do - is_expected.to have_body_text namespace_project_merge_request_path(project.namespace, project, merge_request) - end - - it 'contains the source branch for the merge request' do - is_expected.to have_body_text merge_request.source_branch - end - - it 'contains the target branch for the merge request' do - is_expected.to have_body_text merge_request.target_branch + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request) + is_expected.to have_body_text(namespace_project_merge_request_path(project.namespace, project, merge_request)) + is_expected.to have_body_text(merge_request.source_branch) + is_expected.to have_body_text(merge_request.target_branch) + end end context 'when enabled email_author_in_body' do @@ -275,20 +246,13 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do - is_expected.to have_referable_subject(merge_request, reply: true) - end - - it 'contains the name of the previous assignee' do - is_expected.to have_html_escaped_body_text previous_assignee.name - end - - it 'contains the name of the new assignee' do - is_expected.to have_html_escaped_body_text assignee.name - end - - it 'contains a link to the merge request' do - is_expected.to have_body_text namespace_project_merge_request_path(project.namespace, project, merge_request) + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_html_escaped_body_text(previous_assignee.name) + is_expected.to have_body_text(namespace_project_merge_request_path(project.namespace, project, merge_request)) + is_expected.to have_html_escaped_body_text(assignee.name) + end end end @@ -309,16 +273,10 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do + it 'has the correct subject and body' do is_expected.to have_referable_subject(merge_request, reply: true) - end - - it 'contains the names of the added labels' do - is_expected.to have_body_text 'foo, bar, and baz' - end - - it 'contains a link to the merge request' do - is_expected.to have_body_text namespace_project_merge_request_path(project.namespace, project, merge_request) + is_expected.to have_body_text('foo, bar, and baz') + is_expected.to have_body_text(namespace_project_merge_request_path(project.namespace, project, merge_request)) end end @@ -338,20 +296,13 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do - is_expected.to have_referable_subject(merge_request, reply: true) - end - - it 'contains the new status' do - is_expected.to have_body_text status - end - - it 'contains the user name' do - is_expected.to have_html_escaped_body_text current_user.name - end - - it 'contains a link to the merge request' do - is_expected.to have_body_text namespace_project_merge_request_path(project.namespace, project, merge_request) + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_body_text(status) + is_expected.to have_html_escaped_body_text(current_user.name) + is_expected.to have_body_text(namespace_project_merge_request_path(project.namespace, project, merge_request)) + end end end @@ -371,16 +322,12 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do - is_expected.to have_referable_subject(merge_request, reply: true) - end - - it 'contains the new status' do - is_expected.to have_body_text 'merged' - end - - it 'contains a link to the merge request' do - is_expected.to have_body_text namespace_project_merge_request_path(project.namespace, project, merge_request) + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_body_text('merged') + is_expected.to have_body_text(namespace_project_merge_request_path(project.namespace, project, merge_request)) + end end end end @@ -395,16 +342,10 @@ describe Notify do it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" - it 'has the correct subject' do - is_expected.to have_subject "#{project.name} | Project was moved" - end - - it 'contains name of project' do + it 'has the correct subject and body' do + is_expected.to have_subject("#{project.name} | Project was moved") is_expected.to have_html_escaped_body_text project.name_with_namespace - end - - it 'contains new user role' do - is_expected.to have_body_text project.ssh_url_to_repo + is_expected.to have_body_text(project.ssh_url_to_repo) end end @@ -597,14 +538,14 @@ describe Notify do shared_examples 'a note email' do it_behaves_like 'it should have Gmail Actions links' - it 'is sent as the author' do + it 'is sent to the given recipient as the author' do sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(note_author.name) - expect(sender.address).to eq(gitlab_sender) - end - it 'is sent to the given recipient' do - is_expected.to deliver_to recipient.notification_email + aggregate_failures do + expect(sender.display_name).to eq(note_author.name) + expect(sender.address).to eq(gitlab_sender) + expect(subject).to deliver_to(recipient.notification_email) + end end it 'contains the message from the note' do @@ -641,12 +582,11 @@ describe Notify do it_behaves_like 'it should show Gmail Actions View Commit link' it_behaves_like 'a user cannot unsubscribe through footer link' - it 'has the correct subject' do - is_expected.to have_subject "Re: #{project.name} | #{commit.title.strip} (#{commit.short_id})" - end - - it 'contains a link to the commit' do - is_expected.to have_body_text commit.short_id + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_subject("Re: #{project.name} | #{commit.title.strip} (#{commit.short_id})") + is_expected.to have_body_text(commit.short_id) + end end end @@ -664,12 +604,11 @@ describe Notify do it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' - it 'has the correct subject' do - is_expected.to have_referable_subject(merge_request, reply: true) - end - - it 'contains a link to the merge request note' do - is_expected.to have_body_text note_on_merge_request_path + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_body_text note_on_merge_request_path + end end end @@ -687,12 +626,11 @@ describe Notify do it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' - it 'has the correct subject' do - is_expected.to have_referable_subject(issue, reply: true) - end - - it 'contains a link to the issue note' do - is_expected.to have_body_text note_on_issue_path + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text(note_on_issue_path) + end end end end @@ -717,14 +655,14 @@ describe Notify do it_behaves_like 'it should have Gmail Actions links' - it 'is sent as the author' do + it 'is sent to the given recipient as the author' do sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(note_author.name) - expect(sender.address).to eq(gitlab_sender) - end - it 'is sent to the given recipient' do - is_expected.to deliver_to recipient.notification_email + aggregate_failures do + expect(sender.display_name).to eq(note_author.name) + expect(sender.address).to eq(gitlab_sender) + expect(subject).to deliver_to(recipient.notification_email) + end end it 'contains the message from the note' do @@ -934,21 +872,20 @@ describe Notify do is_expected.to deliver_to 'new-email@mail.com' end - it 'has the correct subject' do - is_expected.to have_subject 'Confirmation instructions | A Nice Suffix' - end - - it 'includes a link to the site' do - is_expected.to have_body_text example_site_path + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_subject('Confirmation instructions | A Nice Suffix') + is_expected.to have_body_text(example_site_path) + end end end describe 'email on push for a created branch' do let(:example_site_path) { root_path } let(:user) { create(:user) } - let(:tree_path) { namespace_project_tree_path(project.namespace, project, "master") } + let(:tree_path) { namespace_project_tree_path(project.namespace, project, "empty-branch") } - subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :create) } + subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/empty-branch', action: :create) } it_behaves_like 'it should not have Gmail Actions links' it_behaves_like 'a user cannot unsubscribe through footer link' @@ -961,12 +898,11 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do - is_expected.to have_subject "[Git][#{project.full_path}] Pushed new branch master" - end - - it 'contains a link to the branch' do - is_expected.to have_body_text tree_path + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_subject("[Git][#{project.full_path}] Pushed new branch empty-branch") + is_expected.to have_body_text(tree_path) + end end end @@ -988,12 +924,11 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do - is_expected.to have_subject "[Git][#{project.full_path}] Pushed new tag v1.0" - end - - it 'contains a link to the tag' do - is_expected.to have_body_text tree_path + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_subject("[Git][#{project.full_path}] Pushed new tag v1.0") + is_expected.to have_body_text(tree_path) + end end end @@ -1064,24 +999,14 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do - is_expected.to have_subject "[Git][#{project.full_path}][master] #{commits.length} commits: Ruby files modified" - end - - it 'includes commits list' do - is_expected.to have_body_text 'Change some files' - end - - it 'includes diffs with character-level highlighting' do - is_expected.to have_body_text 'def</span> <span class="nf">archive_formats_regex' - end - - it 'contains a link to the diff' do - is_expected.to have_body_text diff_path - end - - it 'does not contain the misleading footer' do - is_expected.not_to have_body_text 'you are a member of' + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_subject("[Git][#{project.full_path}][master] #{commits.length} commits: Ruby files modified") + is_expected.to have_body_text('Change some files') + is_expected.to have_body_text('def</span> <span class="nf">archive_formats_regex') + is_expected.to have_body_text(diff_path) + is_expected.not_to have_body_text('you are a member of') + end end context "when set to send from committer email if domain matches" do @@ -1098,13 +1023,13 @@ describe Notify do end it "is sent from the committer email" do - sender = subject.header[:from].addrs[0] - expect(sender.address).to eq(user.email) - end + from = subject.header[:from].addrs.first + reply = subject.header[:reply_to].addrs.first - it "is set to reply to the committer email" do - sender = subject.header[:reply_to].addrs[0] - expect(sender.address).to eq(user.email) + aggregate_failures do + expect(from.address).to eq(user.email) + expect(reply.address).to eq(user.email) + end end end @@ -1115,13 +1040,13 @@ describe Notify do end it "is sent from the default email" do - sender = subject.header[:from].addrs[0] - expect(sender.address).to eq(gitlab_sender) - end + from = subject.header[:from].addrs.first + reply = subject.header[:reply_to].addrs.first - it "is set to reply to the default email" do - sender = subject.header[:reply_to].addrs[0] - expect(sender.address).to eq(gitlab_sender_reply_to) + aggregate_failures do + expect(from.address).to eq(gitlab_sender) + expect(reply.address).to eq(gitlab_sender_reply_to) + end end end @@ -1132,13 +1057,13 @@ describe Notify do end it "is sent from the default email" do - sender = subject.header[:from].addrs[0] - expect(sender.address).to eq(gitlab_sender) - end + from = subject.header[:from].addrs.first + reply = subject.header[:reply_to].addrs.first - it "is set to reply to the default email" do - sender = subject.header[:reply_to].addrs[0] - expect(sender.address).to eq(gitlab_sender_reply_to) + aggregate_failures do + expect(from.address).to eq(gitlab_sender) + expect(reply.address).to eq(gitlab_sender_reply_to) + end end end end @@ -1166,20 +1091,13 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'has the correct subject' do - is_expected.to have_subject "[Git][#{project.full_path}][master] #{commits.first.title}" - end - - it 'includes commits list' do - is_expected.to have_body_text 'Change some files' - end - - it 'includes diffs with character-level highlighting' do - is_expected.to have_body_text 'def</span> <span class="nf">archive_formats_regex' - end - - it 'contains a link to the diff' do - is_expected.to have_body_text diff_path + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_subject("[Git][#{project.full_path}][master] #{commits.first.title}") + is_expected.to have_body_text('Change some files') + is_expected.to have_body_text('def</span> <span class="nf">archive_formats_regex') + is_expected.to have_body_text(diff_path) + end end end diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb index 22115c6566d..d841bdaa292 100644 --- a/spec/services/boards/issues/list_service_spec.rb +++ b/spec/services/boards/issues/list_service_spec.rb @@ -30,6 +30,7 @@ describe Boards::Issues::ListService, services: true do let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3]) } let!(:closed_issue3) { create(:issue, :closed, project: project) } let!(:closed_issue4) { create(:labeled_issue, :closed, project: project, labels: [p1]) } + let!(:closed_issue5) { create(:labeled_issue, :closed, project: project, labels: [development]) } before do project.team << [user, :developer] @@ -57,7 +58,7 @@ describe Boards::Issues::ListService, services: true do issues = described_class.new(project, user, params).execute - expect(issues).to eq [closed_issue4, closed_issue2, closed_issue3, closed_issue1] + expect(issues).to eq [closed_issue4, closed_issue2, closed_issue5, closed_issue3, closed_issue1] end it 'returns opened issues that have label list applied when listing issues from a label list' do diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index 0475f38fe5e..7a1ac027310 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -138,7 +138,7 @@ describe Issuable::BulkUpdateService, services: true do let(:labels) { [bug, regression] } it 'updates the labels of all issues passed to the labels passed' do - expect(issues.map(&:reload).map(&:label_ids)).to all(eq(labels.map(&:id))) + expect(issues.map(&:reload).map(&:label_ids)).to all(match_array(labels.map(&:id))) end it 'does not update issues not passed in' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ceb3209331f..5ab8f0d981a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -35,7 +35,8 @@ RSpec.configure do |config| config.include Warden::Test::Helpers, type: :request config.include LoginHelpers, type: :feature config.include SearchHelpers, type: :feature - config.include WaitForAjax, type: :feature + config.include WaitForRequests, :js + config.include WaitForAjax, :js config.include StubConfiguration config.include EmailHelpers, type: :mailer config.include TestEnv diff --git a/spec/support/notify_shared_examples.rb b/spec/support/notify_shared_examples.rb index a3724b801b3..16a425f2ca2 100644 --- a/spec/support/notify_shared_examples.rb +++ b/spec/support/notify_shared_examples.rb @@ -27,24 +27,14 @@ shared_examples 'a multiple recipients email' do end shared_examples 'an email sent from GitLab' do - it 'is sent from GitLab' do + it 'has the characteristics of an email sent from GitLab' do sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(gitlab_sender_display_name) - expect(sender.address).to eq(gitlab_sender) - end - - it 'has a Reply-To address' do reply_to = subject.header[:reply_to].addresses - expect(reply_to).to eq([gitlab_sender_reply_to]) - end - - context 'when custom suffix for email subject is set' do - before do - stub_config_setting(email_subject_suffix: 'A Nice Suffix') - end - it 'ends the subject with the suffix' do - is_expected.to have_subject /\ \| A Nice Suffix$/ + aggregate_failures do + expect(sender.display_name).to eq(gitlab_sender_display_name) + expect(sender.address).to eq(gitlab_sender) + expect(reply_to).to eq([gitlab_sender_reply_to]) end end end @@ -56,43 +46,40 @@ shared_examples 'an email that contains a header with author username' do end shared_examples 'an email with X-GitLab headers containing project details' do - it 'has X-GitLab-Project* headers' do - is_expected.to have_header 'X-GitLab-Project', /#{project.name}/ - is_expected.to have_header 'X-GitLab-Project-Id', /#{project.id}/ - is_expected.to have_header 'X-GitLab-Project-Path', /#{project.path_with_namespace}/ + it 'has X-GitLab-Project headers' do + aggregate_failures do + is_expected.to have_header('X-GitLab-Project', /#{project.name}/) + is_expected.to have_header('X-GitLab-Project-Id', /#{project.id}/) + is_expected.to have_header('X-GitLab-Project-Path', /#{project.path_with_namespace}/) + end end end shared_examples 'a new thread email with reply-by-email enabled' do - let(:regex) { /\A<reply\-(.*)@#{Gitlab.config.gitlab.host}>\Z/ } - - it 'has a Message-ID header' do - is_expected.to have_header 'Message-ID', "<#{model.class.model_name.singular_route_key}_#{model.id}@#{Gitlab.config.gitlab.host}>" - end + it 'has the characteristics of a threaded email' do + host = Gitlab.config.gitlab.host + route_key = "#{model.class.model_name.singular_route_key}_#{model.id}" - it 'has a References header' do - is_expected.to have_header 'References', regex + aggregate_failures do + is_expected.to have_header('Message-ID', "<#{route_key}@#{host}>") + is_expected.to have_header('References', /\A<reply\-.*@#{host}>\Z/ ) + end end end shared_examples 'a thread answer email with reply-by-email enabled' do include_examples 'an email with X-GitLab headers containing project details' - let(:regex) { /\A<#{model.class.model_name.singular_route_key}_#{model.id}@#{Gitlab.config.gitlab.host}> <reply\-(.*)@#{Gitlab.config.gitlab.host}>\Z/ } - - it 'has a Message-ID header' do - is_expected.to have_header 'Message-ID', /\A<(.*)@#{Gitlab.config.gitlab.host}>\Z/ - end - - it 'has a In-Reply-To header' do - is_expected.to have_header 'In-Reply-To', "<#{model.class.model_name.singular_route_key}_#{model.id}@#{Gitlab.config.gitlab.host}>" - end - it 'has a References header' do - is_expected.to have_header 'References', regex - end + it 'has the characteristics of a threaded reply' do + host = Gitlab.config.gitlab.host + route_key = "#{model.class.model_name.singular_route_key}_#{model.id}" - it 'has a subject that begins with Re: ' do - is_expected.to have_subject /^Re: / + aggregate_failures do + is_expected.to have_header('Message-ID', /\A<.*@#{host}>\Z/) + is_expected.to have_header('In-Reply-To', "<#{route_key}@#{host}>") + is_expected.to have_header('References', /\A<#{route_key}@#{host}> <reply\-.*@#{host}>\Z/ ) + is_expected.to have_subject(/^Re: /) + end end end @@ -136,80 +123,77 @@ shared_examples 'an answer to an existing thread with reply-by-email enabled' do end end -shared_examples 'a new user email' do - it 'is sent to the new user' do - is_expected.to deliver_to new_user_address - end - - it 'has the correct subject' do - is_expected.to have_subject /^Account was created for you$/i - end - - it 'contains the new user\'s login name' do - is_expected.to have_body_text /#{new_user_address}/ - end -end - shared_examples 'it should have Gmail Actions links' do - it { is_expected.to have_body_text '<script type="application/ld+json">' } - it { is_expected.to have_body_text /ViewAction/ } + it do + aggregate_failures do + is_expected.to have_body_text('<script type="application/ld+json">') + is_expected.to have_body_text('ViewAction') + end + end end shared_examples 'it should not have Gmail Actions links' do - it { is_expected.not_to have_body_text '<script type="application/ld+json">' } - it { is_expected.not_to have_body_text /ViewAction/ } + it do + aggregate_failures do + is_expected.not_to have_body_text('<script type="application/ld+json">') + is_expected.not_to have_body_text('ViewAction') + end + end end shared_examples 'it should show Gmail Actions View Issue link' do it_behaves_like 'it should have Gmail Actions links' - it { is_expected.to have_body_text /View Issue/ } + it { is_expected.to have_body_text('View Issue') } end shared_examples 'it should show Gmail Actions View Merge request link' do it_behaves_like 'it should have Gmail Actions links' - it { is_expected.to have_body_text /View Merge request/ } + it { is_expected.to have_body_text('View Merge request') } end shared_examples 'it should show Gmail Actions View Commit link' do it_behaves_like 'it should have Gmail Actions links' - it { is_expected.to have_body_text /View Commit/ } + it { is_expected.to have_body_text('View Commit') } end shared_examples 'an unsubscribeable thread' do it_behaves_like 'an unsubscribeable thread with incoming address without %{key}' - it 'has a List-Unsubscribe header in the correct format' do - is_expected.to have_header 'List-Unsubscribe', /unsubscribe/ - is_expected.to have_header 'List-Unsubscribe', /mailto/ - is_expected.to have_header 'List-Unsubscribe', /^<.+,.+>$/ + it 'has a List-Unsubscribe header in the correct format, and a body link' do + aggregate_failures do + is_expected.to have_header('List-Unsubscribe', /unsubscribe/) + is_expected.to have_header('List-Unsubscribe', /mailto/) + is_expected.to have_header('List-Unsubscribe', /^<.+,.+>$/) + is_expected.to have_body_text('unsubscribe') + end end - - it { is_expected.to have_body_text /unsubscribe/ } end shared_examples 'an unsubscribeable thread with incoming address without %{key}' do include_context 'reply-by-email is enabled with incoming address without %{key}' - it 'has a List-Unsubscribe header in the correct format' do - is_expected.to have_header 'List-Unsubscribe', /unsubscribe/ - is_expected.not_to have_header 'List-Unsubscribe', /mailto/ - is_expected.to have_header 'List-Unsubscribe', /^<[^,]+>$/ + it 'has a List-Unsubscribe header in the correct format, and a body link' do + aggregate_failures do + is_expected.to have_header('List-Unsubscribe', /unsubscribe/) + is_expected.not_to have_header('List-Unsubscribe', /mailto/) + is_expected.to have_header('List-Unsubscribe', /^<[^,]+>$/) + is_expected.to have_body_text('unsubscribe') + end end - - it { is_expected.to have_body_text /unsubscribe/ } end shared_examples 'a user cannot unsubscribe through footer link' do - it 'does not have a List-Unsubscribe header' do - is_expected.not_to have_header 'List-Unsubscribe', /unsubscribe/ + it 'does not have a List-Unsubscribe header or a body link' do + aggregate_failures do + is_expected.not_to have_header('List-Unsubscribe', /unsubscribe/) + is_expected.not_to have_body_text('unsubscribe') + end end - - it { is_expected.not_to have_body_text /unsubscribe/ } end shared_examples 'an email with a labels subscriptions link in its footer' do - it { is_expected.to have_body_text /label subscriptions/ } + it { is_expected.to have_body_text('label subscriptions') } end diff --git a/spec/support/slack_mattermost_notifications_shared_examples.rb b/spec/support/slack_mattermost_notifications_shared_examples.rb index 704922b6cf4..b902fe90707 100644 --- a/spec/support/slack_mattermost_notifications_shared_examples.rb +++ b/spec/support/slack_mattermost_notifications_shared_examples.rb @@ -324,5 +324,24 @@ RSpec.shared_examples 'slack or mattermost notifications' do it_behaves_like 'call Slack/Mattermost API' end end + + context 'only notify for the default branch' do + context 'when enabled' do + let(:pipeline) do + create(:ci_pipeline, project: project, status: 'failed', ref: 'not-the-default-branch') + end + + before do + chat_service.notify_only_default_branch = true + end + + it 'does not call the Slack/Mattermost API for pipeline events' do + data = Gitlab::DataBuilder::Pipeline.build(pipeline) + result = chat_service.execute(data) + + expect(result).to be_falsy + end + end + end end end diff --git a/spec/support/wait_for_requests.rb b/spec/support/wait_for_requests.rb new file mode 100644 index 00000000000..0bfa7f72ff8 --- /dev/null +++ b/spec/support/wait_for_requests.rb @@ -0,0 +1,32 @@ +module WaitForRequests + extend self + + # This is inspired by http://www.salsify.com/blog/engineering/tearing-capybara-ajax-tests + def wait_for_requests_complete + Gitlab::Testing::RequestBlockerMiddleware.block_requests! + wait_for('pending AJAX requests complete') do + Gitlab::Testing::RequestBlockerMiddleware.num_active_requests.zero? + end + ensure + Gitlab::Testing::RequestBlockerMiddleware.allow_requests! + end + + # Waits until the passed block returns true + def wait_for(condition_name, max_wait_time: Capybara.default_max_wait_time, polling_interval: 0.01) + wait_until = Time.now + max_wait_time.seconds + loop do + break if yield + if Time.now > wait_until + raise "Condition not met: #{condition_name}" + else + sleep(polling_interval) + end + end + end +end + +RSpec.configure do |config| + config.after(:each, :js) do + wait_for_requests_complete + end +end diff --git a/vendor/assets/javascripts/js.cookie.js b/vendor/assets/javascripts/js.cookie.js deleted file mode 100644 index 92dbba162c4..00000000000 --- a/vendor/assets/javascripts/js.cookie.js +++ /dev/null @@ -1,156 +0,0 @@ -/*! - * JavaScript Cookie v2.1.3 - * https://github.com/js-cookie/js-cookie - * - * Copyright 2006, 2015 Klaus Hartl & Fagner Brack - * Released under the MIT license - */ -;(function (factory) { - var registeredInModuleLoader = false; - if (typeof define === 'function' && define.amd) { - define(factory); - registeredInModuleLoader = true; - } - if (typeof exports === 'object') { - module.exports = factory(); - registeredInModuleLoader = true; - } - if (!registeredInModuleLoader) { - var OldCookies = window.Cookies; - var api = window.Cookies = factory(); - api.noConflict = function () { - window.Cookies = OldCookies; - return api; - }; - } -}(function () { - function extend () { - var i = 0; - var result = {}; - for (; i < arguments.length; i++) { - var attributes = arguments[ i ]; - for (var key in attributes) { - result[key] = attributes[key]; - } - } - return result; - } - - function init (converter) { - function api (key, value, attributes) { - var result; - if (typeof document === 'undefined') { - return; - } - - // Write - - if (arguments.length > 1) { - attributes = extend({ - path: '/' - }, api.defaults, attributes); - - if (typeof attributes.expires === 'number') { - var expires = new Date(); - expires.setMilliseconds(expires.getMilliseconds() + attributes.expires * 864e+5); - attributes.expires = expires; - } - - try { - result = JSON.stringify(value); - if (/^[\{\[]/.test(result)) { - value = result; - } - } catch (e) {} - - if (!converter.write) { - value = encodeURIComponent(String(value)) - .replace(/%(23|24|26|2B|3A|3C|3E|3D|2F|3F|40|5B|5D|5E|60|7B|7D|7C)/g, decodeURIComponent); - } else { - value = converter.write(value, key); - } - - key = encodeURIComponent(String(key)); - key = key.replace(/%(23|24|26|2B|5E|60|7C)/g, decodeURIComponent); - key = key.replace(/[\(\)]/g, escape); - - return (document.cookie = [ - key, '=', value, - attributes.expires ? '; expires=' + attributes.expires.toUTCString() : '', // use expires attribute, max-age is not supported by IE - attributes.path ? '; path=' + attributes.path : '', - attributes.domain ? '; domain=' + attributes.domain : '', - attributes.secure ? '; secure' : '' - ].join('')); - } - - // Read - - if (!key) { - result = {}; - } - - // To prevent the for loop in the first place assign an empty array - // in case there are no cookies at all. Also prevents odd result when - // calling "get()" - var cookies = document.cookie ? document.cookie.split('; ') : []; - var rdecode = /(%[0-9A-Z]{2})+/g; - var i = 0; - - for (; i < cookies.length; i++) { - var parts = cookies[i].split('='); - var cookie = parts.slice(1).join('='); - - if (cookie.charAt(0) === '"') { - cookie = cookie.slice(1, -1); - } - - try { - var name = parts[0].replace(rdecode, decodeURIComponent); - cookie = converter.read ? - converter.read(cookie, name) : converter(cookie, name) || - cookie.replace(rdecode, decodeURIComponent); - - if (this.json) { - try { - cookie = JSON.parse(cookie); - } catch (e) {} - } - - if (key === name) { - result = cookie; - break; - } - - if (!key) { - result[name] = cookie; - } - } catch (e) {} - } - - return result; - } - - api.set = api; - api.get = function (key) { - return api.call(api, key); - }; - api.getJSON = function () { - return api.apply({ - json: true - }, [].slice.call(arguments)); - }; - api.defaults = {}; - - api.remove = function (key, attributes) { - api(key, '', extend(attributes, { - expires: -1 - })); - }; - - api.withConverter = init; - - return api; - } - - return init(function () {}); -}));
\ No newline at end of file diff --git a/yarn.lock b/yarn.lock index fb3bfce4d11..f254668646c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4450,6 +4450,10 @@ verror@1.3.6: dependencies: extsprintf "1.0.2" +visibilityjs@^1.2.4: + version "1.2.4" + resolved "https://registry.yarnpkg.com/visibilityjs/-/visibilityjs-1.2.4.tgz#bff8663da62c8c10ad4ee5ae6a1ae6fac4259d63" + vm-browserify@0.0.4: version "0.0.4" resolved "https://registry.yarnpkg.com/vm-browserify/-/vm-browserify-0.0.4.tgz#5d7ea45bbef9e4a6ff65f95438e0a87c357d5a73" |