From fbdaf0e2a517660c0e4e3960f20b2d3568c33e78 Mon Sep 17 00:00:00 2001 From: Jeroen van Baarsen Date: Thu, 28 May 2015 11:06:30 +0200 Subject: [PATCH 01/71] Update noteable after a new note is added **What does this do?** It makes sure that whenever a new note is added to an noteable item, the updated_at of that item is also updated. **Why is this needed?** At this moment when you post a comment on an issue or add a label to an issue, the updated_at is not changed. Because of this the filtering for least recently updated is not really useful (since it only takes in account the original text from the noteable). Signed-off-by: Jeroen van Baarsen --- app/models/note.rb | 2 +- spec/lib/gitlab/note_data_builder_spec.rb | 4 ++++ spec/models/note_spec.rb | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/note.rb b/app/models/note.rb index d5f716b3de..6a74d62b71 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -31,7 +31,7 @@ class Note < ActiveRecord::Base participant :author, :mentioned_users belongs_to :project - belongs_to :noteable, polymorphic: true + belongs_to :noteable, polymorphic: true, touch: true belongs_to :author, class_name: "User" delegate :name, to: :project, prefix: true diff --git a/spec/lib/gitlab/note_data_builder_spec.rb b/spec/lib/gitlab/note_data_builder_spec.rb index 448cd0c688..5826144e66 100644 --- a/spec/lib/gitlab/note_data_builder_spec.rb +++ b/spec/lib/gitlab/note_data_builder_spec.rb @@ -36,6 +36,7 @@ describe 'Gitlab::NoteDataBuilder' do let(:note) { create(:note_on_issue, noteable_id: issue.id) } it 'returns the note and issue-specific data' do + data[:issue]["updated_at"] = fixed_time expect(data).to have_key(:issue) expect(data[:issue]).to eq(issue.hook_attrs) end @@ -46,6 +47,7 @@ describe 'Gitlab::NoteDataBuilder' do let(:note) { create(:note_on_merge_request, noteable_id: merge_request.id) } it 'returns the note and merge request data' do + data[:merge_request]["updated_at"] = fixed_time expect(data).to have_key(:merge_request) expect(data[:merge_request]).to eq(merge_request.hook_attrs) end @@ -56,6 +58,7 @@ describe 'Gitlab::NoteDataBuilder' do let(:note) { create(:note_on_merge_request_diff, noteable_id: merge_request.id) } it 'returns the note and merge request diff data' do + data[:merge_request]["updated_at"] = fixed_time expect(data).to have_key(:merge_request) expect(data[:merge_request]).to eq(merge_request.hook_attrs) end @@ -66,6 +69,7 @@ describe 'Gitlab::NoteDataBuilder' do let!(:note) { create(:note_on_project_snippet, noteable_id: snippet.id) } it 'returns the note and project snippet data' do + data[:snippet]["updated_at"] = fixed_time expect(data).to have_key(:snippet) expect(data[:snippet]).to eq(snippet.hook_attrs) end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index ddacba5826..9037992bb0 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -22,7 +22,7 @@ require 'spec_helper' describe Note do describe 'associations' do it { is_expected.to belong_to(:project) } - it { is_expected.to belong_to(:noteable) } + it { is_expected.to belong_to(:noteable).touch(true) } it { is_expected.to belong_to(:author).class_name('User') } end From 7f3eb42f4ec90315062e5ba08a0f48e5a21ec360 Mon Sep 17 00:00:00 2001 From: Daniel Gerhardt Date: Thu, 4 Jun 2015 21:08:35 +0200 Subject: [PATCH 02/71] Fix external issue tracker hook/test for HTTPS URLs If HTTPS was used for 'project_url', an error was raised because a HTTP connection was established to the default HTTPS port. The code has been corrected and simplified by using HTTParty. Additionally, the request now is made directly to the 'project_url' instead of the extracted root path. --- CHANGELOG | 1 + .../project_services/issue_tracker_service.rb | 15 +++++---------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index fe6b0bcee9..f63b36c246 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.12.0 (unreleased) + - Fix external issue tracker hook/test for HTTPS URLs (Daniel Gerhardt) - Don't notify users mentioned in code blocks or blockquotes. - Omit link to generate labels if user does not have access to create them (Stan Hu) - Disable changing of the source branch in merge request update API (Stan Hu) diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index c8ab9d63b7..936e574ccc 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -81,18 +81,13 @@ class IssueTrackerService < Service result = false begin - url = URI.parse(self.project_url) + response = HTTParty.head(self.project_url, verify: true) - if url.host && url.port - http = Net::HTTP.start(url.host, url.port, { open_timeout: 5, read_timeout: 5 }) - response = http.head("/") - - if response - message = "#{self.type} received response #{response.code} when attempting to connect to #{self.project_url}" - result = true - end + if response + message = "#{self.type} received response #{response.code} when attempting to connect to #{self.project_url}" + result = true end - rescue Timeout::Error, SocketError, Errno::ECONNRESET, Errno::ECONNREFUSED => error + rescue HTTParty::Error, Timeout::Error, SocketError, Errno::ECONNRESET, Errno::ECONNREFUSED => error message = "#{self.type} had an error when trying to connect to #{self.project_url}: #{error.message}" end Rails.logger.info(message) From 84a414fe534ebb60c8e7396c245486be521e2a11 Mon Sep 17 00:00:00 2001 From: Eric Maziade Date: Fri, 5 Jun 2015 11:50:37 -0400 Subject: [PATCH 03/71] Add session expiration delay configuration through UI application settings --- CHANGELOG | 3 ++- app/controllers/admin/application_settings_controller.rb | 1 + app/models/application_setting.rb | 2 ++ app/views/admin/application_settings/_form.html.haml | 4 ++++ config/initializers/1_settings.rb | 1 + config/initializers/session_store.rb | 2 +- ...21_add_session_expire_seconds_for_application_settings.rb | 5 +++++ db/schema.rb | 3 ++- lib/gitlab/current_settings.rb | 3 ++- spec/models/application_setting_spec.rb | 1 + 10 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20150604202921_add_session_expire_seconds_for_application_settings.rb diff --git a/CHANGELOG b/CHANGELOG index 1fd938a34c..77deb92f3b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.12.0 (unreleased) + - Add session expiration delay configuration through UI application settings - Don't notify users mentioned in code blocks or blockquotes. - Disable changing of the source branch in merge request update API (Stan Hu) - Shorten merge request WIP text. @@ -1497,4 +1498,4 @@ v 0.8.0 - stability - security fixes - increased test coverage - - email notification + - email notification \ No newline at end of file diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index a01e2a907d..2601867cf0 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -40,6 +40,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :home_page_url, :after_sign_out_path, :max_attachment_size, + :session_expire_seconds, :default_project_visibility, :default_snippet_visibility, :restricted_signup_domains_raw, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 80463ee884..ce06e022c3 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -15,6 +15,7 @@ # twitter_sharing_enabled :boolean default(TRUE) # restricted_visibility_levels :text # max_attachment_size :integer default(10), not null +# session_expire_seconds :integer default(604800), not null # default_project_visibility :integer # default_snippet_visibility :integer # restricted_signup_domains :text @@ -61,6 +62,7 @@ class ApplicationSetting < ActiveRecord::Base sign_in_text: Settings.extra['sign_in_text'], restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], max_attachment_size: Settings.gitlab['max_attachment_size'], + session_expire_seconds: Settings.gitlab['session_expire_seconds'], default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], restricted_signup_domains: Settings.gitlab['restricted_signup_domains'] diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 188a08940a..9de29e50d1 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -83,6 +83,10 @@ = f.label :max_attachment_size, 'Maximum attachment size (MB)', class: 'control-label col-sm-2' .col-sm-10 = f.number_field :max_attachment_size, class: 'form-control' + .form-group + = f.label :session_expire_seconds, 'Session duration (seconds)', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :session_expire_seconds, class: 'form-control' .form-group = f.label :restricted_signup_domains, 'Restricted domains for sign-ups', class: 'control-label col-sm-2' .col-sm-10 diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index c234bd69e9..9b39dff046 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -128,6 +128,7 @@ Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e Settings.gitlab['default_projects_features'] ||= {} Settings.gitlab['webhook_timeout'] ||= 10 Settings.gitlab['max_attachment_size'] ||= 10 +Settings.gitlab['session_expire_seconds'] ||= 604800 Settings.gitlab.default_projects_features['issues'] = true if Settings.gitlab.default_projects_features['issues'].nil? Settings.gitlab.default_projects_features['merge_requests'] = true if Settings.gitlab.default_projects_features['merge_requests'].nil? Settings.gitlab.default_projects_features['wiki'] = true if Settings.gitlab.default_projects_features['wiki'].nil? diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index b2d59f1c4b..1603f7561c 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -6,6 +6,6 @@ Gitlab::Application.config.session_store( key: '_gitlab_session', secure: Gitlab.config.gitlab.https, httponly: true, - expire_after: 1.week, + expire_after: ActiveRecord::Base.connected? && ActiveRecord::Base.connection.table_exists?('application_settings') ? ApplicationSetting.current.session_expire_seconds : Settings.gitlab['session_expire_seconds'], path: (Rails.application.config.relative_url_root.nil?) ? '/' : Rails.application.config.relative_url_root ) diff --git a/db/migrate/20150604202921_add_session_expire_seconds_for_application_settings.rb b/db/migrate/20150604202921_add_session_expire_seconds_for_application_settings.rb new file mode 100644 index 0000000000..8096efc686 --- /dev/null +++ b/db/migrate/20150604202921_add_session_expire_seconds_for_application_settings.rb @@ -0,0 +1,5 @@ +class AddSessionExpireSecondsForApplicationSettings < ActiveRecord::Migration + def change + add_column :application_settings, :session_expire_seconds, :integer, default: 604800, null: false + end +end \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index aea0742cf3..d2ad55d7a9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150529150354) do +ActiveRecord::Schema.define(version: 20150604202921) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -35,6 +35,7 @@ ActiveRecord::Schema.define(version: 20150529150354) do t.text "restricted_signup_domains" t.boolean "user_oauth_applications", default: true t.string "after_sign_out_path" + t.integer "session_expire_seconds", default: 604800, null: false end create_table "broadcast_messages", force: true do |t| diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index d8f696d247..56bb073642 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -21,7 +21,8 @@ module Gitlab gravatar_enabled: Settings.gravatar['enabled'], sign_in_text: Settings.extra['sign_in_text'], restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], - max_attachment_size: Settings.gitlab['max_attachment_size'] + max_attachment_size: Settings.gitlab['max_attachment_size'], + session_expire_seconds: Settings.gitlab['session_expire_seconds'] ) end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 116c318121..f4e1c65b63 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -15,6 +15,7 @@ # twitter_sharing_enabled :boolean default(TRUE) # restricted_visibility_levels :text # max_attachment_size :integer default(10), not null +# session_expire_seconds :integer default(604800), not null # default_project_visibility :integer # default_snippet_visibility :integer # restricted_signup_domains :text From 1d080f57454fda46eb60700a8693cb968e6d557f Mon Sep 17 00:00:00 2001 From: themaze75 Date: Fri, 5 Jun 2015 17:16:32 +0000 Subject: [PATCH 04/71] session_expire_seconds => session_expire_delay delay is in seconds more legible code in session_store Added `GitLab restart required` help block to session_expire_delay --- app/controllers/admin/application_settings_controller.rb | 2 +- app/models/application_setting.rb | 8 ++++++-- app/views/admin/application_settings/_form.html.haml | 5 +++-- config/initializers/1_settings.rb | 2 +- config/initializers/session_store.rb | 8 ++++++-- ...add_session_expire_seconds_for_application_settings.rb | 5 ----- ...1_add_session_expire_delay_for_application_settings.rb | 5 +++++ db/schema.rb | 2 +- lib/gitlab/current_settings.rb | 2 +- spec/models/application_setting_spec.rb | 2 +- 10 files changed, 25 insertions(+), 16 deletions(-) delete mode 100644 db/migrate/20150604202921_add_session_expire_seconds_for_application_settings.rb create mode 100644 db/migrate/20150609141121_add_session_expire_delay_for_application_settings.rb diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 2601867cf0..c7c643db40 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -40,7 +40,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :home_page_url, :after_sign_out_path, :max_attachment_size, - :session_expire_seconds, + :session_expire_delay, :default_project_visibility, :default_snippet_visibility, :restricted_signup_domains_raw, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index ce06e022c3..29f8fac470 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -15,7 +15,7 @@ # twitter_sharing_enabled :boolean default(TRUE) # restricted_visibility_levels :text # max_attachment_size :integer default(10), not null -# session_expire_seconds :integer default(604800), not null +# session_expire_delay :integer default(10080), not null # default_project_visibility :integer # default_snippet_visibility :integer # restricted_signup_domains :text @@ -27,6 +27,10 @@ class ApplicationSetting < ActiveRecord::Base serialize :restricted_visibility_levels serialize :restricted_signup_domains, Array attr_accessor :restricted_signup_domains_raw + + validates :session_expire_delay, + presence: true, + numericality: { only_integer: true, greater_than_or_equal_to: 0 } validates :home_page_url, allow_blank: true, @@ -62,7 +66,7 @@ class ApplicationSetting < ActiveRecord::Base sign_in_text: Settings.extra['sign_in_text'], restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], max_attachment_size: Settings.gitlab['max_attachment_size'], - session_expire_seconds: Settings.gitlab['session_expire_seconds'], + session_expire_delay: Settings.gitlab['session_expire_delay'], default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], restricted_signup_domains: Settings.gitlab['restricted_signup_domains'] diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 9de29e50d1..d5a49fc41f 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -84,9 +84,10 @@ .col-sm-10 = f.number_field :max_attachment_size, class: 'form-control' .form-group - = f.label :session_expire_seconds, 'Session duration (seconds)', class: 'control-label col-sm-2' + = f.label :session_expire_delay, 'Session duration (minutes)', class: 'control-label col-sm-2' .col-sm-10 - = f.number_field :session_expire_seconds, class: 'form-control' + = f.number_field :session_expire_delay, class: 'form-control' + %span.help-block#session_expire_delay_help_block GitLab restart is required to apply changes .form-group = f.label :restricted_signup_domains, 'Restricted domains for sign-ups', class: 'control-label col-sm-2' .col-sm-10 diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 9b39dff046..f050a7ea1a 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -128,7 +128,7 @@ Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e Settings.gitlab['default_projects_features'] ||= {} Settings.gitlab['webhook_timeout'] ||= 10 Settings.gitlab['max_attachment_size'] ||= 10 -Settings.gitlab['session_expire_seconds'] ||= 604800 +Settings.gitlab['session_expire_delay'] ||= 10080 Settings.gitlab.default_projects_features['issues'] = true if Settings.gitlab.default_projects_features['issues'].nil? Settings.gitlab.default_projects_features['merge_requests'] = true if Settings.gitlab.default_projects_features['merge_requests'].nil? Settings.gitlab.default_projects_features['wiki'] = true if Settings.gitlab.default_projects_features['wiki'].nil? diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index 1603f7561c..43077fb575 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -1,11 +1,15 @@ # Be sure to restart your server when you modify this file. +if ActiveRecord::Base.connection.active? && ActiveRecord::Base.connection.table_exists?('application_settings') + Settings.gitlab['session_expire_delay'] = ApplicationSetting.current.session_expire_delay +end + Gitlab::Application.config.session_store( :redis_store, # Using the cookie_store would enable session replay attacks. servers: Gitlab::Application.config.cache_store[1].merge(namespace: 'session:gitlab'), # re-use the Redis config from the Rails cache store key: '_gitlab_session', secure: Gitlab.config.gitlab.https, httponly: true, - expire_after: ActiveRecord::Base.connected? && ActiveRecord::Base.connection.table_exists?('application_settings') ? ApplicationSetting.current.session_expire_seconds : Settings.gitlab['session_expire_seconds'], + expire_after: Settings.gitlab['session_expire_delay'] * 60, path: (Rails.application.config.relative_url_root.nil?) ? '/' : Rails.application.config.relative_url_root -) +) \ No newline at end of file diff --git a/db/migrate/20150604202921_add_session_expire_seconds_for_application_settings.rb b/db/migrate/20150604202921_add_session_expire_seconds_for_application_settings.rb deleted file mode 100644 index 8096efc686..0000000000 --- a/db/migrate/20150604202921_add_session_expire_seconds_for_application_settings.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddSessionExpireSecondsForApplicationSettings < ActiveRecord::Migration - def change - add_column :application_settings, :session_expire_seconds, :integer, default: 604800, null: false - end -end \ No newline at end of file diff --git a/db/migrate/20150609141121_add_session_expire_delay_for_application_settings.rb b/db/migrate/20150609141121_add_session_expire_delay_for_application_settings.rb new file mode 100644 index 0000000000..ffa22e6d5e --- /dev/null +++ b/db/migrate/20150609141121_add_session_expire_delay_for_application_settings.rb @@ -0,0 +1,5 @@ +class AddSessionExpireDelayForApplicationSettings < ActiveRecord::Migration + def change + add_column :application_settings, :session_expire_delay, :integer, default: 10080, null: false + end +end \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index d2ad55d7a9..04f887274d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -35,7 +35,7 @@ ActiveRecord::Schema.define(version: 20150604202921) do t.text "restricted_signup_domains" t.boolean "user_oauth_applications", default: true t.string "after_sign_out_path" - t.integer "session_expire_seconds", default: 604800, null: false + t.integer "session_expire_delay", default: 10080, null: false end create_table "broadcast_messages", force: true do |t| diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 56bb073642..931d51c55d 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -22,7 +22,7 @@ module Gitlab sign_in_text: Settings.extra['sign_in_text'], restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], max_attachment_size: Settings.gitlab['max_attachment_size'], - session_expire_seconds: Settings.gitlab['session_expire_seconds'] + session_expire_delay: Settings.gitlab['session_expire_delay'] ) end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index f4e1c65b63..d648f4078b 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -15,7 +15,7 @@ # twitter_sharing_enabled :boolean default(TRUE) # restricted_visibility_levels :text # max_attachment_size :integer default(10), not null -# session_expire_seconds :integer default(604800), not null +# session_expire_delay :integer default(10080), not null # default_project_visibility :integer # default_snippet_visibility :integer # restricted_signup_domains :text From c2087098bedc690f52b40c66fc73dcae6b3ea935 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Fri, 12 Jun 2015 05:15:11 +0200 Subject: [PATCH 05/71] Fix typo on account page. --- app/views/profiles/accounts/show.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index a26d4e0c75..ed009c8656 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -113,4 +113,4 @@ Your account is currently an owner in these groups: %strong #{@user.solo_owned_groups.map(&:name).join(', ')} %p - You must transfer ownership or delete these groups before you can delete yur account. + You must transfer ownership or delete these groups before you can delete your account. From d26ae2914992f5c655815f658ab885b8a4060475 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 12 Jun 2015 14:24:22 +0200 Subject: [PATCH 06/71] Install git 2.4.3 Git 2.1.2 does not protect against pushes trying the '.Git/config' attack (CVE-2014-9390). Going to 2.4.3 is perhaps a big jump but why not use the latest? (Famous last words.) --- doc/install/installation.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/install/installation.md b/doc/install/installation.md index badea4de21..9815375ade 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -88,8 +88,8 @@ Is the system packaged Git too old? Remove it and compile from source. # Download and compile from source cd /tmp - curl -L --progress https://www.kernel.org/pub/software/scm/git/git-2.1.2.tar.gz | tar xz - cd git-2.1.2/ + curl -L --progress https://www.kernel.org/pub/software/scm/git/git-2.4.3.tar.gz | tar xz + cd git-2.4.3/ ./configure make prefix=/usr/local all From f04134becd64893cad240ec2b81fe39e7d4cd912 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 12 Jun 2015 14:29:18 +0200 Subject: [PATCH 07/71] New source installs should use 7-12-stable Even though the 7-12-stable branch does not exist yet. --- doc/install/installation.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/install/installation.md b/doc/install/installation.md index 9815375ade..ff0361c5e5 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -195,9 +195,9 @@ We recommend using a PostgreSQL database. For MySQL check [MySQL setup guide](da ### Clone the Source # Clone GitLab repository - sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 7-11-stable gitlab + sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 7-12-stable gitlab -**Note:** You can change `7-11-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server! +**Note:** You can change `7-12-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server! ### Configure It From 6ec4da5d01aa41f51a2fb6c7ecffa44f78993582 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 12 Jun 2015 15:24:11 +0200 Subject: [PATCH 08/71] Add 7.12 update guides --- ...r-7.x-to-7.11.md => 6.x-or-7.x-to-7.12.md} | 25 ++-- doc/update/7.11-to-7.12.md | 130 ++++++++++++++++++ 2 files changed, 145 insertions(+), 10 deletions(-) rename doc/update/{6.x-or-7.x-to-7.11.md => 6.x-or-7.x-to-7.12.md} (94%) create mode 100644 doc/update/7.11-to-7.12.md diff --git a/doc/update/6.x-or-7.x-to-7.11.md b/doc/update/6.x-or-7.x-to-7.12.md similarity index 94% rename from doc/update/6.x-or-7.x-to-7.11.md rename to doc/update/6.x-or-7.x-to-7.12.md index b1daa648f1..5705fb360d 100644 --- a/doc/update/6.x-or-7.x-to-7.11.md +++ b/doc/update/6.x-or-7.x-to-7.12.md @@ -1,7 +1,7 @@ -# From 6.x or 7.x to 7.11 -*Make sure you view this [upgrade guide from the `master` branch](../../../master/doc/update/6.x-or-7.x-to-7.11.md) for the most up to date instructions.* +# From 6.x or 7.x to 7.12 +*Make sure you view this [upgrade guide from the `master` branch](../../../master/doc/update/6.x-or-7.x-to-7.12.md) for the most up to date instructions.* -This allows you to upgrade any version of GitLab from 6.0 and up (including 7.0 and up) to 7.11. +This allows you to upgrade any version of GitLab from 6.0 and up (including 7.0 and up) to 7.12. ## Global issue numbers @@ -71,7 +71,7 @@ sudo -u git -H git checkout -- db/schema.rb # local changes will be restored aut For GitLab Community Edition: ```bash -sudo -u git -H git checkout 7-11-stable +sudo -u git -H git checkout 7-12-stable ``` OR @@ -79,7 +79,7 @@ OR For GitLab Enterprise Edition: ```bash -sudo -u git -H git checkout 7-11-stable-ee +sudo -u git -H git checkout 7-12-stable-ee ``` ## 4. Install additional packages @@ -162,11 +162,11 @@ sudo cp lib/support/init.d/gitlab /etc/init.d/gitlab TIP: to see what changed in `gitlab.yml.example` in this release use next command: ``` -git diff 6-0-stable:config/gitlab.yml.example 7-11-stable:config/gitlab.yml.example +git diff 6-0-stable:config/gitlab.yml.example 7-12-stable:config/gitlab.yml.example ``` -* Make `/home/git/gitlab/config/gitlab.yml` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-11-stable/config/gitlab.yml.example but with your settings. -* Make `/home/git/gitlab/config/unicorn.rb` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-11-stable/config/unicorn.rb.example but with your settings. +* Make `/home/git/gitlab/config/gitlab.yml` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-12-stable/config/gitlab.yml.example but with your settings. +* Make `/home/git/gitlab/config/unicorn.rb` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-12-stable/config/unicorn.rb.example but with your settings. * Make `/home/git/gitlab-shell/config.yml` the same as https://gitlab.com/gitlab-org/gitlab-shell/blob/v2.6.0/config.yml.example but with your settings. * Copy rack attack middleware config @@ -182,10 +182,15 @@ sudo cp lib/support/logrotate/gitlab /etc/logrotate.d/gitlab ### Change Nginx settings -* HTTP setups: Make `/etc/nginx/sites-available/gitlab` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-11-stable/lib/support/nginx/gitlab but with your settings. -* HTTPS setups: Make `/etc/nginx/sites-available/gitlab-ssl` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-11-stable/lib/support/nginx/gitlab-ssl but with your settings. +* HTTP setups: Make `/etc/nginx/sites-available/gitlab` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-12-stable/lib/support/nginx/gitlab but with your settings. +* HTTPS setups: Make `/etc/nginx/sites-available/gitlab-ssl` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-12-stable/lib/support/nginx/gitlab-ssl but with your settings. * A new `location /uploads/` section has been added that needs to have the same content as the existing `location @gitlab` section. +### Check the version of /usr/local/bin/git + +If you installed Git from source into /usr/local/bin/git then please [check +your version](7.11-to-7.12.md). + ## 9. Start application sudo service gitlab start diff --git a/doc/update/7.11-to-7.12.md b/doc/update/7.11-to-7.12.md new file mode 100644 index 0000000000..bf0f7743e3 --- /dev/null +++ b/doc/update/7.11-to-7.12.md @@ -0,0 +1,130 @@ +# From 7.11 to 7.12 + +### 0. Double-check your Git version + +**This notice applies only to /usr/local/bin/git** + +If you compiled Git from source on your GitLab server then please double-check +that you are using a version that protects against CVE-2014-9390. For six +months after this vulnerability became known the GitLab installation guide +still contained instructions that would install an outdated, 'vulnerable' Git +version. + +Run the following command to get your current Git version. + +``` +/usr/local/bin/git --version +``` + +If you see 'No such file or directory' then you did not install Git according +to the outdated instructions from the GitLab installation guide and you can go +to the next step 'Stop server' below. + +If you see a version string then it should be v1.8.5.6, v1.9.5, v2.0.5, v2.1.4, +v2.2.1 or newer. If you followed the GitLab installation guide to the letter +then you probably have v2.1.2. You can use the [instructions in the GitLab +source installation +guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/installation.md#1-packages-dependencies) +to install a newer version of Git. + +### 1. Stop server + + sudo service gitlab stop + +### 2. Backup + +```bash +cd /home/git/gitlab +sudo -u git -H bundle exec rake gitlab:backup:create RAILS_ENV=production +``` + +### 3. Get latest code + +```bash +sudo -u git -H git fetch --all +sudo -u git -H git checkout -- db/schema.rb # local changes will be restored automatically +``` + +For GitLab Community Edition: + +```bash +sudo -u git -H git checkout 7-12-stable +``` + +OR + +For GitLab Enterprise Edition: + +```bash +sudo -u git -H git checkout 7-12-stable-ee +``` + +### 4. Update gitlab-shell + +```bash +cd /home/git/gitlab-shell +sudo -u git -H git fetch +sudo -u git -H git checkout v2.6.3 +``` + +### 5. Install libs, migrations, etc. + +```bash +cd /home/git/gitlab + +# MySQL installations (note: the line below states '--without ... postgres') +sudo -u git -H bundle install --without development test postgres --deployment + +# PostgreSQL installations (note: the line below states '--without ... mysql') +sudo -u git -H bundle install --without development test mysql --deployment + +# Run database migrations +sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production + +# Clean up assets and cache +sudo -u git -H bundle exec rake assets:clean assets:precompile cache:clear RAILS_ENV=production + +# Update init.d script +sudo cp lib/support/init.d/gitlab /etc/init.d/gitlab +``` + +### 6. Update config files + +#### New configuration options for `gitlab.yml` + +There are new configuration options available for [`gitlab.yml`](config/gitlab.yml.example). View them with the command below and apply them to your current `gitlab.yml`. + +``` +git diff origin/7-11-stable:config/gitlab.yml.example origin/7-12-stable:config/gitlab.yml.example +`````` + +### 7. Start application + + sudo service gitlab start + sudo service nginx restart + +### 8. Check application status + +Check if GitLab and its environment are configured correctly: + + sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production + +To make sure you didn't miss anything run a more thorough check with: + + sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production + +If all items are green, then congratulations, the upgrade is complete! + +## Things went south? Revert to previous version (7.11) + +### 1. Revert the code to the previous version +Follow the [upgrade guide from 7.10 to 7.11](7.10-to-7.11.md), except for the database migration +(The backup is already migrated to the previous version) + +### 2. Restore from the backup: + +```bash +cd /home/git/gitlab +sudo -u git -H bundle exec rake gitlab:backup:restore RAILS_ENV=production +``` +If you have more than one backup *.tar file(s) please add `BACKUP=timestamp_of_backup` to the command above. From 5019a7f3141b412579292b3c6efd7238d9bda73f Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 12 Jun 2015 15:32:43 +0200 Subject: [PATCH 09/71] Move mention of git 2.1.2 to intro paragraph --- doc/update/7.11-to-7.12.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/doc/update/7.11-to-7.12.md b/doc/update/7.11-to-7.12.md index bf0f7743e3..cc14a13592 100644 --- a/doc/update/7.11-to-7.12.md +++ b/doc/update/7.11-to-7.12.md @@ -7,8 +7,8 @@ If you compiled Git from source on your GitLab server then please double-check that you are using a version that protects against CVE-2014-9390. For six months after this vulnerability became known the GitLab installation guide -still contained instructions that would install an outdated, 'vulnerable' Git -version. +still contained instructions that would install the outdated, 'vulnerable' Git +version 2.1.2. Run the following command to get your current Git version. @@ -21,9 +21,8 @@ to the outdated instructions from the GitLab installation guide and you can go to the next step 'Stop server' below. If you see a version string then it should be v1.8.5.6, v1.9.5, v2.0.5, v2.1.4, -v2.2.1 or newer. If you followed the GitLab installation guide to the letter -then you probably have v2.1.2. You can use the [instructions in the GitLab -source installation +v2.2.1 or newer. You can use the [instructions in the GitLab source +installation guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/installation.md#1-packages-dependencies) to install a newer version of Git. From d45112258e066da307975c17c5ca19ea9a17222b Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 11 Jun 2015 22:39:50 -0700 Subject: [PATCH 10/71] Fix post-receive errors on a push when an external issue tracker is configured Closes #1700 Closes #1720 --- CHANGELOG | 1 + app/services/git_push_service.rb | 26 +++++++++++++--------- app/services/issues/close_service.rb | 2 +- spec/services/git_push_service_spec.rb | 9 ++++++++ spec/services/issues/close_service_spec.rb | 10 +++++++++ 5 files changed, 37 insertions(+), 11 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 9d558b15ab..94e3248c88 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.12.0 (unreleased) + - Fix post-receive errors on a push when an external issue tracker is configured (Stan Hu) - Update oauth button logos for Twitter and Google to recommended assets - Update browser gem to version 0.8.0 for IE11 support (Stan Hu) - Fix timeout when rendering file with thousands of lines. diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index cde65349d5..68d3b915fc 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -88,18 +88,24 @@ class GitPushService end end - # Create cross-reference notes for any other references. Omit any issues that were referenced in an - # issue-closing phrase, or have already been mentioned from this commit (probably from this commit - # being pushed to a different branch). - refs = commit.references(project, user) - issues_to_close - refs.reject! { |r| commit.has_mentioned?(r) } + if project.default_issues_tracker? + create_cross_reference_notes(commit, issues_to_close) + end + end + end - if refs.present? - author ||= commit_user(commit) + def create_cross_reference_notes(commit, issues_to_close) + # Create cross-reference notes for any other references. Omit any issues that were referenced in an + # issue-closing phrase, or have already been mentioned from this commit (probably from this commit + # being pushed to a different branch). + refs = commit.references(project, user) - issues_to_close + refs.reject! { |r| commit.has_mentioned?(r) } - refs.each do |r| - Note.create_cross_reference_note(r, commit, author) - end + if refs.present? + author ||= commit_user(commit) + + refs.each do |r| + Note.create_cross_reference_note(r, commit, author) end end end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 138465859c..3d85f97b7e 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -1,7 +1,7 @@ module Issues class CloseService < Issues::BaseService def execute(issue, commit = nil) - if issue.close + if project.default_issues_tracker? && issue.close event_service.close_issue(issue, current_user) create_note(issue, commit) notification_service.close_issue(issue, current_user) diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index e7558f2876..d0941fa2e0 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -233,6 +233,15 @@ describe GitPushService do expect(Issue.find(issue.id)).to be_opened end + + it "doesn't close issues when external issue tracker is in use" do + allow(project).to receive(:default_issues_tracker?).and_return(false) + + # The push still shouldn't create cross-reference notes. + expect { + service.execute(project, user, @oldrev, @newrev, 'refs/heads/hurf') + }.not_to change { Note.where(project_id: project.id, system: true).count } + end end describe "empty project" do diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 0e5ae724bf..db547ce0d5 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -31,5 +31,15 @@ describe Issues::CloseService do expect(note.note).to include "Status changed to closed" end end + + context "external issue tracker" do + before do + allow(project).to receive(:default_issues_tracker?).and_return(false) + @issue = Issues::CloseService.new(project, user, {}).execute(issue) + end + + it { expect(@issue).to be_valid } + it { expect(@issue).to be_opened } + end end end From 313438b327b49b5055772368c141617e06602b5b Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 12 Jun 2015 16:17:04 +0200 Subject: [PATCH 11/71] Add info about regex anchors to shell command docs. --- doc/development/shell_commands.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/doc/development/shell_commands.md b/doc/development/shell_commands.md index 821027f43f..2d1d0fb415 100644 --- a/doc/development/shell_commands.md +++ b/doc/development/shell_commands.md @@ -177,3 +177,33 @@ File.open(full_path) do # Etc. ``` A check like this could have avoided CVE-2013-4583. + +## Properly anchor regular expressions to the start and end of strings + +When using regular expressions to validate user input that is passed as an argument to a shell command, make sure to use the `\A` and `\z` anchors that designate the start and end of the string, rather than `^` and `$`, or no anchors at all. + +If you don't, an attacker could use this to execute commands with potentially harmful effect. + +For example, when a project's `import_url` is validated like below, the user could trick GitLab into cloning from a Git repository on the local filesystem. + +```ruby +validates :import_url, format: { with: URI.regexp(%w(ssh git http https)) } +# URI.regexp(%w(ssh git http https)) roughly evaluates to /(ssh|git|http|https):(something_that_looks_like_a_url)/ +``` + +Suppose the user submits the following as their import URL: + +``` +file://git:/tmp/lol +``` + +Since there are no anchors in the used regular expression, the `git:/tmp/lol` in the value would match, and the validation would pass. + +When importing, GitLab would execute the following command, passing the `import_url` as an argument: + + +```sh +git clone file://git:/tmp/lol +``` + +Git would simply ignore the `git:` part, interpret the path as `file:///tmp/lol` and import the repository into the new project, in turn potentially giving the attacker access to any repository in the system, whether private or not. From 0a03b9b717f76e6f54a39aa7f08fc9c91c3000a9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 12 Jun 2015 18:15:08 +0200 Subject: [PATCH 12/71] Remove visibility icon from projects list on dashboard and group page Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG | 3 +++ app/assets/stylesheets/pages/dashboard.scss | 4 ---- app/views/shared/_project.html.haml | 2 -- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 9d558b15ab..baa2880d36 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,8 @@ Please view this file on the master branch, on stable branches it's out of date. +v 7.13.0 (unreleased) + - Remove project visibility icons from dashboard projects list + v 7.12.0 (unreleased) - Update oauth button logos for Twitter and Google to recommended assets - Update browser gem to version 0.8.0 for IE11 support (Stan Hu) diff --git a/app/assets/stylesheets/pages/dashboard.scss b/app/assets/stylesheets/pages/dashboard.scss index 09e8d57a10..9a3b543ad1 100644 --- a/app/assets/stylesheets/pages/dashboard.scss +++ b/app/assets/stylesheets/pages/dashboard.scss @@ -28,10 +28,6 @@ font-size: 14px; line-height: 24px; - .str-truncated { - max-width: 76%; - } - a { display: block; padding: 8px 15px; diff --git a/app/views/shared/_project.html.haml b/app/views/shared/_project.html.haml index 4537f8eec8..6bd61455d2 100644 --- a/app/views/shared/_project.html.haml +++ b/app/views/shared/_project.html.haml @@ -3,8 +3,6 @@ - if avatar .dash-project-avatar = project_icon(project, alt: '', class: 'avatar project-avatar s40') - .dash-project-access-icon - = visibility_level_icon(project.visibility_level) %span.str-truncated %span.namespace-name - if project.namespace From 567a25b63032a82fd188177ea7a29a92ca2dc381 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 13 Jun 2015 00:22:55 -0400 Subject: [PATCH 13/71] Ensure `session_expire_delay` field exists before accessing it Closes #1798 --- app/models/application_setting.rb | 8 ++++---- config/initializers/session_store.rb | 8 ++++---- db/schema.rb | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 29f8fac470..fee5269409 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -27,10 +27,10 @@ class ApplicationSetting < ActiveRecord::Base serialize :restricted_visibility_levels serialize :restricted_signup_domains, Array attr_accessor :restricted_signup_domains_raw - - validates :session_expire_delay, - presence: true, - numericality: { only_integer: true, greater_than_or_equal_to: 0 } + + validates :session_expire_delay, + presence: true, + numericality: { only_integer: true, greater_than_or_equal_to: 0 } validates :home_page_url, allow_blank: true, diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index 43077fb575..6d274cd95a 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -1,8 +1,8 @@ # Be sure to restart your server when you modify this file. -if ActiveRecord::Base.connection.active? && ActiveRecord::Base.connection.table_exists?('application_settings') - Settings.gitlab['session_expire_delay'] = ApplicationSetting.current.session_expire_delay -end +require 'gitlab/current_settings' +include Gitlab::CurrentSettings +Settings.gitlab['session_expire_delay'] = current_application_settings.session_expire_delay Gitlab::Application.config.session_store( :redis_store, # Using the cookie_store would enable session replay attacks. @@ -12,4 +12,4 @@ Gitlab::Application.config.session_store( httponly: true, expire_after: Settings.gitlab['session_expire_delay'] * 60, path: (Rails.application.config.relative_url_root.nil?) ? '/' : Rails.application.config.relative_url_root -) \ No newline at end of file +) diff --git a/db/schema.rb b/db/schema.rb index 04f887274d..9a9d4a85e4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150604202921) do +ActiveRecord::Schema.define(version: 20150609141121) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -29,13 +29,13 @@ ActiveRecord::Schema.define(version: 20150604202921) do t.boolean "twitter_sharing_enabled", default: true t.text "restricted_visibility_levels" t.boolean "version_check_enabled", default: true - t.integer "max_attachment_size", default: 10, null: false + t.integer "max_attachment_size", default: 10, null: false t.integer "default_project_visibility" t.integer "default_snippet_visibility" t.text "restricted_signup_domains" t.boolean "user_oauth_applications", default: true t.string "after_sign_out_path" - t.integer "session_expire_delay", default: 10080, null: false + t.integer "session_expire_delay", default: 10080, null: false end create_table "broadcast_messages", force: true do |t| @@ -496,12 +496,12 @@ ActiveRecord::Schema.define(version: 20150604202921) do t.string "bitbucket_access_token" t.string "bitbucket_access_token_secret" t.string "location" - t.string "public_email", default: "", null: false t.string "encrypted_otp_secret" t.string "encrypted_otp_secret_iv" t.string "encrypted_otp_secret_salt" t.boolean "otp_required_for_login" t.text "otp_backup_codes" + t.string "public_email", default: "", null: false end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree From 27a8d84094aef3fb50bc98e46c98b6148d28ac4a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 13 Jun 2015 00:01:03 -0400 Subject: [PATCH 14/71] Update Redcarpet to 3.3.0 Fixes #1432 --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 645e0b5cf7..c8f187e4fb 100644 --- a/Gemfile +++ b/Gemfile @@ -94,7 +94,7 @@ gem "seed-fu" gem 'html-pipeline', '~> 1.11.0' gem 'task_list', '1.0.2', require: 'task_list/railtie' gem 'github-markup' -gem 'redcarpet', '~> 3.2.3' +gem 'redcarpet', '~> 3.3.0' gem 'RedCloth' gem 'rdoc', '~>3.6' gem 'org-ruby', '= 0.9.12' diff --git a/Gemfile.lock b/Gemfile.lock index 1de29ad8f8..fd7cbd508e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -499,7 +499,7 @@ GEM trollop rdoc (3.12.2) json (~> 1.4) - redcarpet (3.2.3) + redcarpet (3.3.1) redis (3.1.0) redis-actionpack (4.0.0) actionpack (~> 4) @@ -810,7 +810,7 @@ DEPENDENCIES rails (~> 4.1.0) raphael-rails (~> 2.1.2) rdoc (~> 3.6) - redcarpet (~> 3.2.3) + redcarpet (~> 3.3.0) redis-rails request_store rerun (~> 0.10.0) From 821fc4b03479a193b055c91b8a655d226bc46c17 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 5 Jun 2015 14:00:21 -0400 Subject: [PATCH 15/71] Add Profiles::PreferencesController --- .../profiles/preferences_controller.rb | 29 ++++++++ app/views/layouts/nav/_profile.html.haml | 7 +- app/views/profiles/preferences/show.html.haml | 1 + app/views/profiles/preferences/update.js.erb | 1 + config/routes.rb | 1 + features/profile/active_tab.feature | 6 +- features/steps/profile/active_tab.rb | 4 +- features/steps/shared/paths.rb | 4 +- .../profiles/preferences_controller_spec.rb | 70 +++++++++++++++++++ spec/features/security/profile_access_spec.rb | 4 +- spec/routing/routing_spec.rb | 15 +++- 11 files changed, 127 insertions(+), 15 deletions(-) create mode 100644 app/controllers/profiles/preferences_controller.rb create mode 100644 app/views/profiles/preferences/show.html.haml create mode 100644 app/views/profiles/preferences/update.js.erb create mode 100644 spec/controllers/profiles/preferences_controller_spec.rb diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb new file mode 100644 index 0000000000..897e6fe074 --- /dev/null +++ b/app/controllers/profiles/preferences_controller.rb @@ -0,0 +1,29 @@ +class Profiles::PreferencesController < Profiles::ApplicationController + before_action :user + + def show + end + + def update + if @user.update_attributes(preferences_params) + flash[:notice] = 'Preferences saved.' + else + # TODO (rspeicher): There's no validation on these values, so can it fail? + end + + respond_to do |format| + format.html { redirect_to profile_preferences_path } + format.js + end + end + + private + + def user + @user = current_user + end + + def preferences_params + params.require(:user).permit(:color_scheme_id, :theme_id) + end +end diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index ac37fd4c1c..121665bd53 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -38,11 +38,12 @@ %span SSH Keys %span.count= current_user.keys.count - = nav_link(path: 'profiles#design') do - = link_to design_profile_path, title: 'Design', data: {placement: 'right'} do + = nav_link(controller: :preferences) do + = link_to profile_preferences_path, title: 'Preferences', data: {placement: 'right'} do + -# TODO (rspeicher): Better icon? = icon('image fw') %span - Design + Preferences = nav_link(path: 'profiles#history') do = link_to history_profile_path, title: 'History', data: {placement: 'right'} do = icon('history fw') diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml new file mode 100644 index 0000000000..1333ed77b7 --- /dev/null +++ b/app/views/profiles/preferences/show.html.haml @@ -0,0 +1 @@ +TODO diff --git a/app/views/profiles/preferences/update.js.erb b/app/views/profiles/preferences/update.js.erb new file mode 100644 index 0000000000..70b786d12e --- /dev/null +++ b/app/views/profiles/preferences/update.js.erb @@ -0,0 +1 @@ +// TODO diff --git a/config/routes.rb b/config/routes.rb index f4a104664f..9b1a746f54 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -222,6 +222,7 @@ Gitlab::Application.routes.draw do put :reset end end + resource :preferences, only: [:show, :update] resources :keys resources :emails, only: [:index, :create, :destroy] resource :avatar, only: [:destroy] diff --git a/features/profile/active_tab.feature b/features/profile/active_tab.feature index 7801ae5b8c..1fa4ac88dd 100644 --- a/features/profile/active_tab.feature +++ b/features/profile/active_tab.feature @@ -18,9 +18,9 @@ Feature: Profile Active Tab Then the active main tab should be SSH Keys And no other main tabs should be active - Scenario: On Profile Design - Given I visit profile design page - Then the active main tab should be Design + Scenario: On Profile Preferences + Given I visit profile preferences page + Then the active main tab should be Preferences And no other main tabs should be active Scenario: On Profile History diff --git a/features/steps/profile/active_tab.rb b/features/steps/profile/active_tab.rb index 8595ee876a..79e3b55f6e 100644 --- a/features/steps/profile/active_tab.rb +++ b/features/steps/profile/active_tab.rb @@ -15,8 +15,8 @@ class Spinach::Features::ProfileActiveTab < Spinach::FeatureSteps ensure_active_main_tab('SSH Keys') end - step 'the active main tab should be Design' do - ensure_active_main_tab('Design') + step 'the active main tab should be Preferences' do + ensure_active_main_tab('Preferences') end step 'the active main tab should be History' do diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 09ae7e3a30..3bd0d60281 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -123,8 +123,8 @@ module SharedPaths visit profile_keys_path end - step 'I visit profile design page' do - visit design_profile_path + step 'I visit profile preferences page' do + visit profile_preferences_path end step 'I visit profile history page' do diff --git a/spec/controllers/profiles/preferences_controller_spec.rb b/spec/controllers/profiles/preferences_controller_spec.rb new file mode 100644 index 0000000000..87503b1ed4 --- /dev/null +++ b/spec/controllers/profiles/preferences_controller_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe Profiles::PreferencesController do + let(:user) { create(:user) } + + before do + sign_in(user) + + allow(subject).to receive(:current_user).and_return(user) + end + + describe 'GET show' do + it 'renders' do + get :show + expect(response).to render_template :show + end + + it 'assigns user' do + get :show + expect(assigns[:user]).to eq user + end + end + + describe 'PATCH update' do + def go(params: {}, format: :js) + params.reverse_merge!( + color_scheme_id: '1', + theme_id: '1' + ) + + patch :update, user: params, format: format + end + + context 'on successful update' do + it 'sets the flash' do + go + expect(flash[:notice]).to eq 'Preferences saved.' + end + + it "changes the user's preferences" do + prefs = { + color_scheme_id: '1', + theme_id: '2' + }.with_indifferent_access + + expect(user).to receive(:update_attributes).with(prefs) + + go params: prefs + end + end + + context 'on unsuccessful update' do + # TODO (rspeicher): Can this happen? + end + + context 'as js' do + it 'renders' do + go + expect(response).to render_template :update + end + end + + context 'as html' do + it 'redirects' do + go format: :html + expect(response).to redirect_to(profile_preferences_path) + end + end + end +end diff --git a/spec/features/security/profile_access_spec.rb b/spec/features/security/profile_access_spec.rb index 2512a9c0e3..2b09771851 100644 --- a/spec/features/security/profile_access_spec.rb +++ b/spec/features/security/profile_access_spec.rb @@ -36,8 +36,8 @@ describe "Profile access", feature: true do it { is_expected.to be_denied_for :visitor } end - describe "GET /profile/design" do - subject { design_profile_path } + describe "GET /profile/preferences" do + subject { profile_preferences_path } it { is_expected.to be_allowed_for @u1 } it { is_expected.to be_allowed_for :admin } diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 953c8dd8dd..199851be48 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -102,7 +102,6 @@ end # profile_token GET /profile/token(.:format) profile#token # profile_reset_private_token PUT /profile/reset_private_token(.:format) profile#reset_private_token # profile GET /profile(.:format) profile#show -# profile_design GET /profile/design(.:format) profile#design # profile_update PUT /profile/update(.:format) profile#update describe ProfilesController, "routing" do it "to #account" do @@ -120,9 +119,19 @@ describe ProfilesController, "routing" do it "to #show" do expect(get("/profile")).to route_to('profiles#show') end +end - it "to #design" do - expect(get("/profile/design")).to route_to('profiles#design') +# profile_preferences GET /profile/preferences(.:format) profiles/preferences#show +# PATCH /profile/preferences(.:format) profiles/preferences#update +# PUT /profile/preferences(.:format) profiles/preferences#update +describe Profiles::PreferencesController, 'routing' do + it 'to #show' do + expect(get('/profile/preferences')).to route_to('profiles/preferences#show') + end + + it 'to #update' do + expect(put('/profile/preferences')).to route_to('profiles/preferences#update') + expect(patch('/profile/preferences')).to route_to('profiles/preferences#update') end end From 44d68159999a0ee30f7714470c1ef5b0c4a717fa Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 2 Jun 2015 17:30:21 -0400 Subject: [PATCH 16/71] Allow login_as helper to accept a User object --- spec/support/login_helpers.rb | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb index 791d2a1fd6..1bd6855201 100644 --- a/spec/support/login_helpers.rb +++ b/spec/support/login_helpers.rb @@ -1,9 +1,25 @@ module LoginHelpers - # Internal: Create and log in as a user of the specified role + # Internal: Log in as a specific user or a new user of a specific role # - # role - User role (e.g., :admin, :user) - def login_as(role) - @user = create(role) + # user_or_role - User object, or a role to create (e.g., :admin, :user) + # + # Examples: + # + # # Create a user automatically + # login_as(:user) + # + # # Create an admin automatically + # login_as(:admin) + # + # # Provide an existing User record + # user = create(:user) + # login_as(user) + def login_as(user_or_role) + if user_or_role.kind_of?(User) + @user = user_or_role + else + @user = create(user_or_role) + end login_with(@user) end From 0c0c31ff34b3010c2c269ec56ef48dd305c6f74a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 5 Jun 2015 13:57:01 -0400 Subject: [PATCH 17/71] Move the "Design" templates and logic to Preferences --- app/assets/javascripts/profile.js.coffee | 2 +- .../profiles/preferences_controller.rb | 5 +- app/controllers/profiles_controller.rb | 22 ++++--- app/views/profiles/design.html.haml | 56 ------------------ app/views/profiles/preferences/show.html.haml | 57 ++++++++++++++++++- app/views/profiles/preferences/update.js.erb | 4 +- app/views/profiles/update.js.erb | 3 - config/routes.rb | 1 - features/profile/profile.feature | 13 ----- features/steps/profile/profile.rb | 21 ------- spec/features/profiles/preferences_spec.rb | 33 +++++++++++ 11 files changed, 112 insertions(+), 105 deletions(-) delete mode 100644 app/views/profiles/design.html.haml delete mode 100644 app/views/profiles/update.js.erb create mode 100644 spec/features/profiles/preferences_spec.rb diff --git a/app/assets/javascripts/profile.js.coffee b/app/assets/javascripts/profile.js.coffee index 40459a9a15..a402973a54 100644 --- a/app/assets/javascripts/profile.js.coffee +++ b/app/assets/javascripts/profile.js.coffee @@ -4,7 +4,7 @@ class @Profile # Submit the form $('.edit_user').submit() - new Flash("Appearance settings saved", "notice") + new Flash('Preferences saved.', 'notice') $('.update-username form').on 'ajax:before', -> $('.loading-gif').show() diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb index 897e6fe074..8b2630d164 100644 --- a/app/controllers/profiles/preferences_controller.rb +++ b/app/controllers/profiles/preferences_controller.rb @@ -24,6 +24,9 @@ class Profiles::PreferencesController < Profiles::ApplicationController end def preferences_params - params.require(:user).permit(:color_scheme_id, :theme_id) + params.require(:user).permit( + :color_scheme_id, + :theme_id + ) end end diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index f4366c18e7..88e8799627 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -8,9 +8,6 @@ class ProfilesController < Profiles::ApplicationController def show end - def design - end - def applications @applications = current_user.oauth_applications @authorized_tokens = current_user.oauth_authorized_tokens @@ -65,10 +62,21 @@ class ProfilesController < Profiles::ApplicationController def user_params params.require(:user).permit( - :email, :password, :password_confirmation, :bio, :name, - :username, :skype, :linkedin, :twitter, :website_url, - :color_scheme_id, :theme_id, :avatar, :hide_no_ssh_key, - :hide_no_password, :location, :public_email + :avatar, + :bio, + :email, + :hide_no_password, + :hide_no_ssh_key, + :linkedin, + :location, + :name, + :password, + :password_confirmation, + :public_email, + :skype, + :twitter, + :username, + :website_url ) end end diff --git a/app/views/profiles/design.html.haml b/app/views/profiles/design.html.haml deleted file mode 100644 index f450ec1c01..0000000000 --- a/app/views/profiles/design.html.haml +++ /dev/null @@ -1,56 +0,0 @@ -- page_title "Design" -%h3.page-title - = page_title -%p.light - Appearance settings will be saved to your profile and made available across all devices. -%hr - -= form_for @user, url: profile_path, remote: true, method: :put do |f| - .panel.panel-default.application-theme - .panel-heading - Application theme - .panel-body - .themes_opts - = label_tag do - .prev.default - = f.radio_button :theme_id, 1 - Graphite - - = label_tag do - .prev.classic - = f.radio_button :theme_id, 2 - Charcoal - - = label_tag do - .prev.modern - = f.radio_button :theme_id, 3 - Green - - = label_tag do - .prev.gray - = f.radio_button :theme_id, 4 - Gray - - = label_tag do - .prev.violet - = f.radio_button :theme_id, 5 - Violet - - = label_tag do - .prev.blue - = f.radio_button :theme_id, 6 - Blue - %br - .clearfix - - .panel.panel-default.code-preview-theme - .panel-heading - Code preview theme - .panel-body - .code_highlight_opts - - color_schemes.each do |color_scheme_id, color_scheme| - = label_tag do - .prev - = image_tag "#{color_scheme}-scheme-preview.png" - = f.radio_button :color_scheme_id, color_scheme_id - = color_scheme.gsub(/[-_]+/, ' ').humanize diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 1333ed77b7..2fc47227c3 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -1 +1,56 @@ -TODO +- page_title "Design" +%h3.page-title + = page_title +%p.light + Appearance settings will be saved to your profile and made available across all devices. +%hr + += form_for @user, url: profile_preferences_path, remote: true, method: :put do |f| + .panel.panel-default.application-theme + .panel-heading + Application theme + .panel-body + .themes_opts + = label_tag do + .prev.default + = f.radio_button :theme_id, 1 + Graphite + + = label_tag do + .prev.classic + = f.radio_button :theme_id, 2 + Charcoal + + = label_tag do + .prev.modern + = f.radio_button :theme_id, 3 + Green + + = label_tag do + .prev.gray + = f.radio_button :theme_id, 4 + Gray + + = label_tag do + .prev.violet + = f.radio_button :theme_id, 5 + Violet + + = label_tag do + .prev.blue + = f.radio_button :theme_id, 6 + Blue + %br + .clearfix + + .panel.panel-default.code-preview-theme + .panel-heading + Code preview theme + .panel-body + .code_highlight_opts + - color_schemes.each do |color_scheme_id, color_scheme| + = label_tag do + .prev + = image_tag "#{color_scheme}-scheme-preview.png" + = f.radio_button :color_scheme_id, color_scheme_id + = color_scheme.gsub(/[-_]+/, ' ').humanize diff --git a/app/views/profiles/preferences/update.js.erb b/app/views/profiles/preferences/update.js.erb index 70b786d12e..db37619136 100644 --- a/app/views/profiles/preferences/update.js.erb +++ b/app/views/profiles/preferences/update.js.erb @@ -1 +1,3 @@ -// TODO +// Remove body class for any previous theme, re-add current one +$('body').removeClass('<%= Gitlab::Theme.body_classes %>') +$('body').addClass('<%= app_theme %> <%= theme_type %>') diff --git a/app/views/profiles/update.js.erb b/app/views/profiles/update.js.erb deleted file mode 100644 index db37619136..0000000000 --- a/app/views/profiles/update.js.erb +++ /dev/null @@ -1,3 +0,0 @@ -// Remove body class for any previous theme, re-add current one -$('body').removeClass('<%= Gitlab::Theme.body_classes %>') -$('body').addClass('<%= app_theme %> <%= theme_type %>') diff --git a/config/routes.rb b/config/routes.rb index 9b1a746f54..52c98541da 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -203,7 +203,6 @@ Gitlab::Application.routes.draw do resource :profile, only: [:show, :update] do member do get :history - get :design get :applications put :reset_private_token diff --git a/features/profile/profile.feature b/features/profile/profile.feature index d586167cdf..0dd0afde8b 100644 --- a/features/profile/profile.feature +++ b/features/profile/profile.feature @@ -84,16 +84,3 @@ Feature: Profile Then I visit profile applications page And I click to remove application Then I see that application is removed - - @javascript - Scenario: I change my application theme - Given I visit profile design page - When I change my application theme - Then I should see the theme change immediately - And I should receive feedback that the changes were saved - - @javascript - Scenario: I change my code preview theme - Given I visit profile design page - When I change my code preview theme - Then I should receive feedback that the changes were saved diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb index 32e6859eff..649aea8e3f 100644 --- a/features/steps/profile/profile.rb +++ b/features/steps/profile/profile.rb @@ -114,27 +114,6 @@ class Spinach::Features::Profile < Spinach::FeatureSteps expect(page).to have_content "#{current_user.name} closed issue" end - step "I change my application theme" do - page.within '.application-theme' do - choose "Violet" - end - end - - step "I change my code preview theme" do - page.within '.code-preview-theme' do - choose "Solarized dark" - end - end - - step "I should see the theme change immediately" do - expect(page).to have_selector('body.ui_color') - expect(page).not_to have_selector('body.ui_basic') - end - - step "I should receive feedback that the changes were saved" do - expect(page).to have_content("saved") - end - step 'my password is expired' do current_user.update_attributes(password_expires_at: Time.now - 1.hour) end diff --git a/spec/features/profiles/preferences_spec.rb b/spec/features/profiles/preferences_spec.rb new file mode 100644 index 0000000000..0e033652a9 --- /dev/null +++ b/spec/features/profiles/preferences_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe 'Profile > Preferences' do + let(:user) { create(:user) } + + before do + login_as(user) + end + + describe 'User changes their application theme', js: true do + let(:default_class) { Gitlab::Theme.css_class_by_id(nil) } + let(:theme_5_class) { Gitlab::Theme.css_class_by_id(5) } + + before do + visit profile_preferences_path + end + + it 'changes immediately' do + expect(page).to have_selector("body.#{default.css_class}") + + choose "user_theme_id_#{theme.id}" + + expect(page).not_to have_selector("body.#{default.css_class}") + expect(page).to have_selector("body.#{theme.css_class}") + end + end + + describe 'User changes their syntax highlighting theme' do + before do + visit profile_preferences_path + end + end +end From 658b42b1fa79c77b1acef67a645b36a2928a71bd Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 5 Jun 2015 15:45:46 -0400 Subject: [PATCH 18/71] Consolidate theme stylesheets into one Since they're all defined by a mixin, it didn't provide any benefit to have them in separate files. This also adds variables defining the basic color of each theme so we can re-use them in the previews. --- app/assets/stylesheets/application.scss | 16 ++--- app/assets/stylesheets/pages/profile.scss | 61 ------------------- .../pages/profiles/preferences.scss | 57 +++++++++++++++++ app/assets/stylesheets/pages/themes.scss | 0 .../stylesheets/themes/gitlab-theme.scss | 41 +++++++++++++ app/assets/stylesheets/themes/ui_basic.scss | 8 --- app/assets/stylesheets/themes/ui_blue.scss | 6 -- app/assets/stylesheets/themes/ui_color.scss | 6 -- app/assets/stylesheets/themes/ui_gray.scss | 6 -- app/assets/stylesheets/themes/ui_mars.scss | 6 -- app/assets/stylesheets/themes/ui_modern.scss | 6 -- 11 files changed, 106 insertions(+), 107 deletions(-) create mode 100644 app/assets/stylesheets/pages/profiles/preferences.scss delete mode 100644 app/assets/stylesheets/pages/themes.scss delete mode 100644 app/assets/stylesheets/themes/ui_basic.scss delete mode 100644 app/assets/stylesheets/themes/ui_blue.scss delete mode 100644 app/assets/stylesheets/themes/ui_color.scss delete mode 100644 app/assets/stylesheets/themes/ui_gray.scss delete mode 100644 app/assets/stylesheets/themes/ui_mars.scss delete mode 100644 app/assets/stylesheets/themes/ui_modern.scss diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 015ff2ce4e..1a5f11df7d 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -35,26 +35,26 @@ */ @import "font-awesome"; +/** + * UI themes: + */ +@import "themes/**/*"; + /** * Generic css (forms, nav etc): */ -@import "generic/*"; +@import "generic/**/*"; /** * Page specific styles (issues, projects etc): */ -@import "pages/*"; +@import "pages/**/*"; /** * Code highlight */ -@import "highlight/*"; - -/** - * UI themes: - */ -@import "themes/*"; +@import "highlight/**/*"; /** * Styles for JS behaviors. diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 5a5fbc468a..8e4f0eb2b2 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -17,67 +17,6 @@ } } -/* - * Appearance settings - * - */ -.themes_opts { - label { - margin-right: 20px; - text-align: center; - - .prev { - height: 80px; - width: 160px; - margin-bottom: 10px; - @include border-radius(4px); - - &.classic { - background: #31363e; - } - - &.default { - background: #888888; - } - - &.modern { - background: #009871; - } - - &.gray { - background: #373737; - } - - &.violet { - background: #548; - } - - &.blue { - background: #2980b9; - } - } - } -} - -.code_highlight_opts { - margin-top: 10px; - - label { - margin-right: 20px; - text-align: center; - - .prev { - width: 160px; - margin-bottom: 10px; - - img { - max-width: 100%; - @include border-radius(4px); - } - } - } -} - .oauth-buttons { .btn-group { margin-right: 10px; diff --git a/app/assets/stylesheets/pages/profiles/preferences.scss b/app/assets/stylesheets/pages/profiles/preferences.scss new file mode 100644 index 0000000000..846a940f1c --- /dev/null +++ b/app/assets/stylesheets/pages/profiles/preferences.scss @@ -0,0 +1,57 @@ +.application-theme { + label { + margin-right: 20px; + text-align: center; + + .application-theme-preview { + height: 80px; + width: 160px; + margin-bottom: 10px; + @include border-radius(4px); + + &.ui_blue { + background: $theme-blue; + } + + &.ui_charcoal { + background: $theme-charcoal; + } + + &.ui_graphite { + background: $theme-graphite; + } + + &.ui_gray { + background: $theme-gray; + } + + &.ui_green { + background: $theme-green; + } + + &.ui_violet { + background: $theme-violet; + } + } + } +} + +.code_highlight_opts { + margin-top: 10px; + + label { + margin-right: 20px; + text-align: center; + + .prev { + width: 160px; + margin-bottom: 10px; + + img { + max-width: 100%; + @include border-radius(4px); + } + } + } +} + diff --git a/app/assets/stylesheets/pages/themes.scss b/app/assets/stylesheets/pages/themes.scss deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/app/assets/stylesheets/themes/gitlab-theme.scss b/app/assets/stylesheets/themes/gitlab-theme.scss index 10fcaf18fa..7cabeaefb9 100644 --- a/app/assets/stylesheets/themes/gitlab-theme.scss +++ b/app/assets/stylesheets/themes/gitlab-theme.scss @@ -1,3 +1,11 @@ +/** + * Styles the GitLab application with a specific color theme + * + * $color-light - + * $color - + * $color-darker - + * $color-dark - + */ @mixin gitlab-theme($color-light, $color, $color-darker, $color-dark) { header { &.navbar-gitlab { @@ -77,3 +85,36 @@ } } } + +$theme-blue: #2980B9; +$theme-charcoal: #474D57; +$theme-graphite: #888888; +$theme-gray: #373737; +$theme-green: #019875; +$theme-violet: #554488; + +body { + &.ui_blue { + @include gitlab-theme(#BECDE9, $theme-blue, #1970A9, #096099); + } + + &.ui_charcoal { + @include gitlab-theme(#979DA7, $theme-charcoal, #373D47, #24272D); + } + + &.ui_graphite { + @include gitlab-theme(#CCCCCC, $theme-graphite, #777777, #666666); + } + + &.ui_gray { + @include gitlab-theme(#979797, $theme-gray, #272727, #222222); + } + + &.ui_green { + @include gitlab-theme(#AADDCC, $theme-green, #018865, #017855); + } + + &.ui_violet { + @include gitlab-theme(#9988CC, $theme-violet, #443366, #332255); + } +} diff --git a/app/assets/stylesheets/themes/ui_basic.scss b/app/assets/stylesheets/themes/ui_basic.scss deleted file mode 100644 index 63e8dce1e9..0000000000 --- a/app/assets/stylesheets/themes/ui_basic.scss +++ /dev/null @@ -1,8 +0,0 @@ -/** - * This file represent some UI that can be changed - * during web app restyle or theme select. - * - */ -.ui_basic { - @include gitlab-theme(#CCCCCC, #888888, #777777, #666666); -} diff --git a/app/assets/stylesheets/themes/ui_blue.scss b/app/assets/stylesheets/themes/ui_blue.scss deleted file mode 100644 index cf995622b6..0000000000 --- a/app/assets/stylesheets/themes/ui_blue.scss +++ /dev/null @@ -1,6 +0,0 @@ -/** - * Blue GitLab UI theme - */ -.ui_blue { - @include gitlab-theme(#BECDE9, #2980b9, #1970a9, #096099); -} diff --git a/app/assets/stylesheets/themes/ui_color.scss b/app/assets/stylesheets/themes/ui_color.scss deleted file mode 100644 index 6babccec0d..0000000000 --- a/app/assets/stylesheets/themes/ui_color.scss +++ /dev/null @@ -1,6 +0,0 @@ -/** - * Violet GitLab UI theme - */ -.ui_color { - @include gitlab-theme(#98C, #548, #436, #325); -} diff --git a/app/assets/stylesheets/themes/ui_gray.scss b/app/assets/stylesheets/themes/ui_gray.scss deleted file mode 100644 index f8e4a6ea7d..0000000000 --- a/app/assets/stylesheets/themes/ui_gray.scss +++ /dev/null @@ -1,6 +0,0 @@ -/** - * Gray GitLab UI theme - */ -.ui_gray { - @include gitlab-theme(#979797, #373737, #272727, #222222); -} diff --git a/app/assets/stylesheets/themes/ui_mars.scss b/app/assets/stylesheets/themes/ui_mars.scss deleted file mode 100644 index fda96b64cd..0000000000 --- a/app/assets/stylesheets/themes/ui_mars.scss +++ /dev/null @@ -1,6 +0,0 @@ -/** - * Classic GitLab UI theme - */ -.ui_mars { - @include gitlab-theme(#979DA7, #474D57, #373D47, #24272D); -} diff --git a/app/assets/stylesheets/themes/ui_modern.scss b/app/assets/stylesheets/themes/ui_modern.scss deleted file mode 100644 index 8261e80b35..0000000000 --- a/app/assets/stylesheets/themes/ui_modern.scss +++ /dev/null @@ -1,6 +0,0 @@ -/** - * Modern GitLab UI theme - */ -.ui_modern { - @include gitlab-theme(#ADC, #019875, #018865, #017855); -} From 844d72716e2175dcd5e39b4d1eecb9e3560aa0b9 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 5 Jun 2015 15:50:36 -0400 Subject: [PATCH 19/71] Add Gitlab::Themes module; remove Gitlab::Theme Now we can simply loop through all themes, among other things. This also removes the `dark_theme` / `light_theme` classes and the `theme_type` helper, since they weren't used anywhere. --- .../pages/profiles/preferences.scss | 9 +-- app/helpers/application_helper.rb | 7 +- app/views/profiles/preferences/show.html.haml | 35 ++-------- app/views/profiles/preferences/update.js.erb | 4 +- config/initializers/1_settings.rb | 2 +- lib/gitlab/theme.rb | 50 -------------- lib/gitlab/themes.rb | 67 +++++++++++++++++++ spec/features/profiles/preferences_spec.rb | 6 +- spec/lib/gitlab/themes_spec.rb | 51 ++++++++++++++ spec/models/user_spec.rb | 4 +- 10 files changed, 137 insertions(+), 98 deletions(-) delete mode 100644 lib/gitlab/theme.rb create mode 100644 lib/gitlab/themes.rb create mode 100644 spec/lib/gitlab/themes_spec.rb diff --git a/app/assets/stylesheets/pages/profiles/preferences.scss b/app/assets/stylesheets/pages/profiles/preferences.scss index 846a940f1c..e8c8036281 100644 --- a/app/assets/stylesheets/pages/profiles/preferences.scss +++ b/app/assets/stylesheets/pages/profiles/preferences.scss @@ -3,12 +3,13 @@ margin-right: 20px; text-align: center; - .application-theme-preview { - height: 80px; - width: 160px; - margin-bottom: 10px; + .preview { @include border-radius(4px); + height: 80px; + margin-bottom: 10px; + width: 160px; + &.ui_blue { background: $theme-blue; } diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index a539ec49f7..62794bc5f4 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -139,11 +139,8 @@ module ApplicationHelper end def app_theme - Gitlab::Theme.css_class_by_id(current_user.try(:theme_id)) - end - - def theme_type - Gitlab::Theme.type_css_class_by_id(current_user.try(:theme_id)) + theme = Gitlab::Themes.by_id(current_user.try(:theme_id)) + theme.css_class end def user_color_scheme_class diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 2fc47227c3..a30f302dea 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -10,38 +10,11 @@ .panel-heading Application theme .panel-body - .themes_opts + - Gitlab::Themes.each do |theme| = label_tag do - .prev.default - = f.radio_button :theme_id, 1 - Graphite - - = label_tag do - .prev.classic - = f.radio_button :theme_id, 2 - Charcoal - - = label_tag do - .prev.modern - = f.radio_button :theme_id, 3 - Green - - = label_tag do - .prev.gray - = f.radio_button :theme_id, 4 - Gray - - = label_tag do - .prev.violet - = f.radio_button :theme_id, 5 - Violet - - = label_tag do - .prev.blue - = f.radio_button :theme_id, 6 - Blue - %br - .clearfix + .preview{class: theme.css_class} + = f.radio_button :theme_id, theme.id + = theme.name .panel.panel-default.code-preview-theme .panel-heading diff --git a/app/views/profiles/preferences/update.js.erb b/app/views/profiles/preferences/update.js.erb index db37619136..830df22855 100644 --- a/app/views/profiles/preferences/update.js.erb +++ b/app/views/profiles/preferences/update.js.erb @@ -1,3 +1,3 @@ // Remove body class for any previous theme, re-add current one -$('body').removeClass('<%= Gitlab::Theme.body_classes %>') -$('body').addClass('<%= app_theme %> <%= theme_type %>') +$('body').removeClass('<%= Gitlab::Themes.body_classes %>') +$('body').addClass('<%= app_theme %>') diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 9c622b7301..7b5d488f59 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -103,7 +103,7 @@ Settings['gitlab'] ||= Settingslogic.new({}) Settings.gitlab['default_projects_limit'] ||= 10 Settings.gitlab['default_branch_protection'] ||= 2 Settings.gitlab['default_can_create_group'] = true if Settings.gitlab['default_can_create_group'].nil? -Settings.gitlab['default_theme'] = Gitlab::Theme::MARS if Settings.gitlab['default_theme'].nil? +Settings.gitlab['default_theme'] = Gitlab::Themes::APPLICATION_DEFAULT if Settings.gitlab['default_theme'].nil? Settings.gitlab['host'] ||= 'localhost' Settings.gitlab['ssh_host'] ||= Settings.gitlab.host Settings.gitlab['https'] = false if Settings.gitlab['https'].nil? diff --git a/lib/gitlab/theme.rb b/lib/gitlab/theme.rb deleted file mode 100644 index e5a1f1b44d..0000000000 --- a/lib/gitlab/theme.rb +++ /dev/null @@ -1,50 +0,0 @@ -module Gitlab - class Theme - BASIC = 1 unless const_defined?(:BASIC) - MARS = 2 unless const_defined?(:MARS) - MODERN = 3 unless const_defined?(:MODERN) - GRAY = 4 unless const_defined?(:GRAY) - COLOR = 5 unless const_defined?(:COLOR) - BLUE = 6 unless const_defined?(:BLUE) - - def self.classes - @classes ||= { - BASIC => 'ui_basic', - MARS => 'ui_mars', - MODERN => 'ui_modern', - GRAY => 'ui_gray', - COLOR => 'ui_color', - BLUE => 'ui_blue' - } - end - - def self.css_class_by_id(id) - id ||= Gitlab.config.gitlab.default_theme - classes[id] - end - - def self.types - @types ||= { - BASIC => 'light_theme', - MARS => 'dark_theme', - MODERN => 'dark_theme', - GRAY => 'dark_theme', - COLOR => 'dark_theme', - BLUE => 'light_theme' - } - end - - def self.type_css_class_by_id(id) - id ||= Gitlab.config.gitlab.default_theme - types[id] - end - - # Convenience method to get a space-separated String of all the theme - # classes that might be applied to the `body` element - # - # Returns a String - def self.body_classes - (classes.values + types.values).uniq.join(' ') - end - end -end diff --git a/lib/gitlab/themes.rb b/lib/gitlab/themes.rb new file mode 100644 index 0000000000..5209df9279 --- /dev/null +++ b/lib/gitlab/themes.rb @@ -0,0 +1,67 @@ +module Gitlab + # Module containing GitLab's application theme definitions and helper methods + # for accessing them. + module Themes + # Theme ID used when no `default_theme` configuration setting is provided. + APPLICATION_DEFAULT = 2 + + # Struct class representing a single Theme + Theme = Struct.new(:id, :name, :css_class) + + # All available Themes + THEMES = [ + Theme.new(1, 'Graphite', 'ui_graphite'), + Theme.new(2, 'Charcoal', 'ui_charcoal'), + Theme.new(3, 'Green', 'ui_green'), + Theme.new(4, 'Gray', 'ui_gray'), + Theme.new(5, 'Violet', 'ui_violet'), + Theme.new(6, 'Blue', 'ui_blue') + ].freeze + + # Convenience method to get a space-separated String of all the theme + # classes that might be applied to the `body` element + # + # Returns a String + def self.body_classes + THEMES.collect(&:css_class).uniq.join(' ') + end + + # Get a Theme by its ID + # + # If the ID is invalid, returns the default Theme. + # + # id - Integer ID + # + # Returns a Theme + def self.by_id(id) + THEMES.detect { |t| t.id == id } || default + end + + # Get the default Theme + # + # Returns a Theme + def self.default + by_id(default_id) + end + + # Iterate through each Theme + # + # Yields the Theme object + def self.each(&block) + THEMES.each(&block) + end + + private + + def self.default_id + id = Gitlab.config.gitlab.default_theme.to_i + + # Prevent an invalid configuration setting from causing an infinite loop + if id < THEMES.first.id || id > THEMES.last.id + APPLICATION_DEFAULT + else + id + end + end + end +end diff --git a/spec/features/profiles/preferences_spec.rb b/spec/features/profiles/preferences_spec.rb index 0e033652a9..7bbb591a4e 100644 --- a/spec/features/profiles/preferences_spec.rb +++ b/spec/features/profiles/preferences_spec.rb @@ -8,14 +8,14 @@ describe 'Profile > Preferences' do end describe 'User changes their application theme', js: true do - let(:default_class) { Gitlab::Theme.css_class_by_id(nil) } - let(:theme_5_class) { Gitlab::Theme.css_class_by_id(5) } + let(:default) { Gitlab::Themes.default } + let(:theme) { Gitlab::Themes.by_id(5) } before do visit profile_preferences_path end - it 'changes immediately' do + it 'reflects the changes immediately' do expect(page).to have_selector("body.#{default.css_class}") choose "user_theme_id_#{theme.id}" diff --git a/spec/lib/gitlab/themes_spec.rb b/spec/lib/gitlab/themes_spec.rb new file mode 100644 index 0000000000..9c6c3fd810 --- /dev/null +++ b/spec/lib/gitlab/themes_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe Gitlab::Themes do + describe '.body_classes' do + it 'returns a space-separated list of class names' do + css = described_class.body_classes + + expect(css).to include('ui_graphite') + expect(css).to include(' ui_charcoal ') + expect(css).to include(' ui_blue') + end + end + + describe '.by_id' do + it 'returns a Theme by its ID' do + expect(described_class.by_id(1).name).to eq 'Graphite' + expect(described_class.by_id(6).name).to eq 'Blue' + end + end + + describe '.default' do + it 'returns the default application theme' do + allow(described_class).to receive(:default_id).and_return(2) + expect(described_class.default.id).to eq 2 + end + + it 'prevents an infinite loop when configuration default is invalid' do + default = described_class::APPLICATION_DEFAULT + themes = described_class::THEMES + + config = double(default_theme: 0).as_null_object + allow(Gitlab).to receive(:config).and_return(config) + expect(described_class.default.id).to eq default + + config = double(default_theme: themes.size + 5).as_null_object + allow(Gitlab).to receive(:config).and_return(config) + expect(described_class.default.id).to eq default + end + end + + describe '.each' do + it 'passes the block to the THEMES Array' do + ids = [] + described_class.each { |theme| ids << theme.id } + expect(ids).not_to be_empty + + # TODO (rspeicher): RSpec 3.x + # expect(described_class.each).to yield_with_arg(described_class::Theme) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f1b8afa585..9ff4288684 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -329,12 +329,12 @@ describe User do end describe 'with default overrides' do - let(:user) { User.new(projects_limit: 123, can_create_group: false, can_create_team: true, theme_id: Gitlab::Theme::BASIC) } + let(:user) { User.new(projects_limit: 123, can_create_group: false, can_create_team: true, theme_id: 1) } it "should apply defaults to user" do expect(user.projects_limit).to eq(123) expect(user.can_create_group).to be_falsey - expect(user.theme_id).to eq(Gitlab::Theme::BASIC) + expect(user.theme_id).to eq(1) end end end From 8112f7550b70c83bde2f74ed48e7781c5424ebb9 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 5 Jun 2015 15:53:57 -0400 Subject: [PATCH 20/71] Add PreferencesHelper module Consolidates the helpers related to user preferences. Renames `app_theme` to `user_application_theme` to better explain what it is. --- app/helpers/application_helper.rb | 29 ----------- app/helpers/gitlab_markdown_helper.rb | 1 + app/helpers/preferences_helper.rb | 31 ++++++++++++ app/views/layouts/application.html.haml | 6 +-- app/views/layouts/errors.html.haml | 2 +- app/views/profiles/preferences/update.js.erb | 2 +- spec/helpers/application_helper_spec.rb | 21 -------- spec/helpers/preferences_helper_spec.rb | 53 ++++++++++++++++++++ 8 files changed, 90 insertions(+), 55 deletions(-) create mode 100644 app/helpers/preferences_helper.rb create mode 100644 spec/helpers/preferences_helper_spec.rb diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 62794bc5f4..10d7aa1120 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -2,26 +2,6 @@ require 'digest/md5' require 'uri' module ApplicationHelper - COLOR_SCHEMES = { - 1 => 'white', - 2 => 'dark', - 3 => 'solarized-light', - 4 => 'solarized-dark', - 5 => 'monokai', - } - COLOR_SCHEMES.default = 'white' - - # Helper method to access the COLOR_SCHEMES - # - # The keys are the `color_scheme_ids` - # The values are the `name` of the scheme. - # - # The preview images are `name-scheme-preview.png` - # The stylesheets should use the css class `.name` - def color_schemes - COLOR_SCHEMES.freeze - end - # Check if a particular controller is the current one # # args - One or more controller names to check @@ -138,15 +118,6 @@ module ApplicationHelper Emoji.names.to_s end - def app_theme - theme = Gitlab::Themes.by_id(current_user.try(:theme_id)) - theme.css_class - end - - def user_color_scheme_class - COLOR_SCHEMES[current_user.try(:color_scheme_id)] if defined?(current_user) - end - # Define whenever show last push event # with suggestion to create MR def show_last_push_widget?(event) diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index 2777944fc9..9aabe01f60 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -2,6 +2,7 @@ require 'nokogiri' module GitlabMarkdownHelper include Gitlab::Markdown + include PreferencesHelper # Use this in places where you would normally use link_to(gfm(...), ...). # diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb new file mode 100644 index 0000000000..04873b9bd0 --- /dev/null +++ b/app/helpers/preferences_helper.rb @@ -0,0 +1,31 @@ +# Helper methods for per-User preferences +module PreferencesHelper + COLOR_SCHEMES = { + 1 => 'white', + 2 => 'dark', + 3 => 'solarized-light', + 4 => 'solarized-dark', + 5 => 'monokai', + } + COLOR_SCHEMES.default = 'white' + + # Helper method to access the COLOR_SCHEMES + # + # The keys are the `color_scheme_ids` + # The values are the `name` of the scheme. + # + # The preview images are `name-scheme-preview.png` + # The stylesheets should use the css class `.name` + def color_schemes + COLOR_SCHEMES.freeze + end + + def user_application_theme + theme = Gitlab::Themes.by_id(current_user.try(:theme_id)) + theme.css_class + end + + def user_color_scheme_class + COLOR_SCHEMES[current_user.try(:color_scheme_id)] if defined?(current_user) + end +end diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 173033f7ea..678ed3c2c1 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -1,10 +1,10 @@ !!! 5 %html{ lang: "en"} = render "layouts/head" - %body{class: "#{app_theme}", :'data-page' => body_data_page} - / Ideally this would be inside the head, but turbolinks only evaluates page-specific JS in the body. + %body{class: "#{user_application_theme}", 'data-page' => body_data_page} + -# Ideally this would be inside the head, but turbolinks only evaluates page-specific JS in the body. = yield :scripts_body_top - + - if current_user = render "layouts/header/default", title: header_title - else diff --git a/app/views/layouts/errors.html.haml b/app/views/layouts/errors.html.haml index 2e3a2b16eb..2af265a229 100644 --- a/app/views/layouts/errors.html.haml +++ b/app/views/layouts/errors.html.haml @@ -1,7 +1,7 @@ !!! 5 %html{ lang: "en"} = render "layouts/head" - %body{class: "#{app_theme} application"} + %body{class: "#{user_application_theme} application"} = render "layouts/header/empty" .container.navless-container = render "layouts/flash" diff --git a/app/views/profiles/preferences/update.js.erb b/app/views/profiles/preferences/update.js.erb index 830df22855..cd2c5b632e 100644 --- a/app/views/profiles/preferences/update.js.erb +++ b/app/views/profiles/preferences/update.js.erb @@ -1,3 +1,3 @@ // Remove body class for any previous theme, re-add current one $('body').removeClass('<%= Gitlab::Themes.body_classes %>') -$('body').addClass('<%= app_theme %>') +$('body').addClass('<%= user_application_theme %>') diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 3307ac776f..47e10197f5 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -185,27 +185,6 @@ describe ApplicationHelper do end end - describe 'user_color_scheme_class' do - context 'with current_user is nil' do - it 'should return a string' do - allow(self).to receive(:current_user).and_return(nil) - expect(user_color_scheme_class).to be_kind_of(String) - end - end - - context 'with a current_user' do - (1..5).each do |color_scheme_id| - context "with color_scheme_id == #{color_scheme_id}" do - it 'should return a string' do - current_user = double(:color_scheme_id => color_scheme_id) - allow(self).to receive(:current_user).and_return(current_user) - expect(user_color_scheme_class).to be_kind_of(String) - end - end - end - end - end - describe 'simple_sanitize' do let(:a_tag) { 'Foo' } diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb new file mode 100644 index 0000000000..095b016e6e --- /dev/null +++ b/spec/helpers/preferences_helper_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe PreferencesHelper do + describe 'user_application_theme' do + context 'with a user' do + it "returns user's theme's css_class" do + user = double('user', theme_id: 3) + allow(self).to receive(:current_user).and_return(user) + expect(user_application_theme).to eq 'ui_green' + end + + it 'returns the default when id is invalid' do + user = double('user', theme_id: Gitlab::Themes::THEMES.size + 5) + + allow(Gitlab.config.gitlab).to receive(:default_theme).and_return(2) + allow(self).to receive(:current_user).and_return(user) + + expect(user_application_theme).to eq 'ui_charcoal' + end + end + + context 'without a user' do + before do + allow(self).to receive(:current_user).and_return(nil) + end + + it 'returns the default theme' do + expect(user_application_theme).to eq Gitlab::Themes.default.css_class + end + end + end + + describe 'user_color_scheme_class' do + context 'with current_user is nil' do + it 'should return a string' do + allow(self).to receive(:current_user).and_return(nil) + expect(user_color_scheme_class).to be_kind_of(String) + end + end + + context 'with a current_user' do + (1..5).each do |color_scheme_id| + context "with color_scheme_id == #{color_scheme_id}" do + it 'should return a string' do + current_user = double(:color_scheme_id => color_scheme_id) + allow(self).to receive(:current_user).and_return(current_user) + expect(user_color_scheme_class).to be_kind_of(String) + end + end + end + end + end +end From 1044dfbfd29866b101cfbdc278788bd4f55a7276 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 5 Jun 2015 16:39:01 -0400 Subject: [PATCH 21/71] Remove js handler from Profiles#update It was only used for the appearance live updating, which is now handled by Profiles::Preferences#update --- app/controllers/profiles_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 88e8799627..b4af9e490e 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -26,7 +26,6 @@ class ProfilesController < Profiles::ApplicationController respond_to do |format| format.html { redirect_to :back } - format.js end end From 5f20574db5c7b91d54e829010c1fda093ea8a25e Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 5 Jun 2015 17:30:40 -0400 Subject: [PATCH 22/71] Fix a few remaining references to the old Theme names/IDs --- app/views/layouts/devise.html.haml | 2 +- config/gitlab.yml.example | 13 +++++++------ db/fixtures/production/001_admin.rb | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/views/layouts/devise.html.haml b/app/views/layouts/devise.html.haml index d406f5764a..1987bf1592 100644 --- a/app/views/layouts/devise.html.haml +++ b/app/views/layouts/devise.html.haml @@ -1,7 +1,7 @@ !!! 5 %html{ lang: "en"} = render "layouts/head" - %body.ui_mars.login-page.application + %body.ui_charcoal.login-page.application = render "layouts/header/empty" = render "layouts/broadcast" .container.navless-container diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 96b6ed01f4..c32ac2042d 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -62,12 +62,13 @@ production: &base # default_can_create_group: false # default: true # username_changing_enabled: false # default: true - User can change her username/namespace - ## Default theme - ## BASIC = 1 - ## MARS = 2 - ## MODERN = 3 - ## GRAY = 4 - ## COLOR = 5 + ## Default theme ID + ## 1 - Graphite + ## 2 - Charcoal + ## 3 - Green + ## 4 - Gray + ## 5 - Violet + ## 6 - Blue # default_theme: 2 # default: 2 ## Automatic issue closing diff --git a/db/fixtures/production/001_admin.rb b/db/fixtures/production/001_admin.rb index 8b560ee09e..1c8740f6ba 100644 --- a/db/fixtures/production/001_admin.rb +++ b/db/fixtures/production/001_admin.rb @@ -12,7 +12,7 @@ admin = User.create( username: 'root', password: password, password_expires_at: expire_time, - theme_id: Gitlab::Theme::MARS + theme_id: Gitlab::Themes::APPLICATION_DEFAULT ) From 0e21436aaf9ea2ed988c3e515e0db73df70534c4 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 5 Jun 2015 18:01:45 -0400 Subject: [PATCH 23/71] Simplify the javascript behavior for Preference updating --- app/assets/javascripts/profile.js.coffee | 9 +++------ app/views/profiles/preferences/show.html.haml | 14 ++++++-------- app/views/profiles/preferences/update.js.erb | 1 + spec/features/profiles/preferences_spec.rb | 8 ++++++++ 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/profile.js.coffee b/app/assets/javascripts/profile.js.coffee index a402973a54..bb0b66b86e 100644 --- a/app/assets/javascripts/profile.js.coffee +++ b/app/assets/javascripts/profile.js.coffee @@ -1,10 +1,8 @@ class @Profile constructor: -> - $('.edit_user .application-theme input, .edit_user .code-preview-theme input').click -> - # Submit the form - $('.edit_user').submit() - - new Flash('Preferences saved.', 'notice') + # Automatically submit the Preferences form when any of its radio buttons change + $('.js-preferences-form').on 'change.preference', 'input[type=radio]', -> + $(this).parents('form').submit() $('.update-username form').on 'ajax:before', -> $('.loading-gif').show() @@ -18,7 +16,6 @@ class @Profile $('.update-notifications').on 'ajax:complete', -> $(this).find('.btn-save').enable() - $('.js-choose-user-avatar-button').bind "click", -> form = $(this).closest("form") form.find(".js-user-avatar-input").click() diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index a30f302dea..bfce0d8ef9 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -5,7 +5,7 @@ Appearance settings will be saved to your profile and made available across all devices. %hr -= form_for @user, url: profile_preferences_path, remote: true, method: :put do |f| += form_for @user, url: profile_preferences_path, remote: true, method: :put, html: {class: 'js-preferences-form'} do |f| .panel.panel-default.application-theme .panel-heading Application theme @@ -20,10 +20,8 @@ .panel-heading Code preview theme .panel-body - .code_highlight_opts - - color_schemes.each do |color_scheme_id, color_scheme| - = label_tag do - .prev - = image_tag "#{color_scheme}-scheme-preview.png" - = f.radio_button :color_scheme_id, color_scheme_id - = color_scheme.gsub(/[-_]+/, ' ').humanize + - color_schemes.each do |color_scheme_id, color_scheme| + = label_tag do + .preview= image_tag "#{color_scheme}-scheme-preview.png" + = f.radio_button :color_scheme_id, color_scheme_id + = color_scheme.tr('-_', ' ').titleize diff --git a/app/views/profiles/preferences/update.js.erb b/app/views/profiles/preferences/update.js.erb index cd2c5b632e..e952d8f47e 100644 --- a/app/views/profiles/preferences/update.js.erb +++ b/app/views/profiles/preferences/update.js.erb @@ -1,3 +1,4 @@ // Remove body class for any previous theme, re-add current one $('body').removeClass('<%= Gitlab::Themes.body_classes %>') $('body').addClass('<%= user_application_theme %>') +new Flash('<%= flash.discard(:notice) %>', 'notice') diff --git a/spec/features/profiles/preferences_spec.rb b/spec/features/profiles/preferences_spec.rb index 7bbb591a4e..a946064a87 100644 --- a/spec/features/profiles/preferences_spec.rb +++ b/spec/features/profiles/preferences_spec.rb @@ -15,6 +15,14 @@ describe 'Profile > Preferences' do visit profile_preferences_path end + it 'creates a flash message' do + choose "user_theme_id_#{theme.id}" + + within('.flash-container') do + expect(page).to have_content('Preferences saved.') + end + end + it 'reflects the changes immediately' do expect(page).to have_selector("body.#{default.css_class}") From 35339bb11f2c410b91a5e8b9ae1d2fef19cac695 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 5 Jun 2015 18:13:12 -0400 Subject: [PATCH 24/71] Update markup/styling for syntax highlight theme preference --- .../stylesheets/pages/profiles/preferences.scss | 12 +++++------- app/views/profiles/preferences/show.html.haml | 4 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/app/assets/stylesheets/pages/profiles/preferences.scss b/app/assets/stylesheets/pages/profiles/preferences.scss index e8c8036281..e5859fe738 100644 --- a/app/assets/stylesheets/pages/profiles/preferences.scss +++ b/app/assets/stylesheets/pages/profiles/preferences.scss @@ -37,22 +37,20 @@ } } -.code_highlight_opts { - margin-top: 10px; - +.syntax-theme { label { margin-right: 20px; text-align: center; - .prev { - width: 160px; + .preview { margin-bottom: 10px; + width: 160px; img { - max-width: 100%; @include border-radius(4px); + + max-width: 100%; } } } } - diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index bfce0d8ef9..680c0930bb 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -16,9 +16,9 @@ = f.radio_button :theme_id, theme.id = theme.name - .panel.panel-default.code-preview-theme + .panel.panel-default.syntax-theme .panel-heading - Code preview theme + Syntax highlighting theme .panel-body - color_schemes.each do |color_scheme_id, color_scheme| = label_tag do From 2bc4fd2d047c1c4c4637f045ed3a51d414359c2a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 03:23:28 -0400 Subject: [PATCH 25/71] Add `dashboard` attribute to User model --- app/controllers/profiles/preferences_controller.rb | 1 + app/models/user.rb | 7 ++++++- db/migrate/20150610065936_add_dashboard_to_users.rb | 9 +++++++++ db/schema.rb | 3 ++- spec/controllers/profiles/preferences_controller_spec.rb | 2 ++ spec/models/user_spec.rb | 3 ++- 6 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20150610065936_add_dashboard_to_users.rb diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb index 8b2630d164..e43a247f72 100644 --- a/app/controllers/profiles/preferences_controller.rb +++ b/app/controllers/profiles/preferences_controller.rb @@ -26,6 +26,7 @@ class Profiles::PreferencesController < Profiles::ApplicationController def preferences_params params.require(:user).permit( :color_scheme_id, + :dashboard, :theme_id ) end diff --git a/app/models/user.rb b/app/models/user.rb index 8be0b62270..6ac287203b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,12 +50,13 @@ # bitbucket_access_token :string(255) # bitbucket_access_token_secret :string(255) # location :string(255) +# public_email :string(255) default(""), not null # encrypted_otp_secret :string(255) # encrypted_otp_secret_iv :string(255) # encrypted_otp_secret_salt :string(255) # otp_required_for_login :boolean # otp_backup_codes :text -# public_email :string(255) default(""), not null +# dashboard :integer default(0) # require 'carrierwave/orm/activerecord' @@ -701,4 +702,8 @@ class User < ActiveRecord::Base def can_be_removed? !solo_owned_groups.present? end + + # User's Dashboard preference + # Note: When adding an option, it MUST go on the end of the array. + enum dashboard: [:projects, :stars] end diff --git a/db/migrate/20150610065936_add_dashboard_to_users.rb b/db/migrate/20150610065936_add_dashboard_to_users.rb new file mode 100644 index 0000000000..2628e45072 --- /dev/null +++ b/db/migrate/20150610065936_add_dashboard_to_users.rb @@ -0,0 +1,9 @@ +class AddDashboardToUsers < ActiveRecord::Migration + def up + add_column :users, :dashboard, :integer, default: 0 + end + + def down + remove_column :users, :dashboard + end +end diff --git a/db/schema.rb b/db/schema.rb index 9a9d4a85e4..f063a4868b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150609141121) do +ActiveRecord::Schema.define(version: 20150610065936) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -502,6 +502,7 @@ ActiveRecord::Schema.define(version: 20150609141121) do t.boolean "otp_required_for_login" t.text "otp_backup_codes" t.string "public_email", default: "", null: false + t.integer "dashboard", default: 0 end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree diff --git a/spec/controllers/profiles/preferences_controller_spec.rb b/spec/controllers/profiles/preferences_controller_spec.rb index 87503b1ed4..646aa0320b 100644 --- a/spec/controllers/profiles/preferences_controller_spec.rb +++ b/spec/controllers/profiles/preferences_controller_spec.rb @@ -25,6 +25,7 @@ describe Profiles::PreferencesController do def go(params: {}, format: :js) params.reverse_merge!( color_scheme_id: '1', + dashboard: 'stars', theme_id: '1' ) @@ -40,6 +41,7 @@ describe Profiles::PreferencesController do it "changes the user's preferences" do prefs = { color_scheme_id: '1', + dashboard: 'stars', theme_id: '2' }.with_indifferent_access diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9ff4288684..f3e278e5c5 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -50,12 +50,13 @@ # bitbucket_access_token :string(255) # bitbucket_access_token_secret :string(255) # location :string(255) +# public_email :string(255) default(""), not null # encrypted_otp_secret :string(255) # encrypted_otp_secret_iv :string(255) # encrypted_otp_secret_salt :string(255) # otp_required_for_login :boolean # otp_backup_codes :text -# public_email :string(255) default(""), not null +# dashboard :integer default(0) # require 'spec_helper' From 94d3c1433df9380ca83f1f35a540074ff0690410 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 03:59:39 -0400 Subject: [PATCH 26/71] Add RootController This controller is now the target for `root_url`. It sub-classes DashboardController so we can render the old default without a redirect if the user hasn't customized their dashboard location. --- app/controllers/root_controller.rb | 18 ++++++++++++ app/views/layouts/nav/_dashboard.html.haml | 2 +- config/routes.rb | 2 +- spec/controllers/root_controller_spec.rb | 32 ++++++++++++++++++++++ spec/routing/routing_spec.rb | 10 +++++-- 5 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 app/controllers/root_controller.rb create mode 100644 spec/controllers/root_controller_spec.rb diff --git a/app/controllers/root_controller.rb b/app/controllers/root_controller.rb new file mode 100644 index 0000000000..7606d2d0fb --- /dev/null +++ b/app/controllers/root_controller.rb @@ -0,0 +1,18 @@ +# RootController +# +# This controller exists solely to handle requests to `root_url`. When a user is +# logged in and has customized their `dashboard` setting, they will be +# redirected to their preferred location. +# +# For users who haven't customized the setting, we simply delegate to +# `DashboardController#show`, which is the default. +class RootController < DashboardController + def show + case current_user.try(:dashboard) + when 'stars' + redirect_to starred_dashboard_projects_path + else + super + end + end +end diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml index d46dba4a24..83e6fe863f 100644 --- a/app/views/layouts/nav/_dashboard.html.haml +++ b/app/views/layouts/nav/_dashboard.html.haml @@ -1,6 +1,6 @@ %ul.nav.nav-sidebar = nav_link(path: 'dashboard#show', html_options: {class: 'home'}) do - = link_to root_path, title: 'Home', class: 'shortcuts-activity', data: {placement: 'right'} do + = link_to dashboard_path, title: 'Home', class: 'shortcuts-activity', data: {placement: 'right'} do = icon('dashboard fw') %span Your Projects diff --git a/config/routes.rb b/config/routes.rb index 52c98541da..d60bc796fd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -293,7 +293,7 @@ Gitlab::Application.routes.draw do get '/users/auth/:provider/omniauth_error' => 'omniauth_callbacks#omniauth_error', as: :omniauth_error end - root to: "dashboard#show" + root to: "root#show" # # Project Area diff --git a/spec/controllers/root_controller_spec.rb b/spec/controllers/root_controller_spec.rb new file mode 100644 index 0000000000..abbbf6855f --- /dev/null +++ b/spec/controllers/root_controller_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe RootController do + describe 'GET show' do + context 'with a user' do + let(:user) { create(:user) } + + before do + sign_in(user) + allow(subject).to receive(:current_user).and_return(user) + end + + context 'who has customized their dashboard setting' do + before do + user.update_attribute(:dashboard, 'stars') + end + + it 'redirects to their specified dashboard' do + get :show + expect(response).to redirect_to starred_dashboard_projects_path + end + end + + context 'who uses the default dashboard setting' do + it 'renders the default dashboard' do + get :show + expect(response).to render_template 'dashboard/show' + end + end + end + end +end diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 199851be48..f268e4755d 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -204,11 +204,9 @@ end # dashboard GET /dashboard(.:format) dashboard#show # dashboard_issues GET /dashboard/issues(.:format) dashboard#issues # dashboard_merge_requests GET /dashboard/merge_requests(.:format) dashboard#merge_requests -# root / dashboard#show describe DashboardController, "routing" do it "to #index" do expect(get("/dashboard")).to route_to('dashboard#show') - expect(get("/")).to route_to('dashboard#show') end it "to #issues" do @@ -220,6 +218,14 @@ describe DashboardController, "routing" do end end +# root / root#show +describe RootController, 'routing' do + it 'to #show' do + expect(get('/')).to route_to('root#show') + end +end + + # new_user_session GET /users/sign_in(.:format) devise/sessions#new # user_session POST /users/sign_in(.:format) devise/sessions#create # destroy_user_session DELETE /users/sign_out(.:format) devise/sessions#destroy From 6de3958364f8d2adb68b8beecd53e4af6b17c353 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 04:36:09 -0400 Subject: [PATCH 27/71] Account for RootController for dashboard navigation and Dispatch JS --- app/assets/javascripts/dispatcher.js.coffee | 2 +- app/views/layouts/nav/_dashboard.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index da56e3cdbc..b7ebe6a5c8 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -55,7 +55,7 @@ class Dispatcher when 'projects:merge_requests:index' shortcut_handler = new ShortcutsNavigation() MergeRequests.init() - when 'dashboard:show' + when 'dashboard:show', 'root:show' new Dashboard() new Activities() when 'dashboard:projects:starred' diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml index 83e6fe863f..687c1fc3dd 100644 --- a/app/views/layouts/nav/_dashboard.html.haml +++ b/app/views/layouts/nav/_dashboard.html.haml @@ -1,5 +1,5 @@ %ul.nav.nav-sidebar - = nav_link(path: 'dashboard#show', html_options: {class: 'home'}) do + = nav_link(path: ['dashboard#show', 'root#show'], html_options: {class: 'home'}) do = link_to dashboard_path, title: 'Home', class: 'shortcuts-activity', data: {placement: 'right'} do = icon('dashboard fw') %span From 1eb9a02f21d45f2fa301576723bbb0a23b5ba22d Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 04:42:02 -0400 Subject: [PATCH 28/71] Add a form field to customize the dashboard preference --- app/helpers/preferences_helper.rb | 19 +++++++++++++++++++ app/views/profiles/preferences/show.html.haml | 19 +++++++++++++++++-- spec/helpers/preferences_helper_spec.rb | 17 +++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb index 04873b9bd0..c67a34270f 100644 --- a/app/helpers/preferences_helper.rb +++ b/app/helpers/preferences_helper.rb @@ -20,6 +20,25 @@ module PreferencesHelper COLOR_SCHEMES.freeze end + # Populates the dashboard preference select field with more user-friendly + # values. + def dashboard_choices + orig = User.dashboards.keys + + choices = [ + ['Projects (default)', orig[0]], + ['Starred Projects', orig[1]] + ] + + if orig.size != choices.size + # Assure that anyone adding new options updates this method too + raise RuntimeError, "`User` defines #{orig.size} dashboard choices," + + " but #{__method__} defined #{choices.size}" + else + choices + end + end + def user_application_theme theme = Gitlab::Themes.by_id(current_user.try(:theme_id)) theme.css_class diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 680c0930bb..8f7c57c12b 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -1,8 +1,10 @@ -- page_title "Design" +- page_title 'Preferences' %h3.page-title = page_title %p.light - Appearance settings will be saved to your profile and made available across all devices. + These settings allow you to customize the appearance and behavior of the site. + They are saved with your account and will persist to any device you use to + access the site. %hr = form_for @user, url: profile_preferences_path, remote: true, method: :put, html: {class: 'js-preferences-form'} do |f| @@ -25,3 +27,16 @@ .preview= image_tag "#{color_scheme}-scheme-preview.png" = f.radio_button :color_scheme_id, color_scheme_id = color_scheme.tr('-_', ' ').titleize + + .panel.panel-default + .panel-heading + Behavior + .panel-body + .form-group + = f.label :dashboard, class: 'control-label' + .col-sm-10 + = f.select :dashboard, dashboard_choices, {}, class: 'form-control' + %p.help-block.hint + This setting allows you to customize the default Dashboard page. + .panel-footer + = f.submit 'Save', class: 'btn btn-save' diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb index 095b016e6e..e8d8c4ceb8 100644 --- a/spec/helpers/preferences_helper_spec.rb +++ b/spec/helpers/preferences_helper_spec.rb @@ -30,6 +30,23 @@ describe PreferencesHelper do end end + describe 'dashboard_choices' do + it 'raises an exception when defined choices may be missing' do + dashboards = User.dashboards + expect(User).to receive(:dashboards). + and_return(dashboards.merge(foo: 'foo')) + + expect { dashboard_choices }.to raise_error + end + + it 'provides better option descriptions' do + choices = dashboard_choices + + expect(choices[0]).to eq ['Projects (default)', 'projects'] + expect(choices[1]).to eq ['Starred Projects', 'stars'] + end + end + describe 'user_color_scheme_class' do context 'with current_user is nil' do it 'should return a string' do From bf9dd4327e36b6ab0b5440dcff747fff27aa5c22 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 17:08:10 -0400 Subject: [PATCH 29/71] Add feature specs for default dashboard preference --- app/views/profiles/preferences/show.html.haml | 2 +- app/views/profiles/preferences/update.js.erb | 5 ++ spec/features/profiles/preferences_spec.rb | 59 +++++++++++++++---- 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 8f7c57c12b..547977596f 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -33,7 +33,7 @@ Behavior .panel-body .form-group - = f.label :dashboard, class: 'control-label' + = f.label :dashboard, 'Default Dashboard', class: 'control-label' .col-sm-10 = f.select :dashboard, dashboard_choices, {}, class: 'form-control' %p.help-block.hint diff --git a/app/views/profiles/preferences/update.js.erb b/app/views/profiles/preferences/update.js.erb index e952d8f47e..6c4b0ce757 100644 --- a/app/views/profiles/preferences/update.js.erb +++ b/app/views/profiles/preferences/update.js.erb @@ -1,4 +1,9 @@ // Remove body class for any previous theme, re-add current one $('body').removeClass('<%= Gitlab::Themes.body_classes %>') $('body').addClass('<%= user_application_theme %>') + +// Re-enable the "Save" button +$('input[type=submit]').enable() + +// Show the notice flash message new Flash('<%= flash.discard(:notice) %>', 'notice') diff --git a/spec/features/profiles/preferences_spec.rb b/spec/features/profiles/preferences_spec.rb index a946064a87..1f07fde7af 100644 --- a/spec/features/profiles/preferences_spec.rb +++ b/spec/features/profiles/preferences_spec.rb @@ -5,22 +5,25 @@ describe 'Profile > Preferences' do before do login_as(user) + visit profile_preferences_path end describe 'User changes their application theme', js: true do let(:default) { Gitlab::Themes.default } let(:theme) { Gitlab::Themes.by_id(5) } - before do - visit profile_preferences_path - end - it 'creates a flash message' do choose "user_theme_id_#{theme.id}" - within('.flash-container') do - expect(page).to have_content('Preferences saved.') - end + expect_preferences_saved_message + end + + it 'updates their preference' do + choose "user_theme_id_#{theme.id}" + + visit page.current_path + + expect(page).to have_checked_field("user_theme_id_#{theme.id}") end it 'reflects the changes immediately' do @@ -33,9 +36,45 @@ describe 'Profile > Preferences' do end end - describe 'User changes their syntax highlighting theme' do - before do - visit profile_preferences_path + describe 'User changes their syntax highlighting theme', js: true do + it 'creates a flash message' do + choose 'user_color_scheme_id_5' + + expect_preferences_saved_message + end + + it 'updates their preference' do + choose 'user_color_scheme_id_5' + + visit page.current_path + + expect(page).to have_checked_field('user_color_scheme_id_5') + end + end + + describe 'User changes their default dashboard' do + it 'creates a flash message' do + select 'Starred Projects', from: 'user_dashboard' + click_button 'Save' + + expect_preferences_saved_message + end + + it 'updates their preference' do + select 'Starred Projects', from: 'user_dashboard' + click_button 'Save' + + click_link 'Dashboard' + expect(page.current_path).to eq starred_dashboard_projects_path + + click_link 'Your Projects' + expect(page.current_path).to eq dashboard_path + end + end + + def expect_preferences_saved_message + within('.flash-container') do + expect(page).to have_content('Preferences saved.') end end end From fb5271ddf11be8074d5882b86a4b4dfec12150d4 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 17:38:11 -0400 Subject: [PATCH 30/71] Make the dashboard choice text match the text in the sidebar --- app/helpers/preferences_helper.rb | 4 ++-- spec/helpers/preferences_helper_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb index c67a34270f..6a894186ea 100644 --- a/app/helpers/preferences_helper.rb +++ b/app/helpers/preferences_helper.rb @@ -26,8 +26,8 @@ module PreferencesHelper orig = User.dashboards.keys choices = [ - ['Projects (default)', orig[0]], - ['Starred Projects', orig[1]] + ['Your Projects (default)', orig[0]], + ['Starred Projects', orig[1]] ] if orig.size != choices.size diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb index e8d8c4ceb8..32a9593cc0 100644 --- a/spec/helpers/preferences_helper_spec.rb +++ b/spec/helpers/preferences_helper_spec.rb @@ -42,8 +42,8 @@ describe PreferencesHelper do it 'provides better option descriptions' do choices = dashboard_choices - expect(choices[0]).to eq ['Projects (default)', 'projects'] - expect(choices[1]).to eq ['Starred Projects', 'stars'] + expect(choices[0]).to eq ['Your Projects (default)', 'projects'] + expect(choices[1]).to eq ['Starred Projects', 'stars'] end end From 1562f017b6305eeef2ac83e370660699d8789965 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 18:19:06 -0400 Subject: [PATCH 31/71] Spec the failure cases for PreferencesController#update --- .../profiles/preferences_controller.rb | 13 ++++++++---- .../profiles/preferences_controller_spec.rb | 20 +++++++++++++++++-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb index e43a247f72..538b09ca54 100644 --- a/app/controllers/profiles/preferences_controller.rb +++ b/app/controllers/profiles/preferences_controller.rb @@ -5,10 +5,15 @@ class Profiles::PreferencesController < Profiles::ApplicationController end def update - if @user.update_attributes(preferences_params) - flash[:notice] = 'Preferences saved.' - else - # TODO (rspeicher): There's no validation on these values, so can it fail? + begin + if @user.update_attributes(preferences_params) + flash[:notice] = 'Preferences saved.' + else + flash[:alert] = 'Failed to save preferences.' + end + rescue ArgumentError => e + # Raised when `dashboard` is given an invalid value. + flash[:alert] = "Failed to save preferences (#{e.message})." end respond_to do |format| diff --git a/spec/controllers/profiles/preferences_controller_spec.rb b/spec/controllers/profiles/preferences_controller_spec.rb index 646aa0320b..1f0943c93d 100644 --- a/spec/controllers/profiles/preferences_controller_spec.rb +++ b/spec/controllers/profiles/preferences_controller_spec.rb @@ -51,8 +51,24 @@ describe Profiles::PreferencesController do end end - context 'on unsuccessful update' do - # TODO (rspeicher): Can this happen? + context 'on failed update' do + it 'sets the flash' do + expect(user).to receive(:update_attributes).and_return(false) + + go + + expect(flash[:alert]).to eq('Failed to save preferences.') + end + end + + context 'on invalid dashboard setting' do + it 'sets the flash' do + prefs = {dashboard: 'invalid'} + + go params: prefs + + expect(flash[:alert]).to match(/\AFailed to save preferences \(.+\)\.\z/) + end end context 'as js' do From b9c85393c36027f31bb2e4fd6861cd034fef7fcc Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 17:37:48 -0400 Subject: [PATCH 32/71] Add docs for Profile > Preferences Also converts doc/README.md to Unix line endings --- doc/README.md | 71 +++++++++++++++++++------------------- doc/profile/preferences.md | 33 ++++++++++++++++++ doc/profile/profile.md | 3 ++ 3 files changed, 72 insertions(+), 35 deletions(-) create mode 100644 doc/profile/preferences.md create mode 100644 doc/profile/profile.md diff --git a/doc/README.md b/doc/README.md index 7a2181edde..451abf8b79 100644 --- a/doc/README.md +++ b/doc/README.md @@ -1,35 +1,36 @@ -# Documentation - -## User documentation - -- [API](api/README.md) Automate GitLab via a simple and powerful API. -- [GitLab as OAuth2 authentication service provider](integration/oauth_provider.md). It allows you to login to other applications from GitLab. -- [Importing to GitLab](workflow/importing/README.md). -- [Markdown](markdown/markdown.md) GitLab's advanced formatting system. -- [Permissions](permissions/permissions.md) Learn what each role in a project (guest/reporter/developer/master/owner) can do. -- [Project Services](project_services/project_services.md) Integrate a project with external services, such as CI and chat. -- [Public access](public_access/public_access.md) Learn how you can allow public and internal access to projects. -- [SSH](ssh/README.md) Setup your ssh keys and deploy keys for secure access to your projects. -- [Web hooks](web_hooks/web_hooks.md) Let GitLab notify you when new code has been pushed to your project. -- [Workflow](workflow/README.md) Using GitLab functionality and importing projects from GitHub and SVN. - -## Administrator documentation - -- [Custom git hooks](hooks/custom_hooks.md) Custom git hooks (on the filesystem) for when web hooks aren't enough. -- [Install](install/README.md) Requirements, directory structures and installation from source. -- [Integration](integration/README.md) How to integrate with systems such as JIRA, Redmine, LDAP and Twitter. -- [Issue closing](customization/issue_closing.md) Customize how to close an issue from commit messages. -- [Libravatar](customization/libravatar.md) Use Libravatar for user avatars. -- [Log system](logs/logs.md) Log system. -- [Operations](operations/README.md) Keeping GitLab up and running -- [Raketasks](raketasks/README.md) Backups, maintenance, automatic web hook setup and the importing of projects. -- [Security](security/README.md) Learn what you can do to further secure your GitLab instance. -- [System hooks](system_hooks/system_hooks.md) Notifications when users, projects and keys are changed. -- [Update](update/README.md) Update guides to upgrade your installation. -- [Welcome message](customization/welcome_message.md) Add a custom welcome message to the sign-in page. - -## Contributor documentation - -- [Development](development/README.md) Explains the architecture and the guidelines for shell commands. -- [Legal](legal/README.md) Contributor license agreements. -- [Release](release/README.md) How to make the monthly and security releases. \ No newline at end of file +# Documentation + +## User documentation + +- [API](api/README.md) Automate GitLab via a simple and powerful API. +- [GitLab as OAuth2 authentication service provider](integration/oauth_provider.md). It allows you to login to other applications from GitLab. +- [Importing to GitLab](workflow/importing/README.md). +- [Markdown](markdown/markdown.md) GitLab's advanced formatting system. +- [Permissions](permissions/permissions.md) Learn what each role in a project (guest/reporter/developer/master/owner) can do. +- [Profile Settings](profile/profile.md) +- [Project Services](project_services/project_services.md) Integrate a project with external services, such as CI and chat. +- [Public access](public_access/public_access.md) Learn how you can allow public and internal access to projects. +- [SSH](ssh/README.md) Setup your ssh keys and deploy keys for secure access to your projects. +- [Web hooks](web_hooks/web_hooks.md) Let GitLab notify you when new code has been pushed to your project. +- [Workflow](workflow/README.md) Using GitLab functionality and importing projects from GitHub and SVN. + +## Administrator documentation + +- [Custom git hooks](hooks/custom_hooks.md) Custom git hooks (on the filesystem) for when web hooks aren't enough. +- [Install](install/README.md) Requirements, directory structures and installation from source. +- [Integration](integration/README.md) How to integrate with systems such as JIRA, Redmine, LDAP and Twitter. +- [Issue closing](customization/issue_closing.md) Customize how to close an issue from commit messages. +- [Libravatar](customization/libravatar.md) Use Libravatar for user avatars. +- [Log system](logs/logs.md) Log system. +- [Operations](operations/README.md) Keeping GitLab up and running +- [Raketasks](raketasks/README.md) Backups, maintenance, automatic web hook setup and the importing of projects. +- [Security](security/README.md) Learn what you can do to further secure your GitLab instance. +- [System hooks](system_hooks/system_hooks.md) Notifications when users, projects and keys are changed. +- [Update](update/README.md) Update guides to upgrade your installation. +- [Welcome message](customization/welcome_message.md) Add a custom welcome message to the sign-in page. + +## Contributor documentation + +- [Development](development/README.md) Explains the architecture and the guidelines for shell commands. +- [Legal](legal/README.md) Contributor license agreements. +- [Release](release/README.md) How to make the monthly and security releases. diff --git a/doc/profile/preferences.md b/doc/profile/preferences.md new file mode 100644 index 0000000000..0c12eb0c65 --- /dev/null +++ b/doc/profile/preferences.md @@ -0,0 +1,33 @@ +# Profile Preferences + +Settings in the **Profile > Preferences** page allow the user to customize +various aspects of the site to their liking. + +## Application theme + +Changing this settings allows the user to customize the color scheme used for +the navigation bar on the left side of the screen. + +The default is **Charcoal**. + +## Syntax highlighting theme + +Changing this setting allows the user to customize the theme used when viewing +syntax highlighted code on the site. + +The default is **White**. + +## Behavior + +### Default Dashboard + +For users who have access to a large number of projects but only keep up with a +select few, the amount of activity on the default Dashboard page can be +overwhelming. + +Changing this setting to allows the user to redefine what their default +dashboard will be. Setting it to **Starred Projects** will make that Dashboard +view the default when signing in or clicking the application logo in the upper +left. + +The default is **Projects**. diff --git a/doc/profile/profile.md b/doc/profile/profile.md new file mode 100644 index 0000000000..032d62cf88 --- /dev/null +++ b/doc/profile/profile.md @@ -0,0 +1,3 @@ +# Profile Settings + +- [Preferences](preferences.md) From cdf893bc1597c21b90f8c401bacf79a7c70787fb Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 11 Jun 2015 22:54:21 -0400 Subject: [PATCH 33/71] Move 2FA docs from "Workflow" to "Profile Settings" --- doc/README.md | 2 +- doc/{workflow => profile}/2fa.png | Bin doc/{workflow => profile}/2fa_auth.png | Bin doc/profile/README.md | 4 +++ doc/profile/profile.md | 3 -- .../two_factor_authentication.md | 0 doc/workflow/README.md | 31 +++++++++--------- 7 files changed, 20 insertions(+), 20 deletions(-) rename doc/{workflow => profile}/2fa.png (100%) rename doc/{workflow => profile}/2fa_auth.png (100%) create mode 100644 doc/profile/README.md delete mode 100644 doc/profile/profile.md rename doc/{workflow => profile}/two_factor_authentication.md (100%) diff --git a/doc/README.md b/doc/README.md index 451abf8b79..2845961325 100644 --- a/doc/README.md +++ b/doc/README.md @@ -7,7 +7,7 @@ - [Importing to GitLab](workflow/importing/README.md). - [Markdown](markdown/markdown.md) GitLab's advanced formatting system. - [Permissions](permissions/permissions.md) Learn what each role in a project (guest/reporter/developer/master/owner) can do. -- [Profile Settings](profile/profile.md) +- [Profile Settings](profile/README.md) - [Project Services](project_services/project_services.md) Integrate a project with external services, such as CI and chat. - [Public access](public_access/public_access.md) Learn how you can allow public and internal access to projects. - [SSH](ssh/README.md) Setup your ssh keys and deploy keys for secure access to your projects. diff --git a/doc/workflow/2fa.png b/doc/profile/2fa.png similarity index 100% rename from doc/workflow/2fa.png rename to doc/profile/2fa.png diff --git a/doc/workflow/2fa_auth.png b/doc/profile/2fa_auth.png similarity index 100% rename from doc/workflow/2fa_auth.png rename to doc/profile/2fa_auth.png diff --git a/doc/profile/README.md b/doc/profile/README.md new file mode 100644 index 0000000000..6f8359d87f --- /dev/null +++ b/doc/profile/README.md @@ -0,0 +1,4 @@ +# Profile Settings + +- [Preferences](preferences.md) +- [Two-factor Authentication (2FA)](two_factor_authentication.md) diff --git a/doc/profile/profile.md b/doc/profile/profile.md deleted file mode 100644 index 032d62cf88..0000000000 --- a/doc/profile/profile.md +++ /dev/null @@ -1,3 +0,0 @@ -# Profile Settings - -- [Preferences](preferences.md) diff --git a/doc/workflow/two_factor_authentication.md b/doc/profile/two_factor_authentication.md similarity index 100% rename from doc/workflow/two_factor_authentication.md rename to doc/profile/two_factor_authentication.md diff --git a/doc/workflow/README.md b/doc/workflow/README.md index 70a8179c8e..f1959d3013 100644 --- a/doc/workflow/README.md +++ b/doc/workflow/README.md @@ -1,16 +1,15 @@ -# Workflow - -- [Authorization for merge requests](authorization_for_merge_requests.md) -- [Change your time zone](timezone.md) -- [Feature branch workflow](workflow.md) -- [GitLab Flow](gitlab_flow.md) -- [Groups](groups.md) -- [Keyboard shortcuts](shortcuts.md) -- [Labels](labels.md) -- [Notifications](notifications.md) -- [Project Features](project_features.md) -- [Project forking workflow](forking_workflow.md) -- [Protected branches](protected_branches.md) -- [Two-factor Authentication (2FA)](two_factor_authentication.md) -- [Web Editor](web_editor.md) -- ["Work In Progress" Merge Requests](wip_merge_requests.md) \ No newline at end of file +# Workflow + +- [Authorization for merge requests](authorization_for_merge_requests.md) +- [Change your time zone](timezone.md) +- [Feature branch workflow](workflow.md) +- [GitLab Flow](gitlab_flow.md) +- [Groups](groups.md) +- [Keyboard shortcuts](shortcuts.md) +- [Labels](labels.md) +- [Notifications](notifications.md) +- [Project Features](project_features.md) +- [Project forking workflow](forking_workflow.md) +- [Protected branches](protected_branches.md) +- [Web Editor](web_editor.md) +- ["Work In Progress" Merge Requests](wip_merge_requests.md) From 13d9544d5de831794c0a7647203e26df3b6b9471 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 18:23:18 -0400 Subject: [PATCH 34/71] CHANGELOG for custom Dashboard page --- CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index c5a625ed1d..cca283a04c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -54,6 +54,8 @@ v 7.12.0 (unreleased) - Improve group removing logic - Trigger create-hooks on backup restore task - Add option to automatically link omniauth and LDAP identities + - Rename "Design" profile settings page to "Preferences". + - Allow users to customize their default Dashboard page. v 7.11.4 - Fix missing bullets when creating lists From d0463d2c6a275559f187df4b45e1e62aaffe4ec9 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 11 Jun 2015 23:08:47 -0400 Subject: [PATCH 35/71] Fix alignment of Behavior form; add documentation link --- app/views/profiles/preferences/show.html.haml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 547977596f..073e8f22aa 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -7,7 +7,7 @@ access the site. %hr -= form_for @user, url: profile_preferences_path, remote: true, method: :put, html: {class: 'js-preferences-form'} do |f| += form_for @user, url: profile_preferences_path, remote: true, method: :put, html: {class: 'js-preferences-form form-horizontal'} do |f| .panel.panel-default.application-theme .panel-heading Application theme @@ -33,7 +33,9 @@ Behavior .panel-body .form-group - = f.label :dashboard, 'Default Dashboard', class: 'control-label' + = f.label :dashboard, class: 'control-label' do + Default Dashboard + = link_to('(?)', help_page_path('profile', 'preferences') + '#default-dashboard', target: '_blank') .col-sm-10 = f.select :dashboard, dashboard_choices, {}, class: 'form-control' %p.help-block.hint From c0cb77e413fa38625a96323a527ff4dc56d59573 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 12 Jun 2015 20:20:53 -0400 Subject: [PATCH 36/71] Remove redundant help text from custom dashboard selection --- app/views/profiles/preferences/show.html.haml | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 073e8f22aa..aa99280fde 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -38,7 +38,5 @@ = link_to('(?)', help_page_path('profile', 'preferences') + '#default-dashboard', target: '_blank') .col-sm-10 = f.select :dashboard, dashboard_choices, {}, class: 'form-control' - %p.help-block.hint - This setting allows you to customize the default Dashboard page. .panel-footer = f.submit 'Save', class: 'btn btn-save' From d2894a39e5b3a8504cb290a2f41b0aac5fc8b0a8 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 12 Jun 2015 20:39:48 -0400 Subject: [PATCH 37/71] Refactor dashboard_choices --- app/helpers/preferences_helper.rb | 29 ++++++++++++++----------- spec/helpers/preferences_helper_spec.rb | 18 ++++++++------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb index 6a894186ea..bceff4fd52 100644 --- a/app/helpers/preferences_helper.rb +++ b/app/helpers/preferences_helper.rb @@ -20,22 +20,25 @@ module PreferencesHelper COLOR_SCHEMES.freeze end - # Populates the dashboard preference select field with more user-friendly - # values. + # Maps `dashboard` values to more user-friendly option text + DASHBOARD_CHOICES = { + projects: 'Your Projects (default)', + stars: 'Starred Projects' + }.with_indifferent_access.freeze + + # Returns an Array usable by a select field for more user-friendly option text def dashboard_choices - orig = User.dashboards.keys + defined = User.dashboards - choices = [ - ['Your Projects (default)', orig[0]], - ['Starred Projects', orig[1]] - ] - - if orig.size != choices.size - # Assure that anyone adding new options updates this method too - raise RuntimeError, "`User` defines #{orig.size} dashboard choices," + - " but #{__method__} defined #{choices.size}" + if defined.size != DASHBOARD_CHOICES.size + # Ensure that anyone adding new options updates this method too + raise RuntimeError, "`User` defines #{defined.size} dashboard choices," + + " but `DASHBOARD_CHOICES` defined #{DASHBOARD_CHOICES.size}." else - choices + defined.map do |key, _| + # Use `fetch` so `KeyError` gets raised when a key is missing + [DASHBOARD_CHOICES.fetch(key), key] + end end end diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb index 32a9593cc0..920de8c432 100644 --- a/spec/helpers/preferences_helper_spec.rb +++ b/spec/helpers/preferences_helper_spec.rb @@ -32,18 +32,20 @@ describe PreferencesHelper do describe 'dashboard_choices' do it 'raises an exception when defined choices may be missing' do - dashboards = User.dashboards - expect(User).to receive(:dashboards). - and_return(dashboards.merge(foo: 'foo')) + expect(User).to receive(:dashboards).and_return(foo: 'foo') + expect { dashboard_choices }.to raise_error(RuntimeError) + end - expect { dashboard_choices }.to raise_error + it 'raises an exception when defined choices may be using the wrong key' do + expect(User).to receive(:dashboards).and_return(foo: 'foo', bar: 'bar') + expect { dashboard_choices }.to raise_error(KeyError) end it 'provides better option descriptions' do - choices = dashboard_choices - - expect(choices[0]).to eq ['Your Projects (default)', 'projects'] - expect(choices[1]).to eq ['Starred Projects', 'stars'] + expect(dashboard_choices).to match_array [ + ['Your Projects (default)', 'projects'], + ['Starred Projects', 'stars'] + ] end end From 8ae13c7a51556a3bd4aa2f0eefc2693972166aa8 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 12 Jun 2015 20:53:58 -0400 Subject: [PATCH 38/71] Refactor RootController --- app/controllers/root_controller.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/controllers/root_controller.rb b/app/controllers/root_controller.rb index 7606d2d0fb..fdfe00dc13 100644 --- a/app/controllers/root_controller.rb +++ b/app/controllers/root_controller.rb @@ -7,12 +7,22 @@ # For users who haven't customized the setting, we simply delegate to # `DashboardController#show`, which is the default. class RootController < DashboardController + before_action :redirect_to_custom_dashboard, only: [:show] + def show - case current_user.try(:dashboard) + super + end + + private + + def redirect_to_custom_dashboard + return unless current_user + + case current_user.dashboard when 'stars' redirect_to starred_dashboard_projects_path else - super + return end end end From 75a40ad5bcee37f43fcfe89789dbd8c65be56c21 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 13 Jun 2015 18:19:24 -0400 Subject: [PATCH 39/71] Change `foo.should_not` syntax to `expect(foo).not_to` in specs --- spec/lib/gitlab/ldap/access_spec.rb | 2 +- spec/models/project_services/irker_service_spec.rb | 2 +- spec/services/destroy_group_service_spec.rb | 4 ++-- spec/services/projects/destroy_service_spec.rb | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 2189e313d6..fc346ea29d 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -56,7 +56,7 @@ describe Gitlab::LDAP::Access do it "should unblock user in GitLab" do access.allowed? - user.should_not be_blocked + expect(user).not_to be_blocked end end end diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index 49face26bb..96ab95c05c 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -43,7 +43,7 @@ describe IrkerService do let(:_recipients) { 'a b c d' } it 'should add an error if there is too many recipients' do subject.send :check_recipients_count - subject.errors.should_not be_blank + expect(subject.errors).not_to be_blank end end diff --git a/spec/services/destroy_group_service_spec.rb b/spec/services/destroy_group_service_spec.rb index 24e439503e..2a7372b986 100644 --- a/spec/services/destroy_group_service_spec.rb +++ b/spec/services/destroy_group_service_spec.rb @@ -12,8 +12,8 @@ describe DestroyGroupService do destroy_group(group, user) end - it { Group.all.should_not include(group) } - it { Project.all.should_not include(project) } + it { expect(Group.all).not_to include(group) } + it { expect(Project.all).not_to include(project) } end context 'file system' do diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index cdf576cc0c..381008def5 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -12,7 +12,7 @@ describe Projects::DestroyService do Sidekiq::Testing.inline! { destroy_project(project, user, {}) } end - it { Project.all.should_not include(project) } + it { expect(Project.all).not_to include(project) } it { Dir.exists?(path).should be_falsey } it { Dir.exists?(remove_path).should be_falsey } end @@ -23,7 +23,7 @@ describe Projects::DestroyService do Sidekiq::Testing.fake! { destroy_project(project, user, {}) } end - it { Project.all.should_not include(project) } + it { expect(Project.all).not_to include(project) } it { Dir.exists?(path).should be_falsey } it { Dir.exists?(remove_path).should be_truthy } end From 422236c71eb8f1c88e83331b6eb74211fd7ccf49 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 13 Jun 2015 18:24:51 -0400 Subject: [PATCH 40/71] Change `foo.should` syntax to `expect(foo).to` in specs --- spec/controllers/autocomplete_controller_spec.rb | 16 ++++++++-------- spec/features/notes_on_merge_requests_spec.rb | 2 +- spec/lib/gitlab/ldap/access_spec.rb | 4 ++-- spec/lib/gitlab/o_auth/auth_hash_spec.rb | 14 +++++++------- spec/models/external_wiki_service_spec.rb | 2 +- .../project_services/irker_service_spec.rb | 8 ++++---- spec/requests/api/issues_spec.rb | 8 ++++---- spec/requests/api/merge_requests_spec.rb | 6 +++--- spec/requests/api/milestones_spec.rb | 6 +++--- spec/requests/api/projects_spec.rb | 6 +++--- spec/services/destroy_group_service_spec.rb | 8 ++++---- spec/services/projects/destroy_service_spec.rb | 8 ++++---- spec/services/projects/transfer_service_spec.rb | 6 ++---- 13 files changed, 46 insertions(+), 48 deletions(-) diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index a0909cec3b..1ea1227b28 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -15,9 +15,9 @@ describe AutocompleteController do let(:body) { JSON.parse(response.body) } - it { body.should be_kind_of(Array) } - it { body.size.should eq(1) } - it { body.first["username"].should == user.username } + it { expect(body).to be_kind_of(Array) } + it { expect(body.size).to eq(1) } + it { expect(body.first["username"]).to eq user.username } end context 'group members' do @@ -32,9 +32,9 @@ describe AutocompleteController do let(:body) { JSON.parse(response.body) } - it { body.should be_kind_of(Array) } - it { body.size.should eq(1) } - it { body.first["username"].should == user.username } + it { expect(body).to be_kind_of(Array) } + it { expect(body.size).to eq(1) } + it { expect(body.first["username"]).to eq user.username } end context 'all users' do @@ -45,7 +45,7 @@ describe AutocompleteController do let(:body) { JSON.parse(response.body) } - it { body.should be_kind_of(Array) } - it { body.size.should eq(User.count) } + it { expect(body).to be_kind_of(Array) } + it { expect(body.size).to eq(User.count) } end end diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index c47368b1fd..b69b59d415 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -90,7 +90,7 @@ describe 'Comments' do #within(".current-note-edit-form") do #fill_in "note[note]", with: "Some new content" #find(".btn-cancel").click - #find(".js-note-text", visible: false).text.should == note.note + #expect(find(".js-note-text", visible: false).text).to eq note.note #end #end diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index fc346ea29d..038ac7e0d7 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -23,7 +23,7 @@ describe Gitlab::LDAP::Access do it "should block user in GitLab" do access.allowed? - user.should be_blocked + expect(user).to be_blocked end end @@ -44,7 +44,7 @@ describe Gitlab::LDAP::Access do it "does not unblock user in GitLab" do access.allowed? - user.should be_blocked + expect(user).to be_blocked end end diff --git a/spec/lib/gitlab/o_auth/auth_hash_spec.rb b/spec/lib/gitlab/o_auth/auth_hash_spec.rb index 678086ffa1..165cde4f16 100644 --- a/spec/lib/gitlab/o_auth/auth_hash_spec.rb +++ b/spec/lib/gitlab/o_auth/auth_hash_spec.rb @@ -80,31 +80,31 @@ describe Gitlab::OAuth::AuthHash do context 'auth_hash constructed with ASCII-8BIT encoding' do it 'forces utf8 encoding on uid' do - auth_hash.uid.encoding.should eql Encoding::UTF_8 + expect(auth_hash.uid.encoding).to eql Encoding::UTF_8 end it 'forces utf8 encoding on provider' do - auth_hash.provider.encoding.should eql Encoding::UTF_8 + expect(auth_hash.provider.encoding).to eql Encoding::UTF_8 end it 'forces utf8 encoding on name' do - auth_hash.name.encoding.should eql Encoding::UTF_8 + expect(auth_hash.name.encoding).to eql Encoding::UTF_8 end it 'forces utf8 encoding on full_name' do - auth_hash.full_name.encoding.should eql Encoding::UTF_8 + expect(auth_hash.full_name.encoding).to eql Encoding::UTF_8 end it 'forces utf8 encoding on username' do - auth_hash.username.encoding.should eql Encoding::UTF_8 + expect(auth_hash.username.encoding).to eql Encoding::UTF_8 end it 'forces utf8 encoding on email' do - auth_hash.email.encoding.should eql Encoding::UTF_8 + expect(auth_hash.email.encoding).to eql Encoding::UTF_8 end it 'forces utf8 encoding on password' do - auth_hash.password.encoding.should eql Encoding::UTF_8 + expect(auth_hash.password.encoding).to eql Encoding::UTF_8 end end end diff --git a/spec/models/external_wiki_service_spec.rb b/spec/models/external_wiki_service_spec.rb index f2e77fc88c..4bd5b0be61 100644 --- a/spec/models/external_wiki_service_spec.rb +++ b/spec/models/external_wiki_service_spec.rb @@ -52,7 +52,7 @@ describe ExternalWikiService do it 'should replace the wiki url' do wiki_path = get_project_wiki_path(project) - wiki_path.should match('https://gitlab.com') + expect(wiki_path).to match('https://gitlab.com') end end end diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index 96ab95c05c..4c437aab12 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -51,7 +51,7 @@ describe IrkerService do let(:_recipients) { 'a b c' } it 'should not add an error if there is 3 recipients' do subject.send :check_recipients_count - subject.errors.should be_blank + expect(subject.errors).to be_blank end end end @@ -96,11 +96,11 @@ describe IrkerService do conn = @irker_server.accept conn.readlines.each do |line| msg = JSON.load(line.chomp("\n")) - msg.keys.should match_array(['to', 'privmsg']) + expect(msg.keys).to match_array(['to', 'privmsg']) if msg['to'].is_a?(String) - msg['to'].should == 'irc://chat.freenode.net/#commits' + expect(msg['to']).to eq 'irc://chat.freenode.net/#commits' else - msg['to'].should match_array(['irc://chat.freenode.net/#commits']) + expect(msg['to']).to match_array(['irc://chat.freenode.net/#commits']) end end conn.close diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 8770786f49..5e65ad18c0 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -196,10 +196,10 @@ describe API::API, api: true do it 'should return a project issue by iid' do get api("/projects/#{project.id}/issues?iid=#{issue.iid}", user) - response.status.should == 200 - json_response.first['title'].should == issue.title - json_response.first['id'].should == issue.id - json_response.first['iid'].should == issue.iid + expect(response.status).to eq 200 + expect(json_response.first['title']).to eq issue.title + expect(json_response.first['id']).to eq issue.id + expect(json_response.first['iid']).to eq issue.iid end it "should return 404 if issue id not found" do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 0ed5883914..38c67bc997 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -118,9 +118,9 @@ describe API::API, api: true do it 'should return merge_request by iid' do url = "/projects/#{project.id}/merge_requests?iid=#{merge_request.iid}" get api(url, user) - response.status.should == 200 - json_response.first['title'].should == merge_request.title - json_response.first['id'].should == merge_request.id + expect(response.status).to eq 200 + expect(json_response.first['title']).to eq merge_request.title + expect(json_response.first['id']).to eq merge_request.id end it "should return a 404 error if merge_request_id not found" do diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb index 6890dd1f3a..db0f6e3c0f 100644 --- a/spec/requests/api/milestones_spec.rb +++ b/spec/requests/api/milestones_spec.rb @@ -32,9 +32,9 @@ describe API::API, api: true do it 'should return a project milestone by iid' do get api("/projects/#{project.id}/milestones?iid=#{milestone.iid}", user) - response.status.should == 200 - json_response.first['title'].should == milestone.title - json_response.first['id'].should == milestone.id + expect(response.status).to eq 200 + expect(json_response.first['title']).to eq milestone.title + expect(json_response.first['id']).to eq milestone.id end it 'should return 401 error if user not authenticated' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index dbfd72e5f1..8fb1509c8b 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -60,9 +60,9 @@ describe API::API, api: true do it 'should include the project labels as the tag_list' do get api('/projects', user) - response.status.should == 200 - json_response.should be_an Array - json_response.first.keys.should include('tag_list') + expect(response.status).to eq 200 + expect(json_response).to be_an Array + expect(json_response.first.keys).to include('tag_list') end context 'and using search' do diff --git a/spec/services/destroy_group_service_spec.rb b/spec/services/destroy_group_service_spec.rb index 2a7372b986..e28564b386 100644 --- a/spec/services/destroy_group_service_spec.rb +++ b/spec/services/destroy_group_service_spec.rb @@ -23,8 +23,8 @@ describe DestroyGroupService do Sidekiq::Testing.inline! { destroy_group(group, user) } end - it { gitlab_shell.exists?(group.path).should be_falsey } - it { gitlab_shell.exists?(remove_path).should be_falsey } + it { expect(gitlab_shell.exists?(group.path)).to be_falsey } + it { expect(gitlab_shell.exists?(remove_path)).to be_falsey } end context 'Sidekiq fake' do @@ -33,8 +33,8 @@ describe DestroyGroupService do Sidekiq::Testing.fake! { destroy_group(group, user) } end - it { gitlab_shell.exists?(group.path).should be_falsey } - it { gitlab_shell.exists?(remove_path).should be_truthy } + it { expect(gitlab_shell.exists?(group.path)).to be_falsey } + it { expect(gitlab_shell.exists?(remove_path)).to be_truthy } end end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 381008def5..e83eef0b1a 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -13,8 +13,8 @@ describe Projects::DestroyService do end it { expect(Project.all).not_to include(project) } - it { Dir.exists?(path).should be_falsey } - it { Dir.exists?(remove_path).should be_falsey } + it { expect(Dir.exists?(path)).to be_falsey } + it { expect(Dir.exists?(remove_path)).to be_falsey } end context 'Sidekiq fake' do @@ -24,8 +24,8 @@ describe Projects::DestroyService do end it { expect(Project.all).not_to include(project) } - it { Dir.exists?(path).should be_falsey } - it { Dir.exists?(remove_path).should be_truthy } + it { expect(Dir.exists?(path)).to be_falsey } + it { expect(Dir.exists?(remove_path)).to be_truthy } end def destroy_project(project, user, params) diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 5650626fb1..79acba78bd 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -20,8 +20,7 @@ describe Projects::TransferService do @result = transfer_project(project, user, new_namespace_id: nil) end - it { expect(@result).not_to be_nil } # { result.should be_false } passes on nil - it { expect(@result).to be_falsey } + it { expect(@result).to eq false } it { expect(project.namespace).to eq(user.namespace) } end @@ -30,8 +29,7 @@ describe Projects::TransferService do @result = transfer_project(project, user, new_namespace_id: group.id) end - it { expect(@result).not_to be_nil } # { result.should be_false } passes on nil - it { expect(@result).to be_falsey } + it { expect(@result).to eq false } it { expect(project.namespace).to eq(user.namespace) } end From 37066267e75cbd10bad3046c61efdfd82bf6338b Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 13 Jun 2015 18:30:09 -0400 Subject: [PATCH 41/71] Use `page.all` and `page.within` in specs --- spec/features/admin/admin_hooks_spec.rb | 2 +- spec/features/issues_spec.rb | 4 +-- spec/features/notes_on_merge_requests_spec.rb | 32 +++++++++---------- spec/features/search_spec.rb | 2 +- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/spec/features/admin/admin_hooks_spec.rb b/spec/features/admin/admin_hooks_spec.rb index 00906e8087..7265cdac7a 100644 --- a/spec/features/admin/admin_hooks_spec.rb +++ b/spec/features/admin/admin_hooks_spec.rb @@ -12,7 +12,7 @@ describe "Admin::Hooks", feature: true do describe "GET /admin/hooks" do it "should be ok" do visit admin_root_path - within ".sidebar-wrapper" do + page.within ".sidebar-wrapper" do click_on "Hooks" end expect(current_path).to eq(admin_hooks_path) diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 66d73b2505..d803a1805d 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -311,10 +311,10 @@ describe 'Issues', feature: true do end def first_issue - all('ul.issues-list li').first.text + page.all('ul.issues-list li').first.text end def last_issue - all('ul.issues-list li').last.text + page.all('ul.issues-list li').last.text end end diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index b69b59d415..219bb3129e 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -22,20 +22,20 @@ describe 'Comments' do is_expected.to have_css('.js-main-target-form', visible: true, count: 1) expect(find('.js-main-target-form input[type=submit]').value). to eq('Add Comment') - within('.js-main-target-form') do + page.within('.js-main-target-form') do expect(page).not_to have_link('Cancel') end end describe 'with text' do before do - within('.js-main-target-form') do + page.within('.js-main-target-form') do fill_in 'note[note]', with: 'This is awesome' end end it 'should have enable submit button and preview button' do - within('.js-main-target-form') do + page.within('.js-main-target-form') do expect(page).not_to have_css('.js-comment-button[disabled]') expect(page).to have_css('.js-md-preview-button', visible: true) end @@ -45,7 +45,7 @@ describe 'Comments' do describe 'when posting a note' do before do - within('.js-main-target-form') do + page.within('.js-main-target-form') do fill_in 'note[note]', with: 'This is awsome!' find('.js-md-preview-button').click click_button 'Add Comment' @@ -54,11 +54,11 @@ describe 'Comments' do it 'should be added and form reset' do is_expected.to have_content('This is awsome!') - within('.js-main-target-form') do + page.within('.js-main-target-form') do expect(page).to have_no_field('note[note]', with: 'This is awesome!') expect(page).to have_css('.js-md-preview', visible: :hidden) end - within('.js-main-target-form') do + page.within('.js-main-target-form') do is_expected.to have_css('.js-note-text', visible: true) end end @@ -66,7 +66,7 @@ describe 'Comments' do describe 'when editing a note', js: true do it 'should contain the hidden edit form' do - within("#note_#{note.id}") do + page.within("#note_#{note.id}") do is_expected.to have_css('.note-edit-form', visible: false) end end @@ -78,7 +78,7 @@ describe 'Comments' do end it 'should show the note edit form and hide the note body' do - within("#note_#{note.id}") do + page.within("#note_#{note.id}") do expect(find('.current-note-edit-form', visible: true)).to be_visible expect(find('.note-edit-form', visible: true)).to be_visible expect(find(:css, '.note-body > .note-text', visible: false)).not_to be_visible @@ -95,12 +95,12 @@ describe 'Comments' do #end it 'appends the edited at time to the note' do - within('.current-note-edit-form') do + page.within('.current-note-edit-form') do fill_in 'note[note]', with: 'Some new content' find('.btn-save').click end - within("#note_#{note.id}") do + page.within("#note_#{note.id}") do is_expected.to have_css('.note_edited_ago') expect(find('.note_edited_ago').text). to match(/less than a minute ago/) @@ -115,7 +115,7 @@ describe 'Comments' do end it 'shows the delete link' do - within('.note-attachment') do + page.within('.note-attachment') do is_expected.to have_css('.js-note-attachment-delete') end end @@ -150,7 +150,7 @@ describe 'Comments' do it { is_expected.to have_css('.js-temp-notes-holder') } it 'has .new_note css class' do - within('.js-temp-notes-holder') do + page.within('.js-temp-notes-holder') do expect(subject).to have_css('.new_note') end end @@ -166,7 +166,7 @@ describe 'Comments' do end it 'should be removed when canceled' do - within(".diff-file form[rel$='#{line_code}']") do + page.within(".diff-file form[rel$='#{line_code}']") do find('.js-close-discussion-note-form').trigger('click') end @@ -186,11 +186,11 @@ describe 'Comments' do describe 'previewing them separately' do before do # add two separate texts and trigger previews on both - within("tr[id='#{line_code}'] + .js-temp-notes-holder") do + page.within("tr[id='#{line_code}'] + .js-temp-notes-holder") do fill_in 'note[note]', with: 'One comment on line 7' find('.js-md-preview-button').click end - within("tr[id='#{line_code_2}'] + .js-temp-notes-holder") do + page.within("tr[id='#{line_code_2}'] + .js-temp-notes-holder") do fill_in 'note[note]', with: 'Another comment on line 10' find('.js-md-preview-button').click end @@ -199,7 +199,7 @@ describe 'Comments' do describe 'posting a note' do before do - within("tr[id='#{line_code_2}'] + .js-temp-notes-holder") do + page.within("tr[id='#{line_code_2}'] + .js-temp-notes-holder") do fill_in 'note[note]', with: 'Another comment on line 10' click_button('Add Comment') end diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index 73987739a7..479334f45d 100644 --- a/spec/features/search_spec.rb +++ b/spec/features/search_spec.rb @@ -7,7 +7,7 @@ describe "Search", feature: true do @project.team << [@user, :reporter] visit search_path - within '.search-holder' do + page.within '.search-holder' do fill_in "search", with: @project.name[0..3] click_button "Search" end From eccdbe367efa638c287ec426f763ec03791a7cf3 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sun, 14 Jun 2015 17:35:03 +0200 Subject: [PATCH 42/71] Highlight Applications nav item when validation fails. --- app/views/layouts/nav/_profile.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index ac37fd4c1c..c795cf79b6 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -9,7 +9,7 @@ = icon('gear fw') %span Account - = nav_link(path: ['profiles#applications', 'applications#edit', 'applications#show', 'applications#new']) do + = nav_link(path: ['profiles#applications', 'applications#edit', 'applications#show', 'applications#new', 'applications#create']) do = link_to applications_profile_path, title: 'Applications', data: {placement: 'right'} do = icon('cloud fw') %span From 92bb845e1ed18395615ba8a252de4a8b123c1914 Mon Sep 17 00:00:00 2001 From: Daniel Gerhardt Date: Sun, 14 Jun 2015 17:41:11 +0200 Subject: [PATCH 43/71] Fix hooks for web based events with external issue references The creation of cross references for external issues (which would fail) is now prevented. Fixes #1650, GH-9333. --- CHANGELOG | 1 + app/services/system_note_service.rb | 4 +++- spec/services/system_note_service_spec.rb | 9 +++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index c5a625ed1d..c9b406c059 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.12.0 (unreleased) - Update oauth button logos for Twitter and Google to recommended assets + - Fix hooks for web based events with external issue references (Daniel Gerhardt) - Update browser gem to version 0.8.0 for IE11 support (Stan Hu) - Fix timeout when rendering file with thousands of lines. - Add "Remember me" checkbox to LDAP signin form. diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index b6801a9233..8253c1f780 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -212,13 +212,15 @@ class SystemNoteService # Check if a cross-reference is disallowed # # This method prevents adding a "mentioned in !1" note on every single commit - # in a merge request. + # in a merge request. Additionally, it prevents the creation of references to + # external issues (which would fail). # # noteable - Noteable object being referenced # mentioner - Mentionable object # # Returns Boolean def self.cross_reference_disallowed?(noteable, mentioner) + return true if noteable.is_a?(ExternalIssue) return false unless mentioner.is_a?(MergeRequest) return false unless noteable.is_a?(Commit) diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 700286b585..2658576640 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -338,6 +338,15 @@ describe SystemNoteService do to be_falsey end end + + context 'when notable is an ExternalIssue' do + let(:noteable) { ExternalIssue.new('EXT-1234', project) } + it 'is truthy' do + mentioner = noteable.dup + expect(described_class.cross_reference_disallowed?(noteable, mentioner)). + to be_truthy + end + end end describe '.cross_reference_exists?' do From 7b5b89eabdfcb7e6e41dd3b7dc77f192c61c9426 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sun, 14 Jun 2015 21:30:09 +0200 Subject: [PATCH 44/71] Remove button to all projects on Trending Projects page. Fixes #2001. --- app/views/explore/projects/trending.html.haml | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/views/explore/projects/trending.html.haml b/app/views/explore/projects/trending.html.haml index 5ae2653fed..5e24df76a6 100644 --- a/app/views/explore/projects/trending.html.haml +++ b/app/views/explore/projects/trending.html.haml @@ -13,6 +13,3 @@ .public-projects %ul.bordered-list = render @trending_projects - - .center.append-bottom-20 - = link_to 'Show all projects', explore_projects_path, class: 'btn btn-primary' From b377ca083007829134478af4fd91f54acbbcba49 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 14 Jun 2015 15:51:48 -0700 Subject: [PATCH 45/71] Update Irker home page [ci skip] Closes #1713 --- doc/project_services/irker.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/project_services/irker.md b/doc/project_services/irker.md index 780a45bca2..9875bebf66 100644 --- a/doc/project_services/irker.md +++ b/doc/project_services/irker.md @@ -4,7 +4,7 @@ GitLab provides a way to push update messages to an Irker server. When configured, pushes to a project will trigger the service to send data directly to the Irker server. -See the project homepage for further info: http://www.catb.org/esr/irker/ +See the project homepage for further info: https://gitlab.com/esr/irker ## Needed setup From b00f447db4d76d453aa65ebd743da3a6bbe281f2 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sun, 14 Jun 2015 20:33:29 -0400 Subject: [PATCH 46/71] Add `allowing_for_delay` helper method for feature specs --- spec/features/profiles/preferences_spec.rb | 14 +++++---- spec/support/capybara.rb | 33 ++++++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/spec/features/profiles/preferences_spec.rb b/spec/features/profiles/preferences_spec.rb index 1f07fde7af..69d15f4170 100644 --- a/spec/features/profiles/preferences_spec.rb +++ b/spec/features/profiles/preferences_spec.rb @@ -21,9 +21,10 @@ describe 'Profile > Preferences' do it 'updates their preference' do choose "user_theme_id_#{theme.id}" - visit page.current_path - - expect(page).to have_checked_field("user_theme_id_#{theme.id}") + allowing_for_delay do + visit page.current_path + expect(page).to have_checked_field("user_theme_id_#{theme.id}") + end end it 'reflects the changes immediately' do @@ -46,9 +47,10 @@ describe 'Profile > Preferences' do it 'updates their preference' do choose 'user_color_scheme_id_5' - visit page.current_path - - expect(page).to have_checked_field('user_color_scheme_id_5') + allowing_for_delay do + visit page.current_path + expect(page).to have_checked_field('user_color_scheme_id_5') + end end end diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index fed1ab6ee3..3e41aec425 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -19,3 +19,36 @@ unless ENV['CI'] || ENV['CI_SERVER'] # Keep only the screenshots generated from the last failing test suite Capybara::Screenshot.prune_strategy = :keep_last_run end + +module CapybaraHelpers + # Execute a block a certain number of times before considering it a failure + # + # The given block is called, and if it raises a `Capybara::ExpectationNotMet` + # error, we wait `interval` seconds and then try again, until `retries` is + # met. + # + # This allows for better handling of timing-sensitive expectations in a + # sketchy CI environment, for example. + # + # interval - Delay between retries in seconds (default: 0.5) + # retries - Number of times to execute before failing (default: 5) + def allowing_for_delay(interval: 0.5, retries: 5) + tries = 0 + + begin + yield + rescue Capybara::ExpectationNotMet => ex + if tries <= retries + tries += 1 + sleep interval + retry + else + raise ex + end + end + end +end + +RSpec.configure do |config| + config.include CapybaraHelpers, type: :feature +end From c6b71111cce706852f00f90ebabcc881870365e1 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 15 Jun 2015 00:39:13 -0400 Subject: [PATCH 47/71] Fix doc typos --- doc/profile/preferences.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/doc/profile/preferences.md b/doc/profile/preferences.md index 0c12eb0c65..ce5f193678 100644 --- a/doc/profile/preferences.md +++ b/doc/profile/preferences.md @@ -5,8 +5,8 @@ various aspects of the site to their liking. ## Application theme -Changing this settings allows the user to customize the color scheme used for -the navigation bar on the left side of the screen. +Changing this setting allows the user to customize the color scheme used for the +navigation bar on the left side of the screen. The default is **Charcoal**. @@ -25,9 +25,8 @@ For users who have access to a large number of projects but only keep up with a select few, the amount of activity on the default Dashboard page can be overwhelming. -Changing this setting to allows the user to redefine what their default -dashboard will be. Setting it to **Starred Projects** will make that Dashboard -view the default when signing in or clicking the application logo in the upper -left. +Changing this setting allows the user to redefine what their default dashboard +will be. Setting it to **Starred Projects** will make that Dashboard view the +default when signing in or clicking the application logo in the upper left. -The default is **Projects**. +The default is **Your Projects**. From 86d35ed3d4dd564ec7f5f8551fe5e65f5c4e5cd2 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Mon, 15 Jun 2015 11:40:00 +0200 Subject: [PATCH 48/71] Update SSL ciphers per logjam vulnerability recommendations. --- CHANGELOG | 1 + lib/support/nginx/gitlab-ssl | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 2d41e45527..462a316766 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.13.0 (unreleased) - Remove project visibility icons from dashboard projects list + - Update ssl_ciphers in Nginx example to remove DHE settings. This will deny forward secrecy for Android 2.3.7, Java 6 and OpenSSL 0.9.8 v 7.12.0 (unreleased) - Fix post-receive errors on a push when an external issue tracker is configured (Stan Hu) diff --git a/lib/support/nginx/gitlab-ssl b/lib/support/nginx/gitlab-ssl index 187a27e93b..5c94ec6343 100644 --- a/lib/support/nginx/gitlab-ssl +++ b/lib/support/nginx/gitlab-ssl @@ -73,7 +73,7 @@ server { ssl_certificate_key /etc/nginx/ssl/gitlab.key; # GitLab needs backwards compatible ciphers to retain compatibility with Java IDEs - ssl_ciphers "ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA:ECDHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES256-GCM-SHA384:AES128-GCM-SHA256:AES256-SHA256:AES128-SHA256:AES256-SHA:AES128-SHA:DES-CBC3-SHA:HIGH:!aNULL:!eNULL:!EXPORT:!DES:!MD5:!PSK:!RC4"; + ssl_ciphers "ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA:ECDHE-RSA-AES128-SHA:ECDHE-RSA-DES-CBC3-SHA:AES256-GCM-SHA384:AES128-GCM-SHA256:AES256-SHA256:AES128-SHA256:AES256-SHA:AES128-SHA:DES-CBC3-SHA:!aNULL:!eNULL:!EXPORT:!DES:!MD5:!PSK:!RC4"; ssl_protocols TLSv1 TLSv1.1 TLSv1.2; ssl_prefer_server_ciphers on; ssl_session_cache shared:SSL:10m; From 9eec51d914bc79fed479a4e3e7b86fda58ad77c8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Jun 2015 11:28:50 +0000 Subject: [PATCH 49/71] Move CHANGELOG item to 7.13 --- CHANGELOG | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index cca283a04c..7ab449c773 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,9 @@ Please view this file on the master branch, on stable branches it's out of date. +v 7.13.0 (unreleased) + - Rename "Design" profile settings page to "Preferences". + - Allow users to customize their default Dashboard page. + v 7.12.0 (unreleased) - Update oauth button logos for Twitter and Google to recommended assets - Update browser gem to version 0.8.0 for IE11 support (Stan Hu) @@ -54,8 +58,6 @@ v 7.12.0 (unreleased) - Improve group removing logic - Trigger create-hooks on backup restore task - Add option to automatically link omniauth and LDAP identities - - Rename "Design" profile settings page to "Preferences". - - Allow users to customize their default Dashboard page. v 7.11.4 - Fix missing bullets when creating lists @@ -1511,4 +1513,4 @@ v 0.8.0 - stability - security fixes - increased test coverage - - email notification + - email notification \ No newline at end of file From 7a7c6b469b89ac651006b4e0007664fc9dc12dd3 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Jun 2015 14:54:41 +0200 Subject: [PATCH 50/71] Fix text align in last push event Signed-off-by: Dmitriy Zaporozhets --- app/views/events/_event_last_push.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/events/_event_last_push.html.haml b/app/views/events/_event_last_push.html.haml index d2f0005142..501412642d 100644 --- a/app/views/events/_event_last_push.html.haml +++ b/app/views/events/_event_last_push.html.haml @@ -4,7 +4,7 @@ %span You pushed to = link_to namespace_project_commits_path(event.project.namespace, event.project, event.ref_name) do %strong= event.ref_name - at + %span at %strong= link_to_project event.project #{time_ago_with_tooltip(event.created_at)} From 8f1eb7fed28361817289391f11a5773c8ebdbf6d Mon Sep 17 00:00:00 2001 From: Sytse Sijbrandij Date: Mon, 15 Jun 2015 13:54:36 +0000 Subject: [PATCH 51/71] Prepare blog post in advance --- doc/release/monthly.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/release/monthly.md b/doc/release/monthly.md index 97418db747..8d475296a2 100644 --- a/doc/release/monthly.md +++ b/doc/release/monthly.md @@ -142,7 +142,8 @@ Tweet about the RC release: ## Prepare the blog post -1. Start with a complete copy of the [release blog template](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/doc/release_blog_template.md) and fill it out. +1. The blog post template for this release should already exist and might have comments that were added during the month. +1. Fill out as much of the blog post template as you can. 1. Make sure the blog post contains information about the GitLab CI release. 1. Check the changelog of CE and EE for important changes. 1. Also check the CI changelog @@ -155,6 +156,7 @@ Tweet about the RC release: 1. Create a merge request on [GitLab.com](https://gitlab.com/gitlab-com/www-gitlab-com/tree/master) 1. Assign to one reviewer who will fix spelling issues by editing the branch (either with a git client or by using the online editor) 1. Comment to the reviewer: '@person Please mention the whole team as soon as you are done (3 workdays before release at the latest)' +1. Create a complete copy of the [release blog template](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/doc/release_blog_template.md) for the release after this. ## Create CE, EE, CI stable versions @@ -212,4 +214,4 @@ Consider creating a post on Hacker News. ## Create a WIP blogpost for the next release -Create a WIP blogpost using [release blog template](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/doc/release_blog_template.md). +Create a WIP blogpost using [release blog template](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/doc/release_blog_template.md). \ No newline at end of file From 83ce6cddc783da584eb327daf808431618485ac1 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Mon, 15 Jun 2015 16:52:01 +0200 Subject: [PATCH 52/71] The changelogs are kept up to date continuously --- doc/release/monthly.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/doc/release/monthly.md b/doc/release/monthly.md index 97418db747..0d1677f651 100644 --- a/doc/release/monthly.md +++ b/doc/release/monthly.md @@ -30,9 +30,6 @@ All steps from issue template are explained below ``` Xth: (7 working days before the 22nd) -- [ ] Update the CE changelog (#LINK) -- [ ] Update the EE changelog (#LINK) -- [ ] Update the CI changelog (#LINK) - [ ] Triage the omnibus-gitlab milestone Xth: (6 working days before the 22nd) From fc4160eb382ce62c7ea5ea6084a0b557034ffcab Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Mon, 15 Jun 2015 16:53:01 +0200 Subject: [PATCH 53/71] Add 'build RC1 packages' to monthly release steps --- doc/release/monthly.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/release/monthly.md b/doc/release/monthly.md index 0d1677f651..6432241d48 100644 --- a/doc/release/monthly.md +++ b/doc/release/monthly.md @@ -38,6 +38,7 @@ Xth: (6 working days before the 22nd) - [ ] Determine QA person and notify this person - [ ] Check the tasks in [how to rc1 guide](https://dev.gitlab.org/gitlab/gitlabhq/blob/master/doc/release/howto_rc1.md) and delegate tasks if necessary - [ ] Create CE, EE, CI RC1 versions (#LINK) +- [ ] Build RC1 packages (EE first) (#LINK) Xth: (5 working days before the 22nd) From fe51fa26a777f239dc4a09f531b54162f9f949fc Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Jun 2015 17:54:22 +0200 Subject: [PATCH 54/71] Dont set checkout sha for removed branch/tag Signed-off-by: Dmitriy Zaporozhets --- lib/gitlab/push_data_builder.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/push_data_builder.rb index f97784f5ab..d010ade704 100644 --- a/lib/gitlab/push_data_builder.rb +++ b/lib/gitlab/push_data_builder.rb @@ -27,7 +27,7 @@ module Gitlab # Get latest 20 commits ASC commits_limited = commits.last(20) - + # For performance purposes maximum 20 latest commits # will be passed as post receive hook data. commit_attrs = commits_limited.map(&:hook_attrs) @@ -70,8 +70,11 @@ module Gitlab end def checkout_sha(repository, newrev, ref) + # Checkout sha is nil when we remove branch or tag + return if Gitlab::Git.blank_ref?(newrev) + # Find sha for tag, except when it was deleted. - if Gitlab::Git.tag_ref?(ref) && !Gitlab::Git.blank_ref?(newrev) + if Gitlab::Git.tag_ref?(ref) tag_name = Gitlab::Git.ref_name(ref) tag = repository.find_tag(tag_name) From 0674bf2339131c104b233cb9c7ffeeb025f2b461 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Jun 2015 17:54:42 +0200 Subject: [PATCH 55/71] Look for .gitlab-ci.yml only if checkout_sha is present Signed-off-by: Dmitriy Zaporozhets --- .../project_services/gitlab_ci_service.rb | 22 +++++++++++-------- .../gitlab_ci_service_spec.rb | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/models/project_services/gitlab_ci_service.rb b/app/models/project_services/gitlab_ci_service.rb index a935475468..19b5859d5c 100644 --- a/app/models/project_services/gitlab_ci_service.rb +++ b/app/models/project_services/gitlab_ci_service.rb @@ -40,10 +40,14 @@ class GitlabCiService < CiService def execute(data) return unless supported_events.include?(data[:object_kind]) - ci_yaml_file = ci_yaml_file(data) + sha = data[:checkout_sha] - if ci_yaml_file - data.merge!(ci_yaml_file: ci_yaml_file) + if sha.present? + file = ci_yaml_file(sha) + + if file && file.data + data.merge!(ci_yaml_file: file.data) + end end service_hook.execute(data) @@ -129,15 +133,15 @@ class GitlabCiService < CiService private - def ci_yaml_file(data) - ref = data[:checkout_sha] - repo = project.repository - commit = repo.commit(ref) - blob = Gitlab::Git::Blob.find(repo, commit.id, ".gitlab-ci.yml") - blob && blob.data + def ci_yaml_file(sha) + repository.blob_at(sha, '.gitlab-ci.yml') end def fork_registration_path project_url.sub(/projects\/\d*/, "#{API_PREFIX}/forks") end + + def repository + project.repository + end end diff --git a/spec/models/project_services/gitlab_ci_service_spec.rb b/spec/models/project_services/gitlab_ci_service_spec.rb index ebd8b545aa..c92cf3cdae 100644 --- a/spec/models/project_services/gitlab_ci_service_spec.rb +++ b/spec/models/project_services/gitlab_ci_service_spec.rb @@ -58,7 +58,7 @@ describe GitlabCiService do service_hook = double service_hook.should_receive(:execute) @service.should_receive(:service_hook).and_return(service_hook) - @service.should_receive(:ci_yaml_file).with(push_sample_data) + @service.should_receive(:ci_yaml_file).with(push_sample_data[:checkout_sha]) @service.execute(push_sample_data) end From 4c317531b4c23a09a4992dc8f3e2fb29f19cdeef Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Mon, 15 Jun 2015 18:04:30 +0200 Subject: [PATCH 56/71] If kerberos is enabled require it. --- config/initializers/7_omniauth.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/config/initializers/7_omniauth.rb b/config/initializers/7_omniauth.rb index 6f1f267bf9..f29216a2cf 100644 --- a/config/initializers/7_omniauth.rb +++ b/config/initializers/7_omniauth.rb @@ -17,3 +17,10 @@ OmniAuth.config.allowed_request_methods << :get if Gitlab.config.omniauth.auto_s OmniAuth.config.before_request_phase do |env| OmniAuth::RequestForgeryProtection.new(env).call end + +if Gitlab.config.omniauth.enabled + Gitlab.config.omniauth.providers.each do |provider| + next unless provider['name'] == 'kerberos' + require 'omniauth-kerberos' + end +end From f715bc7b70e8299c55b96cafce6918f2a61f247c Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 15 Jun 2015 12:00:13 +0300 Subject: [PATCH 57/71] new syntax of .gitlab-ci.yml --- .gitlab-ci.yml | 62 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1411a9194b..8fba268be7 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,3 +1,4 @@ +# This file is generated by GitLab CI before_script: - export PATH=$HOME/bin:/usr/local/bin:/usr/bin:/bin - ruby -v @@ -7,33 +8,44 @@ before_script: - echo $PATH - cp config/database.yml.mysql config/database.yml - cp config/gitlab.yml.example config/gitlab.yml - - ! 'sed "s/username\:.*$/username\: runner/" -i config/database.yml' - - ! 'sed "s/password\:.*$/password\: ''password''/" -i config/database.yml' + - 'sed "s/username\:.*$/username\: runner/" -i config/database.yml' + - 'sed "s/password\:.*$/password\: ''password''/" -i config/database.yml' - sed "s/gitlabhq_test/gitlabhq_test_$((RANDOM/5000))/" -i config/database.yml - touch log/application.log - touch log/test.log - bundle install --without postgres production --jobs $(nproc) - bundle exec rake db:create RAILS_ENV=test -jobs: -- script: - - RAILS_ENV=test SIMPLECOV=true bundle exec rake spec - name: Rspec - runner: ruby,mysql -- script: - - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach - name: Spinach - runner: ruby,mysql -- script: - - RAILS_ENV=test SIMPLECOV=true bundle exec rake jasmine:ci - name: Jasmine - runner: ruby,mysql -- script: - - bundle exec rubocop - name: Rubocop - runner: ruby,mysql -- script: - - bundle exec rake brakeman - name: Brakeman - runner: ruby,mysql -deploy_jobs: [] -skip_refs: '' +Rspec: + script: + - RAILS_ENV=test SIMPLECOV=true bundle exec rake spec + tags: + - ruby + - mysql + +Spinach: + script: + - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach + tags: + - ruby + - mysql + +Jasmine: + script: + - RAILS_ENV=test SIMPLECOV=true bundle exec rake jasmine:ci + tags: + - ruby + - mysql + +Rubocop: + script: + - bundle exec rubocop + tags: + - ruby + - mysql + +Brakeman: + script: + - bundle exec rake brakeman + tags: + - ruby + - mysql \ No newline at end of file From f18a24f0ac96d2cafffc7dbf08bb644d5243a421 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sun, 14 Jun 2015 17:37:58 +0200 Subject: [PATCH 58/71] Fix layout issue when New Application validation fails. --- .../doorkeeper/applications/_form.html.haml | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/app/views/doorkeeper/applications/_form.html.haml b/app/views/doorkeeper/applications/_form.html.haml index a5fec2fabd..a157c9af29 100644 --- a/app/views/doorkeeper/applications/_form.html.haml +++ b/app/views/doorkeeper/applications/_form.html.haml @@ -1,17 +1,22 @@ = form_for application, url: doorkeeper_submit_path(application), html: {class: 'form-horizontal', role: 'form'} do |f| - if application.errors.any? - .alert.alert-danger{"data-alert" => ""} - %p Whoops! Check your form for possible errors - = content_tag :div, class: "form-group#{' has-error' if application.errors[:name].present?}" do - = f.label :name, class: 'col-sm-2 control-label' + .alert.alert-danger + %ul + - application.errors.full_messages.each do |msg| + %li= msg + + .form-group + = f.label :name, class: 'control-label' + .col-sm-10 - = f.text_field :name, class: 'form-control' - = doorkeeper_errors_for application, :name - = content_tag :div, class: "form-group#{' has-error' if application.errors[:redirect_uri].present?}" do - = f.label :redirect_uri, class: 'col-sm-2 control-label' + = f.text_field :name, class: 'form-control', required: true + + .form-group + = f.label :redirect_uri, class: 'control-label' + .col-sm-10 - = f.text_area :redirect_uri, class: 'form-control' - = doorkeeper_errors_for application, :redirect_uri + = f.text_area :redirect_uri, class: 'form-control', required: true + %span.help-block Use one line per URI - if Doorkeeper.configuration.native_redirect_uri From 4cf7d8d956acf5b3cbefa1f4b52d8c0db8d0b04c Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sun, 14 Jun 2015 17:38:27 +0200 Subject: [PATCH 59/71] Fix consistency issues on New Application page. --- app/views/doorkeeper/applications/_form.html.haml | 5 +++-- app/views/doorkeeper/applications/new.html.haml | 7 ++++++- features/steps/profile/profile.rb | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/views/doorkeeper/applications/_form.html.haml b/app/views/doorkeeper/applications/_form.html.haml index a157c9af29..98a61ab211 100644 --- a/app/views/doorkeeper/applications/_form.html.haml +++ b/app/views/doorkeeper/applications/_form.html.haml @@ -24,6 +24,7 @@ Use %code= Doorkeeper.configuration.native_redirect_uri for local tests + .form-actions - = f.submit 'Submit', class: "btn btn-primary wide" - = link_to "Cancel", applications_profile_path, class: "btn btn-default" + = f.submit 'Submit', class: "btn btn-create" + = link_to "Cancel", applications_profile_path, class: "btn btn-cancel" diff --git a/app/views/doorkeeper/applications/new.html.haml b/app/views/doorkeeper/applications/new.html.haml index 655845e4af..fd32a468b4 100644 --- a/app/views/doorkeeper/applications/new.html.haml +++ b/app/views/doorkeeper/applications/new.html.haml @@ -1,2 +1,7 @@ -%h3.page-title New application +- page_title "New Application" + +%h3.page-title New Application + +%hr + = render 'form', application: @application \ No newline at end of file diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb index 32e6859eff..8b07a13d31 100644 --- a/features/steps/profile/profile.rb +++ b/features/steps/profile/profile.rb @@ -197,7 +197,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps end step 'I should see application form' do - expect(page).to have_content "New application" + expect(page).to have_content "New Application" end step 'I fill application form out and submit' do From d8209aebdc6e082412161f75d0c6c8e5d1093f6f Mon Sep 17 00:00:00 2001 From: Nicolas Date: Tue, 16 Jun 2015 03:12:45 +0200 Subject: [PATCH 60/71] Fix layout issue in header title truncation. --- app/assets/stylesheets/generic/header.scss | 19 +++++++++---------- app/assets/stylesheets/generic/mobile.scss | 3 +-- app/views/layouts/header/_default.html.haml | 5 ++--- app/views/layouts/header/_public.html.haml | 4 ++-- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/app/assets/stylesheets/generic/header.scss b/app/assets/stylesheets/generic/header.scss index 5e8701830e..8f17232592 100644 --- a/app/assets/stylesheets/generic/header.scss +++ b/app/assets/stylesheets/generic/header.scss @@ -98,16 +98,16 @@ header { height: $header-height; .title { - position: relative; - float: left; margin: 0; - margin-left: 35px; + padding: 0 15px 0 35px; + overflow: hidden; font-size: 18px; line-height: $header-height; font-weight: bold; color: #444; - - @include str-truncated(37%); + text-overflow: ellipsis; + vertical-align: top; + white-space: nowrap; a { color: #444; @@ -116,6 +116,10 @@ header { } } } + + .navbar-collapse { + float: right; + } } .search { @@ -167,10 +171,6 @@ header { } @media (max-width: $screen-md-max) { - header .container .title { - max-width: 43%; - } - .header-collapsed, .header-expanded { @include collapsed-header; } @@ -190,7 +190,6 @@ header { font-size: 18px; .title { - max-width: 70%; } .navbar-nav { diff --git a/app/assets/stylesheets/generic/mobile.scss b/app/assets/stylesheets/generic/mobile.scss index f04c8eef90..a49775daf8 100644 --- a/app/assets/stylesheets/generic/mobile.scss +++ b/app/assets/stylesheets/generic/mobile.scss @@ -61,8 +61,7 @@ } .container .title { - margin-left: 15px !important; - max-width: 70% !important; + padding-left: 15px !important; } .issue-info, .merge-request-info { diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 8b4510d651..4ec50f3589 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -5,9 +5,6 @@ = brand_header_logo %h3 GitLab .header-content - %h1.title - = title - %button.navbar-toggle %span.sr-only Toggle navigation = icon('bars') @@ -43,4 +40,6 @@ = link_to destroy_user_session_path, class: 'logout', method: :delete, title: 'Sign out', data: {toggle: 'tooltip', placement: 'bottom'} do = icon('sign-out') + %h1.title= title + = render 'shared/outdated_browser' diff --git a/app/views/layouts/header/_public.html.haml b/app/views/layouts/header/_public.html.haml index 6a031722aa..2c5884a5b6 100644 --- a/app/views/layouts/header/_public.html.haml +++ b/app/views/layouts/header/_public.html.haml @@ -5,10 +5,10 @@ = brand_header_logo %h3 GitLab .header-content - %h1.title= title - - unless current_controller?('sessions') .pull-right = link_to "Sign in", new_session_path(:user, redirect_to_referer: 'yes'), class: 'btn btn-sign-in btn-success btn-sm' + %h1.title= title + = render 'shared/outdated_browser' From 6000c7199e2cec97022911ea24c1a66f615f3434 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 16 Jun 2015 10:00:02 +0200 Subject: [PATCH 61/71] Improve DB dump instructions --- doc/development/db_dump.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/development/db_dump.md b/doc/development/db_dump.md index 4ad3bd534e..21f1b3edec 100644 --- a/doc/development/db_dump.md +++ b/doc/development/db_dump.md @@ -4,6 +4,9 @@ Sometimes it is useful to import the database from a production environment into a staging environment for testing. The procedure below assumes you have SSH+sudo access to both the production environment and the staging VM. +**Destroy your staging VM** when you are done with it. It is important to avoid +data leaks. + On the staging VM, add the following line to `/etc/gitlab/gitlab.rb` to speed up large database imports. @@ -12,6 +15,8 @@ large database imports. echo "postgresql['checkpoint_segments'] = 64" | sudo tee -a /etc/gitlab/gitlab.rb sudo touch /etc/gitlab/skip-auto-migrations sudo gitlab-ctl reconfigure +sudo gitlab-ctl stop unicorn +sudo gitlab-ctl stop sidekiq ``` Next, we let the production environment stream a compressed SQL dump to our From c6c3577bc6e49be3d0062983b0a5abdbcd39012a Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 16 Jun 2015 13:44:52 +0200 Subject: [PATCH 62/71] Use explicit if. --- config/initializers/7_omniauth.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/config/initializers/7_omniauth.rb b/config/initializers/7_omniauth.rb index f29216a2cf..df73ec1304 100644 --- a/config/initializers/7_omniauth.rb +++ b/config/initializers/7_omniauth.rb @@ -20,7 +20,8 @@ end if Gitlab.config.omniauth.enabled Gitlab.config.omniauth.providers.each do |provider| - next unless provider['name'] == 'kerberos' - require 'omniauth-kerberos' + if provider['name'] == 'kerberos' + require 'omniauth-kerberos' + end end end From a7932fe2fd63da4864afb01bff859f4e1fbe9576 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 5 Jun 2015 15:24:05 -0700 Subject: [PATCH 63/71] Support commenting on a diff in side-by-side view Closes https://github.com/gitlabhq/gitlabhq/issues/9283 --- CHANGELOG | 2 + app/assets/javascripts/notes.js.coffee | 43 +++++++++++++++---- app/controllers/projects/notes_controller.rb | 17 +++++++- app/helpers/notes_helper.rb | 10 +++-- app/views/projects/commit/show.html.haml | 2 +- .../projects/diffs/_parallel_view.html.haml | 6 ++- .../_diff_notes_with_reply_parallel.html.haml | 32 +++++++------- app/views/projects/notes/_form.html.haml | 2 + .../projects/notes/_notes_with_form.html.haml | 4 +- .../project/commits/diff_comments.feature | 14 ++++++ features/steps/project/commits/commits.rb | 9 +--- features/steps/shared/diff_note.rb | 40 +++++++++++++++++ 12 files changed, 138 insertions(+), 43 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e881fa42a2..86de9314d8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,8 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.13.0 (unreleased) + - Support commenting on diffs in side-by-side mode (Stan Hu) + - Fix JavaScript error when clicking on the comment button on a diff line that has a comment already (Stan Hu) - Remove project visibility icons from dashboard projects list - Rename "Design" profile settings page to "Preferences". - Allow users to customize their default Dashboard page. diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 21656f5914..1c05a2b9fe 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -8,11 +8,12 @@ class @Notes @interval: null - constructor: (notes_url, note_ids, last_fetched_at) -> + constructor: (notes_url, note_ids, last_fetched_at, view) -> @notes_url = notes_url @notes_url = gon.relative_url_root + @notes_url if gon.relative_url_root? @note_ids = note_ids @last_fetched_at = last_fetched_at + @view = view @noteable_url = document.URL @initRefresh() @setupMainTargetNoteForm() @@ -131,6 +132,8 @@ class @Notes isNewNote: (note) -> $.inArray(note.id, @note_ids) == -1 + isParallelView: -> + @view == 'parallel' ### Render note in discussion area. @@ -391,6 +394,7 @@ class @Notes setupDiscussionNoteForm: (dataHolder, form) => # setup note target form.attr "rel", dataHolder.data("discussionId") + form.find("#line_type").val dataHolder.data("lineType") form.find("#note_commit_id").val dataHolder.data("commitId") form.find("#note_line_code").val dataHolder.data("lineCode") form.find("#note_noteable_type").val dataHolder.data("noteableType") @@ -411,19 +415,40 @@ class @Notes form = $(".js-new-note-form") row = $(link).closest("tr") nextRow = row.next() + hasNotes = nextRow.is(".notes_holder") + addForm = false + targetContent = ".notes_content" + rowCssToAdd = "" - # does it already have notes? - if nextRow.is(".notes_holder") - replyButton = nextRow.find(".js-discussion-reply-button") - if replyButton.length > 0 - $.proxy(@replyToDiscussionNote, replyButton).call() + # In parallel view, look inside the correct left/right pane + if @isParallelView() + lineType = $(link).data("lineType") + targetContent += "." + lineType + rowCssToAdd = "" + + if hasNotes + notesContent = nextRow.find(targetContent) + if notesContent.length + replyButton = notesContent.find(".js-discussion-reply-button:visible") + if replyButton.length + e.target = replyButton[0] + $.proxy(@replyToDiscussionNote, replyButton[0], e).call() + else + # In parallel view, the form may not be present in one of the panes + noteForm = notesContent.find(".js-discussion-note-form") + if noteForm.length == 0 + addForm = true else # add a notes row and insert the form - row.after "" - form.clone().appendTo row.next().find(".notes_content") + row.after rowCssToAdd + addForm = true + + if addForm + newForm = form.clone() + newForm.appendTo row.next().find(targetContent) # show the form - @setupDiscussionNoteForm $(link), row.next().find("form") + @setupDiscussionNoteForm $(link), newForm ### Called in response to "cancel" on a diff note form. diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 496b85cb46..f3e521adb6 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -77,11 +77,24 @@ class Projects::NotesController < Projects::ApplicationController end def note_to_discussion_html(note) + if params[:view] == 'parallel' + template = "projects/notes/_diff_notes_with_reply_parallel" + locals = + if params[:line_type] == 'old' + { notes_left: [note], notes_right: [] } + else + { notes_left: [], notes_right: [note] } + end + else + template = "projects/notes/_diff_notes_with_reply" + locals = { notes: [note] } + end + render_to_string( - "projects/notes/_diff_notes_with_reply", + template, layout: false, formats: [:html], - locals: { notes: [note] } + locals: locals ) end diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 271b53aa2b..a7c1fa0b07 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -47,7 +47,7 @@ module NotesHelper }.to_json end - def link_to_new_diff_note(line_code) + def link_to_new_diff_note(line_code, line_type = nil) discussion_id = Note.build_discussion_id( @comments_target[:noteable_type], @comments_target[:noteable_id] || @comments_target[:commit_id], @@ -59,7 +59,8 @@ module NotesHelper noteable_id: @comments_target[:noteable_id], commit_id: @comments_target[:commit_id], line_code: line_code, - discussion_id: discussion_id + discussion_id: discussion_id, + line_type: line_type } button_tag(class: 'btn add-diff-note js-add-diff-note-button', @@ -69,7 +70,7 @@ module NotesHelper end end - def link_to_reply_diff(note) + def link_to_reply_diff(note, line_type = nil) return unless current_user data = { @@ -77,7 +78,8 @@ module NotesHelper noteable_id: note.noteable_id, commit_id: note.commit_id, line_code: note.line_code, - discussion_id: note.discussion_id + discussion_id: note.discussion_id, + line_type: line_type } button_tag class: 'btn reply-btn js-discussion-reply-button', diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml index fc91f71e8d..60b112e67d 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -1,4 +1,4 @@ - page_title "#{@commit.title} (#{@commit.short_id})", "Commits" = render "commit_box" = render "projects/diffs/diffs", diffs: @diffs, project: @project -= render "projects/notes/notes_with_form" += render "projects/notes/notes_with_form", view: params[:view] diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml index 75f3a80f0d..cb41dd852d 100644 --- a/app/views/projects/diffs/_parallel_view.html.haml +++ b/app/views/projects/diffs/_parallel_view.html.haml @@ -18,6 +18,8 @@ - elsif type_left == 'old' || type_left.nil? %td.old_line{id: line_code_left, class: "#{type_left}"} = link_to raw(line_number_left), "##{line_code_left}", id: line_code_left + - if @comments_allowed && can?(current_user, :write_note, @project) + = link_to_new_diff_note(line_code_left, 'old') %td.line_content{class: "parallel noteable_line #{type_left} #{line_code_left}", "line_code" => line_code_left }= raw line_content_left - if type_right == 'new' @@ -29,12 +31,14 @@ %td.new_line{id: new_line_code, class: "#{new_line_class}", data: { linenumber: line_number_right }} = link_to raw(line_number_right), "##{new_line_code}", id: new_line_code + - if @comments_allowed && can?(current_user, :write_note, @project) + = link_to_new_diff_note(line_code_right, 'new') %td.line_content.parallel{class: "noteable_line #{new_line_class} #{new_line_code}", "line_code" => new_line_code}= raw line_content_right - if @reply_allowed - comments_left, comments_right = organize_comments(type_left, type_right, line_code_left, line_code_right) - if comments_left.present? || comments_right.present? - = render "projects/notes/diff_notes_with_reply_parallel", notes1: comments_left, notes2: comments_right + = render "projects/notes/diff_notes_with_reply_parallel", notes_left: comments_left, notes_right: comments_right - if diff_file.diff.diff.blank? && diff_file.mode_changed? .file-mode-changed diff --git a/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml b/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml index 789f3e19fd..c6726cbafa 100644 --- a/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml +++ b/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml @@ -1,34 +1,34 @@ -- note1 = notes1.present? ? notes1.first : nil -- note2 = notes2.present? ? notes2.first : nil +- note1 = notes_left.present? ? notes_left.first : nil +- note2 = notes_right.present? ? notes_right.first : nil %tr.notes_holder - if note1 - %td.notes_line + %td.notes_line.old %span.btn.disabled %i.fa.fa-comment - = notes1.count - %td.notes_content.parallel + = notes_left.count + %td.notes_content.parallel.old %ul.notes{ rel: note1.discussion_id } - = render notes1 + = render notes_left .discussion-reply-holder - = link_to_reply_diff(note1) + = link_to_reply_diff(note1, 'old') - else - %td= "" - %td= "" + %td.notes_line.old= "" + %td.notes_content.parallel.old= "" - if note2 - %td.notes_line + %td.notes_line.new %span.btn.disabled %i.fa.fa-comment - = notes2.count - %td.notes_content.parallel + = notes_right.count + %td.notes_content.parallel.new %ul.notes{ rel: note2.discussion_id } - = render notes2 + = render notes_right .discussion-reply-holder - = link_to_reply_diff(note2) + = link_to_reply_diff(note2, 'new') - else - %td= "" - %td= "" + %td.notes_line.new= "" + %td.notes_content.parallel.new= "" diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index f28b3e9b50..3fb044d736 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -1,4 +1,6 @@ = form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new_note js-new-note-form common-note-form gfm-form" }, authenticity_token: true do |f| + = hidden_field_tag :view, params[:view] + = hidden_field_tag :line_type = note_target_fields(@note) = f.hidden_field :commit_id = f.hidden_field :line_code diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml index 813e37276b..a202e74a89 100644 --- a/app/views/projects/notes/_notes_with_form.html.haml +++ b/app/views/projects/notes/_notes_with_form.html.haml @@ -4,7 +4,7 @@ .js-main-target-form - if can? current_user, :write_note, @project - = render "projects/notes/form" + = render "projects/notes/form", view: params[:view] :javascript - new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}) + new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{params[:view]}") diff --git a/features/project/commits/diff_comments.feature b/features/project/commits/diff_comments.feature index 56b9a13678..4a2b870e08 100644 --- a/features/project/commits/diff_comments.feature +++ b/features/project/commits/diff_comments.feature @@ -77,3 +77,17 @@ Feature: Project Commits Diff Comments And I submit the diff comment Then I should not see the diff comment form And I should see a discussion reply button + + @javascript + Scenario: I can add a comment on a side-by-side commit diff (left side) + Given I open a diff comment form + And I click side-by-side diff button + When I leave a diff comment in a parallel view on the left side like "Old comment" + Then I should see a diff comment on the left side saying "Old comment" + + @javascript + Scenario: I can add a comment on a side-by-side commit diff (right side) + Given I open a diff comment form + And I click side-by-side diff button + When I leave a diff comment in a parallel view on the right side like "New comment" + Then I should see a diff comment on the right side saying "New comment" diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index 4b19e3beed..e6330ec457 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -2,6 +2,7 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps include SharedAuthentication include SharedProject include SharedPaths + include SharedDiffNote include RepoHelpers step 'I see project commits' do @@ -88,14 +89,6 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps expect(links[1]['href']).to match %r{blob/#{sample_image_commit.new_blob_id}} end - step 'I click side-by-side diff button' do - click_link "Side-by-side" - end - - step 'I see side-by-side diff button' do - expect(page).to have_content "Side-by-side" - end - step 'I see inline diff button' do expect(page).to have_content "Inline" end diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index a716ca5837..c4f89ca31c 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -28,6 +28,22 @@ module SharedDiffNote end end + step 'I leave a diff comment in a parallel view on the left side like "Old comment"' do + click_parallel_diff_line(sample_commit.line_code, 'old') + page.within("#{diff_file_selector} form[rel$='#{sample_commit.line_code}']") do + fill_in "note[note]", with: "Old comment" + find(".js-comment-button").trigger("click") + end + end + + step 'I leave a diff comment in a parallel view on the right side like "New comment"' do + click_parallel_diff_line(sample_commit.line_code, 'new') + page.within("#{diff_file_selector} form[rel$='#{sample_commit.line_code}']") do + fill_in "note[note]", with: "New comment" + find(".js-comment-button").trigger("click") + end + end + step 'I preview a diff comment text like "Should fix it :smile:"' do click_diff_line(sample_commit.line_code) page.within("#{diff_file_selector} form[rel$='#{sample_commit.line_code}']") do @@ -102,6 +118,18 @@ module SharedDiffNote end end + step 'I should see a diff comment on the left side saying "Old comment"' do + page.within("#{diff_file_selector} .notes_content.parallel.old") do + expect(page).to have_content("Old comment") + end + end + + step 'I should see a diff comment on the right side saying "New comment"' do + page.within("#{diff_file_selector} .notes_content.parallel.new") do + expect(page).to have_content("New comment") + end + end + step 'I should see a discussion reply button' do page.within(diff_file_selector) do expect(page).to have_button('Reply') @@ -157,6 +185,14 @@ module SharedDiffNote end end + step 'I click side-by-side diff button' do + click_link "Side-by-side" + end + + step 'I see side-by-side diff button' do + expect(page).to have_content "Side-by-side" + end + def diff_file_selector ".diff-file:nth-of-type(1)" end @@ -164,4 +200,8 @@ module SharedDiffNote def click_diff_line(code) find("button[data-line-code='#{code}']").click end + + def click_parallel_diff_line(code, line_type) + find("button[data-line-code='#{code}'][data-line-type='#{line_type}']").trigger('click') + end end From 7400cfc11484b35945cefe8bd42ea02f2675b775 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 15 Jun 2015 18:48:54 -0400 Subject: [PATCH 64/71] Bootlint: Use offset grid classes instead of empty divs --- app/views/admin/groups/_form.html.haml | 3 +-- app/views/admin/projects/show.html.haml | 3 +-- app/views/groups/edit.html.haml | 3 +-- app/views/groups/new.html.haml | 3 +-- app/views/profiles/show.html.haml | 3 +-- app/views/projects/_issuable_form.html.haml | 2 +- app/views/projects/blob/_remove.html.haml | 3 +-- app/views/projects/edit.html.haml | 3 +-- app/views/projects/labels/_form.html.haml | 2 +- app/views/projects/wikis/_form.html.haml | 3 +-- 10 files changed, 10 insertions(+), 18 deletions(-) diff --git a/app/views/admin/groups/_form.html.haml b/app/views/admin/groups/_form.html.haml index 9e7751830a..8de2ba74a7 100644 --- a/app/views/admin/groups/_form.html.haml +++ b/app/views/admin/groups/_form.html.haml @@ -12,8 +12,7 @@ - if @group.new_record? .form-group - .col-sm-2 - .col-sm-10 + .col-sm-offset-2.col-sm-10 .alert.alert-info = render 'shared/group_tips' .form-actions diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 4c2865ac3f..5260eadf95 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -92,8 +92,7 @@ = namespace_select_tag :new_namespace_id, selected: params[:namespace_id], class: 'input-large' .form-group - .col-sm-2 - .col-sm-10 + .col-sm-offset-2.col-sm-10 = f.submit 'Transfer', class: 'btn btn-primary' .col-md-6 diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index 85179d4c4a..aa13ed85b5 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -11,8 +11,7 @@ = render 'shared/group_form', f: f .form-group - .col-sm-2 - .col-sm-10 + .col-sm-offset-2.col-sm-10 = image_tag group_icon(@group), alt: '', class: 'avatar group-avatar s160' %p.light - if @group.avatar? diff --git a/app/views/groups/new.html.haml b/app/views/groups/new.html.haml index edb882bea1..0665cdf387 100644 --- a/app/views/groups/new.html.haml +++ b/app/views/groups/new.html.haml @@ -13,8 +13,7 @@ = render 'shared/choose_group_avatar_button', f: f .form-group - .col-sm-2 - .col-sm-10 + .col-sm-offset-2.col-sm-10 = render 'shared/group_tips' .form-actions diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 6534afb0e8..37a3952635 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -109,6 +109,5 @@ .row .col-md-7 .form-group - .col-sm-2   - .col-sm-10 + .col-sm-offset-2.col-sm-10 = f.submit 'Save changes', class: "btn btn-success" diff --git a/app/views/projects/_issuable_form.html.haml b/app/views/projects/_issuable_form.html.haml index 491e2107da..4d93c89c93 100644 --- a/app/views/projects/_issuable_form.html.haml +++ b/app/views/projects/_issuable_form.html.haml @@ -1,6 +1,6 @@ - if issuable.errors.any? .row - .col-sm-10.col-sm-offset-2 + .col-sm-offset-2.col-sm-10 .alert.alert-danger - issuable.errors.full_messages.each do |msg| %span= msg diff --git a/app/views/projects/blob/_remove.html.haml b/app/views/projects/blob/_remove.html.haml index 09559a4967..fb6be0e089 100644 --- a/app/views/projects/blob/_remove.html.haml +++ b/app/views/projects/blob/_remove.html.haml @@ -13,8 +13,7 @@ = render 'shared/commit_message_container', params: params, placeholder: 'Removed this file because...' .form-group - .col-sm-2 - .col-sm-10 + .col-sm-offset-2.col-sm-10 = button_tag 'Remove file', class: 'btn btn-remove btn-remove-file' = link_to "Cancel", '#', class: "btn btn-cancel", "data-dismiss" => "modal" diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 2765f63c6b..3fecd25c32 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -80,8 +80,7 @@ %legend Project avatar: .form-group - .col-sm-2 - .col-sm-10 + .col-sm-offset-2.col-sm-10 - if @project.avatar? = project_icon("#{@project.namespace.to_param}/#{@project.to_param}", alt: '', class: 'avatar project-avatar s160') %p.light diff --git a/app/views/projects/labels/_form.html.haml b/app/views/projects/labels/_form.html.haml index 261d52dedc..d791ed3410 100644 --- a/app/views/projects/labels/_form.html.haml +++ b/app/views/projects/labels/_form.html.haml @@ -1,7 +1,7 @@ = form_for [@project.namespace.becomes(Namespace), @project, @label], html: { class: 'form-horizontal label-form' } do |f| -if @label.errors.any? .row - .col-sm-10.col-sm-offset-2 + .col-sm-offset-2.col-sm-10 .alert.alert-danger - @label.errors.full_messages.each do |msg| %span= msg diff --git a/app/views/projects/wikis/_form.html.haml b/app/views/projects/wikis/_form.html.haml index 2a8ceaa284..904600499a 100644 --- a/app/views/projects/wikis/_form.html.haml +++ b/app/views/projects/wikis/_form.html.haml @@ -12,8 +12,7 @@ = f.select :format, options_for_select(ProjectWiki::MARKUPS, {selected: @page.format}), {}, class: "form-control" .row - .col-sm-2 - .col-sm-10 + .col-sm-offset-2.col-sm-10 %p.cgray To link to a (new) page you can just type %code [Link Title](page-slug) From e3c1818a3e57fc1aee3cacc3123099f1deefe8a2 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 15 Jun 2015 18:53:06 -0400 Subject: [PATCH 65/71] Bootlint: Modals no longer need a '.hide' class --- app/views/help/_shortcuts.html.haml | 2 +- app/views/projects/_bitbucket_import_modal.html.haml | 2 +- app/views/projects/_github_import_modal.html.haml | 2 +- app/views/projects/_gitlab_import_modal.html.haml | 2 +- app/views/projects/blob/_remove.html.haml | 2 +- app/views/projects/merge_requests/show/_how_to_merge.html.haml | 2 +- app/views/projects/wikis/_new.html.haml | 2 +- app/views/shared/_confirm_modal.html.haml | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/views/help/_shortcuts.html.haml b/app/views/help/_shortcuts.html.haml index ae072bacfb..825acb0ae3 100644 --- a/app/views/help/_shortcuts.html.haml +++ b/app/views/help/_shortcuts.html.haml @@ -1,4 +1,4 @@ -#modal-shortcuts.modal.hide{tabindex: -1} +#modal-shortcuts.modal{tabindex: -1} .modal-dialog .modal-content .modal-header diff --git a/app/views/projects/_bitbucket_import_modal.html.haml b/app/views/projects/_bitbucket_import_modal.html.haml index 07d4d60276..745163e79a 100644 --- a/app/views/projects/_bitbucket_import_modal.html.haml +++ b/app/views/projects/_bitbucket_import_modal.html.haml @@ -1,4 +1,4 @@ -%div#bitbucket_import_modal.modal.hide +%div#bitbucket_import_modal.modal .modal-dialog .modal-content .modal-header diff --git a/app/views/projects/_github_import_modal.html.haml b/app/views/projects/_github_import_modal.html.haml index e88a0f7d68..de58b27df2 100644 --- a/app/views/projects/_github_import_modal.html.haml +++ b/app/views/projects/_github_import_modal.html.haml @@ -1,4 +1,4 @@ -%div#github_import_modal.modal.hide +%div#github_import_modal.modal .modal-dialog .modal-content .modal-header diff --git a/app/views/projects/_gitlab_import_modal.html.haml b/app/views/projects/_gitlab_import_modal.html.haml index 52212b6ae0..ae6c25f937 100644 --- a/app/views/projects/_gitlab_import_modal.html.haml +++ b/app/views/projects/_gitlab_import_modal.html.haml @@ -1,4 +1,4 @@ -%div#gitlab_import_modal.modal.hide +%div#gitlab_import_modal.modal .modal-dialog .modal-content .modal-header diff --git a/app/views/projects/blob/_remove.html.haml b/app/views/projects/blob/_remove.html.haml index fb6be0e089..b8d8451880 100644 --- a/app/views/projects/blob/_remove.html.haml +++ b/app/views/projects/blob/_remove.html.haml @@ -1,4 +1,4 @@ -#modal-remove-blob.modal.hide +#modal-remove-blob.modal .modal-dialog .modal-content .modal-header diff --git a/app/views/projects/merge_requests/show/_how_to_merge.html.haml b/app/views/projects/merge_requests/show/_how_to_merge.html.haml index 6474d32ac0..22f601ac99 100644 --- a/app/views/projects/merge_requests/show/_how_to_merge.html.haml +++ b/app/views/projects/merge_requests/show/_how_to_merge.html.haml @@ -1,4 +1,4 @@ -%div#modal_merge_info.modal.hide +%div#modal_merge_info.modal .modal-dialog .modal-content .modal-header diff --git a/app/views/projects/wikis/_new.html.haml b/app/views/projects/wikis/_new.html.haml index b2c085f34b..dace172438 100644 --- a/app/views/projects/wikis/_new.html.haml +++ b/app/views/projects/wikis/_new.html.haml @@ -1,4 +1,4 @@ -%div#modal-new-wiki.modal.hide +%div#modal-new-wiki.modal .modal-dialog .modal-content .modal-header diff --git a/app/views/shared/_confirm_modal.html.haml b/app/views/shared/_confirm_modal.html.haml index 30ba361c86..5f51b0d450 100644 --- a/app/views/shared/_confirm_modal.html.haml +++ b/app/views/shared/_confirm_modal.html.haml @@ -1,4 +1,4 @@ -#modal-confirm-danger.modal.hide{tabindex: -1} +#modal-confirm-danger.modal{tabindex: -1} .modal-dialog .modal-content .modal-header From 09b4e11d26d056ed2489baff65f319fc33143570 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 15 Jun 2015 18:53:50 -0400 Subject: [PATCH 66/71] Bootlint: Add missing `type` attribute for buttons --- app/views/layouts/header/_default.html.haml | 2 +- app/views/shared/_clone_panel.html.haml | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 4ec50f3589..1403b86f37 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -5,7 +5,7 @@ = brand_header_logo %h3 GitLab .header-content - %button.navbar-toggle + %button.navbar-toggle{type: 'button'} %span.sr-only Toggle navigation = icon('bars') diff --git a/app/views/shared/_clone_panel.html.haml b/app/views/shared/_clone_panel.html.haml index 3f489a04e7..6de2aed29e 100644 --- a/app/views/shared/_clone_panel.html.haml +++ b/app/views/shared/_clone_panel.html.haml @@ -3,6 +3,7 @@ .input-group-addon.git-protocols .input-group-btn %button{ | + type: 'button', | class: "btn btn-sm #{ 'active' if default_clone_protocol == 'ssh' }#{ ' has_tooltip' if current_user && current_user.require_ssh_key? }", | :"data-clone" => project.ssh_url_to_repo, | :"data-title" => "Add an SSH key to your profile
to pull or push via SSH", @@ -11,6 +12,7 @@ SSH .input-group-btn %button{ | + type: 'button', | class: "btn btn-sm #{ 'active' if default_clone_protocol == 'http' }#{ ' has_tooltip' if current_user && current_user.require_password? }", | :"data-clone" => project.http_url_to_repo, | :"data-title" => "Set a password on your account
to pull or push via #{gitlab_config.protocol.upcase}", From 0d7a0675ac7a14b4209fc617f578b83008981fd0 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 15 Jun 2015 18:58:58 -0400 Subject: [PATCH 67/71] Bootlint: Fix incorrectly nested `form-group` element --- app/views/shared/snippets/_form.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/shared/snippets/_form.html.haml b/app/views/shared/snippets/_form.html.haml index 2feeeecc48..fe25133abb 100644 --- a/app/views/shared/snippets/_form.html.haml +++ b/app/views/shared/snippets/_form.html.haml @@ -12,8 +12,8 @@ = render 'shared/visibility_level', f: f, visibility_level: visibility_level, can_change_visibility_level: true, form_model: @snippet - .form-group - .file-editor + .file-editor + .form-group = f.label :file_name, "File", class: 'control-label' .col-sm-10 .file-holder.snippet From dce64418544b27033cb1c1d1169087393ad019f7 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 16 Jun 2015 20:21:38 -0400 Subject: [PATCH 68/71] Bump rails to 4.1.11, and make the version requirement exact --- Gemfile | 2 +- Gemfile.lock | 63 +++++++++++++++++++++++++++------------------------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/Gemfile b/Gemfile index c8f187e4fb..309dff855c 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ source "https://rubygems.org" -gem "rails", "~> 4.1.0" +gem 'rails', '4.1.11' # Default values for AR models gem "default_value_for", "~> 3.0.0" diff --git a/Gemfile.lock b/Gemfile.lock index fd7cbd508e..a910519107 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,31 +4,31 @@ GEM CFPropertyList (2.3.1) RedCloth (4.2.9) ace-rails-ap (2.0.1) - actionmailer (4.1.9) - actionpack (= 4.1.9) - actionview (= 4.1.9) + actionmailer (4.1.11) + actionpack (= 4.1.11) + actionview (= 4.1.11) mail (~> 2.5, >= 2.5.4) - actionpack (4.1.9) - actionview (= 4.1.9) - activesupport (= 4.1.9) + actionpack (4.1.11) + actionview (= 4.1.11) + activesupport (= 4.1.11) rack (~> 1.5.2) rack-test (~> 0.6.2) - actionview (4.1.9) - activesupport (= 4.1.9) + actionview (4.1.11) + activesupport (= 4.1.11) builder (~> 3.1) erubis (~> 2.7.0) - activemodel (4.1.9) - activesupport (= 4.1.9) + activemodel (4.1.11) + activesupport (= 4.1.11) builder (~> 3.1) - activerecord (4.1.9) - activemodel (= 4.1.9) - activesupport (= 4.1.9) + activerecord (4.1.11) + activemodel (= 4.1.11) + activesupport (= 4.1.11) arel (~> 5.0.0) activeresource (4.0.0) activemodel (~> 4.0) activesupport (~> 4.0) rails-observers (~> 0.1.1) - activesupport (4.1.9) + activesupport (4.1.11) i18n (~> 0.6, >= 0.6.9) json (~> 1.7, >= 1.7.7) minitest (~> 5.1) @@ -341,7 +341,7 @@ GEM turbolinks jquery-ui-rails (4.2.1) railties (>= 3.2.16) - json (1.8.2) + json (1.8.3) jwt (0.1.13) multi_json (>= 1.5) kaminari (0.15.1) @@ -366,7 +366,7 @@ GEM mini_portile (0.6.2) minitest (5.3.5) mousetrap-rails (1.4.6) - multi_json (1.10.1) + multi_json (1.11.1) multi_xml (0.5.5) multipart-post (1.2.0) mysql2 (0.3.16) @@ -449,7 +449,7 @@ GEM quiet_assets (1.0.2) railties (>= 3.1, < 5.0) racc (1.4.10) - rack (1.5.3) + rack (1.5.4) rack-accept (0.4.5) rack (>= 0.4) rack-attack (4.3.0) @@ -469,21 +469,21 @@ GEM rack rack-test (0.6.3) rack (>= 1.0) - rails (4.1.9) - actionmailer (= 4.1.9) - actionpack (= 4.1.9) - actionview (= 4.1.9) - activemodel (= 4.1.9) - activerecord (= 4.1.9) - activesupport (= 4.1.9) + rails (4.1.11) + actionmailer (= 4.1.11) + actionpack (= 4.1.11) + actionview (= 4.1.11) + activemodel (= 4.1.11) + activerecord (= 4.1.11) + activesupport (= 4.1.11) bundler (>= 1.3.0, < 2.0) - railties (= 4.1.9) + railties (= 4.1.11) sprockets-rails (~> 2.0) rails-observers (0.1.2) activemodel (~> 4.0) - railties (4.1.9) - actionpack (= 4.1.9) - activesupport (= 4.1.9) + railties (4.1.11) + actionpack (= 4.1.11) + activesupport (= 4.1.11) rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) rainbow (2.0.0) @@ -633,7 +633,7 @@ GEM multi_json (~> 1.0) rack (~> 1.0) tilt (~> 1.1, != 1.3.0) - sprockets-rails (2.2.4) + sprockets-rails (2.3.1) actionpack (>= 3.0) activesupport (>= 3.0) sprockets (>= 2.8, < 4.0) @@ -807,7 +807,7 @@ DEPENDENCIES rack-cors rack-mini-profiler rack-oauth2 (~> 1.0.5) - rails (~> 4.1.0) + rails (= 4.1.11) raphael-rails (~> 2.1.2) rdoc (~> 3.6) redcarpet (~> 3.3.0) @@ -855,3 +855,6 @@ DEPENDENCIES virtus webmock (~> 1.21.0) wikicloth (= 0.8.1) + +BUNDLED WITH + 1.10.4 From 68e77a36236752131803369e6d9132a6a70c0405 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 16 Jun 2015 20:28:02 -0400 Subject: [PATCH 69/71] Bump jquery-rails gem version --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 309dff855c..a4a4ba74e6 100644 --- a/Gemfile +++ b/Gemfile @@ -193,7 +193,7 @@ gem 'font-awesome-rails', '~> 4.2' gem 'gitlab_emoji', '~> 0.1' gem 'gon', '~> 5.0.0' gem 'jquery-atwho-rails', '~> 1.0.0' -gem 'jquery-rails', '3.1.2' +gem 'jquery-rails', '3.1.3' gem 'jquery-scrollto-rails' gem 'jquery-ui-rails' gem 'nprogress-rails' diff --git a/Gemfile.lock b/Gemfile.lock index a910519107..9d29448515 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -331,7 +331,7 @@ GEM inflecto (0.0.2) ipaddress (0.8.0) jquery-atwho-rails (1.0.1) - jquery-rails (3.1.2) + jquery-rails (3.1.3) railties (>= 3.0, < 5.0) thor (>= 0.14, < 2.0) jquery-scrollto-rails (1.4.3) @@ -777,7 +777,7 @@ DEPENDENCIES html-pipeline (~> 1.11.0) httparty jquery-atwho-rails (~> 1.0.0) - jquery-rails (= 3.1.2) + jquery-rails (= 3.1.3) jquery-scrollto-rails jquery-turbolinks jquery-ui-rails From bdab1c6c4621d70dc71129fa9d42696884875821 Mon Sep 17 00:00:00 2001 From: Karen Carias Date: Wed, 17 Jun 2015 01:02:01 +0000 Subject: [PATCH 70/71] added info about migrating to gitlab.com --- doc/workflow/importing/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/workflow/importing/README.md b/doc/workflow/importing/README.md index 2b2e903742..1939565771 100644 --- a/doc/workflow/importing/README.md +++ b/doc/workflow/importing/README.md @@ -4,3 +4,6 @@ 2. [GitHub](import_projects_from_github.md) 3. [GitLab.com](import_projects_from_gitlab_com.md) 4. [SVN](migrating_from_svn.md) + +### Note +* If you'd like to migrate from a self-hosted GitLab instance to GitLab.com, you can copy your repos by changing the remote and pushing to the new server; but issues and merge requests can't be imported. \ No newline at end of file From a209e62e62954231de60690618b21824735e69d0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 19 Jun 2015 10:37:51 +0200 Subject: [PATCH 71/71] Fix jira integration for EE Signed-off-by: Dmitriy Zaporozhets --- app/models/project.rb | 8 ++++++++ app/services/git_push_service.rb | 2 +- app/services/system_note_service.rb | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index a308428c36..5084e5e2d6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -752,4 +752,12 @@ class Project < ActiveRecord::Base errors.add(:base, 'Failed create wiki') false end + + def reference_issue_tracker? + default_issues_tracker? || jira_tracker_active? + end + + def jira_tracker_active? + jira_tracker? && jira_service.active + end end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 8324d35feb..df7ccd89cf 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -92,7 +92,7 @@ class GitPushService end end - if project.default_issues_tracker? + if project.reference_issue_tracker? create_cross_reference_notes(commit, issues_to_close) end end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index b0495da3fc..d6dcd4cafd 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -224,7 +224,7 @@ class SystemNoteService # # Returns Boolean def self.cross_reference_disallowed?(noteable, mentioner) - return true if noteable.is_a?(ExternalIssue) + return true if noteable.is_a?(ExternalIssue) && !noteable.project.jira_tracker_active? return false unless mentioner.is_a?(MergeRequest) return false unless noteable.is_a?(Commit)