Commit 797af064 authored by Grzegorz Bizon's avatar Grzegorz Bizon

Merge branch 'master' into feature/issue-move

* master:
  Fix bug where wrong commit ID was being used in a merge request diff to show old image
  Remove CHANGELOG item that was added during merge resolution
  Improve the "easy WIP & un-WIP from link" feature
  Fix specs
  \#to_branch_name now uses the iid as postfix
  Add label description in tooltip to labels in issue index and sidebar
  Easily (un)mark merge request as WIP using link
  Use specialized system notes when MR is (un)marked as WIP
  another attempt to fix oauth issue
  attempting to fix omniauth problem

Conflicts:
	app/assets/javascripts/issuable_form.js.coffee
parents 6eb31056 01fe50a7
......@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.6.0 (unreleased)
- Add ability to move issue to another project
- Fix bug where wrong commit ID was being used in a merge request diff to show old image (Stan Hu)
- Add confidential issues
- Bump gitlab_git to 9.0.3 (Stan Hu)
- Support Golang subpackage fetching (Stan Hu)
......@@ -9,6 +10,8 @@ v 8.6.0 (unreleased)
- New branch button appears on issues where applicable
- Contributions to forked projects are included in calendar
- Improve the formatting for the user page bio (Connor Shea)
- Easily (un)mark merge request as WIP using link
- Use specialized system notes when MR is (un)marked as WIP
- Removed the default password from the initial admin account created during
setup. A password can be provided during setup (see installation docs), or
GitLab will ask the user to create a new one upon first visit.
......@@ -21,6 +24,7 @@ v 8.6.0 (unreleased)
- Memoize @group in Admin::GroupsController (Yatish Mehta)
- Indicate how much an MR diverged from the target branch (Pierre de La Morinerie)
- Added omniauth-auth0 Gem (Daniel Carraro)
- Add label description in tooltip to labels in issue index and sidebar
- Strip leading and trailing spaces in URL validator (evuez)
- Add "last_sign_in_at" and "confirmed_at" to GET /users/* API endpoints for admins (evuez)
- Return empty array instead of 404 when commit has no statuses in commit status API
......
class @IssuableForm
ISSUE_MOVE_CONFIRM_MSG = 'Are you sure you want to move this issue to another project?'
issueMoveConfirmMsg: 'Are you sure you want to move this issue to another project?'
wipRegex: /^\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i
constructor: (@form) ->
GitLab.GfmAutoComplete.setup()
......@@ -17,6 +18,8 @@ class @IssuableForm
@form.on "submit", @handleSubmit
@form.on "click", ".btn-cancel", @resetAutosave
@initWip()
initAutosave: ->
new Autosave @titleField, [
document.location.pathname,
......@@ -32,10 +35,48 @@ class @IssuableForm
handleSubmit: =>
if (parseInt(@issueMoveField?.val()) ? 0) > 0
return false unless confirm(ISSUE_MOVE_CONFIRM_MSG)
return false unless confirm(@issueMoveConfirmMsg)
@resetAutosave()
resetAutosave: =>
@titleField.data("autosave").reset()
@descriptionField.data("autosave").reset()
initWip: ->
@$wipExplanation = @form.find(".js-wip-explanation")
@$noWipExplanation = @form.find(".js-no-wip-explanation")
return unless @$wipExplanation.length and @$noWipExplanation.length
@form.on "click", ".js-toggle-wip", @toggleWip
@titleField.on "keyup blur", @renderWipExplanation
@renderWipExplanation()
workInProgress: ->
@wipRegex.test @titleField.val()
renderWipExplanation: =>
if @workInProgress()
@$wipExplanation.show()
@$noWipExplanation.hide()
else
@$wipExplanation.hide()
@$noWipExplanation.show()
toggleWip: (event) =>
event.preventDefault()
if @workInProgress()
@removeWip()
else
@addWip()
@renderWipExplanation()
removeWip: ->
@titleField.val @titleField.val().replace(@wipRegex, "")
addWip: ->
@titleField.val "WIP: #{@titleField.val()}"
......@@ -7,6 +7,7 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
if pre_auth.authorizable?
if skip_authorization? || matching_token?
auth = authorization.authorize
session.delete(:user_return_to)
redirect_to auth.redirect_uri
else
render "doorkeeper/authorizations/new"
......
......@@ -5,7 +5,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
before_action :module_enabled
before_action :merge_request, only: [
:edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check,
:ci_status, :cancel_merge_when_build_succeeds
:ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip
]
before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits, :builds]
before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds]
......@@ -20,7 +20,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
before_action :authorize_create_merge_request!, only: [:new, :create]
# Allow modify merge_request
before_action :authorize_update_merge_request!, only: [:close, :edit, :update, :sort]
before_action :authorize_update_merge_request!, only: [:close, :edit, :update, :remove_wip, :sort]
def index
terms = params['issue_search']
......@@ -164,6 +164,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end
end
def remove_wip
MergeRequests::UpdateService.new(project, current_user, title: @merge_request.wipless_title).execute(@merge_request)
redirect_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request),
notice: "The merge request can now be merged."
end
def merge_check
@merge_request.check_if_can_be_merged
......
......@@ -32,7 +32,7 @@ module LabelsHelper
# link_to_label(label) { "My Custom Label Text" }
#
# Returns a String
def link_to_label(label, project: nil, type: :issue, &block)
def link_to_label(label, project: nil, type: :issue, tooltip: true, &block)
project ||= @project || label.project
link = send("namespace_project_#{type.to_s.pluralize}_path",
project.namespace,
......@@ -42,7 +42,7 @@ module LabelsHelper
if block_given?
link_to link, &block
else
link_to render_colored_label(label), link
link_to render_colored_label(label, tooltip: tooltip), link
end
end
......@@ -50,23 +50,24 @@ module LabelsHelper
@project.labels.pluck(:title)
end
def render_colored_label(label, label_suffix = '')
def render_colored_label(label, label_suffix = '', tooltip: true)
label_color = label.color || Label::DEFAULT_COLOR
text_color = text_color_for_bg(label_color)
# Intentionally not using content_tag here so that this method can be called
# by LabelReferenceFilter
span = %(<span class="label color-label") +
%(style="background-color: #{label_color}; color: #{text_color}">) +
span = %(<span class="label color-label #{"has_tooltip" if tooltip}" ) +
%(style="background-color: #{label_color}; color: #{text_color}" ) +
%(title="#{escape_once(label.description)}" data-container="body">) +
%(#{escape_once(label.name)}#{label_suffix}</span>)
span.html_safe
end
def render_colored_cross_project_label(label)
def render_colored_cross_project_label(label, tooltip: true)
label_suffix = label.project.name_with_namespace
label_suffix = " <i>in #{escape_once(label_suffix)}</i>"
render_colored_label(label, label_suffix)
render_colored_label(label, label_suffix, tooltip: tooltip)
end
def suggested_colors
......
......@@ -109,9 +109,8 @@ class Issue < ActiveRecord::Base
def related_branches
return [] if self.project.empty_repo?
self.project.repository.branch_names.select do |branch|
branch =~ /\A#{iid}-(?!\d+-stable)/i
end
self.project.repository.branch_names.select { |branch| branch.end_with?("-#{iid}") }
end
# Reset issue events cache
......@@ -154,7 +153,7 @@ class Issue < ActiveRecord::Base
end
def to_branch_name
"#{iid}-#{title.parameterize}"
"#{title.parameterize}-#{iid}"
end
def can_be_worked_on?(current_user)
......
......@@ -277,8 +277,14 @@ class MergeRequest < ActiveRecord::Base
self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
end
WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze
def work_in_progress?
!!(title =~ /\A\[?WIP(\]|:| )/i)
title =~ WIP_REGEX
end
def wipless_title
self.title.sub(WIP_REGEX, "")
end
def mergeable?
......
......@@ -5,6 +5,19 @@ module MergeRequests
SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
end
def create_title_change_note(issuable, old_title)
removed_wip = old_title =~ MergeRequest::WIP_REGEX && !issuable.work_in_progress?
added_wip = old_title !~ MergeRequest::WIP_REGEX && issuable.work_in_progress?
if removed_wip
SystemNoteService.remove_merge_request_wip(issuable, issuable.project, current_user)
elsif added_wip
SystemNoteService.add_merge_request_wip(issuable, issuable.project, current_user)
else
super
end
end
def hook_data(merge_request, action)
hook_data = merge_request.to_hook_data(current_user)
merge_request_url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id)
......
......@@ -51,7 +51,7 @@ module MergeRequests
# be interpreted as the use wants to close that issue on this project
# Pattern example: 112-fix-mep-mep
# Will lead to appending `Closes #112` to the description
if match = merge_request.source_branch.match(/\A(\d+)-/)
if match = merge_request.source_branch.match(/-(\d+)\z/)
iid = match[1]
closes_issue = "Closes ##{iid}"
......
......@@ -144,6 +144,18 @@ class SystemNoteService
create_note(noteable: noteable, project: project, author: author, note: body)
end
def self.remove_merge_request_wip(noteable, project, author)
body = 'Unmarked this merge request as a Work In Progress'
create_note(noteable: noteable, project: project, author: author, note: body)
end
def self.add_merge_request_wip(noteable, project, author)
body = 'Marked this merge request as a **Work In Progress**'
create_note(noteable: noteable, project: project, author: author, note: body)
end
# Called when the title of a Noteable is changed
#
# noteable - Noteable object that responds to `title`
......@@ -210,7 +222,7 @@ class SystemNoteService
# Called when a branch is created from the 'new branch' button on a issue
# Example note text:
#
# "Started branch `201-issue-branch-button`"
# "Started branch `issue-branch-button-201`"
def self.new_issue_branch(issue, project, author, branch)
h = Gitlab::Application.routes.url_helpers
link = h.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch)
......
%li{id: dom_id(label)}
.label-row
= render_colored_label(label)
= render_colored_label(label, tooltip: false)
= markdown(label.description, pipeline: :single_line)
.pull-right
= link_to 'Edit', edit_admin_label_path(label), class: 'btn btn-sm'
......
......@@ -20,4 +20,4 @@
- next unless blob
= render 'projects/diffs/file', i: index, project: project,
diff_file: diff_file, diff_commit: diff_commit, blob: blob
diff_file: diff_file, diff_commit: diff_commit, blob: blob, diff_refs: diff_refs
......@@ -53,6 +53,6 @@
= render "projects/diffs/text_file", diff_file: diff_file, index: i
- elsif blob.image?
- old_file = project.repository.prev_blob_for_diff(diff_commit, diff_file)
= render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i
= render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i, diff_refs: diff_refs
- else
.nothing-here-block No preview for this file type
- diff = diff_file.diff
- file_raw_path = namespace_project_raw_path(@project.namespace, @project, tree_join(@commit.id, diff.new_path))
- old_file_raw_path = namespace_project_raw_path(@project.namespace, @project, tree_join(@commit.parent_id, diff.old_path))
- old_commit_id = diff_refs.first.id
- old_file_raw_path = namespace_project_raw_path(@project.namespace, @project, tree_join(old_commit_id, diff.old_path))
- if diff.renamed_file || diff.new_file || diff.deleted_file
.image
%span.wrap
......@@ -12,7 +13,7 @@
%div.two-up.view
%span.wrap
.frame.deleted
%a{href: namespace_project_blob_path(@project.namespace, @project, tree_join(@commit.parent_id, diff.old_path))}
%a{href: namespace_project_blob_path(@project.namespace, @project, tree_join(old_commit_id, diff.old_path))}
%img{src: old_file_raw_path}
%p.image-info.hide
%span.meta-filesize= "#{number_to_human_size old_file.size}"
......
%h4
This merge request is currently a Work In Progress
%p
When this merge request is ready, remove the "WIP" prefix from the title to allow it to be merged.
- if can?(current_user, :update_merge_request, @merge_request)
%p
When this merge request is ready,
= link_to remove_wip_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), method: :post do
remove the
%code WIP:
prefix from the title
to allow it to be merged.
%span.label-row
= link_to_label(label)
= link_to_label(label, tooltip: false)
%span.prepend-left-10
= markdown(label.description, pipeline: :single_line)
......@@ -13,12 +13,21 @@
- if issuable.is_a?(MergeRequest)
%p.help-block
- if issuable.work_in_progress?
Remove the <code>WIP</code> prefix from the title to allow this
<strong>Work In Progress</strong> merge request to be merged when it's ready.
- else
Start the title with <code>[WIP]</code> or <code>WIP:</code> to prevent a
<strong>Work In Progress</strong> merge request from being merged before it's ready.
.js-wip-explanation
%a.js-toggle-wip{href: ""}
Remove the
%code WIP:
prefix from the title
to allow this
%strong Work In Progress
merge request to be merged when it's ready.
.js-no-wip-explanation
%a.js-toggle-wip{href: ""}
Start the title with
%code WIP:
to prevent a
%strong Work In Progress
merge request from being merged before it's ready.
.form-group.detail-page-description
= f.label :description, 'Description', class: 'control-label'
.col-sm-10
......
......@@ -5,7 +5,7 @@
%li
%span.label-row
= link_to milestones_label_path(options) do
- render_colored_label(label)
- render_colored_label(label, tooltip: false)
%span.prepend-left-10
= markdown(label.description, pipeline: :single_line)
......
......@@ -623,6 +623,7 @@ Rails.application.routes.draw do
post :cancel_merge_when_build_succeeds
get :ci_status
post :toggle_subscription
post :remove_wip
end
collection do
......
......@@ -11,7 +11,7 @@ describe LabelsHelper do
end
it 'uses the instance variable' do
expect(link_to_label(label)).to match %r{<a href="/#{@project.to_reference}/issues\?label_name=#{label.name}">.*</a>}
expect(link_to_label(label)).to match %r{<a href="/#{@project.to_reference}/issues\?label_name=#{label.name}"><span class="[\w\s\-]*has_tooltip".*</span></a>}
end
end
......@@ -39,6 +39,14 @@ describe LabelsHelper do
end
end
context 'with a tooltip argument' do
context 'set to false' do
it 'does not include the has_tooltip class' do
expect(link_to_label(label, tooltip: false)).not_to match %r{has_tooltip}
end
end
end
context 'with block' do
it 'passes the block to link_to' do
link = link_to_label(label) { 'Foo' }
......@@ -49,7 +57,7 @@ describe LabelsHelper do
context 'without block' do
it 'uses render_colored_label as the link content' do
expect(self).to receive(:render_colored_label).
with(label).and_return('Foo')
with(label, tooltip: true).and_return('Foo')
expect(link_to_label(label)).to match('Foo')
end
end
......
......@@ -56,7 +56,7 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
describe 'label span element' do
it 'includes default classes' do
doc = reference_filter("Label #{reference}")
expect(doc.css('a span').first.attr('class')).to eq 'label color-label'
expect(doc.css('a span').first.attr('class')).to eq 'label color-label has_tooltip'
end
it 'includes a style attribute' do
......
......@@ -181,7 +181,7 @@ describe Issue, models: true do
end
describe '#related_branches' do
it "should " do
it "selects the right branches" do
allow(subject.project.repository).to receive(:branch_names).
and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name])
......@@ -201,10 +201,10 @@ describe Issue, models: true do
end
describe "#to_branch_name" do
let(:issue) { build(:issue, title: 'a' * 30) }
let(:issue) { create(:issue, title: 'a' * 30) }
it "starts with the issue iid" do
expect(issue.to_branch_name).to match /\A#{issue.iid}-a+\z/
expect(issue.to_branch_name).to match /-#{issue.iid}\z/
end
end
end
......@@ -216,24 +216,11 @@ describe MergeRequest, models: true do
end
describe "#work_in_progress?" do
it "detects the 'WIP ' prefix" do
subject.title = "WIP #{subject.title}"
['WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP [WIP] WIP: WIP '].each do |wip_prefix|
it "detects the '#{wip_prefix}' prefix" do
subject.title = "#{wip_prefix}#{subject.title}"
expect(subject).to be_work_in_progress
end
it "detects the 'WIP: ' prefix" do
subject.title = "WIP: #{subject.title}"
expect(subject).to be_work_in_progress
end
it "detects the '[WIP] ' prefix" do
subject.title = "[WIP] #{subject.title}"
expect(subject).to be_work_in_progress
end
it "detects the '[WIP]' prefix" do
subject.title = "[WIP]#{subject.title}"
expect(subject).to be_work_in_progress
end
it "doesn't detect WIP for words starting with WIP" do
......@@ -241,6 +228,11 @@ describe MergeRequest, models: true do
expect(subject).not_to be_work_in_progress
end
it "doesn't detect WIP for words containing with WIP" do
subject.title = "WupWipwap #{subject.title}"
expect(subject).not_to be_work_in_progress
end
it "doesn't detect WIP by default" do
expect(subject).not_to be_work_in_progress
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment