Read about our upcoming Code of Conduct on this issue

Commit ac9c5e57 authored by GitLab Bot's avatar GitLab Bot
Browse files

Add latest changes from gitlab-org/gitlab@master

parent 5cbace3fa287
......@@ -22,6 +22,7 @@ gem 'rugged', '~> 0.28'
gem 'grape-path-helpers', '~> 1.1'
gem 'faraday', '~> 0.12'
gem 'marginalia', '~> 1.8.0'
# Authentication libraries
gem 'devise', '~> 4.6'
......
......@@ -591,6 +591,9 @@ GEM
mail_room (0.9.1)
marcel (0.3.3)
mimemagic (~> 0.3.2)
marginalia (1.8.0)
actionpack (>= 2.3)
activerecord (>= 2.3)
memoist (0.16.0)
memoizable (0.4.2)
thread_safe (~> 0.3, >= 0.3.1)
......@@ -1243,6 +1246,7 @@ DEPENDENCIES
lograge (~> 0.5)
loofah (~> 2.2)
mail_room (~> 0.9.1)
marginalia (~> 1.8.0)
memory_profiler (~> 0.9)
method_source (~> 0.8)
mimemagic (~> 0.3.2)
......
......@@ -155,7 +155,7 @@ export default {
{{ cherryPickLabel }}
</a>
</div>
<section class="mr-info-list">
<section class="mr-info-list" data-qa-selector="merged_status_content">
<p>
{{ s__('mrWidget|The changes were merged into') }}
<span class="label-branch">
......
......@@ -249,9 +249,10 @@ export default {
type="button"
class="btn btn-sm btn-info dropdown-toggle js-merge-moment"
data-toggle="dropdown"
data-qa-selector="merge_moment_dropdown"
:aria-label="__('Select merge moment')"
>
<i class="fa fa-chevron-down qa-merge-moment-dropdown" aria-hidden="true"></i>
<i class="fa fa-chevron-down" aria-hidden="true"></i>
</button>
<ul
v-if="shouldShowMergeImmediatelyDropdown"
......@@ -272,7 +273,8 @@ export default {
</li>
<li>
<a
class="accept-merge-request qa-merge-immediately-option"
class="accept-merge-request"
data-qa-selector="merge_immediately_option"
href="#"
@click.prevent="handleMergeButtonClick(false, true)"
>
......
......@@ -44,7 +44,7 @@
- if group_sidebar_link?(:contribution_analytics)
= nav_link(path: 'analytics#show') do
= link_to group_analytics_path(@group), title: _('Contribution Analytics'), data: { placement: 'right' } do
= link_to group_analytics_path(@group), title: _('Contribution Analytics'), data: { placement: 'right', qa_selector: 'contribution_analytics_link' } do
%span
= _('Contribution Analytics')
......
......@@ -6,7 +6,7 @@
- if is_current_user
- if can_update
= link_to "Close #{display_issuable_type}", close_issuable_path(issuable), method: button_method,
class: "d-none d-sm-none d-md-block btn btn-grouped btn-close js-btn-issue-action #{issuable_button_visibility(issuable, true)}", title: "Close #{display_issuable_type}"
class: "d-none d-sm-none d-md-block btn btn-grouped btn-close js-btn-issue-action #{issuable_button_visibility(issuable, true)}", title: "Close #{display_issuable_type}", data: { qa_selector: 'close_issue_button' }
- if can_reopen
= link_to "Reopen #{display_issuable_type}", reopen_issuable_path(issuable), method: button_method,
class: "d-none d-sm-none d-md-block btn btn-grouped btn-reopen js-btn-issue-action #{issuable_button_visibility(issuable, false)}", title: "Reopen #{display_issuable_type}", data: { qa_selector: 'reopen_issue_button' }
......
# frozen_string_literal: true
require 'marginalia'
::Marginalia::Comment.extend(::Gitlab::Marginalia::Comment)
# Patch to include support for 'Marginalia.without_annotation' method.
::Marginalia.singleton_class.prepend(Gitlab::Marginalia::InlineAnnotation)
# Patch to modify 'Marginalia::ActiveRecordInstrumentation.annotate_sql' method with feature check.
# Orignal Marginalia::ActiveRecordInstrumentation is included to ActiveRecord::ConnectionAdapters::PostgreSQLAdapter in the Marginalia Railtie.
# Refer: https://github.com/basecamp/marginalia/blob/v1.8.0/lib/marginalia/railtie.rb#L67
ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(Gitlab::Marginalia::ActiveRecordInstrumentation)
Marginalia::Comment.components = [:application, :controller, :action, :correlation_id, :jid, :job_class, :line]
Gitlab::Marginalia.set_application_name
Gitlab::Marginalia.enable_sidekiq_instrumentation
# frozen_string_literal: true
class CleanupDeployAccessLevelsForRemovedGroups < ActiveRecord::Migration[5.2]
DOWNTIME = false
def up
return unless Gitlab.ee?
delete = <<~SQL
DELETE FROM protected_environment_deploy_access_levels d
USING protected_environments p
WHERE d.protected_environment_id=p.id
AND d.group_id IS NOT NULL
AND NOT EXISTS (SELECT 1 FROM project_group_links WHERE project_id=p.project_id AND group_id=d.group_id)
RETURNING *
SQL
# At the time of writing there are 4 such records on GitLab.com,
# execution time is expected to be around 15ms.
records = execute(delete)
logger = Gitlab::BackgroundMigration::Logger.build
records.to_a.each do |record|
logger.info record.as_json.merge(message: "protected_environments_deploy_access_levels was deleted")
end
end
def down
# There is no pragmatic way to restore
# the records deleted in the `#up` method above.
end
end
......@@ -40,8 +40,8 @@ This saves you time as you don't have to wait for the same errors to be detected
[rss-source]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#source-code-layout
[rss-naming]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#naming-conventions
[doc-guidelines]: ../documentation/index.md "Documentation guidelines"
[js-styleguide]: ../fe_guide/style_guide_js.md "JavaScript styleguide"
[scss-styleguide]: ../fe_guide/style_guide_scss.md "SCSS styleguide"
[js-styleguide]: ../fe_guide/style/javascript.md "JavaScript styleguide"
[scss-styleguide]: ../fe_guide/style/scss.md "SCSS styleguide"
[newlines-styleguide]: ../newlines_styleguide.md "Newlines styleguide"
[testing]: ../testing_guide/index.md
[us-english]: https://en.wikipedia.org/wiki/American_English
......@@ -76,15 +76,18 @@ Read the [frontend's FAQ](frontend_faq.md) for common small pieces of helpful in
## Style Guides
### [JavaScript Style Guide](style_guide_js.md)
See the relevant style guides for our guidelines and for information on linting:
We use eslint to enforce our JavaScript style guides. Our guide is based on
- [JavaScript](style/javascript.md). Our guide is based on
the excellent [Airbnb][airbnb-js-style-guide] style guide with a few small
changes.
- [SCSS](style/scss.md): our SCSS conventions which are enforced through [`scss-lint`](https://github.com/brigade/scss-lint).
- [HTML](style/html.md). Guidelines for writing HTML code consistent with the rest of the codebase.
- [Vue](style/vue.md). Guidelines and conventions for Vue code may be found here.
### [SCSS Style Guide](style_guide_scss.md)
## Tooling
Our SCSS conventions which are enforced through [scss-lint](https://github.com/sds/scss-lint).
Our code is automatically formatted with [Prettier](https://prettier.io) to follow our guidelines. Read our [Tooling guide](tooling.md) for more detail.
## [Performance](performance.md)
......
# HTML style guide
## Buttons
### Button type
Button tags requires a `type` attribute according to the [W3C HTML specification](https://www.w3.org/TR/2011/WD-html5-20110525/the-button-element.html#dom-button-type).
```html
// bad
<button></button>
// good
<button type="button"></button>
```
### Button role
If an HTML element has an `onClick` handler but is not a button, it should have `role="button"`. This is [more accessible](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/button_role).
```html
// bad
<div onClick="doSomething"></div>
// good
<div role="button" onClick="doSomething"></div>
```
## Links
### Blank target
Use `rel="noopener noreferrer"` whenever your links open in a new window, i.e. `target="_blank"`. This prevents a security vulnerability [documented by JitBit](https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/).
```html
// bad
<a href="url" target="_blank"></a>
// good
<a href="url" target="_blank" rel="noopener noreferrer"></a>
```
### Fake links
**Do not use fake links.** Use a button tag if a link only invokes JavaScript click event handlers, which is more semantic.
```html
// bad
<a class="js-do-something" href="#"></a>
// good
<button class="js-do-something" type="button"></button>
```
# GitLab development style guides
See below the relevant style guides, guidelines, linting, and other information for developing GitLab.
## JavaScript style guide
We use `eslint` to enforce our [JavaScript style guides](javascript.md). Our guide is based on
the excellent [AirBnB](https://github.com/airbnb/javascript) style guide with a few small
changes.
## SCSS style guide
Our [SCSS conventions](scss.md) which are enforced through [`scss-lint`](https://github.com/brigade/scss-lint).
## HTML style guide
Guidelines for writing [HTML code](html.md) consistent with the rest of the codebase.
## Vue style guide
Guidelines and conventions for Vue code may be found within the [Vue style guide](vue.md).
---
disqus_identifier: 'https://docs.gitlab.com/ee/development/fe_guide/style_guide_js.html'
---
# JavaScript style guide
We use [Airbnb's JavaScript Style Guide](https://github.com/airbnb/javascript) and it's accompanying
linter to manage most of our JavaScript style guidelines.
In addition to the style guidelines set by Airbnb, we also have a few specific rules
listed below.
> **Tip:**
You can run eslint locally by running `yarn eslint`
## Avoid forEach
Avoid forEach when mutating data. Use `map`, `reduce` or `filter` instead of `forEach`
when mutating data. This will minimize mutations in functions,
which aligns with [Airbnb's style guide](https://github.com/airbnb/javascript#testing--for-real).
```javascript
// bad
users.forEach((user, index) => {
user.id = index;
});
// good
const usersWithId = users.map((user, index) => {
return Object.assign({}, user, { id: index });
});
```
## Limit number of parameters
If your function or method has more than 3 parameters, use an object as a parameter
instead.
```javascript
// bad
function a(p1, p2, p3) {
// ...
};
// good
function a(p) {
// ...
};
```
## Avoid side effects in constructors
Avoid making asynchronous calls, API requests or DOM manipulations in the `constructor`.
Move them into separate functions instead. This will make tests easier to write and
code easier to maintain.
```javascript
// bad
class myClass {
constructor(config) {
this.config = config;
axios.get(this.config.endpoint)
}
}
// good
class myClass {
constructor(config) {
this.config = config;
}
makeRequest() {
axios.get(this.config.endpoint)
}
}
const instance = new myClass();
instance.makeRequest();
```
## Avoid classes to handle DOM events
If the only purpose of the class is to bind a DOM event and handle the callback, prefer
using a function.
```javascript
// bad
class myClass {
constructor(config) {
this.config = config;
}
init() {
document.addEventListener('click', () => {});
}
}
// good
const myFunction = () => {
document.addEventListener('click', () => {
// handle callback here
});
}
```
## Pass element container to constructor
When your class manipulates the DOM, receive the element container as a parameter.
This is more maintainable and performant.
```javascript
// bad
class a {
constructor() {
document.querySelector('.b');
}
}
// good
class a {
constructor(options) {
options.container.querySelector('.b');
}
}
```
## Use ParseInt
Use `ParseInt` when converting a numeric string into a number.
```javascript
// bad
Number('10')
// good
parseInt('10', 10);
```
## CSS Selectors - Use `js-` prefix
If a CSS class is only being used in JavaScript as a reference to the element, prefix
the class name with `js-`.
```html
// bad
<button class="add-user"></button>
// good
<button class="js-add-user"></button>
```
## ES Module Syntax
Use ES module syntax to import modules:
```javascript
// bad
const SomeClass = require('some_class');
// good
import SomeClass from 'some_class';
// bad
module.exports = SomeClass;
// good
export default SomeClass;
```
_Note:_ We still use `require` in `scripts/` and `config/` files.
## Absolute vs relative paths for modules
Use relative paths if the module you are importing is less than two levels up.
```javascript
// bad
import GitLabStyleGuide from '~/guides/GitLabStyleGuide';
// good
import GitLabStyleGuide from '../GitLabStyleGuide';
```
If the module you are importing is two or more levels up, use an absolute path instead:
```javascript
// bad
import GitLabStyleGuide from '../../../guides/GitLabStyleGuide';
// good
import GitLabStyleGuide from '~/GitLabStyleGuide';
```
Additionally, **do not add to global namespace**.
## Do not use `DOMContentLoaded` in non-page modules
Imported modules should act the same each time they are loaded. `DOMContentLoaded`
events are only allowed on modules loaded in the `/pages/*` directory because those
are loaded dynamically with webpack.
## Avoid XSS
Do not use `innerHTML`, `append()` or `html()` to set content. It opens up too many
vulnerabilities.
## ESLint
ESLint behaviour can be found in our [tooling guide](../tooling.md).
## IIFEs
Avoid using IIFEs (Immediately-Invoked Function Expressions). Although
we have a lot of examples of files which wrap their contents in IIFEs,
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.
## Global namespace and side effects
Avoid adding to the global namespace.
```javascript
// bad
window.MyClass = class { /* ... */ };
// good
export default class MyClass { /* ... */ }
```
Top-level side effects are forbidden in any script which contains `export`:
```javascript
// bad
export default class MyClass { /* ... */ }
document.addEventListener("DOMContentLoaded", function(event) {
new MyClass();
}
```
Avoid constructors with side effects whenever possible.
Side effects make constructors difficult to unit test and violate the [Single Responsibility Principle](https://en.wikipedia.org/wiki/Single_responsibility_principle).
```javascript
// Bad
export class Foo {
constructor() {
this.currentState = state.INITIAL;
document.getElementById('root').addEventListener('click', this.handleCallback)
}
handleCallback() {
}
}
// Good
export class Foo {
constructor() {
this.currentState = state.INITIAL;
}
initListener(element) {
element.addEventListener('click', this.handleCallback)
}
handleCallback() {
}
}
```
On the other hand, if a class only needs to extend a third-party or add
event listeners in some specific cases, they should be initialized outside
of the constructor.
## Pure Functions and Data Mutation
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);
```
---
disqus_identifier: 'https://docs.gitlab.com/ee/development/fe_guide/style_guide_scss.html'
---
# SCSS style guide
This style guide recommends best practices for SCSS to make styles easy to read,
easy to maintain, and performant for the end-user.
## Rules
### Utility Classes
As part of the effort for [cleaning up our CSS and moving our components into GitLab-UI](https://gitlab.com/groups/gitlab-org/-/epics/950)
led by the [GitLab UI WG](https://gitlab.com/gitlab-com/www-gitlab-com/merge_requests/20623) we prefer the use of utility classes over adding new CSS. However, complex CSS can be addressed by adding component classes.
#### Where are utility classes defined?
- [Bootstrap's Utility Classes](https://getbootstrap.com/docs/4.3/utilities/)
- [`common.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/framework/common.scss) (old)
- [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/utilities.scss) (new)
#### Where should I put new utility classes?
New utility classes should be added to [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/utilities.scss). Existing classes include:
| Name | Pattern | Example |
|------|---------|---------|
| Background color | `.bg-{variant}-{shade}` | `.bg-warning-400` |
| Text color | `.text-{variant}-{shade}` | `.text-success-500` |
| Font size | `.text-{size}` | `.text-2` |
- `{variant}` is one of 'primary', 'secondary', 'success', 'warning', 'error'
- `{shade}` is one of the shades listed on [colors](https://design.gitlab.com/product-foundations/colors/)
- `{size}` is a number from 1-6 from our [Type scale](https://design.gitlab.com/product-foundations/typography/)
#### When should I create component classes?
We recommend a "utility-first" approach.
1. Start with utility classes.
1. If composing utility classes into a component class removes code duplication and encapsulates a clear responsibility, do it.
This encourages an organic growth of component classes and prevents the creation of one-off unreusable classes. Also, the kind of classes that emerge from "utility-first" tend to be design-centered (e.g. `.button`, `.alert`, `.card`) rather than domain-centered (e.g. `.security-report-widget`, `.commit-header-icon`).
Examples of component classes that were created using "utility-first" include:
- [`.circle-icon-container`](https://gitlab.com/gitlab-org/gitlab/blob/579fa8b8ec7eb38d40c96521f517c9dab8c3b97a/app/assets/stylesheets/framework/icons.scss#L85)
- [`.d-flex-center`](https://gitlab.com/gitlab-org/gitlab/blob/900083d89cd6af391d26ab7922b3f64fa2839bef/app/assets/stylesheets/framework/common.scss#L425)
Inspiration:
- <https://tailwindcss.com/docs/utility-first/>
- <https://tailwindcss.com/docs/extracting-components/>
### Naming
Filenames should use `snake_case`.
CSS classes should use the `lowercase-hyphenated` format rather than
`snake_case` or `camelCase`.