diff options
41 files changed, 189 insertions, 336 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cedfa60b3e..5d3cd42a95d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,25 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 9.5.5 (2017-09-18) + +- [SECURITY] Upgrade mail and nokogiri gems due to security issues. !13662 (Markus Koller) +- [FIXED] Fix division by zero error in blame age mapping. !13803 (Jeff Stubler) +- [FIXED] Fix problems sanitizing URLs with empty passwords. !14083 +- [FIXED] Fix a wrong `X-Gitlab-Event` header when testing webhooks. !14108 +- [FIXED] Fixes the 500 errors caused by a race condition in GPG's tmp directory handling. !14194 (Alexis Reigel) +- [FIXED] Fix Pipeline Triggers to show triggered label and predefined variables (e.g. CI_PIPELINE_TRIGGERED). !14244 +- [FIXED] Fix project feature being deleted when updating project with invalid visibility level. +- [FIXED] Fix new navigation wrapping and causing height to grow. +- [FIXED] Fix buttons with different height in merge request widget. +- [FIXED] Normalize styles for empty state combo button. +- [FIXED] Fix broken svg in jobs dropdown for success status. +- [FIXED] Improve migrations using triggers. +- [FIXED] Disable GitLab Project Import Button if source disabled. +- [CHANGED] Update the GPG verification semantics: A GPG signature must additionally match the committer in order to be verified. !13771 (Alexis Reigel) +- [OTHER] Fix repository equality check and avoid fetching ref if the commit is already available. This affects merge request creation performance. !13685 +- [OTHER] Update documentation for confidential issue. !14117 + ## 9.5.4 (2017-09-06) - [SECURITY] Upgrade mail and nokogiri gems due to security issues. !13662 (Markus Koller) diff --git a/app/assets/javascripts/commit/image_file.js b/app/assets/javascripts/commit/image_file.js index 17d14dc1e79..bb33e9eb783 100644 --- a/app/assets/javascripts/commit/image_file.js +++ b/app/assets/javascripts/commit/image_file.js @@ -134,8 +134,9 @@ width: maxWidth + 1, height: maxHeight + 2 }); + // Set swipeBar left position to match image frame $swipeBar.css({ - left: 0 + left: 1 }); wrapPadding = parseInt($swipeWrap.css('right').replace('px', ''), 10); diff --git a/app/assets/stylesheets/new_nav.scss b/app/assets/stylesheets/new_nav.scss index 58e205537ef..8c5bafac637 100644 --- a/app/assets/stylesheets/new_nav.scss +++ b/app/assets/stylesheets/new_nav.scss @@ -375,8 +375,6 @@ header.navbar-gitlab-new { display: flex; width: 100%; position: relative; - padding-top: $gl-padding; - padding-bottom: $gl-padding; align-items: center; border-bottom: 1px solid $border-color; } @@ -388,6 +386,11 @@ header.navbar-gitlab-new { align-self: center; color: $gl-text-color-secondary; + @media (max-width: $screen-xs-max) { + padding-left: 17px; + border-left: 1px solid $gl-text-color-quaternary; + } + .avatar-tile { margin-right: 4px; border: 1px solid $border-color; diff --git a/app/assets/stylesheets/new_sidebar.scss b/app/assets/stylesheets/new_sidebar.scss index 4d5e3d1eceb..9c404b7e542 100644 --- a/app/assets/stylesheets/new_sidebar.scss +++ b/app/assets/stylesheets/new_sidebar.scss @@ -445,9 +445,8 @@ $new-sidebar-collapsed-width: 50px; background-color: transparent; border: 0; padding: 6px 16px; - margin: 0 16px 0 -15px; + margin: 0 0 0 -15px; height: 46px; - border-right: 1px solid $gl-text-color-quaternary; i { font-size: 20px; @@ -455,7 +454,12 @@ $new-sidebar-collapsed-width: 50px; } @media (max-width: $screen-xs-max) { - display: inline-block; + display: flex; + align-items: center; + + i { + font-size: 18px; + } } } diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 052c005a2e8..46d31e41ada 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -727,6 +727,12 @@ ul.notes { border-bottom-left-radius: 0; } + .btn { + svg path { + fill: $gray-darkest; + } + } + .btn.discussion-create-issue-btn { margin-left: -4px; border-radius: 0; @@ -741,10 +747,6 @@ ul.notes { border: 0; } } - - .new-issue-for-discussion path { - fill: $gray-darkest; - } } } @@ -817,16 +819,6 @@ ul.notes { vertical-align: middle; } -.discussion-next-btn { - svg { - margin: 0; - - path { - fill: $gray-darkest; - } - } -} - // Merge request notes in diffs .diff-file { // Diff is inline diff --git a/app/models/event.rb b/app/models/event.rb index 0b1f053a7e6..0997b056c6a 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,7 +1,7 @@ class Event < ActiveRecord::Base include Sortable include IgnorableColumn - default_scope { reorder(nil).where.not(author_id: nil) } + default_scope { reorder(nil) } CREATED = 1 UPDATED = 2 @@ -77,6 +77,12 @@ class Event < ActiveRecord::Base scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) } + # Authors are required as they're used to display who pushed data. + # + # We're just validating the presence of the ID here as foreign key constraints + # should ensure the ID points to a valid user. + validates :author_id, presence: true + self.inheritance_column = 'action' # "data" will be removed in 10.0 but it may be possible that JOINs happen that diff --git a/app/models/key.rb b/app/models/key.rb index 4fa6cac2fd0..0c41e34d969 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -4,8 +4,6 @@ class Key < ActiveRecord::Base include Gitlab::CurrentSettings include Sortable - LAST_USED_AT_REFRESH_TIME = 1.day.to_i - belongs_to :user before_validation :generate_fingerprint @@ -54,10 +52,7 @@ class Key < ActiveRecord::Base end def update_last_used_at - lease = Gitlab::ExclusiveLease.new("key_update_last_used_at:#{id}", timeout: LAST_USED_AT_REFRESH_TIME) - return unless lease.try_obtain - - UseKeyWorker.perform_async(id) + Keys::LastUsedService.new(self).execute end def add_to_shell diff --git a/app/models/push_event.rb b/app/models/push_event.rb index 708513c7861..83ce9014094 100644 --- a/app/models/push_event.rb +++ b/app/models/push_event.rb @@ -3,12 +3,6 @@ class PushEvent < Event # different "action" value. validate :validate_push_action - # Authors are required as they're used to display who pushed data. - # - # We're just validating the presence of the ID here as foreign key constraints - # should ensure the ID points to a valid user. - validates :author_id, presence: true - # The project is required to build links to commits, commit ranges, etc. # # We're just validating the presence of the ID here as foreign key constraints diff --git a/app/services/keys/last_used_service.rb b/app/services/keys/last_used_service.rb new file mode 100644 index 00000000000..066f3246158 --- /dev/null +++ b/app/services/keys/last_used_service.rb @@ -0,0 +1,33 @@ +module Keys + class LastUsedService + TIMEOUT = 1.day.to_i + + attr_reader :key + + # key - The Key for which to update the last used timestamp. + def initialize(key) + @key = key + end + + def execute + # We _only_ want to update last_used_at and not also updated_at (which + # would be updated when using #touch). + key.update_column(:last_used_at, Time.zone.now) if update? + end + + def update? + last_used = key.last_used_at + + return false if last_used && (Time.zone.now - last_used) <= TIMEOUT + + !!redis_lease.try_obtain + end + + private + + def redis_lease + Gitlab::ExclusiveLease + .new("key_update_last_used_at:#{key.id}", timeout: TIMEOUT) + end + end +end diff --git a/app/views/dashboard/projects/index.html.haml b/app/views/dashboard/projects/index.html.haml index c546252455a..a4dc49d2120 100644 --- a/app/views/dashboard/projects/index.html.haml +++ b/app/views/dashboard/projects/index.html.haml @@ -10,9 +10,6 @@ = render "projects/last_push" %div{ class: container_class } - - if show_callout?('user_callout_dismissed') - = render 'shared/user_callout' - - if has_projects_or_name?(@projects, params) = render 'dashboard/projects_head' = render 'projects' diff --git a/app/views/discussions/_new_issue_for_all_discussions.html.haml b/app/views/discussions/_new_issue_for_all_discussions.html.haml index cab346fb514..50dd5864195 100644 --- a/app/views/discussions/_new_issue_for_all_discussions.html.haml +++ b/app/views/discussions/_new_issue_for_all_discussions.html.haml @@ -1,6 +1,8 @@ - if merge_request.discussions_can_be_resolved_by?(current_user) && can?(current_user, :create_issue, @project) .btn-group{ role: "group", "v-if" => "unresolvedDiscussionCount > 0" } - .btn.btn-default.discussion-create-issue-btn.has-tooltip{ title: "Resolve all discussions in new issue", - "aria-label" => "Resolve all discussions in a new issue", - "data-container" => "body" } - = link_to custom_icon('icon_mr_issue'), new_project_issue_path(@project, merge_request_to_resolve_discussions_of: merge_request.iid), title: "Resolve all discussions in new issue", class: 'new-issue-for-discussion' + = link_to custom_icon('icon_mr_issue'), + new_project_issue_path(@project, merge_request_to_resolve_discussions_of: merge_request.iid), + title: 'Resolve all discussions in new issue', + aria: { label: 'Resolve all discussions in new issue' }, + data: { container: 'body' }, + class: 'new-issue-for-discussion btn btn-default discussion-create-issue-btn has-tooltip' diff --git a/app/views/discussions/_new_issue_for_discussion.html.haml b/app/views/discussions/_new_issue_for_discussion.html.haml index a9bc317b8f8..2bfe118c608 100644 --- a/app/views/discussions/_new_issue_for_discussion.html.haml +++ b/app/views/discussions/_new_issue_for_discussion.html.haml @@ -2,7 +2,9 @@ %new-issue-for-discussion-btn{ ":discussion-id" => "'#{discussion.id}'", "inline-template" => true } .btn-group{ role: "group", "v-if" => "showButton" } - .btn.btn-default.discussion-create-issue-btn.has-tooltip{ title: "Resolve this discussion in a new issue", - "aria-label" => "Resolve this discussion in a new issue", - "data-container" => "body" } - = link_to custom_icon('icon_mr_issue'), new_project_issue_path(@project, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id), title: "Resolve this discussion in a new issue", class: 'new-issue-for-discussion' + = link_to custom_icon('icon_mr_issue'), + new_project_issue_path(@project, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id), + title: 'Resolve this discussion in a new issue', + aria: { label: 'Resolve this discussion in a new issue' }, + data: { container: 'body' }, + class: 'new-issue-for-discussion btn btn-default discussion-create-issue-btn has-tooltip' diff --git a/app/views/shared/_user_callout.html.haml b/app/views/shared/_user_callout.html.haml deleted file mode 100644 index 17ffcba69d8..00000000000 --- a/app/views/shared/_user_callout.html.haml +++ /dev/null @@ -1,13 +0,0 @@ -.user-callout{ data: { uid: 'user_callout_dismissed' } } - .bordered-box.landing.content-block - %button.btn.btn-default.close.js-close-callout{ type: 'button', - 'aria-label' => 'Dismiss customize experience box' } - = icon('times', class: 'dismiss-icon', 'aria-hidden' => 'true') - .svg-container - = custom_icon('icon_customization') - .user-callout-copy - %h4 - Customize your experience - %p - Change syntax themes, default project pages, and more in preferences. - = link_to 'Check it out', profile_preferences_path, class: 'btn btn-primary js-close-callout' diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 879e0f99b14..d0ffcc88d43 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -99,8 +99,6 @@ Snippets %div{ class: container_class } - - if @user == current_user && show_callout?('user_callout_dismissed') - = render 'shared/user_callout' .tab-content #activity.tab-pane .row-content-block.calender-block.white.second-block.hidden-xs diff --git a/app/workers/use_key_worker.rb b/app/workers/use_key_worker.rb deleted file mode 100644 index c9d382cc5d6..00000000000 --- a/app/workers/use_key_worker.rb +++ /dev/null @@ -1,13 +0,0 @@ -class UseKeyWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue - - def perform(key_id) - key = Key.find(key_id) - key.touch(:last_used_at) - rescue ActiveRecord::RecordNotFound - Rails.logger.error("UseKeyWorker: couldn't find key with ID=#{key_id}, skipping job") - - false - end -end diff --git a/changelogs/unreleased/35441-fix-division-by-zero.yml b/changelogs/unreleased/35441-fix-division-by-zero.yml deleted file mode 100644 index 335b2d40494..00000000000 --- a/changelogs/unreleased/35441-fix-division-by-zero.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix division by zero error in blame age mapping -merge_request: 13803 -author: Jeff Stubler -type: fixed diff --git a/changelogs/unreleased/36638-select-project-to-create-issue-button-is-disconnected-from-dropdown-button.yml b/changelogs/unreleased/36638-select-project-to-create-issue-button-is-disconnected-from-dropdown-button.yml deleted file mode 100644 index 76c9c905590..00000000000 --- a/changelogs/unreleased/36638-select-project-to-create-issue-button-is-disconnected-from-dropdown-button.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Normalize styles for empty state combo button -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/36821-fix-new-nav-wrapping-caret-and-increasing-height.yml b/changelogs/unreleased/36821-fix-new-nav-wrapping-caret-and-increasing-height.yml deleted file mode 100644 index 54c7a8c8788..00000000000 --- a/changelogs/unreleased/36821-fix-new-nav-wrapping-caret-and-increasing-height.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix new navigation wrapping and causing height to grow -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/36882-disable-gitlab-project-import-button-if-source-disabled.yml b/changelogs/unreleased/36882-disable-gitlab-project-import-button-if-source-disabled.yml deleted file mode 100644 index a06c84c30e6..00000000000 --- a/changelogs/unreleased/36882-disable-gitlab-project-import-button-if-source-disabled.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Disable GitLab Project Import Button if source disabled -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/37288-fix-wrong-header-when-testing-webhook.yml b/changelogs/unreleased/37288-fix-wrong-header-when-testing-webhook.yml deleted file mode 100644 index d6d21ac4c51..00000000000 --- a/changelogs/unreleased/37288-fix-wrong-header-when-testing-webhook.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix a wrong `X-Gitlab-Event` header when testing webhooks -merge_request: 14108 -author: -type: fixed diff --git a/changelogs/unreleased/37331-button-MR-widget.yml b/changelogs/unreleased/37331-button-MR-widget.yml deleted file mode 100644 index 59bc1bd201e..00000000000 --- a/changelogs/unreleased/37331-button-MR-widget.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix buttons with different height in merge request widget -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/37406-success-status-icon.yml b/changelogs/unreleased/37406-success-status-icon.yml deleted file mode 100644 index faac947f188..00000000000 --- a/changelogs/unreleased/37406-success-status-icon.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix broken svg in jobs dropdown for success status -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/38049-fix-resolve-in-new-issue-btn.yml b/changelogs/unreleased/38049-fix-resolve-in-new-issue-btn.yml new file mode 100644 index 00000000000..a904c656f4f --- /dev/null +++ b/changelogs/unreleased/38049-fix-resolve-in-new-issue-btn.yml @@ -0,0 +1,5 @@ +--- +title: Fix the "resolve discussion in a new issue" button +merge_request: 14357 +author: +type: fixed diff --git a/changelogs/unreleased/check-trigger-permissions.yml b/changelogs/unreleased/check-trigger-permissions.yml deleted file mode 100644 index e0809cea9bf..00000000000 --- a/changelogs/unreleased/check-trigger-permissions.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Improve migrations using triggers -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/docs-confidential-issue.yml b/changelogs/unreleased/docs-confidential-issue.yml deleted file mode 100644 index 841970ef4cf..00000000000 --- a/changelogs/unreleased/docs-confidential-issue.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Update documentation for confidential issue -merge_request: 14117 -author: -type: other diff --git a/changelogs/unreleased/events-redundant-where.yml b/changelogs/unreleased/events-redundant-where.yml new file mode 100644 index 00000000000..a118a2e33e1 --- /dev/null +++ b/changelogs/unreleased/events-redundant-where.yml @@ -0,0 +1,5 @@ +--- +title: Remove redundant WHERE from event queries +merge_request: +author: +type: other diff --git a/changelogs/unreleased/feature-gpg-verification-status.yml b/changelogs/unreleased/feature-gpg-verification-status.yml deleted file mode 100644 index 7518fafcdb8..00000000000 --- a/changelogs/unreleased/feature-gpg-verification-status.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: 'Update the GPG verification semantics: A GPG signature must additionally match - the committer in order to be verified' -merge_request: 13771 -author: Alexis Reigel -type: changed diff --git a/changelogs/unreleased/fix-gpg-tmp-dir-removal-race-condition.yml b/changelogs/unreleased/fix-gpg-tmp-dir-removal-race-condition.yml deleted file mode 100644 index e75f188913f..00000000000 --- a/changelogs/unreleased/fix-gpg-tmp-dir-removal-race-condition.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fixes the 500 errors caused by a race condition in GPG's tmp directory handling -merge_request: 14194 -author: Alexis Reigel -type: fixed diff --git a/changelogs/unreleased/fix-image-diff-swipe-handle.yml b/changelogs/unreleased/fix-image-diff-swipe-handle.yml new file mode 100644 index 00000000000..a4e0c2e8465 --- /dev/null +++ b/changelogs/unreleased/fix-image-diff-swipe-handle.yml @@ -0,0 +1,5 @@ +--- +title: Fix image diff swipe handle offset to correctly align with the frame +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/fix-sm-37559-pipeline-triggered-through-api-not-showing-trigger-variables.yml b/changelogs/unreleased/fix-sm-37559-pipeline-triggered-through-api-not-showing-trigger-variables.yml deleted file mode 100644 index 8aae0f6f5b6..00000000000 --- a/changelogs/unreleased/fix-sm-37559-pipeline-triggered-through-api-not-showing-trigger-variables.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: Fix Pipeline Triggers to show triggered label and predefined variables (e.g. - CI_PIPELINE_TRIGGERED) -merge_request: 14244 -author: -type: fixed diff --git a/changelogs/unreleased/issue_37640.yml b/changelogs/unreleased/issue_37640.yml deleted file mode 100644 index d806ed64bed..00000000000 --- a/changelogs/unreleased/issue_37640.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: Fix project feature being deleted when updating project with invalid visibility - level -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/perf-slow-issuable.yml b/changelogs/unreleased/perf-slow-issuable.yml deleted file mode 100644 index 29d15be1401..00000000000 --- a/changelogs/unreleased/perf-slow-issuable.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: Fix repository equality check and avoid fetching ref if the commit is already - available. This affects merge request creation performance -merge_request: 13685 -author: -type: other diff --git a/changelogs/unreleased/remove-use-key-worker.yml b/changelogs/unreleased/remove-use-key-worker.yml new file mode 100644 index 00000000000..a39bcae66bc --- /dev/null +++ b/changelogs/unreleased/remove-use-key-worker.yml @@ -0,0 +1,5 @@ +--- +title: Stop using Sidekiq for updating Key#last_used_at +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/url-sanitizer-fixes.yml b/changelogs/unreleased/url-sanitizer-fixes.yml deleted file mode 100644 index 769036c829c..00000000000 --- a/changelogs/unreleased/url-sanitizer-fixes.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix problems sanitizing URLs with empty passwords -merge_request: 14083 -author: -type: fixed diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 24c001362c6..d169c38a693 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -38,7 +38,6 @@ - [invalid_gpg_signature_update, 2] - [create_gpg_signature, 2] - [upload_checksum, 1] - - [use_key, 1] - [repository_fork, 1] - [repository_import, 1] - [project_service, 1] diff --git a/spec/features/user_callout_spec.rb b/spec/features/user_callout_spec.rb deleted file mode 100644 index 37d66b618af..00000000000 --- a/spec/features/user_callout_spec.rb +++ /dev/null @@ -1,55 +0,0 @@ -require 'spec_helper' - -describe 'User Callouts', js: true do - let(:user) { create(:user) } - let(:another_user) { create(:user) } - let(:project) { create(:project, path: 'gitlab', name: 'sample') } - - before do - sign_in(user) - project.team << [user, :master] - end - - it 'takes you to the profile preferences when the link is clicked' do - visit dashboard_projects_path - click_link 'Check it out' - expect(current_path).to eq profile_preferences_path - end - - it 'does not show when cookie is set' do - visit dashboard_projects_path - - within('.user-callout') do - find('.close').trigger('click') - end - - visit dashboard_projects_path - - expect(page).not_to have_selector('.user-callout') - end - - describe 'user callout should appear in two routes' do - it 'shows up on the user profile' do - visit user_path(user) - expect(find('.user-callout')).to have_content 'Customize your experience' - end - - it 'shows up on the dashboard projects' do - visit dashboard_projects_path - expect(find('.user-callout')).to have_content 'Customize your experience' - end - end - - it 'hides the user callout when click on the dismiss icon' do - visit user_path(user) - within('.user-callout') do - find('.close').click - end - expect(page).not_to have_selector('.user-callout') - end - - it 'does not show callout on another users profile' do - visit user_path(another_user) - expect(page).not_to have_selector('.user-callout') - end -end diff --git a/spec/javascripts/fixtures/dashboard.rb b/spec/javascripts/fixtures/dashboard.rb deleted file mode 100644 index 7fa351680c9..00000000000 --- a/spec/javascripts/fixtures/dashboard.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'spec_helper' - -describe Dashboard::ProjectsController, '(JavaScript fixtures)', type: :controller do - include JavaScriptFixturesHelpers - - let(:admin) { create(:admin) } - let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} - let(:project) { create(:project, namespace: namespace, path: 'builds-project') } - - render_views - - before(:all) do - clean_frontend_fixtures('dashboard/') - end - - before do - sign_in(admin) - end - - after do - remove_repository(project) - end - - it 'dashboard/user-callout.html.raw' do |example| - rendered = render_template('shared/_user_callout') - store_frontend_fixture(rendered, example.description) - end - - private - - def render_template(template_file_name) - controller.prepend_view_path(JavaScriptFixturesHelpers::FIXTURE_PATH) - controller.render_to_string(template_file_name, layout: false) - end -end diff --git a/spec/javascripts/user_callout_spec.js b/spec/javascripts/user_callout_spec.js deleted file mode 100644 index 69cb93bd850..00000000000 --- a/spec/javascripts/user_callout_spec.js +++ /dev/null @@ -1,49 +0,0 @@ -import Cookies from 'js-cookie'; -import UserCallout from '~/user_callout'; - -const USER_CALLOUT_COOKIE = 'user_callout_dismissed'; - -describe('UserCallout', function () { - const fixtureName = 'dashboard/user-callout.html.raw'; - preloadFixtures(fixtureName); - - beforeEach(() => { - loadFixtures(fixtureName); - Cookies.remove(USER_CALLOUT_COOKIE); - - this.userCallout = new UserCallout(); - this.closeButton = $('.js-close-callout.close'); - this.userCalloutBtn = $('.js-close-callout:not(.close)'); - }); - - it('hides when user clicks on the dismiss-icon', (done) => { - this.closeButton.click(); - expect(Cookies.get(USER_CALLOUT_COOKIE)).toBe('true'); - - setTimeout(() => { - expect( - document.querySelector('.user-callout'), - ).toBeNull(); - - done(); - }); - }); - - it('hides when user clicks on the "check it out" button', () => { - this.userCalloutBtn.click(); - expect(Cookies.get(USER_CALLOUT_COOKIE)).toBe('true'); - }); - - describe('Sets cookie with setCalloutPerProject', () => { - beforeEach(() => { - spyOn(Cookies, 'set').and.callFake(() => {}); - document.querySelector('.user-callout').setAttribute('data-project-path', 'foo/bar'); - this.userCallout = new UserCallout({ setCalloutPerProject: true }); - }); - - it('sets a cookie when the user clicks the close button', () => { - this.userCalloutBtn.click(); - expect(Cookies.set).toHaveBeenCalledWith('user_callout_dismissed', 'true', Object({ expires: 365, path: 'foo/bar' })); - }); - }); -}); diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index dbc4aba8547..8eabc4ca72f 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -37,30 +37,17 @@ describe Key, :mailer do end describe "#update_last_used_at" do - let(:key) { create(:key) } - - context 'when key was not updated during the last day' do - before do - allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) - .and_return('000000') - end - - it 'enqueues a UseKeyWorker job' do - expect(UseKeyWorker).to receive(:perform_async).with(key.id) - key.update_last_used_at - end - end + it 'updates the last used timestamp' do + key = build(:key) + service = double(:service) + + expect(Keys::LastUsedService).to receive(:new) + .with(key) + .and_return(service) - context 'when key was updated during the last day' do - before do - allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) - .and_return(false) - end + expect(service).to receive(:execute) - it 'does not enqueue a UseKeyWorker job' do - expect(UseKeyWorker).not_to receive(:perform_async) - key.update_last_used_at - end + key.update_last_used_at end end end diff --git a/spec/services/keys/last_used_service_spec.rb b/spec/services/keys/last_used_service_spec.rb new file mode 100644 index 00000000000..edaace293ae --- /dev/null +++ b/spec/services/keys/last_used_service_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +describe Keys::LastUsedService do + describe '#execute', :clean_gitlab_redis_shared_state do + it 'updates the key when it has not been used recently' do + key = create(:key, last_used_at: 1.year.ago) + time = Time.zone.now + + Timecop.freeze(time) { described_class.new(key).execute } + + expect(key.last_used_at).to eq(time) + end + + it 'does not update the key when it has been used recently' do + time = 1.minute.ago + key = create(:key, last_used_at: time) + + described_class.new(key).execute + + expect(key.last_used_at).to eq(time) + end + + it 'does not update the updated_at field' do + # Since a lot of these updates could happen in parallel for different keys + # we want these updates to be as lightweight as possible, hence we want to + # make sure we _only_ update last_used_at and not always updated_at. + key = create(:key, last_used_at: 1.year.ago) + + recorder = ActiveRecord::QueryRecorder.new do + described_class.new(key).execute + end + + expect(recorder.count).to eq(1) + expect(recorder.log[0]).not_to include('updated_at') + end + end + + describe '#update?', :clean_gitlab_redis_shared_state do + it 'returns true when no last used timestamp is present' do + key = build(:key, last_used_at: nil) + service = described_class.new(key) + + expect(service.update?).to eq(true) + end + + it 'returns true when the key needs to be updated' do + key = build(:key, last_used_at: 1.year.ago) + service = described_class.new(key) + + expect(service.update?).to eq(true) + end + + it 'returns false when a lease has already been obtained' do + key = build(:key, last_used_at: 1.year.ago) + service = described_class.new(key) + + expect(service.update?).to eq(true) + expect(service.update?).to eq(false) + end + + it 'returns false when the key does not yet need to be updated' do + key = build(:key, last_used_at: 1.minute.ago) + service = described_class.new(key) + + expect(service.update?).to eq(false) + end + end +end diff --git a/spec/workers/use_key_worker_spec.rb b/spec/workers/use_key_worker_spec.rb deleted file mode 100644 index e50c788b82a..00000000000 --- a/spec/workers/use_key_worker_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -describe UseKeyWorker do - describe "#perform" do - it "updates the key's last_used_at attribute to the current time when it exists" do - worker = described_class.new - key = create(:key) - current_time = Time.zone.now - - Timecop.freeze(current_time) do - expect { worker.perform(key.id) } - .to change { key.reload.last_used_at }.from(nil).to be_like_time(current_time) - end - end - - it "returns false and skips the job when the key doesn't exist" do - worker = described_class.new - key = create(:key) - - expect(worker.perform(key.id + 1)).to eq false - end - end -end |