diff options
89 files changed, 478 insertions, 42 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 99e0d1ed987..61825a7bf03 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.120.0 +0.121.0 diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index 39d90a64aab..eb596b251c1 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -11,8 +11,10 @@ export default () => ({ endpoint: '', basePath: '', commit: null, + startVersion: null, diffFiles: [], mergeRequestDiffs: [], + mergeRequestDiff: null, diffLineCommentForms: {}, diffViewType: viewTypeFromQueryString || viewTypeFromCookie || defaultViewType, }); diff --git a/app/assets/javascripts/diffs/store/modules/index.js b/app/assets/javascripts/diffs/store/modules/index.js index 20d1ebbe049..6860e24db6b 100644 --- a/app/assets/javascripts/diffs/store/modules/index.js +++ b/app/assets/javascripts/diffs/store/modules/index.js @@ -3,10 +3,10 @@ import * as getters from '../getters'; import mutations from '../mutations'; import createState from './diff_state'; -export default { +export default () => ({ namespaced: true, state: createState(), getters, actions, mutations, -}; +}); diff --git a/app/assets/javascripts/mr_notes/stores/index.js b/app/assets/javascripts/mr_notes/stores/index.js index dd2019001db..446eb477efc 100644 --- a/app/assets/javascripts/mr_notes/stores/index.js +++ b/app/assets/javascripts/mr_notes/stores/index.js @@ -9,7 +9,7 @@ Vue.use(Vuex); export default new Vuex.Store({ modules: { page: mrPageModule, - notes: notesModule, - diffs: diffsModule, + notes: notesModule(), + diffs: diffsModule(), }, }); diff --git a/app/assets/javascripts/notes/stores/index.js b/app/assets/javascripts/notes/stores/index.js index 0f48b8880f4..f105b7d0d11 100644 --- a/app/assets/javascripts/notes/stores/index.js +++ b/app/assets/javascripts/notes/stores/index.js @@ -1,16 +1,8 @@ import Vue from 'vue'; import Vuex from 'vuex'; -import * as actions from './actions'; -import * as getters from './getters'; -import mutations from './mutations'; -import module from './modules'; +import notesModule from './modules'; Vue.use(Vuex); export default () => - new Vuex.Store({ - state: module.state, - actions, - getters, - mutations, - }); + new Vuex.Store(notesModule()); diff --git a/app/assets/javascripts/notes/stores/modules/index.js b/app/assets/javascripts/notes/stores/modules/index.js index b4cb9267e0f..61dbb075586 100644 --- a/app/assets/javascripts/notes/stores/modules/index.js +++ b/app/assets/javascripts/notes/stores/modules/index.js @@ -2,7 +2,7 @@ import * as actions from '../actions'; import * as getters from '../getters'; import mutations from '../mutations'; -export default { +export default () => ({ state: { discussions: [], targetNoteHash: null, @@ -24,4 +24,4 @@ export default { actions, getters, mutations, -}; +}); diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index 2c04bfea122..16bb54be126 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -214,12 +214,7 @@ export default { [types.SET_DISCUSSION_DIFF_LINES](state, { discussionId, diffLines }) { const discussion = utils.findNoteObjectById(state.discussions, discussionId); - const index = state.discussions.indexOf(discussion); - const discussionWithDiffLines = Object.assign({}, discussion, { - truncated_diff_lines: diffLines, - }); - - state.discussions.splice(index, 1, discussionWithDiffLines); + discussion.truncated_diff_lines = diffLines; }, }; diff --git a/app/finders/access_requests_finder.rb b/app/finders/access_requests_finder.rb index b6ee49df99b..2cc8a978877 100644 --- a/app/finders/access_requests_finder.rb +++ b/app/finders/access_requests_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AccessRequestsFinder attr_accessor :source diff --git a/app/finders/admin/projects_finder.rb b/app/finders/admin/projects_finder.rb index 6b805c432f6..e2b9b0b44c1 100644 --- a/app/finders/admin/projects_finder.rb +++ b/app/finders/admin/projects_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Admin::ProjectsFinder attr_reader :params, :current_user diff --git a/app/finders/branches_finder.rb b/app/finders/branches_finder.rb index 8bb1366867c..970efa79dfb 100644 --- a/app/finders/branches_finder.rb +++ b/app/finders/branches_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class BranchesFinder def initialize(repository, params = {}) @repository = repository diff --git a/app/finders/clusters_finder.rb b/app/finders/clusters_finder.rb index c13f98257bf..b40d6c41b71 100644 --- a/app/finders/clusters_finder.rb +++ b/app/finders/clusters_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ClustersFinder def initialize(project, user, scope) @project = project diff --git a/app/finders/concerns/created_at_filter.rb b/app/finders/concerns/created_at_filter.rb index ac9ac77732c..6b5863a5c53 100644 --- a/app/finders/concerns/created_at_filter.rb +++ b/app/finders/concerns/created_at_filter.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module CreatedAtFilter def by_created_at(items) items = items.created_before(params[:created_before]) if params[:created_before].present? diff --git a/app/finders/concerns/custom_attributes_filter.rb b/app/finders/concerns/custom_attributes_filter.rb index 022a19b0a7f..825c3a6b5b7 100644 --- a/app/finders/concerns/custom_attributes_filter.rb +++ b/app/finders/concerns/custom_attributes_filter.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module CustomAttributesFilter # rubocop: disable CodeReuse/ActiveRecord def by_custom_attributes(items) diff --git a/app/finders/concerns/finder_methods.rb b/app/finders/concerns/finder_methods.rb index ce0ec214b8a..5290313585f 100644 --- a/app/finders/concerns/finder_methods.rb +++ b/app/finders/concerns/finder_methods.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module FinderMethods # rubocop: disable CodeReuse/ActiveRecord def find_by!(*args) diff --git a/app/finders/concerns/finder_with_cross_project_access.rb b/app/finders/concerns/finder_with_cross_project_access.rb index f70a0b60864..e038636f0c4 100644 --- a/app/finders/concerns/finder_with_cross_project_access.rb +++ b/app/finders/concerns/finder_with_cross_project_access.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Module to prepend into finders to specify wether or not the finder requires # cross project access # diff --git a/app/finders/contributed_projects_finder.rb b/app/finders/contributed_projects_finder.rb index 1447def0cdf..c1ef9dfefa7 100644 --- a/app/finders/contributed_projects_finder.rb +++ b/app/finders/contributed_projects_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ContributedProjectsFinder < UnionFinder def initialize(user) @user = user diff --git a/app/finders/environments_finder.rb b/app/finders/environments_finder.rb index 95af6214c7d..419be46fafe 100644 --- a/app/finders/environments_finder.rb +++ b/app/finders/environments_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class EnvironmentsFinder attr_reader :project, :current_user, :params diff --git a/app/finders/events_finder.rb b/app/finders/events_finder.rb index a8d4f56a84c..fd7aeca0d8b 100644 --- a/app/finders/events_finder.rb +++ b/app/finders/events_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class EventsFinder prepend FinderMethods prepend FinderWithCrossProjectAccess diff --git a/app/finders/fork_projects_finder.rb b/app/finders/fork_projects_finder.rb index 8713d43136f..03ace7e8057 100644 --- a/app/finders/fork_projects_finder.rb +++ b/app/finders/fork_projects_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ForkProjectsFinder < ProjectsFinder # rubocop: disable CodeReuse/ActiveRecord def initialize(project, params: {}, current_user: nil) diff --git a/app/finders/group_descendants_finder.rb b/app/finders/group_descendants_finder.rb index 15060a99930..9d57d2d3bc9 100644 --- a/app/finders/group_descendants_finder.rb +++ b/app/finders/group_descendants_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # GroupDescendantsFinder # # Used to find and filter all subgroups and projects of a passed parent group diff --git a/app/finders/group_finder.rb b/app/finders/group_finder.rb index 8fde3a00af7..d2ad8a372b1 100644 --- a/app/finders/group_finder.rb +++ b/app/finders/group_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GroupFinder include Gitlab::Allowable diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb index cab1ff8fb98..eebc67cfa9e 100644 --- a/app/finders/group_members_finder.rb +++ b/app/finders/group_members_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GroupMembersFinder def initialize(group) @group = group diff --git a/app/finders/group_projects_finder.rb b/app/finders/group_projects_finder.rb index 52a347fa598..4155b6af8da 100644 --- a/app/finders/group_projects_finder.rb +++ b/app/finders/group_projects_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # GroupProjectsFinder # # Used to filter Projects by set of params diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb index 436abb332ff..a35a3ed6142 100644 --- a/app/finders/groups_finder.rb +++ b/app/finders/groups_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # GroupsFinder # # Used to filter Groups by a set of params diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 3308fd6d697..251a559878a 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # IssuableFinder # # Used to filter Issues and MergeRequests collections by set of params diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index c347ef1ec19..770e0bfe1a3 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Finders::Issues class # # Used to filter Issues collections by set of params diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb index 47174980258..18cc6891ca4 100644 --- a/app/finders/joined_groups_finder.rb +++ b/app/finders/joined_groups_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class JoinedGroupsFinder < UnionFinder def initialize(user) @user = user diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 17c55f8db50..08fc2968e77 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class LabelsFinder < UnionFinder prepend FinderWithCrossProjectAccess include FinderMethods diff --git a/app/finders/license_template_finder.rb b/app/finders/license_template_finder.rb index fad33f0eca2..196922709f7 100644 --- a/app/finders/license_template_finder.rb +++ b/app/finders/license_template_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # LicenseTemplateFinder # # Used to find license templates, which may come from a variety of external diff --git a/app/finders/members_finder.rb b/app/finders/members_finder.rb index 58e3606692d..48777838d60 100644 --- a/app/finders/members_finder.rb +++ b/app/finders/members_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class MembersFinder attr_reader :project, :current_user, :group diff --git a/app/finders/merge_request_target_project_finder.rb b/app/finders/merge_request_target_project_finder.rb index 66d4c4018f3..5f0589f6c8b 100644 --- a/app/finders/merge_request_target_project_finder.rb +++ b/app/finders/merge_request_target_project_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class MergeRequestTargetProjectFinder include FinderMethods diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index c6403f8f25d..b698a3c7b09 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Finders::MergeRequest class # # Used to filter MergeRequests collections by set of params diff --git a/app/finders/milestones_finder.rb b/app/finders/milestones_finder.rb index 7dc2a5b0b66..47231ea80c7 100644 --- a/app/finders/milestones_finder.rb +++ b/app/finders/milestones_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Search for milestones # # params - Hash diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index 580984329a1..c67c2065440 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class NotesFinder FETCH_OVERLAP = 5.seconds diff --git a/app/finders/personal_access_tokens_finder.rb b/app/finders/personal_access_tokens_finder.rb index 6b610ffc687..5beea92689f 100644 --- a/app/finders/personal_access_tokens_finder.rb +++ b/app/finders/personal_access_tokens_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class PersonalAccessTokensFinder attr_accessor :params diff --git a/app/finders/personal_projects_finder.rb b/app/finders/personal_projects_finder.rb index 16e4e060786..20f5b221a89 100644 --- a/app/finders/personal_projects_finder.rb +++ b/app/finders/personal_projects_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class PersonalProjectsFinder < UnionFinder include Gitlab::Allowable diff --git a/app/finders/pipeline_schedules_finder.rb b/app/finders/pipeline_schedules_finder.rb index d131e1659c1..3beee608268 100644 --- a/app/finders/pipeline_schedules_finder.rb +++ b/app/finders/pipeline_schedules_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class PipelineSchedulesFinder attr_reader :project, :pipeline_schedules diff --git a/app/finders/pipelines_finder.rb b/app/finders/pipelines_finder.rb index b03d730f975..3d0d3219a94 100644 --- a/app/finders/pipelines_finder.rb +++ b/app/finders/pipelines_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class PipelinesFinder attr_reader :project, :pipelines, :params, :current_user diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 0f2fdf317a4..c2404412006 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # ProjectsFinder # # Used to filter Projects by set of params diff --git a/app/finders/runner_jobs_finder.rb b/app/finders/runner_jobs_finder.rb index d4f2474197f..4fca4ec94f3 100644 --- a/app/finders/runner_jobs_finder.rb +++ b/app/finders/runner_jobs_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class RunnerJobsFinder attr_reader :runner, :params diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index 32da3e8f404..715dffb972f 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Snippets Finder # # Used to filter Snippets collections by a set of params diff --git a/app/finders/tags_finder.rb b/app/finders/tags_finder.rb index b474f0805dc..2ffd46245e9 100644 --- a/app/finders/tags_finder.rb +++ b/app/finders/tags_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class TagsFinder def initialize(repository, params) @repository = repository diff --git a/app/finders/template_finder.rb b/app/finders/template_finder.rb index 7cdc7a32acc..c92ee9ca9ac 100644 --- a/app/finders/template_finder.rb +++ b/app/finders/template_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class TemplateFinder VENDORED_TEMPLATES = { dockerfiles: ::Gitlab::Template::DockerfileTemplate, diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 6481893a195..0d9f16fdce7 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # TodosFinder # # Used to filter Todos by set of params diff --git a/app/finders/union_finder.rb b/app/finders/union_finder.rb index 91b845871a8..798c3eba0f3 100644 --- a/app/finders/union_finder.rb +++ b/app/finders/union_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class UnionFinder # rubocop: disable CodeReuse/ActiveRecord def find_union(segments, klass) diff --git a/app/finders/user_recent_events_finder.rb b/app/finders/user_recent_events_finder.rb index 7405d6d27d9..a4daf5b5841 100644 --- a/app/finders/user_recent_events_finder.rb +++ b/app/finders/user_recent_events_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Get user activity feed for projects common for a user and a logged in user # # - current_user: The user viewing the events diff --git a/app/finders/users_finder.rb b/app/finders/users_finder.rb index 46672301851..f2ad9b4bda5 100644 --- a/app/finders/users_finder.rb +++ b/app/finders/users_finder.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # UsersFinder # # Used to filter users by set of params diff --git a/app/graphql/functions/base_function.rb b/app/graphql/functions/base_function.rb index 42fb8f99acc..2512ecbd255 100644 --- a/app/graphql/functions/base_function.rb +++ b/app/graphql/functions/base_function.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Functions class BaseFunction < GraphQL::Function end diff --git a/app/graphql/functions/echo.rb b/app/graphql/functions/echo.rb index e5bf109b8d7..3104486faac 100644 --- a/app/graphql/functions/echo.rb +++ b/app/graphql/functions/echo.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Functions class Echo < BaseFunction argument :text, GraphQL::STRING_TYPE diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb index 8755a1a62e7..06d26309b5b 100644 --- a/app/graphql/gitlab_schema.rb +++ b/app/graphql/gitlab_schema.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GitlabSchema < GraphQL::Schema use BatchLoader::GraphQL use Gitlab::Graphql::Authorize diff --git a/app/graphql/mutations/concerns/mutations/resolves_project.rb b/app/graphql/mutations/concerns/mutations/resolves_project.rb index 0dd1f264a52..da9814e88b0 100644 --- a/app/graphql/mutations/concerns/mutations/resolves_project.rb +++ b/app/graphql/mutations/concerns/mutations/resolves_project.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Mutations module ResolvesProject extend ActiveSupport::Concern diff --git a/app/graphql/mutations/merge_requests/base.rb b/app/graphql/mutations/merge_requests/base.rb index 2149e72e2df..54f01c99d78 100644 --- a/app/graphql/mutations/merge_requests/base.rb +++ b/app/graphql/mutations/merge_requests/base.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Mutations module MergeRequests class Base < BaseMutation diff --git a/app/graphql/resolvers/base_resolver.rb b/app/graphql/resolvers/base_resolver.rb index 89b7f9dad6f..459933af9d3 100644 --- a/app/graphql/resolvers/base_resolver.rb +++ b/app/graphql/resolvers/base_resolver.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Resolvers class BaseResolver < GraphQL::Schema::Resolver end diff --git a/app/graphql/resolvers/concerns/resolves_pipelines.rb b/app/graphql/resolvers/concerns/resolves_pipelines.rb index 9ec45378d8e..8fd26d85994 100644 --- a/app/graphql/resolvers/concerns/resolves_pipelines.rb +++ b/app/graphql/resolvers/concerns/resolves_pipelines.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResolvesPipelines extend ActiveSupport::Concern diff --git a/app/graphql/resolvers/full_path_resolver.rb b/app/graphql/resolvers/full_path_resolver.rb index 4eb28aaed6c..8d3da33e8d2 100644 --- a/app/graphql/resolvers/full_path_resolver.rb +++ b/app/graphql/resolvers/full_path_resolver.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Resolvers module FullPathResolver extend ActiveSupport::Concern diff --git a/app/graphql/resolvers/merge_request_pipelines_resolver.rb b/app/graphql/resolvers/merge_request_pipelines_resolver.rb index 00b51ee1381..b371f1335f8 100644 --- a/app/graphql/resolvers/merge_request_pipelines_resolver.rb +++ b/app/graphql/resolvers/merge_request_pipelines_resolver.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Resolvers class MergeRequestPipelinesResolver < BaseResolver include ::ResolvesPipelines diff --git a/app/graphql/resolvers/merge_request_resolver.rb b/app/graphql/resolvers/merge_request_resolver.rb index 6b7d0ce6e24..b87c95217f7 100644 --- a/app/graphql/resolvers/merge_request_resolver.rb +++ b/app/graphql/resolvers/merge_request_resolver.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Resolvers class MergeRequestResolver < BaseResolver argument :iid, GraphQL::ID_TYPE, diff --git a/app/graphql/resolvers/project_pipelines_resolver.rb b/app/graphql/resolvers/project_pipelines_resolver.rb index 7f175a3b26c..86094c46c2a 100644 --- a/app/graphql/resolvers/project_pipelines_resolver.rb +++ b/app/graphql/resolvers/project_pipelines_resolver.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Resolvers class ProjectPipelinesResolver < BaseResolver include ResolvesPipelines diff --git a/app/graphql/resolvers/project_resolver.rb b/app/graphql/resolvers/project_resolver.rb index ec115bad896..ac7c9b0ce2e 100644 --- a/app/graphql/resolvers/project_resolver.rb +++ b/app/graphql/resolvers/project_resolver.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Resolvers class ProjectResolver < BaseResolver prepend FullPathResolver diff --git a/app/graphql/types/base_enum.rb b/app/graphql/types/base_enum.rb index b45a845f74f..cf43fea45e6 100644 --- a/app/graphql/types/base_enum.rb +++ b/app/graphql/types/base_enum.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class BaseEnum < GraphQL::Schema::Enum end diff --git a/app/graphql/types/base_field.rb b/app/graphql/types/base_field.rb index c5740a334d7..2b2ea64c00b 100644 --- a/app/graphql/types/base_field.rb +++ b/app/graphql/types/base_field.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class BaseField < GraphQL::Schema::Field prepend Gitlab::Graphql::Authorize diff --git a/app/graphql/types/base_input_object.rb b/app/graphql/types/base_input_object.rb index 309e336e6c8..aebed035d3b 100644 --- a/app/graphql/types/base_input_object.rb +++ b/app/graphql/types/base_input_object.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class BaseInputObject < GraphQL::Schema::InputObject end diff --git a/app/graphql/types/base_interface.rb b/app/graphql/types/base_interface.rb index 69e72dc5808..3451a195c33 100644 --- a/app/graphql/types/base_interface.rb +++ b/app/graphql/types/base_interface.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types module BaseInterface include GraphQL::Schema::Interface diff --git a/app/graphql/types/base_object.rb b/app/graphql/types/base_object.rb index 754adf4c04d..82b78abd573 100644 --- a/app/graphql/types/base_object.rb +++ b/app/graphql/types/base_object.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class BaseObject < GraphQL::Schema::Object prepend Gitlab::Graphql::Present diff --git a/app/graphql/types/base_scalar.rb b/app/graphql/types/base_scalar.rb index c0aa38be239..719bc808f47 100644 --- a/app/graphql/types/base_scalar.rb +++ b/app/graphql/types/base_scalar.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class BaseScalar < GraphQL::Schema::Scalar end diff --git a/app/graphql/types/base_union.rb b/app/graphql/types/base_union.rb index 36337fc6ee5..30a5668c0bb 100644 --- a/app/graphql/types/base_union.rb +++ b/app/graphql/types/base_union.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class BaseUnion < GraphQL::Schema::Union end diff --git a/app/graphql/types/ci/pipeline_status_enum.rb b/app/graphql/types/ci/pipeline_status_enum.rb index 2c12e5001d8..c19ddf5bb25 100644 --- a/app/graphql/types/ci/pipeline_status_enum.rb +++ b/app/graphql/types/ci/pipeline_status_enum.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types module Ci class PipelineStatusEnum < BaseEnum diff --git a/app/graphql/types/ci/pipeline_type.rb b/app/graphql/types/ci/pipeline_type.rb index bbb7d9354d0..2bbffad4563 100644 --- a/app/graphql/types/ci/pipeline_type.rb +++ b/app/graphql/types/ci/pipeline_type.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types module Ci class PipelineType < BaseObject diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index 88cd2adc6dc..fb740b6fb1c 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class MergeRequestType < BaseObject expose_permissions Types::PermissionTypes::MergeRequest diff --git a/app/graphql/types/permission_types/base_permission_type.rb b/app/graphql/types/permission_types/base_permission_type.rb index 934ed572e56..26a71e2bfbb 100644 --- a/app/graphql/types/permission_types/base_permission_type.rb +++ b/app/graphql/types/permission_types/base_permission_type.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types module PermissionTypes class BasePermissionType < BaseObject diff --git a/app/graphql/types/permission_types/ci/pipeline.rb b/app/graphql/types/permission_types/ci/pipeline.rb index 942539c7cf7..73e44a33eba 100644 --- a/app/graphql/types/permission_types/ci/pipeline.rb +++ b/app/graphql/types/permission_types/ci/pipeline.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types module PermissionTypes module Ci diff --git a/app/graphql/types/permission_types/merge_request.rb b/app/graphql/types/permission_types/merge_request.rb index 5c21f6ee9c6..13995d3ea8f 100644 --- a/app/graphql/types/permission_types/merge_request.rb +++ b/app/graphql/types/permission_types/merge_request.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types module PermissionTypes class MergeRequest < BasePermissionType diff --git a/app/graphql/types/permission_types/project.rb b/app/graphql/types/permission_types/project.rb index 755699a4415..066ce64a254 100644 --- a/app/graphql/types/permission_types/project.rb +++ b/app/graphql/types/permission_types/project.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types module PermissionTypes class Project < BasePermissionType diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index 97707215b4e..7b879608b34 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class ProjectType < BaseObject expose_permissions Types::PermissionTypes::Project diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 010ec2d7942..7c41716b82a 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class QueryType < BaseObject graphql_name 'Query' diff --git a/app/graphql/types/time_type.rb b/app/graphql/types/time_type.rb index 2333d82ad1e..f045a50e672 100644 --- a/app/graphql/types/time_type.rb +++ b/app/graphql/types/time_type.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class TimeType < BaseScalar graphql_name 'Time' diff --git a/changelogs/unreleased/frozen-string-app-finders-graphql.yml b/changelogs/unreleased/frozen-string-app-finders-graphql.yml new file mode 100644 index 00000000000..ea8c83f64d9 --- /dev/null +++ b/changelogs/unreleased/frozen-string-app-finders-graphql.yml @@ -0,0 +1,5 @@ +--- +title: Enable frozen string in app/graphql + app/finders +merge_request: +author: gfyoung +type: performance diff --git a/changelogs/unreleased/mr-fixed-expanded-state-not-working.yml b/changelogs/unreleased/mr-fixed-expanded-state-not-working.yml new file mode 100644 index 00000000000..b8bf8306546 --- /dev/null +++ b/changelogs/unreleased/mr-fixed-expanded-state-not-working.yml @@ -0,0 +1,5 @@ +--- +title: Fixed resolved discussions not toggling expanded state on changes tab +merge_request: 21676 +author: +type: fixed diff --git a/db/schema.rb b/db/schema.rb index 13814dd569e..d888891c8ea 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -943,8 +943,8 @@ ActiveRecord::Schema.define(version: 20180906101639) do add_index "gpg_signatures", ["project_id"], name: "index_gpg_signatures_on_project_id", using: :btree create_table "group_custom_attributes", force: :cascade do |t| - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false t.integer "group_id", null: false t.string "key", null: false t.string "value", null: false @@ -1550,8 +1550,8 @@ ActiveRecord::Schema.define(version: 20180906101639) do add_index "project_ci_cd_settings", ["project_id"], name: "index_project_ci_cd_settings_on_project_id", unique: true, using: :btree create_table "project_custom_attributes", force: :cascade do |t| - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false t.integer "project_id", null: false t.string "key", null: false t.string "value", null: false diff --git a/doc/development/README.md b/doc/development/README.md index b37403552fe..efe37b8ba0b 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -49,6 +49,7 @@ description: 'Learn how to contribute to GitLab.' - [Working with Merge Request diffs](diffs.md) - [Permissions](permissions.md) - [Prometheus metrics](prometheus_metrics.md) +- [Guidelines for reusing abstractions](reusing_abstractions.md) ## Performance guides diff --git a/doc/development/reusing_abstractions.md b/doc/development/reusing_abstractions.md new file mode 100644 index 00000000000..83d7d42bd1f --- /dev/null +++ b/doc/development/reusing_abstractions.md @@ -0,0 +1,182 @@ +# Guidelines for reusing abstractions + +As GitLab has grown, different patterns emerged across the codebase. Service +classes, serializers, and presenters are just a few. These patterns made it easy +to reuse code, but at the same time make it easy to accidentally reuse the wrong +abstraction in a particular place. + +## Why these guidelines are necessary + +Code reuse is good, but sometimes this can lead to shoehorning the wrong +abstraction into a particular use case. This in turn can have a negative impact +on maintainability, the ability to easily debug problems, or even performance. + +An example would be to use `ProjectsFinder` in `IssuesFinder` to limit issues to +those belonging to a set of projects. While initially this may seem like a good +idea, both classes provide a very high level interface with very little control. +This means that `IssuesFinder` may not be able to produce a better optimised +database query, as a large portion of the query is controlled by the internals +of `ProjectsFinder`. + +To work around this problem, you would use the same code used by +`ProjectsFinder`, instead of using `ProjectsFinder` itself directly. This allows +you to compose your behaviour better, giving you more control over the behaviour +of the code. + +To illustrate, consider the following code from `IssuableFinder#projects`: + +```ruby +return @projects = project if project? + +projects = + if current_user && params[:authorized_only].presence && !current_user_related? + current_user.authorized_projects + elsif group + finder_options = { include_subgroups: params[:include_subgroups], only_owned: true } + GroupProjectsFinder.new(group: group, current_user: current_user, options: finder_options).execute + else + ProjectsFinder.new(current_user: current_user).execute + end + +@projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil) +``` + +Here we determine what projects to scope our data to, using three different +approaches. When a group is specified, we use `GroupProjectsFinder` to retrieve +all the projects of that group. On the surface this seems harmless: it is easy +to use, and we only need two lines of code. + +In reality, things can get hairy very quickly. For example, the query produced +by `GroupProjectsFinder` may start out simple. Over time more and more +functionality is added to this (high level) interface. Instead of _only_ +affecting the cases where this is necessary, it may also start affecting +`IssuableFinder` in a negative way. For example, the query produced by +`GroupProjectsFinder` may include unnecessary conditions. Since we're using a +finder here, we can't easily opt-out of that behaviour. We could add options to +do so, but then we'd need as many options as we have features. Every option adds +two code paths, which means that for four features we have to cover 8 different +code paths. + +A much more reliable (and pleasant) way of dealing with this, is to simply use +the underlying bits that make up `GroupProjectsFinder` directly. This means we +may need a little bit more code in `IssuableFinder`, but it also gives us much +more control and certainty. This means we might end up with something like this: + +```ruby +return @projects = project if project? + +projects = + if current_user && params[:authorized_only].presence && !current_user_related? + current_user.authorized_projects + elsif group + current_user + .owned_groups(subgroups: params[:include_subgroups]) + .projects + .any_additional_method_calls + .that_might_be_necessary + else + current_user + .projects_visible_to_user + .any_additional_method_calls + .that_might_be_necessary + end + +@projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil) +``` + +This is just a sketch, but it shows the general idea: we would use whatever the +`GroupProjectsFinder` and `ProjectsFinder` finders use under the hoods. + +## End goal + +The guidelines in this document are meant to foster _better_ code reuse, by +clearly defining what can be reused where, and what to do when you can not reuse +something. Clearly separating abstractions makes it harder to use the wrong one, +makes it easier to debug the code, and (hopefully) results in fewer performance +problems. + +## Abstractions + +Now let's take a look at the various abstraction levels available, and what they +can (or cannot) reuse. For this we can use the following table, which defines +the various abstractions and what they can (not) reuse: + +| Abstraction | Service classes | Finders | Presenters | Serializers | Model instance method | Model class methods | Active Record | Worker +|:-----------------------|:-----------------|:---------|:------------|:--------------|:------------------------|:----------------------|:----------------|:-------- +| Controller | Yes | Yes | Yes | Yes | Yes | No | No | No +| Service class | Yes | Yes | No | No | Yes | No | No | Yes +| Finder | No | No | No | No | Yes | Yes | No | No +| Presenter | No | Yes | No | No | Yes | Yes | No | No +| Serializer | No | Yes | No | No | Yes | Yes | No | No +| Model class method | No | No | No | No | Yes | Yes | Yes | No +| Model instance method | No | Yes | No | No | Yes | Yes | Yes | Yes +| Worker | Yes | Yes | No | No | Yes | No | No | Yes + +### Controllers + +Everything in `app/controllers`. + +Controllers should not do much work on their own, instead they simply pass input +to other classes and present the results. + +### Grape endpoint + +Everything in `lib/api`. + +### Service classes + +Everything that resides in `app/services`. + +### Finders + +Everything in `app/finders`, typically used for retrieving data from a database. + +Finders can not reuse other finders in an attempt to better control the SQL +queries they produce. + +### Presenters + +Everything in `app/presenters`, used for exposing complex data to a Rails view, +without having to create many instance variables. + +### Serializers + +Everything in `app/serializers`, used for presenting the response to a request, +typically in JSON. + +### Model class methods + +These are class methods defined by _GitLab itself_, including the following +methods provided by Active Record: + +* `find` +* `find_by_id` +* `delete_all` +* `destroy` +* `destroy_all` + +Any other methods such as `find_by(some_column: X)` are not included, and +instead fall under the "Active Record" abstraction. + +### Model instance methods + +Instance methods defined on Active Record models by _GitLab itself_. Methods +provided by Active Record are not included, except for the following methods: + +* `save` +* `update` +* `destroy` +* `delete` + +### Active Record + +The API provided by Active Record itself, such as the `where` method, `save`, +`delete_all`, etc. + +### Worker + +Everything in `app/workers`. + +The scheduling of Sidekiq jobs using `SomeWorker.perform_async`, `perform_in`, +etc. Directly invoking a worker using `SomeWorker.new.perform` should be avoided +at all times in application code, though this is fine to use in tests. diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb index 7c4a440b9a9..c13b0249d94 100644 --- a/spec/factories/clusters/applications/helm.rb +++ b/spec/factories/clusters/applications/helm.rb @@ -46,7 +46,7 @@ FactoryBot.define do factory :clusters_applications_jupyter, class: Clusters::Applications::Jupyter do oauth_application factory: :oauth_application - cluster factory: %i(cluster with_installed_helm provided_by_gcp) + cluster factory: %i(cluster with_installed_helm provided_by_gcp project) end end end diff --git a/spec/javascripts/diffs/components/app_spec.js b/spec/javascripts/diffs/components/app_spec.js index 7237274eb43..7be44a26ded 100644 --- a/spec/javascripts/diffs/components/app_spec.js +++ b/spec/javascripts/diffs/components/app_spec.js @@ -1 +1,71 @@ -// TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 +import Vue from 'vue'; +import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import { TEST_HOST } from 'spec/test_constants'; +import App from '~/diffs/components/app.vue'; +import createDiffsStore from '../create_diffs_store'; + +describe('diffs/components/app', () => { + const oldMrTabs = window.mrTabs; + const Component = Vue.extend(App); + + let vm; + + beforeEach(() => { + // setup globals (needed for component to mount :/) + window.mrTabs = jasmine.createSpyObj('mrTabs', ['resetViewContainer']); + + // setup component + const store = createDiffsStore(); + store.state.diffs.isLoading = false; + + vm = mountComponentWithStore(Component, { + store, + props: { + endpoint: `${TEST_HOST}/diff/endpoint`, + projectPath: 'namespace/project', + currentUser: {}, + }, + }); + }); + + afterEach(() => { + // reset globals + window.mrTabs = oldMrTabs; + + // reset component + vm.$destroy(); + }); + + it('shows comments message, with commit', done => { + vm.$store.state.diffs.commit = {}; + + vm.$nextTick() + .then(() => { + expect(vm.$el).toContainText('Only comments from the following commit are shown below'); + }) + .then(done) + .catch(done.fail); + }); + + it('shows comments message, with old mergeRequestDiff', done => { + vm.$store.state.diffs.mergeRequestDiff = { latest: false }; + + vm.$nextTick() + .then(() => { + expect(vm.$el).toContainText("Not all comments are displayed because you're viewing an old version of the diff."); + }) + .then(done) + .catch(done.fail); + }); + + it('shows comments message, with startVersion', done => { + vm.$store.state.diffs.startVersion = 'test'; + + vm.$nextTick() + .then(() => { + expect(vm.$el).toContainText("Not all comments are displayed because you're comparing two versions of the diff."); + }) + .then(done) + .catch(done.fail); + }); +}); diff --git a/spec/javascripts/diffs/components/changed_files_spec.js b/spec/javascripts/diffs/components/changed_files_spec.js index f737e8fa38e..7f21273a991 100644 --- a/spec/javascripts/diffs/components/changed_files_spec.js +++ b/spec/javascripts/diffs/components/changed_files_spec.js @@ -8,7 +8,7 @@ describe('ChangedFiles', () => { const Component = Vue.extend(changedFiles); const store = new Vuex.Store({ modules: { - diffs: diffsModule, + diffs: diffsModule(), }, }); diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js index 92b2004c4d7..c986ea604b2 100644 --- a/spec/javascripts/diffs/components/diff_file_header_spec.js +++ b/spec/javascripts/diffs/components/diff_file_header_spec.js @@ -16,8 +16,8 @@ describe('diff_file_header', () => { const store = new Vuex.Store({ modules: { - diffs: diffsModule, - notes: notesModule, + diffs: diffsModule(), + notes: notesModule(), }, }); @@ -450,13 +450,14 @@ describe('diff_file_header', () => { propsCopy.diffFile.deletedFile = true; const discussionGetter = () => [diffDiscussionMock]; - notesModule.getters.discussions = discussionGetter; + const notesModuleMock = notesModule(); + notesModuleMock.getters.discussions = discussionGetter; vm = mountComponentWithStore(Component, { props: propsCopy, store: new Vuex.Store({ modules: { - diffs: diffsModule, - notes: notesModule, + diffs: diffsModule(), + notes: notesModuleMock, }, }), }); diff --git a/spec/javascripts/diffs/create_diffs_store.js b/spec/javascripts/diffs/create_diffs_store.js new file mode 100644 index 00000000000..aacde99964c --- /dev/null +++ b/spec/javascripts/diffs/create_diffs_store.js @@ -0,0 +1,15 @@ +import Vue from 'vue'; +import Vuex from 'vuex'; +import diffsModule from '~/diffs/store/modules'; +import notesModule from '~/notes/stores/modules'; + +Vue.use(Vuex); + +export default function createDiffsStore() { + return new Vuex.Store({ + modules: { + diffs: diffsModule(), + notes: notesModule(), + }, + }); +} diff --git a/spec/javascripts/notes/stores/mutation_spec.js b/spec/javascripts/notes/stores/mutation_spec.js index a15ff1a5888..6b7f61468d5 100644 --- a/spec/javascripts/notes/stores/mutation_spec.js +++ b/spec/javascripts/notes/stores/mutation_spec.js @@ -1,3 +1,4 @@ +import Vue from 'vue'; import mutations from '~/notes/stores/mutations'; import { note, @@ -333,7 +334,7 @@ describe('Notes Store mutations', () => { }); }); - describe('SET_NOTES_FETCHING_STATE', () => { + describe('SET_NOTES_FETCHED_STATE', () => { it('should set the given state', () => { const state = { isNotesFetched: false, @@ -343,4 +344,37 @@ describe('Notes Store mutations', () => { expect(state.isNotesFetched).toEqual(true); }); }); + + describe('SET_DISCUSSION_DIFF_LINES', () => { + it('sets truncated_diff_lines', () => { + const state = { + discussions: [ + { + id: 1, + }, + ], + }; + + mutations.SET_DISCUSSION_DIFF_LINES(state, { discussionId: 1, diffLines: ['test'] }); + + expect(state.discussions[0].truncated_diff_lines).toEqual(['test']); + }); + + it('keeps reactivity of discussion', () => { + const state = {}; + Vue.set(state, 'discussions', [ + { + id: 1, + expanded: false, + }, + ]); + const discussion = state.discussions[0]; + + mutations.SET_DISCUSSION_DIFF_LINES(state, { discussionId: 1, diffLines: ['test'] }); + + discussion.expanded = true; + + expect(state.discussions[0].expanded).toBe(true); + }); + }); }); diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index dfe36d7d459..ce4caf0c27d 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1752,7 +1752,7 @@ describe Gitlab::Git::Repository, :seed_helper do describe '#checksum' do it 'calculates the checksum for non-empty repo' do - expect(repository.checksum).to eq '4be7d24ce7e8d845502d599b72d567d23e6a40c0' + expect(repository.checksum).to eq '51d0a9662681f93e1fee547a6b7ba2bcaf716059' end it 'returns 0000000000000000000000000000000000000000 for an empty repo' do diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index 44a64928e94..20e782113ff 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -114,11 +114,7 @@ describe Clusters::Applications::Jupyter do end context 'when cluster belongs to a project' do - let(:project) { create(:project) } - - before do - application.cluster.projects << project - end + let(:project) { application.cluster.first_project } it 'sets GitLab project id' do expect(values).to match(/GITLAB_PROJECT_ID: '?#{project.id}/) |