Allow Members::ApproveAccessRequestService to accept a new `:force` option
## What does this MR do?
See the commit message.
This is a backport of the EE fix for https://gitlab.com/gitlab-org/gitlab-ee/issues/1159: gitlab-org/gitlab-ee!830
See merge request !7168
Pass user instance to Labels::FindOrCreateService or skip_authorization: true
## What does this MR do?
It fixes a bug described in #23694 when `project.owner` was passed to `Labels::FindOrCreateService`. `Labels::FindOrCreateService` expected a user instance and `project.owner` may return a group as well. This MR makes sure that we either pass a user instance or `skip_authorization: true`.
## Are there points in the code the reviewer needs to double check?
- places where we pass `skip_authorization: true`
## Does this MR meet the acceptance criteria?
- Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Fixes#23694
See merge request !7093
Use optimistic locking
## What does this MR do?
Removes the usage of pessimistic locking in favor of optimistic which is way cheaper and doesn't block database operation.
Since this is very simple change it should be safe. If we receive `StaleObjectError` message we will reload object a retry operations in lock.
However, I still believe that we need this one: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7005 as this will reduce a load on Database and FS.
This changes a behavior from:
### Pesimistic locking (previous behavior)
#### For updating
1. SELECT * FOR UPDATE (other updates wait on this)
2. we update ci_pipeline
3. latest_build_status
4. enqueue: (use: transition :created -> :pending)
5. [state_machine] we are in state created, we can go to pending
6. [state_machine] ci_pipeline.status = created
7. [state_machine] ci_pipeline.save
8. [state_machine] after_transition: (if for success): PipelineSuccessWorker on Sidekiq
9. release DB lock
#### If no update is required
1. SELECT * FOR UPDATE (other updates wait on this)
2. we update ci_pipeline
3. latest_build_status
4. we are in pending, we can't transition to pending, because it's forbidden
5. release DB lock
### Optimistic locking (implemented by this MR)
#### For updating
1. latest_build_status
2. enqueue: (use `transition :created -> :pending`)
3. [state_machine] we are in state created, we can go to pending
4. [state_machine] ci_pipeline.status = created
5. [state_machine] ci_pipeline.save
6. [state_machine] [save] where(lock_version: ci_pipeline.lock_version).update_all(status: :created, updated_at: Time.now)
7. [state_machine] [save] unless we_updated_row then raise ObjectInconsistentError
#### If no update is required
1. we update ci_pipeline
2. latest_build_status
3. we are in pending, we can't transition to pending, because it's forbidden
## Why was this MR needed?
We have been seeing a number of problems when we migrated Pipeline/Build processing to Sidekiq. Especially we started seeing a lot of blocking queries.
We used a pessimistic locking which doesn't seem to be required. This effectively allows us to fix our issues with blocked queries by using more efficient method of operation.
## What are the relevant issue numbers?
Issues: https://gitlab.com/gitlab-com/infrastructure/issues/623 and https://gitlab.com/gitlab-com/infrastructure/issues/584, but also there's a bunch of Merge Requests that try to improve behavior of scheduled jobs.
cc @pcarranza @yorickpeterse @stanhu
See merge request !7040
Delete issue board welcome cookie
## What does this MR do?
This was seen in the idea to production demo, because the project had already existed on the exact same URL the cookie already existed which meant the blank state wasn't shown.
This MR deletes the cookie on project create.
See merge request !6899
Replace static fixture by generated one in issue_spec.js
## What does this MR do?
- clean up `issue_spec.js`
- introduce an alternative approach to #19445
- rename `rake teaspoon` to `rake teaspoon:tests`
- introduce `rake teaspoon:fixtures` which generates fixtures using RSpec
- introduce `rake teaspoon` which runs `rake teaspoon:fixtures` and `rake teaspoon:tests`
## Why was this MR needed?
- many duplications
- missing existence checks
- missing conditions
- static fixtures don't match real views
## Reasoning
I want to explain some of my decisions here, so that they stay visible for future discussions.
### Why not HAML?
- same number of HAML templates as number of fixtures (many input files)
- embedded logic less readable
- can not be rendered by JavaScript (because of inline Ruby)
### Why RSpec?
- real controllers for fixtures
- spys available for mocking
- easily report failed fixture generations
### Why not magic_lamp? (#19445)
- introduces another dependency/tool
- needs to run a server concurrently to teaspoon
- makes it harder to use a JavaScript test runner
- static HTML files serve faster
See merge request !6059
Only show one error message for an invalid email
Changes it to only validate the notification_email format if it's different from email.
Closes#5905
See merge request !7158
`ProjectTeam#find_member` doesn't take group links into account. It was
used in two places:
1. An admin view - it can stay here.
2. `ProjectTeam#member?`, which is often used to decide if a user has
access to view something.
This second part broke confidential issues viewing. `IssuesFinder` ends
up delegating to `Project#authorized_for_user?`, which does consider
group links, so users with access to the project via a group link could
see confidential issues on the index page. However, `IssuesPolicy` used
`ProjectTeam#member?`, so the same user couldn't view the issue when
going to it directly.
If notification_email is blank, it's set from email. If an admin
attempted to create a user with an invalid email, an error would be
displayed for both fields. Only validate the notification_email if it's
different from email.
Enable linting for ES6 files
## What does this MR do?
Add `.js.es6` to file extensions checked by ESLint.
## What are the relevant issue numbers?
Follows !5445
See merge request !7156
Sean McGivern
Rémy Coutable
Stan Hu
Guilherme Salazar
Douwe Maan
Robert Speicher
Sean McGivern
Phil Hughes
Winnie
winniehell
blackst0ne
Adam Niedzielski
Ahmad Sherif
Alejandro Rodríguez
Nick Thomas
Annabel Dunstone Gray
Sid Sijbrandij
Dimitrie Hoekstra
Steve Halasz
Jacob Schatz
Luke Bennett