diff --git a/.gitlab/ci/workhorse.gitlab-ci.yml b/.gitlab/ci/workhorse.gitlab-ci.yml index dac59b92843..4ed674948cf 100644 --- a/.gitlab/ci/workhorse.gitlab-ci.yml +++ b/.gitlab/ci/workhorse.gitlab-ci.yml @@ -1,9 +1,10 @@ workhorse:verify: extends: .workhorse:rules:workhorse - image: ${GITLAB_DEPENDENCY_PROXY}golang:1.18 + image: ${GITLAB_DEPENDENCY_PROXY}golang:${GO_VERSION} stage: test needs: [] script: + - go version - make -C workhorse # test build - make -C workhorse verify @@ -12,7 +13,6 @@ workhorse:verify: image: ${REGISTRY_HOST}/${REGISTRY_GROUP}/gitlab-build-images/debian-${DEBIAN_VERSION}-ruby-${RUBY_VERSION}-golang-${GO_VERSION}:git-2.36 variables: GITALY_ADDRESS: "tcp://127.0.0.1:8075" - GO_VERSION: "1.18" stage: test needs: - setup-test-env @@ -27,7 +27,7 @@ workhorse:test go: extends: .workhorse:test parallel: matrix: - - GO_VERSION: ["1.17", "1.18"] + - GO_VERSION: ["1.17", "1.18", "1.19"] script: - make -C workhorse test-coverage coverage: '/\d+.\d+%/' diff --git a/Gemfile b/Gemfile index bdc0c246298..947fc9914a2 100644 --- a/Gemfile +++ b/Gemfile @@ -357,7 +357,7 @@ group :development do gem 'solargraph', '~> 0.46.0', require: false gem 'letter_opener_web', '~> 2.0.0' - gem 'lookbook', '~> 1.0' + gem 'lookbook', '~> 1.0', '>= 1.0.8' # Better errors handler gem 'better_errors', '~> 2.9.1' diff --git a/Gemfile.checksum b/Gemfile.checksum index 02da640c42f..51893fa0557 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -14,7 +14,7 @@ {"name":"activestorage","version":"6.1.6.1","platform":"ruby","checksum":"3fbf4c355a69a46e14676004ad8e06245bdce7f96858e72782715218326aafc5"}, {"name":"activesupport","version":"6.1.6.1","platform":"ruby","checksum":"5fc9fd6fe6f755e7523bb3aaf4370fb91a8416b39e3202939fd8bded4fec606d"}, {"name":"acts-as-taggable-on","version":"9.0.0","platform":"ruby","checksum":"5a409be0eae125b7b02c1a7316264b40d4a583584a13d4ea4a6d82acdb351b86"}, -{"name":"addressable","version":"2.8.0","platform":"ruby","checksum":"f76d29d2d1f54b6c6a49aec58f9583b08d97e088c227a3fcba92f6c6531d5908"}, +{"name":"addressable","version":"2.8.1","platform":"ruby","checksum":"bc724a176ef02118c8a3ed6b5c04c39cf59209607ffcce77b91d0261dbadedfa"}, {"name":"aes_key_wrap","version":"1.1.0","platform":"ruby","checksum":"b935f4756b37375895db45669e79dfcdc0f7901e12d4e08974d5540c8e0776a5"}, {"name":"akismet","version":"3.0.0","platform":"ruby","checksum":"74991b8e3d3257eeea996b47069abb8da2006c84a144255123e8dffd1c86b230"}, {"name":"android_key_attestation","version":"0.3.0","platform":"ruby","checksum":"467eb01a99d2bb48ef9cf24cc13712669d7056cba5a52d009554ff037560570b"}, @@ -90,7 +90,7 @@ {"name":"crass","version":"1.0.6","platform":"ruby","checksum":"dc516022a56e7b3b156099abc81b6d2b08ea1ed12676ac7a5657617f012bd45d"}, {"name":"creole","version":"0.5.0","platform":"ruby","checksum":"951701e2d80760f156b1cb2a93471ca97c076289becc067a33b745133ed32c03"}, {"name":"crystalball","version":"0.7.0","platform":"ruby","checksum":"6e729f372a5071daec877adb40c5df4cb25fe21f350635e2a9624373fc151ef2"}, -{"name":"css_parser","version":"1.11.0","platform":"ruby","checksum":"568926c3193579446ad3e3f9d761c73e2918ee5b3b7757a1a49ec166c67d6de1"}, +{"name":"css_parser","version":"1.12.0","platform":"ruby","checksum":"8b7c04bca32257da0c65bd7b1fa585df5a0fd9f5197ccd78498d5598dd900784"}, {"name":"cvss-suite","version":"3.0.1","platform":"ruby","checksum":"b5ca9e9e94032a42fd0dc28c1e305378b62c949e35ed7111fc4a1d76f68ad3f9"}, {"name":"danger","version":"8.6.1","platform":"ruby","checksum":"d95eb58b41f68d3aaa9bbef697916b6b4d161a38819517c98562531be75cdfd8"}, {"name":"danger-gitlab","version":"8.0.0","platform":"ruby","checksum":"497dd7d0f6513913de651019223d8058cf494df10acbd17de92b175dfa04a3a8"}, @@ -309,7 +309,7 @@ {"name":"lockbox","version":"0.6.2","platform":"ruby","checksum":"0136677875c3d6e27cef87cd7bd66610404e2b3cd7f07f1ac8ed34e48f18dc3c"}, {"name":"lograge","version":"0.11.2","platform":"ruby","checksum":"4cbd1554b86f545d795eff15a0c24fd25057d2ac4e1caa5fc186168b3da932ef"}, {"name":"loofah","version":"2.19.0","platform":"ruby","checksum":"302791371f473611e342f9e469e7f2fbf1155bb1b3a978a83ac7df625298feba"}, -{"name":"lookbook","version":"1.0.3","platform":"ruby","checksum":"c53e130a37588e32f66be3b9418b1efcb51bef69946b276edd3b4348a71cbcd6"}, +{"name":"lookbook","version":"1.0.8","platform":"ruby","checksum":"e4b8789c5ff25c6443394da1d6b62966642c324e19c42d7f4cf3b5da2fe75f77"}, {"name":"lru_redux","version":"1.1.0","platform":"ruby","checksum":"ee71d0ccab164c51de146c27b480a68b3631d5b4297b8ffe8eda1c72de87affb"}, {"name":"lumberjack","version":"1.2.7","platform":"ruby","checksum":"a5c6aae6b4234f1420dbcd80b23e3bca0817bd239440dde097ebe3fa63c63b1f"}, {"name":"mail","version":"2.7.1","platform":"ruby","checksum":"ec2a3d489f7510b90d8eaa3f6abaad7038cf1d663cdf8ee66d0214a0bdf99c03"}, @@ -418,7 +418,7 @@ {"name":"pry-byebug","version":"3.9.0","platform":"ruby","checksum":"3bba08f97fea15b89cc299f3b5136e3b85763cd18cf84960eac4fbfbeb2ede24"}, {"name":"pry-rails","version":"0.3.9","platform":"ruby","checksum":"468662575abb6b67f4a9831219f99290d5eae7bf186e64dd810d0a3e4a8cc4b1"}, {"name":"pry-shell","version":"0.5.1","platform":"ruby","checksum":"2b9000e30677acf5d66f55fa53d31934b7c069d9e0f738a0b84eed03a4ab677d"}, -{"name":"public_suffix","version":"4.0.7","platform":"ruby","checksum":"8be161e2421f8d45b0098c042c06486789731ea93dc3a896d30554ee38b573b8"}, +{"name":"public_suffix","version":"5.0.0","platform":"ruby","checksum":"26ee4fbce33ada25eb117ac71f2c24bf4d8b3414ab6b34f05b4708a3e90f1c6b"}, {"name":"puma","version":"5.6.5","platform":"java","checksum":"29d78fc2bc070b9db285a3334a890c3e0ece9bb369388065f0f340ccb1e57faf"}, {"name":"puma","version":"5.6.5","platform":"ruby","checksum":"661029d15a115e9f6c0641a69c830ffd9f1b9ac63fcd0791d94ccd900e03f863"}, {"name":"puma_worker_killer","version":"0.3.1","platform":"ruby","checksum":"9c5534d296b5e92d1ad4a578f2daf2aa71563003c84f7263f0a8dfd22b5c614a"}, diff --git a/Gemfile.lock b/Gemfile.lock index 6a360b08e6e..b589d5edbff 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -155,8 +155,8 @@ GEM zeitwerk (~> 2.3) acts-as-taggable-on (9.0.0) activerecord (>= 6.0, < 7.1) - addressable (2.8.0) - public_suffix (>= 2.0.2, < 5.0) + addressable (2.8.1) + public_suffix (>= 2.0.2, < 6.0) aes_key_wrap (1.1.0) akismet (3.0.0) android_key_attestation (0.3.0) @@ -301,7 +301,7 @@ GEM creole (0.5.0) crystalball (0.7.0) git - css_parser (1.11.0) + css_parser (1.12.0) addressable cvss-suite (3.0.1) danger (8.6.1) @@ -830,7 +830,7 @@ GEM loofah (2.19.0) crass (~> 1.0.2) nokogiri (>= 1.5.9) - lookbook (1.0.3) + lookbook (1.0.8) actioncable css_parser htmlbeautifier (~> 1.3) @@ -1048,7 +1048,7 @@ GEM pry (~> 0.13.0) tty-markdown tty-prompt - public_suffix (4.0.7) + public_suffix (5.0.0) puma (5.6.5) nio4r (~> 2.0) puma_worker_killer (0.3.1) @@ -1671,7 +1671,7 @@ DEPENDENCIES lockbox (~> 0.6.2) lograge (~> 0.5) loofah (~> 2.19.0) - lookbook (~> 1.0) + lookbook (~> 1.0, >= 1.0.8) lru_redux mail (= 2.7.1) mail-smtp_pool (~> 0.1.0)! @@ -1817,4 +1817,4 @@ DEPENDENCIES yajl-ruby (~> 1.4.3) BUNDLED WITH - 2.3.15 + 2.3.22 diff --git a/app/assets/javascripts/pages/projects/pipeline_schedules/edit/index.js b/app/assets/javascripts/pages/projects/pipeline_schedules/edit/index.js index 6dd21380bec..0edce2db0a3 100644 --- a/app/assets/javascripts/pages/projects/pipeline_schedules/edit/index.js +++ b/app/assets/javascripts/pages/projects/pipeline_schedules/edit/index.js @@ -1,3 +1,8 @@ +import initPipelineSchedulesFormApp from '~/pipeline_schedules/mount_pipeline_schedules_form_app'; import initForm from '../shared/init_form'; -initForm(); +if (gon.features?.pipelineSchedulesVue) { + initPipelineSchedulesFormApp('#pipeline-schedules-form-edit'); +} else { + initForm(); +} diff --git a/app/assets/javascripts/pages/projects/pipeline_schedules/index/index.js b/app/assets/javascripts/pages/projects/pipeline_schedules/index/index.js index 9513f42d9c9..7d0930f6424 100644 --- a/app/assets/javascripts/pages/projects/pipeline_schedules/index/index.js +++ b/app/assets/javascripts/pages/projects/pipeline_schedules/index/index.js @@ -1,9 +1,10 @@ import Vue from 'vue'; import { BV_SHOW_MODAL } from '~/lib/utils/constants'; +import initPipelineSchedulesApp from '~/pipeline_schedules/mount_pipeline_schedules_app'; import PipelineSchedulesTakeOwnershipModal from '~/pipeline_schedules/components/take_ownership_modal.vue'; import PipelineSchedulesCallout from '../shared/components/pipeline_schedules_callout.vue'; -function initPipelineSchedules() { +function initPipelineSchedulesCallout() { const el = document.getElementById('pipeline-schedules-callout'); if (!el) { @@ -15,6 +16,7 @@ function initPipelineSchedules() { // eslint-disable-next-line no-new new Vue({ el, + name: 'PipelineSchedulesCalloutRoot', provide: { docsUrl, illustrationUrl, @@ -25,6 +27,8 @@ function initPipelineSchedules() { }); } +// TODO: move take ownership feature into new Vue app +// located in directory app/assets/javascripts/pipeline_schedules/components function initTakeownershipModal() { const modalId = 'pipeline-take-ownership-modal'; const buttonSelector = 'js-take-ownership-button'; @@ -63,5 +67,10 @@ function initTakeownershipModal() { }); } -initPipelineSchedules(); -initTakeownershipModal(); +initPipelineSchedulesCallout(); + +if (gon.features?.pipelineSchedulesVue) { + initPipelineSchedulesApp(); +} else { + initTakeownershipModal(); +} diff --git a/app/assets/javascripts/pages/projects/pipeline_schedules/new/index.js b/app/assets/javascripts/pages/projects/pipeline_schedules/new/index.js index 6dd21380bec..06084fa729b 100644 --- a/app/assets/javascripts/pages/projects/pipeline_schedules/new/index.js +++ b/app/assets/javascripts/pages/projects/pipeline_schedules/new/index.js @@ -1,3 +1,8 @@ +import initPipelineSchedulesFormApp from '~/pipeline_schedules/mount_pipeline_schedules_form_app'; import initForm from '../shared/init_form'; -initForm(); +if (gon.features?.pipelineSchedulesVue) { + initPipelineSchedulesFormApp('#pipeline-schedules-form-new'); +} else { + initForm(); +} diff --git a/app/assets/javascripts/pipeline_schedules/components/pipeline_schedules.vue b/app/assets/javascripts/pipeline_schedules/components/pipeline_schedules.vue new file mode 100644 index 00000000000..86460e62183 --- /dev/null +++ b/app/assets/javascripts/pipeline_schedules/components/pipeline_schedules.vue @@ -0,0 +1,20 @@ + + + diff --git a/app/assets/javascripts/pipeline_schedules/components/pipeline_schedules_form.vue b/app/assets/javascripts/pipeline_schedules/components/pipeline_schedules_form.vue new file mode 100644 index 00000000000..6e24ac6b8d4 --- /dev/null +++ b/app/assets/javascripts/pipeline_schedules/components/pipeline_schedules_form.vue @@ -0,0 +1,18 @@ + + + diff --git a/app/assets/javascripts/pipeline_schedules/components/table/pipeline_schedules_table.vue b/app/assets/javascripts/pipeline_schedules/components/table/pipeline_schedules_table.vue new file mode 100644 index 00000000000..613da200105 --- /dev/null +++ b/app/assets/javascripts/pipeline_schedules/components/table/pipeline_schedules_table.vue @@ -0,0 +1,13 @@ + + + diff --git a/app/assets/javascripts/pipeline_schedules/mount_pipeline_schedules_app.js b/app/assets/javascripts/pipeline_schedules/mount_pipeline_schedules_app.js new file mode 100644 index 00000000000..8f77e06c19a --- /dev/null +++ b/app/assets/javascripts/pipeline_schedules/mount_pipeline_schedules_app.js @@ -0,0 +1,32 @@ +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createDefaultClient from '~/lib/graphql'; +import PipelineSchedules from './components/pipeline_schedules.vue'; + +Vue.use(VueApollo); + +const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient(), +}); + +export default () => { + const containerEl = document.querySelector('#pipeline-schedules-app'); + + if (!containerEl) { + return false; + } + + const { fullPath } = containerEl.dataset; + + return new Vue({ + el: containerEl, + name: 'PipelineSchedulesRoot', + apolloProvider, + provide: { + fullPath, + }, + render(createElement) { + return createElement(PipelineSchedules); + }, + }); +}; diff --git a/app/assets/javascripts/pipeline_schedules/mount_pipeline_schedules_form_app.js b/app/assets/javascripts/pipeline_schedules/mount_pipeline_schedules_form_app.js new file mode 100644 index 00000000000..d83417ab84a --- /dev/null +++ b/app/assets/javascripts/pipeline_schedules/mount_pipeline_schedules_form_app.js @@ -0,0 +1,32 @@ +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createDefaultClient from '~/lib/graphql'; +import PipelineSchedulesForm from './components/pipeline_schedules_form.vue'; + +Vue.use(VueApollo); + +const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient(), +}); + +export default (selector) => { + const containerEl = document.querySelector(selector); + + if (!containerEl) { + return false; + } + + const { fullPath } = containerEl.dataset; + + return new Vue({ + el: containerEl, + name: 'PipelineSchedulesFormRoot', + apolloProvider, + provide: { + fullPath, + }, + render(createElement) { + return createElement(PipelineSchedulesForm); + }, + }); +}; diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index a23d7fb3e6b..ca787785901 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -10,6 +10,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController before_action :authorize_update_pipeline_schedule!, only: [:edit, :update] before_action :authorize_take_ownership_pipeline_schedule!, only: [:take_ownership] before_action :authorize_admin_pipeline_schedule!, only: [:destroy] + before_action :push_schedule_feature_flag, only: [:index, :new, :edit] feature_category :continuous_integration urgency :low @@ -115,4 +116,8 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController def authorize_admin_pipeline_schedule! return access_denied! unless can?(current_user, :admin_pipeline_schedule, schedule) end + + def push_schedule_feature_flag + push_frontend_feature_flag(:pipeline_schedules_vue, @project) + end end diff --git a/app/models/concerns/integrations/base_data_fields.rb b/app/models/concerns/integrations/base_data_fields.rb index 2870922d90d..4319d63abb9 100644 --- a/app/models/concerns/integrations/base_data_fields.rb +++ b/app/models/concerns/integrations/base_data_fields.rb @@ -5,8 +5,6 @@ module Integrations extend ActiveSupport::Concern included do - # TODO: Once we rename the tables we can't rely on `table_name` anymore. - # https://gitlab.com/gitlab-org/gitlab/-/issues/331953 belongs_to :integration, inverse_of: self.table_name.to_sym, foreign_key: :integration_id validates :integration, presence: true diff --git a/app/views/projects/pipeline_schedules/edit.html.haml b/app/views/projects/pipeline_schedules/edit.html.haml index 642b458eea6..3f843ce6aec 100644 --- a/app/views/projects/pipeline_schedules/edit.html.haml +++ b/app/views/projects/pipeline_schedules/edit.html.haml @@ -7,4 +7,7 @@ = _("Edit Pipeline Schedule") %hr -= render "form" +- if Feature.enabled?(:pipeline_schedules_vue, @project) + #pipeline-schedules-form-edit{ data: { full_path: @project.full_path } } +- else + = render "form" diff --git a/app/views/projects/pipeline_schedules/index.html.haml b/app/views/projects/pipeline_schedules/index.html.haml index a0412650b3a..6c7b4b2f320 100644 --- a/app/views/projects/pipeline_schedules/index.html.haml +++ b/app/views/projects/pipeline_schedules/index.html.haml @@ -3,21 +3,25 @@ - add_page_specific_style 'page_bundles/pipeline_schedules' #pipeline-schedules-callout{ data: { docs_url: help_page_path('ci/pipelines/schedules'), illustration_url: image_path('illustrations/pipeline_schedule_callout.svg') } } -.top-area - - schedule_path_proc = ->(scope) { pipeline_schedules_path(@project, scope: scope) } - = render "tabs", schedule_path_proc: schedule_path_proc, all_schedules: @all_schedules, scope: @scope - - if can?(current_user, :create_pipeline_schedule, @project) - .nav-controls - = link_to new_project_pipeline_schedule_path(@project), class: 'btn gl-button btn-confirm' do - %span= _('New schedule') - -- if @schedules.present? - %ul.content-list - = render partial: "table" +- if Feature.enabled?(:pipeline_schedules_vue, @project) + #pipeline-schedules-app{ data: { full_path: @project.full_path } } - else - = render Pajamas::CardComponent.new(card_options: { class: 'bg-light gl-mt-3 gl-text-center' }) do |c| - - c.body do - = _("No schedules") + .top-area + - schedule_path_proc = ->(scope) { pipeline_schedules_path(@project, scope: scope) } + = render "tabs", schedule_path_proc: schedule_path_proc, all_schedules: @all_schedules, scope: @scope -#pipeline-take-ownership-modal + - if can?(current_user, :create_pipeline_schedule, @project) + .nav-controls + = link_to new_project_pipeline_schedule_path(@project), class: 'btn gl-button btn-confirm' do + %span= _('New schedule') + + - if @schedules.present? + %ul.content-list + = render partial: "table" + - else + = render Pajamas::CardComponent.new(card_options: { class: 'bg-light gl-mt-3 gl-text-center' }) do |c| + - c.body do + = _("No schedules") + + #pipeline-take-ownership-modal diff --git a/app/views/projects/pipeline_schedules/new.html.haml b/app/views/projects/pipeline_schedules/new.html.haml index 3b4acf5b8c5..d3757d0e339 100644 --- a/app/views/projects/pipeline_schedules/new.html.haml +++ b/app/views/projects/pipeline_schedules/new.html.haml @@ -8,4 +8,7 @@ %h1.page-title.gl-font-size-h-display = _("Schedule a new pipeline") -= render "form" +- if Feature.enabled?(:pipeline_schedules_vue, @project) + #pipeline-schedules-form-new{ data: { full_path: @project.full_path } } +- else + = render "form" diff --git a/config/feature_flags/development/pipeline_schedules_vue.yml b/config/feature_flags/development/pipeline_schedules_vue.yml new file mode 100644 index 00000000000..69106660c35 --- /dev/null +++ b/config/feature_flags/development/pipeline_schedules_vue.yml @@ -0,0 +1,8 @@ +--- +name: pipeline_schedules_vue +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/99155 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/375139 +milestone: '15.5' +type: development +group: group::pipeline execution +default_enabled: false diff --git a/db/post_migrate/20220922204106_remove_index_for_requested_non_invited_awaiting_members.rb b/db/post_migrate/20220922204106_remove_index_for_requested_non_invited_awaiting_members.rb new file mode 100644 index 00000000000..033b04a75f8 --- /dev/null +++ b/db/post_migrate/20220922204106_remove_index_for_requested_non_invited_awaiting_members.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class RemoveIndexForRequestedNonInvitedAwaitingMembers < Gitlab::Database::Migration[2.0] + INDEX_NAME = 'index_members_on_non_requested_non_invited_and_state_awaiting' + + disable_ddl_transaction! + + def up + remove_concurrent_index_by_name :members, INDEX_NAME + end + + def down + clause = '((requested_at IS NULL) AND (invite_token IS NULL) AND (access_level > 5) AND (state = 1))' + + add_concurrent_index :members, :source_id, where: clause, name: INDEX_NAME + end +end diff --git a/db/schema_migrations/20220922204106 b/db/schema_migrations/20220922204106 new file mode 100644 index 00000000000..0eabc077b58 --- /dev/null +++ b/db/schema_migrations/20220922204106 @@ -0,0 +1 @@ +e1106d4b77704a1ac4c185e0a6b500966dc61f46569de55650875aa6a89b7f9d \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index eb7e6fab790..d6665c0999d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -29229,8 +29229,6 @@ CREATE INDEX index_members_on_member_namespace_id ON members USING btree (member CREATE INDEX index_members_on_member_role_id ON members USING btree (member_role_id); -CREATE INDEX index_members_on_non_requested_non_invited_and_state_awaiting ON members USING btree (source_id) WHERE ((requested_at IS NULL) AND (invite_token IS NULL) AND (access_level > 5) AND (state = 1)); - CREATE INDEX index_members_on_requested_at ON members USING btree (requested_at); CREATE INDEX index_members_on_source_id_and_source_type ON members USING btree (source_id, source_type); diff --git a/doc/development/reusing_abstractions.md b/doc/development/reusing_abstractions.md index a33ffb722aa..893924dc365 100644 --- a/doc/development/reusing_abstractions.md +++ b/doc/development/reusing_abstractions.md @@ -135,40 +135,70 @@ API endpoints have the same abstraction level as controllers. Everything that resides in `app/services`. -Services should consider inheriting from: +Service classes represent operations that coordinates changes between models +(such as entities and value objects). Changes impact the state of the application. -- `BaseContainerService` for services scoped by container (project or group) -- `BaseProjectService` for services scoped to projects -- `BaseGroupService` for services scoped to groups +1. When an object makes no changes to the state of the application, then it's not a service. + It may be a [finder](#finders) or a value object. +1. When there is no operation, there is no need to execute a service. The class would + probably be better designed as an entity, a value object, or a policy. -or, create a new base class and update the list above. +When implementing a service class, consider: -Legacy classes inherited from `BaseService` for historical reasons. +1. A service class initializer should contain in its arguments: + 1. A [model](#models) instance that is being acted upon. Should be first positional + argument of the initializer. The argument name of the argument is left to the + developer's discretion, such as: `issue`, `project`, `merge_request`. + 1. When service represents an action initiated by a user or executed in the + context of a user, the initializer must have the `current_user:` keyword argument. + Services with `current_user:` argument run high-level business logic. + 1. When service does not have a user context and it's not directly initiated + by a user (like background service or side-effects), the `current_user:` + argument is not needed. This describes low-level domain logic or instance-wide logic. + 1. For all additional data required by a service, the explicit keyword arguments are recommended. + When a service requires too long of a list of arguments, consider splitting them into: + - `params`: A hash with model properties that will be assigned directly. + - `options`: A hash with extra parameters (which need to be processed, + and are not model properties). The `options` hash should be stored in an instance variable. -In Service classes the use of `execute` and `#execute` is preferred over `call` and `#call`. + ```ruby + # merge_request: A model instance that is being acted upon. + # assignee: new MR assignee that will be assigned to the MR + # after the service is executed. + def initialize(merge_request, assignee:) + @merge_request = merge_request + @assignee = assignee + end + ``` -Model properties should be passed to the constructor in a `params` hash, and will be assigned directly. + ```ruby + # issue: A model instance that is being acted upon. + # current_user: Current user. + # params: Model properties. + # options: Configuration for this service. Can be any of the following: + # - notify: Whether to send a notification to the current user. + # - cc: Email address to copy when sending a notification. + def initialize(issue:, current_user:, params: {}, options: {}) + @issue = issue + @current_user = current_user + @params = params + @options = options + end + ``` -To pass extra parameters (which need to be processed, and are not model properties), -include an `options` hash in the constructor and store it in an instance variable: +1. It implements a single public instance method `#execute`, which invokes service class behavior: + - The `#execute` method takes no arguments. All required data is passed into initializer. + - Optional. If needed, the `#execute` method returns its result via [`ServiceResponse`](#serviceresponse). -```ruby -# container: Project, or Group -# current_user: Current user -# params: Model properties from the controller, already allowlisted with strong parameters -# options: Configuration for this service, can be any of the following: -# notify: Whether to send a notifcation to the current user -# cc: Email address to copy when sending a notification -def initialize(container:, current_user: nil, params: {}, options: {}) - super(container, current_user, params) - @options = options -end -``` +Several base classes implement the service classes convention. You may consider inheriting from: -View the [initial discussion](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90008#note_988744060) -and [further discussion](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90853#note_1053425083). +- `BaseContainerService` for services scoped by container (project or group). +- `BaseProjectService` for services scoped to projects. +- `BaseGroupService` for services scoped to groups. -Classes that are not service objects should be [created elsewhere](directory_structure.md#use-namespaces-to-define-bounded-contexts), such as in `lib`. +Classes that are not service objects should be +[created elsewhere](directory_structure.md#use-namespaces-to-define-bounded-contexts), +such as in `lib`. #### ServiceResponse diff --git a/doc/user/project/merge_requests/creating_merge_requests.md b/doc/user/project/merge_requests/creating_merge_requests.md index cb04b93888a..8d462fef15a 100644 --- a/doc/user/project/merge_requests/creating_merge_requests.md +++ b/doc/user/project/merge_requests/creating_merge_requests.md @@ -10,6 +10,9 @@ disqus_identifier: 'https://docs.gitlab.com/ee/gitlab-basics/add-merge-request.h There are many different ways to create a merge request. +NOTE: +Use [branch naming patterns](../repository/branches/index.md#naming) to streamline merge request creation. + ## From the merge request list You can create a merge request from the list of merge requests. diff --git a/doc/user/project/repository/branches/index.md b/doc/user/project/repository/branches/index.md index 699009cbc15..4a9e4095a0e 100644 --- a/doc/user/project/repository/branches/index.md +++ b/doc/user/project/repository/branches/index.md @@ -13,9 +13,10 @@ other. After pushing your changes to a new branch, you can: -- Create a [merge request](../../merge_requests/index.md) -- Perform inline code review -- [Discuss](../../../discussions/index.md) your implementation with your team +- Create a [merge request](../../merge_requests/index.md). You can streamline this process + by following [branch naming patterns](#naming). +- Perform inline code review. +- [Discuss](../../../discussions/index.md) your implementation with your team. - Preview changes submitted to a new branch with [Review Apps](../../../../ci/review_apps/index.md). You can also request [approval](../../merge_requests/approvals/index.md) @@ -42,6 +43,18 @@ See also: - [GitLab Flow](../../../../topics/gitlab_flow.md) documentation. - [Getting started with Git](../../../../topics/git/index.md) and GitLab. +## Naming + +Prefix a branch name with an issue number to streamline merge request creation. +When you create a merge request for a branch with a name beginning with an issue +number, GitLab: + +- Marks the issue as related. If your project is configured with a + [default closing pattern](../../issues/managing_issues.md#default-closing-pattern), + merging this merge request [also closes](../../issues/managing_issues.md#closing-issues-automatically) + the related issue. +- Copies label and milestone metadata from the issue. + ## Compare To compare branches in a repository: diff --git a/doc/user/project/repository/web_editor.md b/doc/user/project/repository/web_editor.md index 05bc0207fa9..f39c1cc7f48 100644 --- a/doc/user/project/repository/web_editor.md +++ b/doc/user/project/repository/web_editor.md @@ -115,6 +115,9 @@ the target branch. Select **Create directory** to finish. There are multiple ways to create a branch from the GitLab web interface. +NOTE: +Use [branch naming patterns](branches/index.md#naming) to streamline merge request creation. + ### Create a new branch from an issue > The **Create merge request** button [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/349566) to open the merge request creation form in GitLab 14.8. diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb index dcc46f5d223..4ed0a11da38 100644 --- a/spec/features/projects/pipeline_schedules_spec.rb +++ b/spec/features/projects/pipeline_schedules_spec.rb @@ -11,6 +11,10 @@ RSpec.describe 'Pipeline Schedules', :js do let(:scope) { nil } let!(:user) { create(:user) } + before do + stub_feature_flags(pipeline_schedules_vue: false) + end + context 'logged in as the pipeline schedule owner' do before do project.add_developer(user) diff --git a/spec/features/projects/pipelines/legacy_pipeline_spec.rb b/spec/features/projects/pipelines/legacy_pipeline_spec.rb index 250a336469c..d93c951791d 100644 --- a/spec/features/projects/pipelines/legacy_pipeline_spec.rb +++ b/spec/features/projects/pipelines/legacy_pipeline_spec.rb @@ -735,6 +735,8 @@ RSpec.describe 'Pipeline', :js do end it 'displays the PipelineSchedule in an inactive state' do + stub_feature_flags(pipeline_schedules_vue: false) + visit project_pipeline_schedules_path(project) page.click_link('Inactive') diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 51a6fbc4d36..0b43e13996f 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -860,6 +860,8 @@ RSpec.describe 'Pipeline', :js do end it 'displays the PipelineSchedule in an inactive state' do + stub_feature_flags(pipeline_schedules_vue: false) + visit project_pipeline_schedules_path(project) page.click_link('Inactive') diff --git a/spec/frontend/fixtures/pipeline_schedules.rb b/spec/frontend/fixtures/pipeline_schedules.rb index 5b7a445557e..b992820d311 100644 --- a/spec/frontend/fixtures/pipeline_schedules.rb +++ b/spec/frontend/fixtures/pipeline_schedules.rb @@ -17,6 +17,7 @@ RSpec.describe Projects::PipelineSchedulesController, '(JavaScript fixtures)', t before do sign_in(user) + stub_feature_flags(pipeline_schedules_vue: false) end it 'pipeline_schedules/edit.html' do diff --git a/spec/frontend/pipeline_schedules/components/pipeline_schedules_form_spec.js b/spec/frontend/pipeline_schedules/components/pipeline_schedules_form_spec.js new file mode 100644 index 00000000000..4b5a9611251 --- /dev/null +++ b/spec/frontend/pipeline_schedules/components/pipeline_schedules_form_spec.js @@ -0,0 +1,25 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlForm } from '@gitlab/ui'; +import PipelineSchedulesForm from '~/pipeline_schedules/components/pipeline_schedules_form.vue'; + +describe('Pipeline schedules form', () => { + let wrapper; + + const createComponent = () => { + wrapper = shallowMount(PipelineSchedulesForm); + }; + + const findForm = () => wrapper.findComponent(GlForm); + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('displays form', () => { + expect(findForm().exists()).toBe(true); + }); +}); diff --git a/spec/frontend/pipeline_schedules/components/pipeline_schedules_spec.js b/spec/frontend/pipeline_schedules/components/pipeline_schedules_spec.js new file mode 100644 index 00000000000..98e53942d19 --- /dev/null +++ b/spec/frontend/pipeline_schedules/components/pipeline_schedules_spec.js @@ -0,0 +1,25 @@ +import { shallowMount } from '@vue/test-utils'; +import PipelineSchedules from '~/pipeline_schedules/components/pipeline_schedules.vue'; +import PipelineSchedulesTable from '~/pipeline_schedules/components/table/pipeline_schedules_table.vue'; + +describe('Pipeline schedules app', () => { + let wrapper; + + const createComponent = () => { + wrapper = shallowMount(PipelineSchedules); + }; + + const findTable = () => wrapper.findComponent(PipelineSchedulesTable); + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('displays table', () => { + expect(findTable().exists()).toBe(true); + }); +}); diff --git a/spec/frontend/pipeline_schedules/components/table/pipeline_schedules_table_spec.js b/spec/frontend/pipeline_schedules/components/table/pipeline_schedules_table_spec.js new file mode 100644 index 00000000000..950b5d64ffe --- /dev/null +++ b/spec/frontend/pipeline_schedules/components/table/pipeline_schedules_table_spec.js @@ -0,0 +1,25 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlTableLite } from '@gitlab/ui'; +import PipelineSchedulesTable from '~/pipeline_schedules/components/table/pipeline_schedules_table.vue'; + +describe('Pipeline schedules table', () => { + let wrapper; + + const createComponent = () => { + wrapper = shallowMount(PipelineSchedulesTable); + }; + + const findTable = () => wrapper.findComponent(GlTableLite); + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('displays table', () => { + expect(findTable().exists()).toBe(true); + }); +});