diff options
author | Lin Jen-Shin <godfat@godfat.org> | 2016-10-04 23:57:43 +0800 |
---|---|---|
committer | Lin Jen-Shin <godfat@godfat.org> | 2016-10-04 23:57:43 +0800 |
commit | aafb0171e6fb17789754ee71bc5c3c6bca94bf7b (patch) | |
tree | 670f6bae32dd8967380371e464a6542d56c7c77b | |
parent | 218331e6af8548eb796870de34e73dd43c78644c (diff) | |
parent | c38b85f37be87b64c08669aebacea4def6396911 (diff) | |
download | gitlab-ce-aafb0171e6fb17789754ee71bc5c3c6bca94bf7b.tar.gz |
Merge remote-tracking branch 'upstream/master' into all-skipped-equals-success
* upstream/master: (55 commits)
Restrict failed login attempts for users with 2FA
Update RuboCop to 0.43.0 and update configuration
Use SELECT 1, instead SELECT COUNT(*) to ask for notes existency
Simplify Mentionable concern instance methods
Fix issues importing services via Import/Export
deployment refs in own folder, new method for creating refs
Update method name
Save a fetchable ref per deployement
GrapeDSL for Namespace endpoint
Remove SCSS rules for short hex chars.
Fix bug when trying to cache closed issues from external issue trackers
Upgrade acts-as-taggable-on from 3.5.0 to 4.0.0.
Remove useless code now that Member#add_user handles it
Combine requestFileSuccess arguments into `opts`
Append issue template to existing description
Invert method's naming
Fix a few things after the initial improvment to Members::DestroyService
Improve Members::DestroyService
Add Container Registry on/off status to admin area
Enable Lint/StringConversionInInterpolation cop and autocorrect offenses
...
85 files changed, 784 insertions, 369 deletions
diff --git a/.csscomb.json b/.csscomb.json index 741cc1488b5..aa6a17f7517 100644 --- a/.csscomb.json +++ b/.csscomb.json @@ -6,7 +6,7 @@ "always-semicolon": true, "color-case": "lower", "block-indent": " ", - "color-shorthand": true, + "color-shorthand": false, "element-case": "lower", "space-before-colon": "", "space-after-colon": " ", diff --git a/.rubocop.yml b/.rubocop.yml index 5bd31ccf329..bec2464c740 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -453,6 +453,10 @@ Style/VariableName: EnforcedStyle: snake_case Enabled: true +# Use the configured style when numbering variables. +Style/VariableNumber: + Enabled: false + # Use when x then ... for one-line cases. Style/WhenThen: Enabled: true @@ -639,6 +643,10 @@ Lint/RescueException: Lint/ShadowedException: Enabled: false +# Checks for Object#to_s usage in string interpolation. +Lint/StringConversionInInterpolation: + Enabled: true + # Do not use prefix `_` for a variable that is used. Lint/UnderscorePrefixedVariableName: Enabled: true diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 87520c67dd5..11b34fafa2a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,21 +1,21 @@ # This configuration was generated by # `rubocop --auto-gen-config --exclude-limit 0` -# on 2016-09-14 15:44:53 -0400 using RuboCop version 0.42.0. +# on 2016-10-04 13:16:20 +0200 using RuboCop version 0.43.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 158 +# Offense count: 160 Lint/AmbiguousRegexpLiteral: Enabled: false -# Offense count: 41 +# Offense count: 40 # Configuration parameters: AllowSafeAssignment. Lint/AssignmentInCondition: Enabled: false -# Offense count: 16 +# Offense count: 18 Lint/HandleExceptions: Enabled: false @@ -23,16 +23,21 @@ Lint/HandleExceptions: Lint/Loop: Enabled: false -# Offense count: 16 +# Offense count: 19 Lint/ShadowingOuterLocalVariable: Enabled: false -# Offense count: 6 +# Offense count: 9 +# Cop supports --auto-correct. +Lint/UnifiedInteger: + Enabled: false + +# Offense count: 13 # Cop supports --auto-correct. -Lint/StringConversionInInterpolation: +Lint/UnneededSplatExpansion: Enabled: false -# Offense count: 49 +# Offense count: 69 # Cop supports --auto-correct. # Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments. Lint/UnusedBlockArgument: @@ -44,32 +49,81 @@ Lint/UnusedBlockArgument: Lint/UnusedMethodArgument: Enabled: false -# Offense count: 9 -# Cop supports --auto-correct. -Performance/PushSplat: - Enabled: false - # Offense count: 2 # Cop supports --auto-correct. Performance/RedundantBlockCall: Enabled: false -# Offense count: 4 +# Offense count: 5 # Cop supports --auto-correct. Performance/RedundantMatch: Enabled: false -# Offense count: 27 +# Offense count: 26 # Cop supports --auto-correct. # Configuration parameters: MaxKeyValuePairs. Performance/RedundantMerge: Enabled: false -# Offense count: 61 +# Offense count: 7 +RSpec/BeEql: + Enabled: false + +# Offense count: 20 +# Configuration parameters: CustomIncludeMethods. +RSpec/EmptyExampleGroup: + Enabled: false + +# Offense count: 16 +RSpec/ExpectActual: + Enabled: false + +# Offense count: 34 +# Configuration parameters: EnforcedStyle, SupportedStyles. +# SupportedStyles: implicit, each, example +RSpec/HookArgument: + Enabled: false + +# Offense count: 168 +RSpec/LeadingSubject: + Enabled: false + +# Offense count: 162 +RSpec/LetSetup: + Enabled: false + +# Offense count: 10 +RSpec/MessageChain: + Enabled: false + +# Offense count: 714 +# Configuration parameters: EnforcedStyle, SupportedStyles. +# SupportedStyles: allow, expect +RSpec/MessageExpectation: + Enabled: false + +# Offense count: 2423 +RSpec/MultipleExpectations: + Max: 36 + +# Offense count: 1504 +RSpec/NamedSubject: + Enabled: false + +# Offense count: 1335 +# Configuration parameters: MaxNesting. +RSpec/NestedGroups: + Enabled: false + +# Offense count: 99 +RSpec/SubjectStub: + Enabled: false + +# Offense count: 64 Rails/OutputSafety: Enabled: false -# Offense count: 129 +# Offense count: 151 # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: strict, flexible Rails/TimeZone: @@ -82,58 +136,63 @@ Rails/TimeZone: Rails/Validation: Enabled: false -# Offense count: 273 +# Offense count: 2 +# Cop supports --auto-correct. +Security/JSONLoad: + Enabled: false + +# Offense count: 284 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth. # SupportedStyles: with_first_parameter, with_fixed_indentation Style/AlignParameters: Enabled: false -# Offense count: 30 +# Offense count: 28 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: always, conditionals Style/AndOr: Enabled: false -# Offense count: 50 +# Offense count: 52 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: percent_q, bare_percent Style/BarePercentLiterals: Enabled: false -# Offense count: 289 +# Offense count: 291 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: braces, no_braces, context_dependent Style/BracesAroundHashParameters: Enabled: false -# Offense count: 5 +# Offense count: 6 Style/CaseEquality: Enabled: false -# Offense count: 19 +# Offense count: 26 # Cop supports --auto-correct. Style/ColonMethodCall: Enabled: false -# Offense count: 3 +# Offense count: 2 # Cop supports --auto-correct. # Configuration parameters: Keywords. # Keywords: TODO, FIXME, OPTIMIZE, HACK, REVIEW Style/CommentAnnotation: Enabled: false -# Offense count: 33 +# Offense count: 30 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, SingleLineConditionsOnly. # SupportedStyles: assign_to_condition, assign_inside_condition Style/ConditionalAssignment: Enabled: false -# Offense count: 881 +# Offense count: 957 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: leading, trailing @@ -144,12 +203,12 @@ Style/DotPosition: Style/DoubleNegation: Enabled: false -# Offense count: 4 +# Offense count: 6 # Cop supports --auto-correct. Style/EachWithObject: Enabled: false -# Offense count: 25 +# Offense count: 26 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: empty, nil, both @@ -161,24 +220,24 @@ Style/EmptyElse: Style/EmptyLiteral: Enabled: false -# Offense count: 135 +# Offense count: 140 # Cop supports --auto-correct. # Configuration parameters: AllowForAlignment, ForceEqualSignAlignment. Style/ExtraSpacing: Enabled: false -# Offense count: 7 +# Offense count: 6 # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: format, sprintf, percent Style/FormatString: Enabled: false -# Offense count: 51 +# Offense count: 201 # Configuration parameters: MinBodyLength. Style/GuardClause: Enabled: false -# Offense count: 9 +# Offense count: 11 Style/IfInsideElse: Enabled: false @@ -188,21 +247,21 @@ Style/IfInsideElse: Style/IfUnlessModifier: Enabled: false -# Offense count: 52 +# Offense count: 53 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth. # SupportedStyles: special_inside_parentheses, consistent, align_brackets Style/IndentArray: Enabled: false -# Offense count: 97 +# Offense count: 95 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth. # SupportedStyles: special_inside_parentheses, consistent, align_braces Style/IndentHash: Enabled: false -# Offense count: 12 +# Offense count: 29 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: line_count_dependent, lambda, literal @@ -214,7 +273,7 @@ Style/Lambda: Style/LineEndConcatenation: Enabled: false -# Offense count: 13 +# Offense count: 15 # Cop supports --auto-correct. Style/MethodCallParentheses: Enabled: false @@ -223,7 +282,7 @@ Style/MethodCallParentheses: Style/MethodMissing: Enabled: false -# Offense count: 85 +# Offense count: 95 # Cop supports --auto-correct. Style/MutableConstant: Enabled: false @@ -240,14 +299,14 @@ Style/NestedParenthesizedCalls: Style/Next: Enabled: false -# Offense count: 8 +# Offense count: 12 # Cop supports --auto-correct. # Configuration parameters: EnforcedOctalStyle, SupportedOctalStyles. # SupportedOctalStyles: zero_with_o, zero_only Style/NumericLiteralPrefix: Enabled: false -# Offense count: 64 +# Offense count: 53 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: predicate, comparison @@ -259,7 +318,7 @@ Style/NumericPredicate: Style/ParallelAssignment: Enabled: false -# Offense count: 264 +# Offense count: 294 # Cop supports --auto-correct. # Configuration parameters: PreferredDelimiters. Style/PercentLiteralDelimiters: @@ -277,7 +336,7 @@ Style/PercentQLiterals: Style/PerlBackrefs: Enabled: false -# Offense count: 35 +# Offense count: 38 # Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist. # NamePrefix: is_, has_, have_ # NamePrefixBlacklist: is_, has_, have_ @@ -285,7 +344,7 @@ Style/PerlBackrefs: Style/PredicateName: Enabled: false -# Offense count: 27 +# Offense count: 26 # Cop supports --auto-correct. Style/PreferredHashMethods: Enabled: false @@ -317,12 +376,12 @@ Style/RedundantException: Style/RedundantFreeze: Enabled: false -# Offense count: 408 +# Offense count: 427 # Cop supports --auto-correct. Style/RedundantSelf: Enabled: false -# Offense count: 93 +# Offense count: 97 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, AllowInnerSlashes. # SupportedStyles: slashes, percent_r, mixed @@ -334,7 +393,12 @@ Style/RegexpLiteral: Style/RescueModifier: Enabled: false -# Offense count: 5 +# Offense count: 114 +# Cop supports --auto-correct. +Style/SafeNavigation: + Enabled: false + +# Offense count: 7 # Cop supports --auto-correct. Style/SelfAssignment: Enabled: false @@ -351,7 +415,7 @@ Style/SingleLineBlockParams: Style/SingleLineMethods: Enabled: false -# Offense count: 124 +# Offense count: 125 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: space, no_space @@ -364,19 +428,19 @@ Style/SpaceBeforeBlockBraces: Style/SpaceBeforeFirstArg: Enabled: false -# Offense count: 141 +# Offense count: 145 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, EnforcedStyleForEmptyBraces, SpaceBeforeBlockParameters. # SupportedStyles: space, no_space Style/SpaceInsideBlockBraces: Enabled: false -# Offense count: 96 +# Offense count: 99 # Cop supports --auto-correct. Style/SpaceInsideBrackets: Enabled: false -# Offense count: 62 +# Offense count: 65 # Cop supports --auto-correct. Style/SpaceInsideParens: Enabled: false @@ -386,21 +450,21 @@ Style/SpaceInsideParens: Style/SpaceInsidePercentLiteralDelimiters: Enabled: false -# Offense count: 40 +# Offense count: 41 # Cop supports --auto-correct. # Configuration parameters: SupportedStyles. # SupportedStyles: use_perl_names, use_english_names Style/SpecialGlobalVars: EnforcedStyle: use_perl_names -# Offense count: 30 +# Offense count: 31 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: single_quotes, double_quotes Style/StringLiteralsInInterpolation: Enabled: false -# Offense count: 32 +# Offense count: 33 # Cop supports --auto-correct. # Configuration parameters: IgnoredMethods. # IgnoredMethods: respond_to, define_method @@ -414,7 +478,7 @@ Style/SymbolProc: Style/TernaryParentheses: Enabled: false -# Offense count: 24 +# Offense count: 29 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyleForMultiline, SupportedStyles. # SupportedStyles: comma, consistent_comma, no_comma diff --git a/.scss-lint.yml b/.scss-lint.yml index 66f9975d4ce..71df6be6a15 100644 --- a/.scss-lint.yml +++ b/.scss-lint.yml @@ -79,7 +79,7 @@ linters: # HEX colors should use three-character values where possible. HexLength: - enabled: true + enabled: false # HEX color values should use lower-case colors to differentiate between # letters and numbers, e.g. `#E3E3E3` vs. `#e3e3e3`. diff --git a/CHANGELOG b/CHANGELOG index 7c9cb58438b..f3d6535c170 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,9 +8,11 @@ v 8.13.0 (unreleased) - Replaced the check sign to arrow in the show build view. !6501 - Add a /wip slash command to toggle the Work In Progress status of a merge request. !6259 (tbalthazar) - Speed-up group milestones show page + - Keep refs for each deployment - Log LDAP lookup errors and don't swallow unrelated exceptions. !6103 (Markus Koller) - Add more tests for calendar contribution (ClemMakesApps) - Avoid database queries on Banzai::ReferenceParser::BaseParser for nodes without references + - Simplify Mentionable concern instance methods - Fix permission for setting an issue's due date - Expose expires_at field when sharing project on API - Fix issue with page scrolling to top when closing or pinning sidebar (lukehowell) @@ -20,6 +22,7 @@ v 8.13.0 (unreleased) - Fix robots.txt disallowing access to groups starting with "s" (Matt Harrison) - Close open merge request without source project (Katarzyna Kobierska Ula Budziszewska) - Fix that manual jobs would no longer block jobs in the next stage. !6604 + - Add configurable email subject suffix (Fu Xu) - Use a ConnectionPool for Rails.cache on Sidekiq servers - Replace `alias_method_chain` with `Module#prepend` - Enable GitLab Import/Export for non-admin users. @@ -27,6 +30,7 @@ v 8.13.0 (unreleased) - Only update issuable labels if they have been changed - Take filters in account in issuable counters. !6496 - Use custom Ruby images to test builds (registry.dev.gitlab.org/gitlab/gitlab-build-images:*) + - Append issue template to existing description !6149 (Joseph Frazier) - Revoke button in Applications Settings underlines on hover. - Add missing values to linter !6276 (Katarzyna Kobierska Ula Budziszewska) - Fix Long commit messages overflow viewport in file tree @@ -42,8 +46,14 @@ v 8.13.0 (unreleased) - Notify the Merger about merge after successful build (Dimitris Karakasilis) - Fix broken repository 500 errors in project list - Close todos when accepting merge requests via the API !6486 (tonygambone) + - Changed Slack service user referencing from full name to username (Sebastian Poxhofer) + - Add Container Registry on/off status to Admin Area !6638 (the-undefined) v 8.12.4 (unreleased) + - Fix type mismatch bug when closing Jira issue + - Fix issues importing services via Import/Export + - Restrict failed login attempts for users with 2FA enabled + - Fix "Copy to clipboard" tooltip to say "Copied!" when clipboard button is clicked. (lukehowell) v 8.12.3 - Update Gitlab Shell to support low IO priority for storage moves @@ -102,6 +112,7 @@ v 8.12.0 - Fix long comments in diffs messing with table width - Add spec covering 'Gitlab::Git::committer_hash' !6433 (dandunckelman) - Fix pagination on user snippets page + - Honor "fixed layout" preference in more places !6422 - Run CI builds with the permissions of users !5735 - Fix sorting of issues in API - Fix download artifacts button links !6407 @@ -118,6 +129,7 @@ v 8.12.0 - Reduce contributions calendar data payload (ClemMakesApps) - Show all pipelines for merge requests even from discarded commits !6414 - Replace contributions calendar timezone payload with dates (ClemMakesApps) + - Changed MR widget build status to pipeline status !6335 - Add `web_url` field to issue, merge request, and snippet API objects (Ben Boeckel) - Enable pipeline events by default !6278 - Move parsing of sidekiq ps into helper !6245 (pascalbetz) @@ -141,6 +153,7 @@ v 8.12.0 - Increase ci_builds artifacts_size column to 8-byte integer to allow larger files - Add textarea autoresize after comment (ClemMakesApps) - Do not write SSH public key 'comments' to authorized_keys !6381 + - Add due date to issue todos - Refresh todos count cache when an Issue/MR is deleted - Fix branches page dropdown sort alignment (ClemMakesApps) - Hides merge request button on branches page is user doesn't have permissions @@ -99,17 +99,17 @@ gem 'unf', '~> 0.1.4' gem 'seed-fu', '~> 2.3.5' # Markdown and HTML processing -gem 'html-pipeline', '~> 1.11.0' -gem 'task_list', '~> 1.0.2', require: 'task_list/railtie' -gem 'github-markup', '~> 1.4' -gem 'redcarpet', '~> 3.3.3' -gem 'RedCloth', '~> 4.3.2' -gem 'rdoc', '~>3.6' -gem 'org-ruby', '~> 0.9.12' -gem 'creole', '~> 0.5.0' -gem 'wikicloth', '0.8.1' -gem 'asciidoctor', '~> 1.5.2' -gem 'rouge', '~> 2.0' +gem 'html-pipeline', '~> 1.11.0' +gem 'deckar01-task_list', '1.0.5', require: 'task_list/railtie' +gem 'github-markup', '~> 1.4' +gem 'redcarpet', '~> 3.3.3' +gem 'RedCloth', '~> 4.3.2' +gem 'rdoc', '~>3.6' +gem 'org-ruby', '~> 0.9.12' +gem 'creole', '~> 0.5.0' +gem 'wikicloth', '0.8.1' +gem 'asciidoctor', '~> 1.5.2' +gem 'rouge', '~> 2.0' # See https://groups.google.com/forum/#!topic/ruby-security-ann/aSbgDiwb24s # and https://groups.google.com/forum/#!topic/ruby-security-ann/Dy7YiKb_pMM @@ -130,7 +130,7 @@ gem 'state_machines-activerecord', '~> 0.4.0' gem 'after_commit_queue', '~> 1.3.0' # Issue tags -gem 'acts-as-taggable-on', '~> 3.4' +gem 'acts-as-taggable-on', '~> 4.0' # Background jobs gem 'sidekiq', '~> 4.2' @@ -295,7 +295,7 @@ group :development, :test do gem 'spring-commands-spinach', '~> 1.1.0' gem 'spring-commands-teaspoon', '~> 0.0.2' - gem 'rubocop', '~> 0.42.0', require: false + gem 'rubocop', '~> 0.43.0', require: false gem 'rubocop-rspec', '~> 1.5.0', require: false gem 'scss_lint', '~> 0.47.0', require: false gem 'haml_lint', '~> 0.18.2', require: false diff --git a/Gemfile.lock b/Gemfile.lock index f15715a20ff..3f756fec929 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -44,8 +44,8 @@ GEM minitest (~> 5.1) thread_safe (~> 0.3, >= 0.3.4) tzinfo (~> 1.1) - acts-as-taggable-on (3.5.0) - activerecord (>= 3.2, < 5) + acts-as-taggable-on (4.0.0) + activerecord (>= 4.0) addressable (2.3.8) after_commit_queue (1.3.0) activerecord (>= 3.0) @@ -157,6 +157,10 @@ GEM database_cleaner (1.5.3) debug_inspector (0.0.2) debugger-ruby_core_source (1.3.8) + deckar01-task_list (1.0.5) + activesupport (~> 4.0) + html-pipeline + rack (~> 1.0) default_value_for (3.0.2) activerecord (>= 3.2.0, < 5.1) descendants_tracker (0.0.4) @@ -483,7 +487,7 @@ GEM orm_adapter (0.5.0) paranoia (2.1.4) activerecord (~> 4.0) - parser (2.3.1.2) + parser (2.3.1.4) ast (~> 2.2) pg (0.18.4) pkg-config (1.1.7) @@ -616,7 +620,7 @@ GEM rspec-retry (0.4.5) rspec-core rspec-support (3.5.0) - rubocop (0.42.0) + rubocop (0.43.0) parser (>= 2.3.1.1, < 3.0) powerpack (~> 0.1) rainbow (>= 1.99.1, < 3.0) @@ -725,8 +729,6 @@ GEM ffi sysexits (1.2.0) systemu (2.6.5) - task_list (1.0.2) - html-pipeline teaspoon (1.1.5) railties (>= 3.2.5, < 6) teaspoon-jasmine (2.2.0) @@ -800,7 +802,7 @@ DEPENDENCIES RedCloth (~> 4.3.2) ace-rails-ap (~> 4.1.0) activerecord-session_store (~> 1.0.0) - acts-as-taggable-on (~> 3.4) + acts-as-taggable-on (~> 4.0) addressable (~> 2.3.8) after_commit_queue (~> 1.3.0) akismet (~> 2.0) @@ -831,6 +833,7 @@ DEPENDENCIES creole (~> 0.5.0) d3_rails (~> 3.5.0) database_cleaner (~> 1.5.0) + deckar01-task_list (= 1.0.5) default_value_for (~> 3.0.0) devise (~> 4.2) devise-two-factor (~> 3.0.0) @@ -935,7 +938,7 @@ DEPENDENCIES rqrcode-rails3 (~> 0.1.7) rspec-rails (~> 3.5.0) rspec-retry (~> 0.4.5) - rubocop (~> 0.42.0) + rubocop (~> 0.43.0) rubocop-rspec (~> 1.5.0) ruby-fogbugz (~> 0.2.1) ruby-prof (~> 0.16.2) @@ -963,7 +966,6 @@ DEPENDENCIES sprockets-es6 (~> 0.9.2) state_machines-activerecord (~> 0.4.0) sys-filesystem (~> 1.1.6) - task_list (~> 1.0.2) teaspoon (~> 1.1.0) teaspoon-jasmine (~> 2.2.0) test_after_commit (~> 0.4.2) @@ -984,4 +986,4 @@ DEPENDENCIES wikicloth (= 0.8.1) BUNDLED WITH - 1.13.1 + 1.13.2 diff --git a/app/assets/javascripts/blob/template_selector.js b/app/assets/javascripts/blob/template_selector.js index 95352164d76..6d41442cdfc 100644 --- a/app/assets/javascripts/blob/template_selector.js +++ b/app/assets/javascripts/blob/template_selector.js @@ -72,9 +72,17 @@ // To be implemented on the extending class // e.g. // Api.gitignoreText item.name, @requestFileSuccess.bind(@) - TemplateSelector.prototype.requestFileSuccess = function(file, skipFocus) { - this.editor.setValue(file.content, 1); - if (!skipFocus) this.editor.focus(); + TemplateSelector.prototype.requestFileSuccess = function(file, opts) { + var oldValue = this.editor.getValue(); + var newValue = file.content; + if (opts == null) { + opts = {}; + } + if (opts.append && oldValue.length && oldValue !== newValue) { + newValue = oldValue + '\n\n' + newValue; + } + this.editor.setValue(newValue, 1); + if (!opts.skipFocus) this.editor.focus(); if (this.editor instanceof jQuery) { this.editor.get(0).dispatchEvent(this.autosizeUpdateEvent); diff --git a/app/assets/javascripts/copy_to_clipboard.js b/app/assets/javascripts/copy_to_clipboard.js index 3e20db7e308..e23bda2fa4e 100644 --- a/app/assets/javascripts/copy_to_clipboard.js +++ b/app/assets/javascripts/copy_to_clipboard.js @@ -26,15 +26,15 @@ }; showTooltip = function(target, title) { - return $(target).tooltip({ - container: 'body', - html: 'true', - placement: 'auto bottom', - title: title, - trigger: 'manual' - }).tooltip('show').one('mouseleave', function() { - return $(this).tooltip('hide'); - }); + var $target = $(target); + var originalTitle = $target.data('original-title'); + + $target + .attr('title', 'Copied!') + .tooltip('fixTitle') + .tooltip('show') + .attr('title', originalTitle) + .tooltip('fixTitle'); }; $(function() { diff --git a/app/assets/javascripts/diff.js b/app/assets/javascripts/diff.js index c8634b78f2b..8086c10ad6b 100644 --- a/app/assets/javascripts/diff.js +++ b/app/assets/javascripts/diff.js @@ -7,6 +7,9 @@ function Diff() { $('.files .diff-file').singleFileDiff(); this.filesCommentButton = $('.files .diff-file').filesCommentButton(); + if (this.diffViewType() === 'parallel') { + $('.content-wrapper .container-fluid').removeClass('container-limited'); + } $(document).off('click', '.js-unfold'); $(document).on('click', '.js-unfold', (function(_this) { return function(event) { @@ -52,6 +55,10 @@ })(this)); } + Diff.prototype.diffViewType = function() { + return $('.inline-parallel-buttons a.active').data('view-type'); + } + Diff.prototype.lineNumbers = function(line) { if (!line.children().length) { return [0, 0]; diff --git a/app/assets/javascripts/merge_conflict_data_provider.js.es6 b/app/assets/javascripts/merge_conflict_data_provider.js.es6 index cd92df8ddc5..13ee794ba38 100644 --- a/app/assets/javascripts/merge_conflict_data_provider.js.es6 +++ b/app/assets/javascripts/merge_conflict_data_provider.js.es6 @@ -7,13 +7,16 @@ const ORIGIN_BUTTON_TITLE = 'Use theirs'; class MergeConflictDataProvider { getInitialData() { + // TODO: remove reliance on jQuery and DOM state introspection const diffViewType = $.cookie('diff_view'); + const fixedLayout = $('.content-wrapper .container-fluid').hasClass('container-limited'); return { isLoading : true, hasError : false, isParallel : diffViewType === 'parallel', diffViewType : diffViewType, + fixedLayout : fixedLayout, isSubmitting : false, conflictsData : {}, resolutionData : {} @@ -192,14 +195,17 @@ class MergeConflictDataProvider { updateViewType(newType) { const vi = this.vueInstance; - if (newType === vi.diffView || !(newType === 'parallel' || newType === 'inline')) { + if (newType === vi.diffViewType || !(newType === 'parallel' || newType === 'inline')) { return; } - vi.diffView = newType; - vi.isParallel = newType === 'parallel'; - $.cookie('diff_view', newType); // TODO: Make sure that cookie path added. - $('.content-wrapper .container-fluid').toggleClass('container-limited'); + vi.diffViewType = newType; + vi.isParallel = newType === 'parallel'; + $.cookie('diff_view', newType, { + path: (gon && gon.relative_url_root) || '/' + }); + $('.content-wrapper .container-fluid') + .toggleClass('container-limited', !vi.isParallel && vi.fixedLayout); } diff --git a/app/assets/javascripts/merge_conflict_resolver.js.es6 b/app/assets/javascripts/merge_conflict_resolver.js.es6 index b56fd5aa658..7e756433bf5 100644 --- a/app/assets/javascripts/merge_conflict_resolver.js.es6 +++ b/app/assets/javascripts/merge_conflict_resolver.js.es6 @@ -60,9 +60,8 @@ class MergeConflictResolver { $('#conflicts .js-syntax-highlight').syntaxHighlight(); }); - if (this.vue.diffViewType === 'parallel') { - $('.content-wrapper .container-fluid').removeClass('container-limited'); - } + $('.content-wrapper .container-fluid') + .toggleClass('container-limited', !this.vue.isParallel && this.vue.fixedLayout); }) } diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index 05644b3d03c..02ff5a382e2 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -36,13 +36,10 @@ }; MergeRequest.prototype.initTabs = function() { - if (this.opts.action !== 'new') { - // `MergeRequests#new` has no tab-persisting or lazy-loading behavior - window.mrTabs = new MergeRequestTabs(this.opts); - } else { - // Show the first tab (Commits) - return $('.merge-request-tabs a[data-toggle="tab"]:first').tab('show'); + if (window.mrTabs) { + window.mrTabs.unbindEvents(); } + window.mrTabs = new MergeRequestTabs(this.opts); }; MergeRequest.prototype.showAllCommits = function() { diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 18bbfa7a459..bec11a198a1 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -56,6 +56,8 @@ MergeRequestTabs.prototype.commitsLoaded = false; + MergeRequestTabs.prototype.fixedLayoutPref = null; + function MergeRequestTabs(opts) { this.opts = opts != null ? opts : {}; this.opts.setUrl = this.opts.setUrl !== undefined ? this.opts.setUrl : true; @@ -70,7 +72,12 @@ MergeRequestTabs.prototype.bindEvents = function() { $(document).on('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown); - return $(document).on('click', '.js-show-tab', this.showTab); + $(document).on('click', '.js-show-tab', this.showTab); + }; + + MergeRequestTabs.prototype.unbindEvents = function() { + $(document).off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown); + $(document).off('click', '.js-show-tab', this.showTab); }; MergeRequestTabs.prototype.showTab = function(event) { @@ -85,11 +92,15 @@ if (action === 'commits') { this.loadCommits($target.attr('href')); this.expandView(); + this.resetViewContainer(); } else if (action === 'diffs') { this.loadDiff($target.attr('href')); if ((typeof bp !== "undefined" && bp !== null) && bp.getBreakpointSize() !== 'lg') { this.shrinkView(); } + if (this.diffViewType() === 'parallel') { + this.expandViewContainer(); + } navBarHeight = $('.navbar-gitlab').outerHeight(); $.scrollTo(".merge-request-details .merge-request-tabs", { offset: -navBarHeight @@ -97,11 +108,14 @@ } else if (action === 'builds') { this.loadBuilds($target.attr('href')); this.expandView(); + this.resetViewContainer(); } else if (action === 'pipelines') { this.loadPipelines($target.attr('href')); this.expandView(); + this.resetViewContainer(); } else { this.expandView(); + this.resetViewContainer(); } if (this.opts.setUrl) { this.setCurrentAction(action); @@ -126,7 +140,7 @@ if (action === 'show') { action = 'notes'; } - return $(".merge-request-tabs a[data-action='" + action + "']").tab('show'); + $(".merge-request-tabs a[data-action='" + action + "']").tab('show').trigger('shown.bs.tab'); }; // Replaces the current Merge Request-specific action in the URL with a new one @@ -209,7 +223,7 @@ gl.utils.localTimeAgo($('.js-timeago', 'div#diffs')); $('#diffs .js-syntax-highlight').syntaxHighlight(); $('#diffs .diff-file').singleFileDiff(); - if (_this.diffViewType() === 'parallel') { + if (_this.diffViewType() === 'parallel' && _this.currentAction === 'diffs') { _this.expandViewContainer(); } _this.diffsLoaded = true; @@ -308,11 +322,21 @@ MergeRequestTabs.prototype.diffViewType = function() { return $('.inline-parallel-buttons a.active').data('view-type'); - // Returns diff view type }; MergeRequestTabs.prototype.expandViewContainer = function() { - return $('.container-fluid').removeClass('container-limited'); + var $wrapper = $('.content-wrapper .container-fluid'); + if (this.fixedLayoutPref === null) { + this.fixedLayoutPref = $wrapper.hasClass('container-limited'); + } + $wrapper.removeClass('container-limited'); + }; + + MergeRequestTabs.prototype.resetViewContainer = function() { + if (this.fixedLayoutPref !== null) { + $('.content-wrapper .container-fluid') + .toggleClass('container-limited', this.fixedLayoutPref); + } }; MergeRequestTabs.prototype.shrinkView = function() { diff --git a/app/assets/javascripts/templates/issuable_template_selector.js.es6 b/app/assets/javascripts/templates/issuable_template_selector.js.es6 index c32ddf80219..017008c8438 100644 --- a/app/assets/javascripts/templates/issuable_template_selector.js.es6 +++ b/app/assets/javascripts/templates/issuable_template_selector.js.es6 @@ -16,7 +16,7 @@ if (initialQuery.name) this.requestFile(initialQuery); $('.reset-template', this.dropdown.parent()).on('click', () => { - if (this.currentTemplate) this.setInputValueToTemplateContent(); + if (this.currentTemplate) this.setInputValueToTemplateContent(false); }); } @@ -26,22 +26,24 @@ this.currentTemplate = currentTemplate; if (err) return; // Error handled by global AJAX error handler this.stopLoadingSpinner(); - this.setInputValueToTemplateContent(); + this.setInputValueToTemplateContent(true); }); return; } - setInputValueToTemplateContent() { + setInputValueToTemplateContent(append) { // `this.requestFileSuccess` sets the value of the description input field - // to the content of the template selected. + // to the content of the template selected. If `append` is true, the + // template content will be appended to the previous value of the field, + // separated by a blank line if the previous value is non-empty. if (this.titleInput.val() === '') { // If the title has not yet been set, focus the title input and - // skip focusing the description input by setting `true` as the 2nd - // argument to `requestFileSuccess`. - this.requestFileSuccess(this.currentTemplate, true); + // skip focusing the description input by setting `true` as the + // `skipFocus` option to `requestFileSuccess`. + this.requestFileSuccess(this.currentTemplate, {skipFocus: true, append}); this.titleInput.focus(); } else { - this.requestFileSuccess(this.currentTemplate); + this.requestFileSuccess(this.currentTemplate, {skipFocus: false, append}); } return; } diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 926247e5e87..3514ee2f35e 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -70,7 +70,8 @@ &.ci-success { color: $gl-success; - a.environment { + a.environment, + a.pipeline { color: inherit; } } diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index d5a8a962662..4c497711fc0 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -23,15 +23,24 @@ module AuthenticatesWithTwoFactor # # Returns nil def prompt_for_two_factor(user) + return locked_user_redirect(user) if user.access_locked? + session[:otp_user_id] = user.id setup_u2f_authentication(user) render 'devise/sessions/two_factor' end + def locked_user_redirect(user) + flash.now[:alert] = 'Invalid Login or password' + render 'devise/sessions/new' + end + def authenticate_with_two_factor user = self.resource = find_user - if user_params[:otp_attempt].present? && session[:otp_user_id] + if user.access_locked? + locked_user_redirect(user) + elsif user_params[:otp_attempt].present? && session[:otp_user_id] authenticate_with_two_factor_via_otp(user) elsif user_params[:device_response].present? && session[:otp_user_id] authenticate_with_two_factor_via_u2f(user) @@ -50,8 +59,9 @@ module AuthenticatesWithTwoFactor remember_me(user) if user_params[:remember_me] == '1' sign_in(user) else + user.increment_failed_attempts! flash.now[:alert] = 'Invalid two-factor code.' - render :two_factor + prompt_for_two_factor(user) end end @@ -65,6 +75,7 @@ module AuthenticatesWithTwoFactor remember_me(user) if user_params[:remember_me] == '1' sign_in(user) else + user.increment_failed_attempts! flash.now[:alert] = 'Authentication via U2F device failed.' prompt_for_two_factor(user) end diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index b8ed2c159a7..c13333641d3 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -15,18 +15,17 @@ module MembershipActions end def leave - @member = membershipable.members.find_by(user_id: current_user) || - membershipable.requesters.find_by(user_id: current_user) - Members::DestroyService.new(@member, current_user).execute + member = Members::DestroyService.new(membershipable, current_user, user_id: current_user.id). + execute(:all) - source_type = @member.real_source_type.humanize(capitalize: false) + source_type = membershipable.class.to_s.humanize(capitalize: false) notice = - if @member.request? + if member.request? "Your access request to the #{source_type} has been withdrawn." else - "You left the \"#{@member.source.human_name}\" #{source_type}." + "You left the \"#{membershipable.human_name}\" #{source_type}." end - redirect_path = @member.request? ? @member.source : [:dashboard, @member.real_source_type.tableize] + redirect_path = member.request? ? member.source : [:dashboard, membershipable.class.to_s.tableize] redirect_to redirect_path, notice: notice end diff --git a/app/controllers/concerns/spammable_actions.rb b/app/controllers/concerns/spammable_actions.rb index 29e243c66a3..99acd98ae13 100644 --- a/app/controllers/concerns/spammable_actions.rb +++ b/app/controllers/concerns/spammable_actions.rb @@ -7,7 +7,7 @@ module SpammableActions def mark_as_spam if SpamService.new(spammable).mark_as_spam! - redirect_to spammable, notice: "#{spammable.class.to_s} was submitted to Akismet successfully." + redirect_to spammable, notice: "#{spammable.class} was submitted to Akismet successfully." else redirect_to spammable, alert: 'Error with Akismet. Please check the logs for more info.' end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 9c323d7705a..18cd800c619 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -40,10 +40,7 @@ class Groups::GroupMembersController < Groups::ApplicationController end def destroy - @group_member = @group.members.find_by(id: params[:id]) || - @group.requesters.find_by(id: params[:id]) - - Members::DestroyService.new(@group_member, current_user).execute + Members::DestroyService.new(@group, current_user, id: params[:id]).execute(:all) respond_to do |format| format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' } diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 2343c7d20ec..f56b256984b 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -55,10 +55,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def destroy - @project_member = @project.members.find_by(id: params[:id]) || - @project.requesters.find_by(id: params[:id]) - - Members::DestroyService.new(@project_member, current_user).execute + Members::DestroyService.new(@project, current_user, params). + execute(:all) respond_to do |format| format.html do diff --git a/app/helpers/page_layout_helper.rb b/app/helpers/page_layout_helper.rb index 22387d66451..7d4d049101a 100644 --- a/app/helpers/page_layout_helper.rb +++ b/app/helpers/page_layout_helper.rb @@ -92,12 +92,8 @@ module PageLayoutHelper end end - def fluid_layout(enabled = false) - if @fluid_layout.nil? - @fluid_layout = (current_user && current_user.layout == "fluid") || enabled - else - @fluid_layout - end + def fluid_layout + current_user && current_user.layout == "fluid" end def blank_container(enabled = false) diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 56477733ea2..e667c9e4e2e 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -139,7 +139,7 @@ module ProjectsHelper end options = options_for_select(options, selected: highest_available_option || @project.project_feature.public_send(field)) - content_tag(:select, options, name: "project[project_feature_attributes][#{field.to_s}]", id: "project_project_feature_attributes_#{field.to_s}", class: "pull-right form-control", data: { field: field }).html_safe + content_tag(:select, options, name: "project[project_feature_attributes][#{field}]", id: "project_project_feature_attributes_#{field}", class: "pull-right form-control", data: { field: field }).html_safe end private diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index 1e86f648203..a9db8bb2b82 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -114,6 +114,26 @@ module TodosHelper selected_type ? selected_type[:text] : default_type end + def todo_due_date(todo) + return unless todo.target.try(:due_date) + + is_due_today = todo.target.due_date.today? + is_overdue = todo.target.overdue? + css_class = + if is_due_today + 'text-warning' + elsif is_overdue + 'text-danger' + else + '' + end + + html = "· ".html_safe + html << content_tag(:span, class: css_class) do + "Due #{is_due_today ? "today" : todo.target.due_date.to_s(:medium)}" + end + end + private def show_todo_state?(todo) diff --git a/app/mailers/devise_mailer.rb b/app/mailers/devise_mailer.rb index 415f6e12885..f7ed61625f4 100644 --- a/app/mailers/devise_mailer.rb +++ b/app/mailers/devise_mailer.rb @@ -3,4 +3,12 @@ class DeviseMailer < Devise::Mailer default reply_to: Gitlab.config.gitlab.email_reply_to layout 'devise_mailer' + + protected + + def subject_for(key) + subject = super + subject << " | #{Gitlab.config.gitlab.email_subject_suffix}" if Gitlab.config.gitlab.email_subject_suffix.present? + subject + end end diff --git a/app/mailers/emails/members.rb b/app/mailers/emails/members.rb index 45311690293..7b617b359ea 100644 --- a/app/mailers/emails/members.rb +++ b/app/mailers/emails/members.rb @@ -45,7 +45,7 @@ module Emails @token = token mail(to: member.invite_email, - subject: "Invitation to join the #{member_source.human_name} #{member_source.model_name.singular}") + subject: subject("Invitation to join the #{member_source.human_name} #{member_source.model_name.singular}")) end def member_invite_accepted_email(member_source_type, member_id) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 29f1c527776..2444702104e 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -92,6 +92,7 @@ class Notify < BaseMailer subject = "" subject << "#{@project.name} | " if @project subject << extra.join(' | ') if extra.present? + subject << " | #{Gitlab.config.gitlab.email_subject_suffix}" if Gitlab.config.gitlab.email_subject_suffix.present? subject end diff --git a/app/models/commit_range.rb b/app/models/commit_range.rb index 656a242c265..ac2477fd973 100644 --- a/app/models/commit_range.rb +++ b/app/models/commit_range.rb @@ -80,7 +80,7 @@ class CommitRange end def inspect - %(#<#{self.class}:#{object_id} #{to_s}>) + %(#<#{self.class}:#{object_id} #{self}>) end def to_s diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index ec9e0f1b1d0..eb2ff0428f6 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -43,19 +43,15 @@ module Mentionable self end - def all_references(current_user = nil, text = nil, extractor: nil) + def all_references(current_user = nil, extractor: nil) extractor ||= Gitlab::ReferenceExtractor. new(project, current_user) - if text - extractor.analyze(text, author: author) - else - self.class.mentionable_attrs.each do |attr, options| - text = __send__(attr) - options = options.merge(cache_key: [self, attr], author: author) + self.class.mentionable_attrs.each do |attr, options| + text = __send__(attr) + options = options.merge(cache_key: [self, attr], author: author) - extractor.analyze(text, options) - end + extractor.analyze(text, options) end extractor @@ -66,8 +62,8 @@ module Mentionable end # Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference. - def referenced_mentionables(current_user = self.author, text = nil) - refs = all_references(current_user, text) + def referenced_mentionables(current_user = self.author) + refs = all_references(current_user) refs = (refs.issues + refs.merge_requests + refs.commits) # We're using this method instead of Array diffing because that requires @@ -77,8 +73,8 @@ module Mentionable end # Create a cross-reference Note for each GFM reference to another Mentionable found in the +mentionable_attrs+. - def create_cross_references!(author = self.author, without = [], text = nil) - refs = referenced_mentionables(author, text) + def create_cross_references!(author = self.author, without = []) + refs = referenced_mentionables(author) # We're using this method instead of Array diffing because that requires # both of the object's `hash` values to be the same, which may not be the @@ -97,10 +93,7 @@ module Mentionable return if changes.empty? - original_text = changes.collect { |_, vals| vals.first }.join(' ') - - preexisting = referenced_mentionables(author, original_text) - create_cross_references!(author, preexisting) + create_cross_references!(author) end private diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 07d7e19e70d..82b27b78229 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -11,7 +11,7 @@ class Deployment < ActiveRecord::Base delegate :name, to: :environment, prefix: true - after_save :keep_around_commit + after_save :create_ref def commit project.commit(sha) @@ -29,8 +29,8 @@ class Deployment < ActiveRecord::Base self == environment.last_deployment end - def keep_around_commit - project.repository.keep_around(self.sha) + def create_ref + project.repository.create_ref(ref, ref_path) end def manual_actions @@ -76,4 +76,10 @@ class Deployment < ActiveRecord::Base where.not(id: self.id). take end + + private + + def ref_path + File.join(environment.ref_path, 'deployments', id.to_s) + end end diff --git a/app/models/environment.rb b/app/models/environment.rb index 49e0a20640c..f0f3ee23223 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -47,4 +47,8 @@ class Environment < ActiveRecord::Base def update_merge_request_metrics? self.name == "production" end + + def ref_path + "refs/environments/#{Shellwords.shellescape(name)}" + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a431d46cc9e..071dfe54ef9 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -523,9 +523,13 @@ class MergeRequest < ActiveRecord::Base # `MergeRequestsClosingIssues` model. This is a performance optimization. # Calculating this information for a number of merge requests requires # running `ReferenceExtractor` on each of them separately. + # This optimization does not apply to issues from external sources. def cache_merge_request_closes_issues!(current_user = self.author) + return if project.has_external_issue_tracker? + transaction do self.merge_requests_closing_issues.delete_all + closes_issues(current_user).each do |issue| self.merge_requests_closing_issues.create!(issue: issue) end diff --git a/app/models/project_services/slack_service/issue_message.rb b/app/models/project_services/slack_service/issue_message.rb index 88e053ec192..cd87a79d0c6 100644 --- a/app/models/project_services/slack_service/issue_message.rb +++ b/app/models/project_services/slack_service/issue_message.rb @@ -11,7 +11,7 @@ class SlackService attr_reader :description def initialize(params) - @user_name = params[:user][:name] + @user_name = params[:user][:username] @project_name = params[:project_name] @project_url = params[:project_url] diff --git a/app/models/project_services/slack_service/merge_message.rb b/app/models/project_services/slack_service/merge_message.rb index 11fc691022b..b7615c96068 100644 --- a/app/models/project_services/slack_service/merge_message.rb +++ b/app/models/project_services/slack_service/merge_message.rb @@ -10,7 +10,7 @@ class SlackService attr_reader :title def initialize(params) - @user_name = params[:user][:name] + @user_name = params[:user][:username] @project_name = params[:project_name] @project_url = params[:project_url] diff --git a/app/models/project_services/slack_service/note_message.rb b/app/models/project_services/slack_service/note_message.rb index 89ba51cb662..9e84e90f38c 100644 --- a/app/models/project_services/slack_service/note_message.rb +++ b/app/models/project_services/slack_service/note_message.rb @@ -10,7 +10,7 @@ class SlackService def initialize(params) params = HashWithIndifferentAccess.new(params) - @user_name = params[:user][:name] + @user_name = params[:user][:username] @project_name = params[:project_name] @project_url = params[:project_url] diff --git a/app/models/project_services/slack_service/wiki_page_message.rb b/app/models/project_services/slack_service/wiki_page_message.rb index f336d9e7691..160ca3ac115 100644 --- a/app/models/project_services/slack_service/wiki_page_message.rb +++ b/app/models/project_services/slack_service/wiki_page_message.rb @@ -9,7 +9,7 @@ class SlackService attr_reader :description def initialize(params) - @user_name = params[:user][:name] + @user_name = params[:user][:username] @project_name = params[:project_name] @project_url = params[:project_url] diff --git a/app/models/repository.rb b/app/models/repository.rb index 51557228ab9..eb574555df6 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -997,6 +997,10 @@ class Repository Gitlab::Popen.popen(args, path_to_repo) end + def create_ref(ref, ref_path) + fetch_ref(path_to_repo, ref, ref_path) + end + def update_branch_with_hooks(current_user, branch) update_autocrlf_option diff --git a/app/models/service.rb b/app/models/service.rb index 80de7175565..66c804f2b06 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -136,6 +136,7 @@ class Service < ActiveRecord::Base end def #{arg}=(value) + self.properties ||= {} updated_properties['#{arg}'] = #{arg} unless #{arg}_changed? self.properties['#{arg}'] = value end diff --git a/app/models/user.rb b/app/models/user.rb index 6996740eebd..7f5a8562907 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -827,6 +827,22 @@ class User < ActiveRecord::Base todos_pending_count(force: true) end + # This is copied from Devise::Models::Lockable#valid_for_authentication?, as our auth + # flow means we don't call that automatically (and can't conveniently do so). + # + # See: + # <https://github.com/plataformatec/devise/blob/v4.0.0/lib/devise/models/lockable.rb#L92> + # + def increment_failed_attempts! + self.failed_attempts ||= 0 + self.failed_attempts += 1 + if attempts_exceeded? + lock_access! unless access_locked? + else + save(validate: false) + end + end + private def projects_union(min_access_level = nil) diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb index ca9db59cac7..b7a244c2029 100644 --- a/app/services/members/authorized_destroy_service.rb +++ b/app/services/members/authorized_destroy_service.rb @@ -14,6 +14,8 @@ module Members if member.request? && member.user != user notification_service.decline_access_request(member) end + + member end end end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 9a2bf82ef51..431da8372c9 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -1,17 +1,42 @@ module Members class DestroyService < BaseService - attr_accessor :member, :current_user + include MembersHelper - def initialize(member, current_user) - @member = member + attr_accessor :source + + ALLOWED_SCOPES = %i[members requesters all] + + def initialize(source, current_user, params = {}) + @source = source @current_user = current_user + @params = params end - def execute - unless member && can?(current_user, "destroy_#{member.type.underscore}".to_sym, member) - raise Gitlab::Access::AccessDeniedError - end + def execute(scope = :members) + raise "scope :#{scope} is not allowed!" unless ALLOWED_SCOPES.include?(scope) + + member = find_member!(scope) + + raise Gitlab::Access::AccessDeniedError unless can_destroy_member?(member) + AuthorizedDestroyService.new(member, current_user).execute end + + private + + def find_member!(scope) + condition = params[:user_id] ? { user_id: params[:user_id] } : { id: params[:id] } + case scope + when :all + source.members.find_by(condition) || + source.requesters.find_by!(condition) + else + source.public_send(scope).find_by!(condition) + end + end + + def can_destroy_member?(member) + member && can?(current_user, action_member_permission(:destroy, member), member) + end end end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 1fb72cf89e9..a2bfa422c9d 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -72,7 +72,7 @@ class SystemHooksService return 'user_add_to_group' if event == :create return 'user_remove_from_group' if event == :destroy else - "#{model.class.name.downcase}_#{event.to_s}" + "#{model.class.name.downcase}_#{event}" end end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 5ccaa5275b7..1ce66d50368 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -246,7 +246,7 @@ module SystemNoteService 'deleted' end - body = "#{verb} #{branch_type.to_s} branch `#{branch}`".capitalize + body = "#{verb} #{branch_type} branch `#{branch}`".capitalize create_note(noteable: noteable, project: project, author: author, note: body) end @@ -347,7 +347,7 @@ module SystemNoteService notes = notes.where(noteable_id: noteable.id) end - notes_for_mentioner(mentioner, noteable, notes).count > 0 + notes_for_mentioner(mentioner, noteable, notes).exists? end # Build an Array of lines detailing each commit added in a merge request diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index e6687f43816..90798c47d97 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -63,6 +63,11 @@ Reply by email %span.light.pull-right = boolean_to_icon Gitlab::IncomingEmail.enabled? + %p + Container Registry + %span.light.pull-right + = boolean_to_icon Gitlab.config.registry.enabled + .col-md-4 %h4 Components diff --git a/app/views/dashboard/todos/_todo.html.haml b/app/views/dashboard/todos/_todo.html.haml index b40395c74de..cc077fad32a 100644 --- a/app/views/dashboard/todos/_todo.html.haml +++ b/app/views/dashboard/todos/_todo.html.haml @@ -19,6 +19,7 @@ (removed) · #{time_ago_with_tooltip(todo.created_at)} + = todo_due_date(todo) .todo-body .todo-note diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 94c53882623..237280872f1 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -1,5 +1,5 @@ %header.navbar.navbar-fixed-top.navbar-gitlab{ class: nav_header_class } - %div{ class: fluid_layout ? "container-fluid" : "container-fluid" } + %div{ class: "container-fluid" } .header-content %button.side-nav-toggle{ type: 'button', "aria-label" => "Toggle global navigation" } %span.sr-only Toggle navigation diff --git a/app/views/profiles/preferences/update.js.erb b/app/views/profiles/preferences/update.js.erb index 4433cab7782..8966dd3fd86 100644 --- a/app/views/profiles/preferences/update.js.erb +++ b/app/views/profiles/preferences/update.js.erb @@ -4,9 +4,9 @@ $('body').addClass('<%= user_application_theme %>') // Toggle container-fluid class if ('<%= current_user.layout %>' === 'fluid') { - $('.content-wrapper').find('.container-fluid').removeClass('container-limited') + $('.content-wrapper .container-fluid').removeClass('container-limited') } else { - $('.content-wrapper').find('.container-fluid').addClass('container-limited') + $('.content-wrapper .container-fluid').addClass('container-limited') } // Re-enable the "Save" button diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 62aff36aadd..576e7ef021a 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -1,7 +1,5 @@ - show_whitespace_toggle = local_assigns.fetch(:show_whitespace_toggle, true) - diff_files = diffs.diff_files -- if diff_view == :parallel - - fluid_layout true .content-block.oneline-block.files-changed .inline-parallel-buttons diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index d03ff9ec7e8..9f34ca9ff4e 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -4,9 +4,6 @@ - content_for :page_specific_javascripts do = page_specific_javascript_tag('diff_notes/diff_notes_bundle.js') -- if diff_view == :parallel - - fluid_layout true - .merge-request{'data-url' => merge_request_path(@merge_request)} = render "projects/merge_requests/show/mr_title" diff --git a/app/views/projects/merge_requests/widget/_heading.html.haml b/app/views/projects/merge_requests/widget/_heading.html.haml index 44e645a7e81..b5f5e11d4c3 100644 --- a/app/views/projects/merge_requests/widget/_heading.html.haml +++ b/app/views/projects/merge_requests/widget/_heading.html.haml @@ -4,14 +4,15 @@ .ci_widget{ class: "ci-#{status}", style: ("display:none" unless @pipeline.status == status) } = ci_icon_for_status(status) %span - CI build + Pipeline + = link_to "##{@pipeline.id}", namespace_project_pipeline_path(@pipeline.project.namespace, @pipeline.project, @pipeline.id), class: 'pipeline' = ci_label_for_status(status) for - commit = @merge_request.diff_head_commit = succeed "." do = link_to @pipeline.short_sha, namespace_project_commit_path(@merge_request.source_project.namespace, @merge_request.source_project, @pipeline.sha), class: "monospace" %span.ci-coverage - = link_to "View details", builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: "js-show-tab", data: {action: 'builds'} + = link_to "View details", pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: "js-show-tab", data: {action: 'pipelines'} - elsif @merge_request.has_ci? - # Compatibility with old CI integrations (ex jenkins) when you request status from CI server via AJAX diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 1470a6e2550..a79356923b2 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -70,6 +70,7 @@ production: &base email_from: example@example.com email_display_name: GitLab email_reply_to: noreply@example.com + email_subject_suffix: '' # Email server smtp settings are in config/initializers/smtp_settings.rb.sample diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 195108b921b..c5ed2162c92 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -186,6 +186,7 @@ Settings.gitlab['email_enabled'] ||= true if Settings.gitlab['email_enabled'].ni Settings.gitlab['email_from'] ||= ENV['GITLAB_EMAIL_FROM'] || "gitlab@#{Settings.gitlab.host}" Settings.gitlab['email_display_name'] ||= ENV['GITLAB_EMAIL_DISPLAY_NAME'] || 'GitLab' Settings.gitlab['email_reply_to'] ||= ENV['GITLAB_EMAIL_REPLY_TO'] || "noreply@#{Settings.gitlab.host}" +Settings.gitlab['email_subject_suffix'] ||= ENV['GITLAB_EMAIL_SUBJECT_SUFFIX'] || "" Settings.gitlab['base_url'] ||= Settings.send(:build_base_gitlab_url) Settings.gitlab['url'] ||= Settings.send(:build_gitlab_url) Settings.gitlab['user'] ||= 'git' diff --git a/doc/administration/environment_variables.md b/doc/administration/environment_variables.md index 7f53915a4d7..b4a953d1ccc 100644 --- a/doc/administration/environment_variables.md +++ b/doc/administration/environment_variables.md @@ -13,15 +13,17 @@ override certain values. Variable | Type | Description -------- | ---- | ----------- -`GITLAB_ROOT_PASSWORD` | string | Sets the password for the `root` user on installation -`GITLAB_HOST` | string | The full URL of the GitLab server (including `http://` or `https://`) -`RAILS_ENV` | string | The Rails environment; can be one of `production`, `development`, `staging` or `test` -`DATABASE_URL` | string | The database URL; is of the form: `postgresql://localhost/blog_development` -`GITLAB_EMAIL_FROM` | string | The e-mail address used in the "From" field in e-mails sent by GitLab -`GITLAB_EMAIL_DISPLAY_NAME` | string | The name used in the "From" field in e-mails sent by GitLab -`GITLAB_EMAIL_REPLY_TO` | string | The e-mail address used in the "Reply-To" field in e-mails sent by GitLab -`GITLAB_UNICORN_MEMORY_MIN` | integer | The minimum memory threshold (in bytes) for the Unicorn worker killer -`GITLAB_UNICORN_MEMORY_MAX` | integer | The maximum memory threshold (in bytes) for the Unicorn worker killer +`GITLAB_ROOT_PASSWORD` | string | Sets the password for the `root` user on installation +`GITLAB_HOST` | string | The full URL of the GitLab server (including `http://` or `https://`) +`RAILS_ENV` | string | The Rails environment; can be one of `production`, `development`, `staging` or `test` +`DATABASE_URL` | string | The database URL; is of the form: `postgresql://localhost/blog_development` +`GITLAB_EMAIL_FROM` | string | The e-mail address used in the "From" field in e-mails sent by GitLab +`GITLAB_EMAIL_DISPLAY_NAME` | string | The name used in the "From" field in e-mails sent by GitLab +`GITLAB_EMAIL_REPLY_TO` | string | The e-mail address used in the "Reply-To" field in e-mails sent by GitLab +`GITLAB_EMAIL_REPLY_TO` | string | The e-mail address used in the "Reply-To" field in e-mails sent by GitLab +`GITLAB_EMAIL_SUBJECT_SUFFIX` | string | The e-mail subject suffix used in e-mails sent by GitLab +`GITLAB_UNICORN_MEMORY_MIN` | integer | The minimum memory threshold (in bytes) for the Unicorn worker killer +`GITLAB_UNICORN_MEMORY_MAX` | integer | The maximum memory threshold (in bytes) for the Unicorn worker killer ## Complete database variables diff --git a/doc/ci/environments.md b/doc/ci/environments.md index d85b8a34ced..e070302fb82 100644 --- a/doc/ci/environments.md +++ b/doc/ci/environments.md @@ -14,6 +14,19 @@ Defining environments in a project's `.gitlab-ci.yml` lets developers track Deployments are created when [jobs] deploy versions of code to [environments]. +### Checkout deployments locally + +Since 8.13, a reference in the git repository is saved for each deployment. So +knowing what the state is of your current environments is only a `git fetch` +away. + +In your git config, append the `[remote "<your-remote>"]` block with an extra +fetch line: + +``` +fetch = +refs/environments/*:refs/remotes/origin/environments/* +``` + ## Defining environments You can create and delete environments manually in the web interface, but we diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb index 7b9de7c9598..d3db7740830 100644 --- a/lib/api/access_requests.rb +++ b/lib/api/access_requests.rb @@ -75,9 +75,8 @@ module API required_attributes! [:user_id] source = find_source(source_type, params[:id]) - access_requester = source.requesters.find_by!(user_id: params[:user_id]) - - ::Members::DestroyService.new(access_requester, current_user).execute + ::Members::DestroyService.new(source, current_user, params). + execute(:requesters) end end end diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb index 2461a783ea8..e9ccba3b465 100644 --- a/lib/api/award_emoji.rb +++ b/lib/api/award_emoji.rb @@ -8,16 +8,19 @@ module API awardable_string = awardable_type.pluralize awardable_id_string = "#{awardable_type}_id" + params do + requires :id, type: String, desc: 'The ID of a project' + requires :"#{awardable_id_string}", type: Integer, desc: "The ID of an Issue, Merge Request or Snippet" + end + [ ":id/#{awardable_string}/:#{awardable_id_string}/award_emoji", ":id/#{awardable_string}/:#{awardable_id_string}/notes/:note_id/award_emoji" ].each do |endpoint| - # Get a list of project +awardable+ award emoji - # - # Parameters: - # id (required) - The ID of a project - # awardable_id (required) - The ID of an issue or MR - # Example Request: - # GET /projects/:id/issues/:awardable_id/award_emoji + + desc 'Get a list of project +awardable+ award emoji' do + detail 'This feature was introduced in 8.9' + success Entities::AwardEmoji + end get endpoint do if can_read_awardable? awards = paginate(awardable.award_emoji) @@ -27,14 +30,13 @@ module API end end - # Get a specific award emoji - # - # Parameters: - # id (required) - The ID of a project - # awardable_id (required) - The ID of an issue or MR - # award_id (required) - The ID of the award - # Example Request: - # GET /projects/:id/issues/:awardable_id/award_emoji/:award_id + desc 'Get a specific award emoji' do + detail 'This feature was introduced in 8.9' + success Entities::AwardEmoji + end + params do + requires :award_id, type: Integer, desc: 'The ID of the award' + end get "#{endpoint}/:award_id" do if can_read_awardable? present awardable.award_emoji.find(params[:award_id]), with: Entities::AwardEmoji @@ -43,17 +45,14 @@ module API end end - # Award a new Emoji - # - # Parameters: - # id (required) - The ID of a project - # awardable_id (required) - The ID of an issue or mr - # name (required) - The name of a award_emoji (without colons) - # Example Request: - # POST /projects/:id/issues/:awardable_id/award_emoji + desc 'Award a new Emoji' do + detail 'This feature was introduced in 8.9' + success Entities::AwardEmoji + end + params do + requires :name, type: String, desc: 'The name of a award_emoji (without colons)' + end post endpoint do - required_attributes! [:name] - not_found!('Award Emoji') unless can_read_awardable? && can_award_awardable? award = awardable.create_award_emoji(params[:name], current_user) @@ -65,14 +64,13 @@ module API end end - # Delete a +awardables+ award emoji - # - # Parameters: - # id (required) - The ID of a project - # awardable_id (required) - The ID of an issue or MR - # award_emoji_id (required) - The ID of an award emoji - # Example Request: - # DELETE /projects/:id/issues/:issue_id/notes/:note_id/award_emoji/:award_id + desc 'Delete a +awardables+ award emoji' do + detail 'This feature was introduced in 8.9' + success Entities::AwardEmoji + end + params do + requires :award_id, type: Integer, desc: 'The ID of an award emoji' + end delete "#{endpoint}/:award_id" do award = awardable.award_emoji.find(params[:award_id]) diff --git a/lib/api/members.rb b/lib/api/members.rb index a18ce769e29..34df55fe192 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -134,7 +134,7 @@ module API if member.nil? { message: "Access revoked", id: params[:user_id].to_i } else - ::Members::DestroyService.new(member, current_user).execute + ::Members::DestroyService.new(source, current_user, params).execute present member.user, with: Entities::Member, member: member end diff --git a/lib/api/namespaces.rb b/lib/api/namespaces.rb index 50d3729449e..fe981d7b9fa 100644 --- a/lib/api/namespaces.rb +++ b/lib/api/namespaces.rb @@ -4,20 +4,18 @@ module API before { authenticate! } resource :namespaces do - # Get a namespaces list - # - # Example Request: - # GET /namespaces + desc 'Get a namespaces list' do + success Entities::Namespace + end + params do + optional :search, type: String, desc: "Search query for namespaces" + end get do - @namespaces = if current_user.admin - Namespace.all - else - current_user.namespaces - end - @namespaces = @namespaces.search(params[:search]) if params[:search].present? - @namespaces = paginate @namespaces + namespaces = current_user.admin ? Namespace.all : current_user.namespaces + + namespaces = namespaces.search(params[:search]) if params[:search].present? - present @namespaces, with: Entities::Namespace + present paginate(namespaces), with: Entities::Namespace end end end diff --git a/lib/banzai/filter/task_list_filter.rb b/lib/banzai/filter/task_list_filter.rb index 4efbcaf5c7f..9fa5f589f3e 100644 --- a/lib/banzai/filter/task_list_filter.rb +++ b/lib/banzai/filter/task_list_filter.rb @@ -2,29 +2,7 @@ require 'task_list/filter' module Banzai module Filter - # Work around a bug in the default TaskList::Filter that adds a `task-list` - # class to every list element, regardless of whether or not it contains a - # task list. - # - # This is a (hopefully) temporary fix, pending a new release of the - # task_list gem. - # - # See https://github.com/github/task_list/pull/60 - module ClassNamesFilter - def add_css_class(node, *new_class_names) - if new_class_names.include?('task-list') - # Don't add class to all lists - return - elsif new_class_names.include?('task-list-item') - super(node.parent, 'task-list') - end - - super(node, *new_class_names) - end - end - class TaskListFilter < TaskList::Filter - prepend ClassNamesFilter end end end diff --git a/lib/gitlab/sidekiq_middleware/arguments_logger.rb b/lib/gitlab/sidekiq_middleware/arguments_logger.rb index 7813091ec7b..82a59a7a87e 100644 --- a/lib/gitlab/sidekiq_middleware/arguments_logger.rb +++ b/lib/gitlab/sidekiq_middleware/arguments_logger.rb @@ -2,7 +2,7 @@ module Gitlab module SidekiqMiddleware class ArgumentsLogger def call(worker, job, queue) - Sidekiq.logger.info "arguments: #{job['args']}" + Sidekiq.logger.info "arguments: #{JSON.dump(job['args'])}" yield end end diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 92b97bf3d0c..a0870891cf4 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -87,10 +87,10 @@ describe Groups::GroupMembersController do context 'when member is not found' do before { sign_in(user) } - it 'returns 403' do + it 'returns 404' do delete :leave, group_id: group - expect(response).to have_http_status(403) + expect(response).to have_http_status(404) end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 5e2a8cf3849..074f85157de 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -135,11 +135,11 @@ describe Projects::ProjectMembersController do context 'when member is not found' do before { sign_in(user) } - it 'returns 403' do + it 'returns 404' do delete :leave, namespace_id: project.namespace, project_id: project - expect(response).to have_http_status(403) + expect(response).to have_http_status(404) end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 8f27e616c3e..48d69377461 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -109,6 +109,44 @@ describe SessionsController do end end + context 'when the user is on their last attempt' do + before do + user.update(failed_attempts: User.maximum_attempts.pred) + end + + context 'when OTP is valid' do + it 'authenticates correctly' do + authenticate_2fa(otp_attempt: user.current_otp) + + expect(subject.current_user).to eq user + end + end + + context 'when OTP is invalid' do + before { authenticate_2fa(otp_attempt: 'invalid') } + + it 'does not authenticate' do + expect(subject.current_user).not_to eq user + end + + it 'warns about invalid login' do + expect(response).to set_flash.now[:alert] + .to /Invalid Login or password/ + end + + it 'locks the user' do + expect(user.reload).to be_access_locked + end + + it 'keeps the user locked on future login attempts' do + post(:create, user: { login: user.username, password: user.password }) + + expect(response) + .to set_flash.now[:alert].to /Invalid Login or password/ + end + end + end + context 'when another user does not have 2FA enabled' do let(:another_user) { create(:user) } diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb index f76c4fe8b57..cd79c4f512d 100644 --- a/spec/features/projects/issuable_templates_spec.rb +++ b/spec/features/projects/issuable_templates_spec.rb @@ -26,7 +26,7 @@ feature 'issuable templates', feature: true, js: true do scenario 'user selects "bug" template' do select_template 'bug' wait_for_ajax - preview_template + preview_template(template_content) save_changes end @@ -42,6 +42,26 @@ feature 'issuable templates', feature: true, js: true do end end + context 'user creates an issue using templates, with a prior description' do + let(:prior_description) { 'test issue description' } + let(:template_content) { 'this is a test "bug" template' } + let(:issue) { create(:issue, author: user, assignee: user, project: project) } + + background do + project.repository.commit_file(user, '.gitlab/issue_templates/bug.md', template_content, 'added issue template', 'master', false) + visit edit_namespace_project_issue_path project.namespace, project, issue + fill_in :'issue[title]', with: 'test issue title' + fill_in :'issue[description]', with: prior_description + end + + scenario 'user selects "bug" template' do + select_template 'bug' + wait_for_ajax + preview_template("#{prior_description}\n\n#{template_content}") + save_changes + end + end + context 'user creates a merge request using templates' do let(:template_content) { 'this is a test "feature-proposal" template' } let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) } @@ -55,7 +75,7 @@ feature 'issuable templates', feature: true, js: true do scenario 'user selects "feature-proposal" template' do select_template 'feature-proposal' wait_for_ajax - preview_template + preview_template(template_content) save_changes end end @@ -82,16 +102,16 @@ feature 'issuable templates', feature: true, js: true do scenario 'user selects template' do select_template 'feature-proposal' wait_for_ajax - preview_template + preview_template(template_content) save_changes end end end end - def preview_template + def preview_template(expected_content) click_link 'Preview' - expect(page).to have_content template_content + expect(page).to have_content expected_content end def save_changes diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index fc555a74f30..bf93c1d1251 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -4,7 +4,7 @@ describe 'Dashboard Todos', feature: true do let(:user) { create(:user) } let(:author) { create(:user) } let(:project) { create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } - let(:issue) { create(:issue) } + let(:issue) { create(:issue, due_date: Date.today) } describe 'GET /dashboard/todos' do context 'User does not have todos' do @@ -28,6 +28,12 @@ describe 'Dashboard Todos', feature: true do expect(page).to have_selector('.todos-list .todo', count: 1) end + it 'shows due date as today' do + page.within first('.todo') do + expect(page).to have_content 'Due today' + end + end + describe 'deleting the todo' do before do first('.done-todo').click diff --git a/spec/finders/access_requests_finder_spec.rb b/spec/finders/access_requests_finder_spec.rb index 626513200e4..8cfea9659cb 100644 --- a/spec/finders/access_requests_finder_spec.rb +++ b/spec/finders/access_requests_finder_spec.rb @@ -16,7 +16,7 @@ describe AccessRequestsFinder, services: true do access_requesters = described_class.new(source).public_send(method_name, user) expect(access_requesters.size).to eq(1) - expect(access_requesters.first).to be_a "#{source.class.to_s}Member".constantize + expect(access_requesters.first).to be_a "#{source.class}Member".constantize expect(access_requesters.first.user).to eq(access_requester) end end diff --git a/spec/lib/banzai/filter/task_list_filter_spec.rb b/spec/lib/banzai/filter/task_list_filter_spec.rb deleted file mode 100644 index 569cbc885c7..00000000000 --- a/spec/lib/banzai/filter/task_list_filter_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'spec_helper' - -describe Banzai::Filter::TaskListFilter, lib: true do - include FilterSpecHelper - - it 'does not apply `task-list` class to non-task lists' do - exp = act = %(<ul><li>Item</li></ul>) - expect(filter(act).to_html).to eq exp - end - - it 'applies `task-list` to single-item task lists' do - act = filter('<ul><li>[ ] Task 1</li></ul>') - - expect(act.to_html).to start_with '<ul class="task-list">' - end -end diff --git a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb index 2ba344092ce..2e19d590d83 100644 --- a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb @@ -27,7 +27,7 @@ describe 'Import/Export attribute configuration', lib: true do relation_names.each do |relation_name| relation_class = relation_class_for_name(relation_name) - expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class.to_s} to exist in safe_model_attributes" + expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class} to exist in safe_model_attributes" current_attributes = parsed_attributes(relation_name, relation_class.attribute_names) safe_attributes = safe_model_attributes[relation_class.to_s] diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index cd8578b6f49..0e4130e8a3a 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -831,6 +831,7 @@ describe Notify do let(:user) { create(:user, email: 'old-email@mail.com') } before do + stub_config_setting(email_subject_suffix: 'A Nice Suffix') perform_enqueued_jobs do user.email = "new-email@mail.com" user.save @@ -847,7 +848,7 @@ describe Notify do end it 'has the correct subject' do - is_expected.to have_subject "Confirmation instructions" + is_expected.to have_subject /^Confirmation instructions/ end it 'includes a link to the site' do diff --git a/spec/mailers/shared/notify.rb b/spec/mailers/shared/notify.rb index 5c9851f14c7..3956d05060b 100644 --- a/spec/mailers/shared/notify.rb +++ b/spec/mailers/shared/notify.rb @@ -37,6 +37,16 @@ shared_examples 'an email sent from GitLab' 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$/ + end + end end shared_examples 'an email that contains a header with author username' do diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 549b0042038..132858950d5 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -1,18 +1,27 @@ require 'spec_helper' describe Mentionable do - include Mentionable + class Example + include Mentionable - def author - nil + attr_accessor :project, :message + attr_mentionable :message + + def author + nil + end end describe 'references' do let(:project) { create(:project) } + let(:mentionable) { Example.new } it 'excludes JIRA references' do allow(project).to receive_messages(jira_tracker?: true) - expect(referenced_mentionables(project, 'JIRA-123')).to be_empty + + mentionable.project = project + mentionable.message = 'JIRA-123' + expect(mentionable.referenced_mentionables).to be_empty end end end @@ -39,9 +48,8 @@ describe Issue, "Mentionable" do let(:user) { create(:user) } def referenced_issues(current_user) - text = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}" - - issue.referenced_mentionables(current_user, text) + issue.title = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}" + issue.referenced_mentionables(current_user) end context 'when the current user can see the issue' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 9d7be2429ed..38b6da50168 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -86,6 +86,30 @@ describe MergeRequest, models: true do end end + describe '#cache_merge_request_closes_issues!' do + before do + subject.project.team << [subject.author, :developer] + subject.target_branch = subject.project.default_branch + end + + it 'caches closed issues' do + issue = create :issue, project: subject.project + commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") + allow(subject).to receive(:commits).and_return([commit]) + + expect { subject.cache_merge_request_closes_issues! }.to change(subject.merge_requests_closing_issues, :count).by(1) + end + + it 'does not cache issues from external trackers' do + subject.project.update_attribute(:has_external_issue_tracker, true) + issue = ExternalIssue.new('JIRA-123', subject.project) + commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") + allow(subject).to receive(:commits).and_return([commit]) + + expect { subject.cache_merge_request_closes_issues! }.not_to change(subject.merge_requests_closing_issues, :count) + end + end + describe '#source_branch_sha' do let(:last_branch_commit) { subject.source_project.repository.commit(subject.source_branch) } @@ -522,7 +546,7 @@ describe MergeRequest, models: true do end it_behaves_like 'an editable mentionable' do - subject { create(:merge_request) } + subject { create(:merge_request, :simple) } let(:backref_text) { "merge request #{subject.to_reference}" } let(:set_mentionable_text) { ->(txt){ subject.description = txt } } diff --git a/spec/models/project_services/slack_service/issue_message_spec.rb b/spec/models/project_services/slack_service/issue_message_spec.rb index 0f8889bdf3c..98c36ec088d 100644 --- a/spec/models/project_services/slack_service/issue_message_spec.rb +++ b/spec/models/project_services/slack_service/issue_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::IssueMessage, models: true do { user: { name: 'Test User', - username: 'Test User' + username: 'test.user' }, project_name: 'project_name', project_url: 'somewhere.com', @@ -40,7 +40,7 @@ describe SlackService::IssueMessage, models: true do context 'open' do it 'returns a message regarding opening of issues' do expect(subject.pretext).to eq( - '<somewhere.com|[project_name>] Issue opened by Test User') + '<somewhere.com|[project_name>] Issue opened by test.user') expect(subject.attachments).to eq([ { title: "#100 Issue title", @@ -60,7 +60,7 @@ describe SlackService::IssueMessage, models: true do it 'returns a message regarding closing of issues' do expect(subject.pretext). to eq( - '<somewhere.com|[project_name>] Issue <url|#100 Issue title> closed by Test User') + '<somewhere.com|[project_name>] Issue <url|#100 Issue title> closed by test.user') expect(subject.attachments).to be_empty end end diff --git a/spec/models/project_services/slack_service/merge_message_spec.rb b/spec/models/project_services/slack_service/merge_message_spec.rb index 224c7ceabe8..c5c052d9af1 100644 --- a/spec/models/project_services/slack_service/merge_message_spec.rb +++ b/spec/models/project_services/slack_service/merge_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::MergeMessage, models: true do { user: { name: 'Test User', - username: 'Test User' + username: 'test.user' }, project_name: 'project_name', project_url: 'somewhere.com', @@ -31,7 +31,7 @@ describe SlackService::MergeMessage, models: true do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'Test User opened <somewhere.com/merge_requests/100|merge request !100> '\ + 'test.user opened <somewhere.com/merge_requests/100|merge request !100> '\ 'in <somewhere.com|project_name>: *Issue title*') expect(subject.attachments).to be_empty end @@ -43,7 +43,7 @@ describe SlackService::MergeMessage, models: true do end it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'Test User closed <somewhere.com/merge_requests/100|merge request !100> '\ + 'test.user closed <somewhere.com/merge_requests/100|merge request !100> '\ 'in <somewhere.com|project_name>: *Issue title*') expect(subject.attachments).to be_empty end diff --git a/spec/models/project_services/slack_service/note_message_spec.rb b/spec/models/project_services/slack_service/note_message_spec.rb index 41b93f08050..38cfe4ad3e3 100644 --- a/spec/models/project_services/slack_service/note_message_spec.rb +++ b/spec/models/project_services/slack_service/note_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::NoteMessage, models: true do @args = { user: { name: 'Test User', - username: 'username', + username: 'test.user', avatar_url: 'http://fakeavatar' }, project_name: 'project_name', @@ -37,7 +37,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on commits' do message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("Test User commented on " \ + expect(message.pretext).to eq("test.user commented on " \ "<url|commit 5f163b2b> in <somewhere.com|project_name>: " \ "*Added a commit message*") expected_attachments = [ @@ -63,7 +63,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on a merge request' do message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("Test User commented on " \ + expect(message.pretext).to eq("test.user commented on " \ "<url|merge request !30> in <somewhere.com|project_name>: " \ "*merge request title*") expected_attachments = [ @@ -90,7 +90,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on an issue' do message = SlackService::NoteMessage.new(@args) expect(message.pretext).to eq( - "Test User commented on " \ + "test.user commented on " \ "<url|issue #20> in <somewhere.com|project_name>: " \ "*issue title*") expected_attachments = [ @@ -115,7 +115,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on a project snippet' do message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("Test User commented on " \ + expect(message.pretext).to eq("test.user commented on " \ "<url|snippet #5> in <somewhere.com|project_name>: " \ "*snippet title*") expected_attachments = [ diff --git a/spec/models/project_services/slack_service/push_message_spec.rb b/spec/models/project_services/slack_service/push_message_spec.rb index cda9ee670b0..17cd05e24f1 100644 --- a/spec/models/project_services/slack_service/push_message_spec.rb +++ b/spec/models/project_services/slack_service/push_message_spec.rb @@ -9,7 +9,7 @@ describe SlackService::PushMessage, models: true do before: 'before', project_name: 'project_name', ref: 'refs/heads/master', - user_name: 'user_name', + user_name: 'test.user', project_url: 'url' } end @@ -26,7 +26,7 @@ describe SlackService::PushMessage, models: true do it 'returns a message regarding pushes' do expect(subject.pretext).to eq( - 'user_name pushed to branch <url/commits/master|master> of '\ + 'test.user pushed to branch <url/commits/master|master> of '\ '<url|project_name> (<url/compare/before...after|Compare changes>)' ) expect(subject.attachments).to eq([ @@ -46,13 +46,13 @@ describe SlackService::PushMessage, models: true do before: Gitlab::Git::BLANK_SHA, project_name: 'project_name', ref: 'refs/tags/new_tag', - user_name: 'user_name', + user_name: 'test.user', project_url: 'url' } end it 'returns a message regarding pushes' do - expect(subject.pretext).to eq('user_name pushed new tag ' \ + expect(subject.pretext).to eq('test.user pushed new tag ' \ '<url/commits/new_tag|new_tag> to ' \ '<url|project_name>') expect(subject.attachments).to be_empty @@ -66,7 +66,7 @@ describe SlackService::PushMessage, models: true do it 'returns a message regarding a new branch' do expect(subject.pretext).to eq( - 'user_name pushed new branch <url/commits/master|master> to '\ + 'test.user pushed new branch <url/commits/master|master> to '\ '<url|project_name>' ) expect(subject.attachments).to be_empty @@ -80,7 +80,7 @@ describe SlackService::PushMessage, models: true do it 'returns a message regarding a removed branch' do expect(subject.pretext).to eq( - 'user_name removed branch master from <url|project_name>' + 'test.user removed branch master from <url|project_name>' ) expect(subject.attachments).to be_empty end diff --git a/spec/models/project_services/slack_service/wiki_page_message_spec.rb b/spec/models/project_services/slack_service/wiki_page_message_spec.rb index 13aea0b0600..093911598b0 100644 --- a/spec/models/project_services/slack_service/wiki_page_message_spec.rb +++ b/spec/models/project_services/slack_service/wiki_page_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::WikiPageMessage, models: true do { user: { name: 'Test User', - username: 'Test User' + username: 'test.user' }, project_name: 'project_name', project_url: 'somewhere.com', @@ -25,7 +25,7 @@ describe SlackService::WikiPageMessage, models: true do it 'returns a message that a new wiki page was created' do expect(subject.pretext).to eq( - 'Test User created <url|wiki page> in <somewhere.com|project_name>: '\ + 'test.user created <url|wiki page> in <somewhere.com|project_name>: '\ '*Wiki page title*') end end @@ -35,7 +35,7 @@ describe SlackService::WikiPageMessage, models: true do it 'returns a message that a wiki page was updated' do expect(subject.pretext).to eq( - 'Test User edited <url|wiki page> in <somewhere.com|project_name>: '\ + 'test.user edited <url|wiki page> in <somewhere.com|project_name>: '\ '*Wiki page title*') end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index db29f4d353b..98c64c079b9 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -320,6 +320,16 @@ describe Repository, models: true do end end + describe '#create_ref' do + it 'redirects the call to fetch_ref' do + ref, ref_path = '1', '2' + + expect(repository).to receive(:fetch_ref).with(repository.path_to_repo, ref, ref_path) + + repository.create_ref(ref, ref_path) + end + end + describe "#changelog" do before do repository.send(:cache).expire(:changelog) diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 05056a4bb47..ed1bc9271ae 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -203,6 +203,23 @@ describe Service, models: true do end end + describe 'initialize service with no properties' do + let(:service) do + GitlabIssueTrackerService.create( + project: create(:project), + title: 'random title' + ) + end + + it 'does not raise error' do + expect { service }.not_to raise_error + end + + it 'creates the properties' do + expect(service.properties).to eq({ "title" => "random title" }) + end + end + describe "callbacks" do let(:project) { create(:project) } let!(:service) do diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb index 905a7311372..b467890a403 100644 --- a/spec/requests/api/access_requests_spec.rb +++ b/spec/requests/api/access_requests_spec.rb @@ -195,7 +195,7 @@ describe API::AccessRequests, api: true do end context 'when authenticated as the access requester' do - it 'returns 200' do + it 'deletes the access requester' do expect do delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", access_requester) @@ -205,7 +205,7 @@ describe API::AccessRequests, api: true do end context 'when authenticated as a master/owner' do - it 'returns 200' do + it 'deletes the access requester' do expect do delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", master) @@ -213,6 +213,16 @@ describe API::AccessRequests, api: true do end.to change { source.requesters.count }.by(-1) end + context 'user_id matches a member, not an access requester' do + it 'returns 404' do + expect do + delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{developer.id}", master) + + expect(response).to have_http_status(404) + end.not_to change { source.requesters.count } + end + end + context 'user_id does not match an existing access requester' do it 'returns 404' do expect do diff --git a/spec/services/members/approve_access_request_service_spec.rb b/spec/services/members/approve_access_request_service_spec.rb index 6fca80b5613..03e296259f9 100644 --- a/spec/services/members/approve_access_request_service_spec.rb +++ b/spec/services/members/approve_access_request_service_spec.rb @@ -26,7 +26,7 @@ describe Members::ApproveAccessRequestService, services: true do it 'returns a <Source>Member' do member = described_class.new(source, user, params).execute - expect(member).to be_a "#{source.class.to_s}Member".constantize + expect(member).to be_a "#{source.class}Member".constantize expect(member.requested_at).to be_nil end diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 2395445e7fd..9995f3488af 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -2,70 +2,111 @@ require 'spec_helper' describe Members::DestroyService, services: true do let(:user) { create(:user) } - let(:project) { create(:project) } - let!(:member) { create(:project_member, source: project) } + let(:member_user) { create(:user) } + let(:project) { create(:project, :public) } + let(:group) { create(:group, :public) } - context 'when member is nil' do - before do - project.team << [user, :developer] + shared_examples 'a service raising ActiveRecord::RecordNotFound' do + it 'raises ActiveRecord::RecordNotFound' do + expect { described_class.new(source, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) end + end - it 'does not destroy the member' do - expect { destroy_member(nil, user) }.to raise_error(Gitlab::Access::AccessDeniedError) + shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do + it 'raises Gitlab::Access::AccessDeniedError' do + expect { described_class.new(source, user, params).execute }.to raise_error(Gitlab::Access::AccessDeniedError) end end - context 'when current user cannot destroy the given member' do - before do - project.team << [user, :developer] + shared_examples 'a service destroying a member' do + it 'destroys the member' do + expect { described_class.new(source, user, params).execute }.to change { source.members.count }.by(-1) + end + + context 'when the given member is an access requester' do + before do + source.members.find_by(user_id: member_user).destroy + source.request_access(member_user) + end + let(:access_requester) { source.requesters.find_by(user_id: member_user) } + + it_behaves_like 'a service raising ActiveRecord::RecordNotFound' + + %i[requesters all].each do |scope| + context "and #{scope} scope is passed" do + it 'destroys the access requester' do + expect { described_class.new(source, user, params).execute(scope) }.to change { source.requesters.count }.by(-1) + end + + it 'calls Member#after_decline_request' do + expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(access_requester) + + described_class.new(source, user, params).execute(scope) + end + + context 'when current user is the member' do + it 'does not call Member#after_decline_request' do + expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(access_requester) + + described_class.new(source, member_user, params).execute(scope) + end + end + end + end end + end + + context 'when no member are found' do + let(:params) { { user_id: 42 } } - it 'does not destroy the member' do - expect { destroy_member(member, user) }.to raise_error(Gitlab::Access::AccessDeniedError) + it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do + let(:source) { project } + end + + it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do + let(:source) { group } end end - context 'when current user can destroy the given member' do + context 'when a member is found' do before do - project.team << [user, :master] + project.team << [member_user, :developer] + group.add_developer(member_user) end + let(:params) { { user_id: member_user.id } } - it 'destroys the member' do - destroy_member(member, user) + context 'when current user cannot destroy the given member' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { project } + end - expect(member).to be_destroyed + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { group } + end end - context 'when the given member is a requester' do + context 'when current user can destroy the given member' do before do - member.update_column(:requested_at, Time.now) + project.team << [user, :master] + group.add_owner(user) end - it 'calls Member#after_decline_request' do - expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(member) - - destroy_member(member, user) + it_behaves_like 'a service destroying a member' do + let(:source) { project } end - context 'when current user is the member' do - it 'does not call Member#after_decline_request' do - expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member) - - destroy_member(member, member.user) - end + it_behaves_like 'a service destroying a member' do + let(:source) { group } end - context 'when current user is the member and ' do - it 'does not call Member#after_decline_request' do - expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member) + context 'when given a :id' do + let(:params) { { id: project.members.find_by!(user_id: user.id).id } } - destroy_member(member, member.user) + it 'destroys the member' do + expect { described_class.new(project, user, params).execute }. + to change { project.members.count }.by(-1) end end end end - - def destroy_member(member, user) - Members::DestroyService.new(member, user).execute - end end diff --git a/spec/services/members/request_access_service_spec.rb b/spec/services/members/request_access_service_spec.rb index dff5b4917ae..0d2d5f03199 100644 --- a/spec/services/members/request_access_service_spec.rb +++ b/spec/services/members/request_access_service_spec.rb @@ -19,7 +19,7 @@ describe Members::RequestAccessService, services: true do it 'returns a <Source>Member' do member = described_class.new(source, user).execute - expect(member).to be_a "#{source.class.to_s}Member".constantize + expect(member).to be_a "#{source.class}Member".constantize expect(member.requested_at).to be_present end end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 31167675d07..e49a0d5e553 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -38,6 +38,42 @@ describe MergeRequests::MergeService, services: true do end end + context 'closes related issues' do + let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } + + before do + allow(project).to receive(:default_branch).and_return(merge_request.target_branch) + end + + it 'closes GitLab issue tracker issues' do + issue = create :issue, project: project + commit = double('commit', safe_message: "Fixes #{issue.to_reference}") + allow(merge_request).to receive(:commits).and_return([commit]) + + service.execute(merge_request) + + expect(issue.reload.closed?).to be_truthy + end + + context 'with JIRA integration' do + include JiraServiceHelper + + let(:jira_tracker) { project.create_jira_service } + + before { jira_service_settings } + + it 'closes issues on JIRA issue tracker' do + jira_issue = ExternalIssue.new('JIRA-123', project) + commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}") + allow(merge_request).to receive(:commits).and_return([commit]) + + expect_any_instance_of(JiraService).to receive(:close_issue).with(merge_request, jira_issue).once + + service.execute(merge_request) + end + end + end + context 'closes related todos' do let(:merge_request) { create(:merge_request, assignee: user, author: user) } let(:project) { merge_request.project } diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index e876d44c166..f57c82809a6 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -9,7 +9,7 @@ shared_context 'mentionable context' do let(:author) { subject.author } let(:mentioned_issue) { create(:issue, project: project) } - let!(:mentioned_mr) { create(:merge_request, :simple, source_project: project) } + let!(:mentioned_mr) { create(:merge_request, source_project: project) } let(:mentioned_commit) { project.commit("HEAD~1") } let(:ext_proj) { create(:project, :public) } @@ -100,6 +100,7 @@ shared_examples 'an editable mentionable' do it 'creates new cross-reference notes when the mentionable text is edited' do subject.save + subject.create_cross_references! new_text = <<-MSG.strip_heredoc These references already existed: @@ -131,6 +132,7 @@ shared_examples 'an editable mentionable' do end # These two issues are new and should receive reference notes + # In the case of MergeRequests remember that cannot mention commits included in the MergeRequest new_issues.each do |newref| expect(SystemNoteService).to receive(:cross_reference). with(newref, subject.local_reference, author) |