diff --git a/app/assets/javascripts/merge_requests/components/compare_app.vue b/app/assets/javascripts/merge_requests/components/compare_app.vue index 8e02048f4948e5eccb67b34b840dc9f64eee8553..20c95174989d30a916d28abd87ad332aeb39f334 100644 --- a/app/assets/javascripts/merge_requests/components/compare_app.vue +++ b/app/assets/javascripts/merge_requests/components/compare_app.vue @@ -23,9 +23,6 @@ export default { currentProject: { default: () => ({}), }, - currentBranch: { - default: () => ({}), - }, inputs: { default: () => ({}), }, @@ -39,6 +36,13 @@ export default { default: '', }, }, + props: { + currentBranch: { + type: Object, + required: false, + default: () => ({}), + }, + }, data() { return { selectedProject: this.currentProject, @@ -57,6 +61,12 @@ export default { return this.commitHtml || this.loading || !this.selectedBranch.value; }, }, + watch: { + currentBranch(newVal) { + this.selectedBranch = newVal; + this.fetchCommit(); + }, + }, mounted() { this.fetchCommit(); }, @@ -67,6 +77,7 @@ export default { selectBranch(branch) { this.selectedBranch = branch; this.fetchCommit(); + this.$emit('select-branch', branch.value); }, async fetchCommit() { if (!this.selectedBranch.value) return; @@ -109,6 +120,7 @@ export default { :default="currentBranch" :toggle-class="toggleClass.branch" :qa-selector="branchQaSelector" + data-testid="compare-dropdown" @selected="selectBranch" /> </div> diff --git a/app/assets/javascripts/merge_requests/components/compare_dropdown.vue b/app/assets/javascripts/merge_requests/components/compare_dropdown.vue index a5a4e683214e437d9a35413fa07fe467e34115be..6f46b41ee5b27f926072e9404e452cbfce6fe64a 100644 --- a/app/assets/javascripts/merge_requests/components/compare_dropdown.vue +++ b/app/assets/javascripts/merge_requests/components/compare_dropdown.vue @@ -70,6 +70,12 @@ export default { ); }, }, + watch: { + default(newVal) { + this.current = newVal; + this.selected = newVal.value; + }, + }, methods: { async fetchData() { if (!this.endpoint) return; diff --git a/app/assets/javascripts/pages/projects/merge_requests/creations/new/branch_finder.js b/app/assets/javascripts/pages/projects/merge_requests/creations/new/branch_finder.js new file mode 100644 index 0000000000000000000000000000000000000000..ee84f54978acc5f5d6005339bec8d267d92583bd --- /dev/null +++ b/app/assets/javascripts/pages/projects/merge_requests/creations/new/branch_finder.js @@ -0,0 +1 @@ +export const findTargetBranch = async () => {}; diff --git a/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js b/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js index f71a1041068e80874fd3d9862ab7e8ec0510a825..9db400b3b0a53cca6f8713e6fc8c4c65b17754ce 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js +++ b/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js @@ -2,6 +2,8 @@ import Vue from 'vue'; import { mountMarkdownEditor } from 'ee_else_ce/vue_shared/components/markdown/mount_markdown_editor'; +import { findTargetBranch } from 'ee_else_ce/pages/projects/merge_requests/creations/new/branch_finder'; + import initPipelines from '~/commit/pipelines/pipelines_bundle'; import MergeRequest from '~/merge_request'; import CompareApp from '~/merge_requests/components/compare_app.vue'; @@ -13,14 +15,15 @@ if (mrNewCompareNode) { const targetCompareEl = document.getElementById('js-target-project-dropdown'); const sourceCompareEl = document.getElementById('js-source-project-dropdown'); const compareEl = document.querySelector('.js-merge-request-new-compare'); + const targetBranch = Vue.observable({ name: '' }); + const currentSourceBranch = JSON.parse(sourceCompareEl.dataset.currentBranch); // eslint-disable-next-line no-new new Vue({ el: sourceCompareEl, name: 'SourceCompareApp', provide: { currentProject: JSON.parse(sourceCompareEl.dataset.currentProject), - currentBranch: JSON.parse(sourceCompareEl.dataset.currentBranch), branchCommitPath: compareEl.dataset.sourceBranchUrl, inputs: { project: { @@ -42,18 +45,34 @@ if (mrNewCompareNode) { }, branchQaSelector: 'source_branch_dropdown', }, + methods: { + async selectedBranch(branchName) { + const targetBranchName = await findTargetBranch(branchName); + + if (targetBranchName) { + targetBranch.name = targetBranchName; + } + }, + }, render(h) { - return h(CompareApp); + return h(CompareApp, { + props: { + currentBranch: currentSourceBranch, + }, + on: { + 'select-branch': this.selectedBranch, + }, + }); }, }); + const currentTargetBranch = JSON.parse(targetCompareEl.dataset.currentBranch); // eslint-disable-next-line no-new new Vue({ el: targetCompareEl, name: 'TargetCompareApp', provide: { currentProject: JSON.parse(targetCompareEl.dataset.currentProject), - currentBranch: JSON.parse(targetCompareEl.dataset.currentBranch), projectsPath: targetCompareEl.dataset.targetProjectsPath, branchCommitPath: compareEl.dataset.targetBranchUrl, inputs: { @@ -75,8 +94,17 @@ if (mrNewCompareNode) { branch: 'js-target-branch gl-font-monospace', }, }, + computed: { + currentBranch() { + if (targetBranch.name) { + return { text: targetBranch.name, value: targetBranch.name }; + } + + return currentTargetBranch; + }, + }, render(h) { - return h(CompareApp); + return h(CompareApp, { props: { currentBranch: this.currentBranch } }); }, }); } else { diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index b018c03eefd194710c3914bbde28aa120a714c3a..a90a16e120cf2a7794d9dc028b2cd40218f8b97e 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -226,6 +226,13 @@ def how_merge_modal_data(merge_request) } end + def mr_compare_form_data(_, merge_request) + { + source_branch_url: project_new_merge_request_branch_from_path(merge_request.source_project), + target_branch_url: project_new_merge_request_branch_to_path(merge_request.source_project) + } + end + private def review_requested_merge_requests_count diff --git a/app/views/projects/merge_requests/creations/_new_compare.html.haml b/app/views/projects/merge_requests/creations/_new_compare.html.haml index 015f6423e7c5f3d9ee87502eea6211d056c402e9..e6bd0b05f003ec4da725108538918dc5f81dd1c4 100644 --- a/app/views/projects/merge_requests/creations/_new_compare.html.haml +++ b/app/views/projects/merge_requests/creations/_new_compare.html.haml @@ -14,7 +14,7 @@ = gitlab_ui_form_for [@project, @merge_request], url: project_new_merge_request_path(@project), method: :get, html: { class: "merge-request-form js-requires-input" } do |f| - if params[:nav_source].present? = hidden_field_tag(:nav_source, params[:nav_source]) - .js-merge-request-new-compare.row{ 'data-source-branch-url': project_new_merge_request_branch_from_path(@source_project), 'data-target-branch-url': project_new_merge_request_branch_to_path(@source_project) } + .js-merge-request-new-compare.row{ data: mr_compare_form_data(current_user, @merge_request) } .col-lg-6 .card-new-merge-request %h2.gl-font-size-h2 diff --git a/ee/app/assets/javascripts/pages/projects/merge_requests/creations/new/branch_finder.js b/ee/app/assets/javascripts/pages/projects/merge_requests/creations/new/branch_finder.js new file mode 100644 index 0000000000000000000000000000000000000000..3f1ed4065b9a5a1e1a8f9f8fc81104c8980991d3 --- /dev/null +++ b/ee/app/assets/javascripts/pages/projects/merge_requests/creations/new/branch_finder.js @@ -0,0 +1,12 @@ +import axios from '~/lib/utils/axios_utils'; + +export const findTargetBranch = async (branch) => { + const path = document.querySelector('.js-merge-request-new-compare')?.dataset + .targetBranchFinderPath; + + if (!path) return null; + + const { data } = await axios.get(path, { params: { branch_name: branch } }); + + return data.target_branch; +}; diff --git a/ee/app/controllers/projects/target_branch_rules_controller.rb b/ee/app/controllers/projects/target_branch_rules_controller.rb index 5bbb4486bac341f02200a1acc75724d30a05762c..5f5c0e7d63ccc9dd1f26f234c6811698428d8be5 100644 --- a/ee/app/controllers/projects/target_branch_rules_controller.rb +++ b/ee/app/controllers/projects/target_branch_rules_controller.rb @@ -9,6 +9,12 @@ class TargetBranchRulesController < Projects::ApplicationController render_404 unless can?(current_user, :read_target_branch_rule, @project) end + def index + service = ::TargetBranchRules::FindService.new(@project, current_user) + + render json: { target_branch: service.execute(params[:branch_name]) }.to_json + end + def create result = TargetBranchRules::CreateService.new(@project, current_user, create_params).execute diff --git a/ee/app/helpers/ee/merge_requests_helper.rb b/ee/app/helpers/ee/merge_requests_helper.rb index 9077b1491fd047f8e6cce9390057d53c074d8e11..ec867993beaf5457527c44140e635264109b1901 100644 --- a/ee/app/helpers/ee/merge_requests_helper.rb +++ b/ee/app/helpers/ee/merge_requests_helper.rb @@ -30,6 +30,15 @@ def diffs_tab_pane_data(project, merge_request, params) super.merge(data) end + override :mr_compare_form_data + def mr_compare_form_data(user, merge_request) + target_branch_finder_path = if can?(user, :read_target_branch_rule, merge_request.project) + project_target_branch_rules_path(merge_request.project) + end + + super.merge({ target_branch_finder_path: target_branch_finder_path }) + end + def summarize_llm_enabled?(project, user) ::Llm::MergeRequests::SummarizeDiffService.enabled?(group: project.root_ancestor, user: user) end diff --git a/ee/config/routes/project.rb b/ee/config/routes/project.rb index 3cf2397b3937bacd5a146c87c8ad56e3ba40fdf8..ecd8a698449027a182848534bff4bb6642ac9491 100644 --- a/ee/config/routes/project.rb +++ b/ee/config/routes/project.rb @@ -42,7 +42,7 @@ end end - resources :target_branch_rules, only: [:create] + resources :target_branch_rules, only: [:index, :create] resources :automations, only: [:index] diff --git a/ee/spec/frontend/pages/projects/merge_requests/creations/new/branch_finder_spec.js b/ee/spec/frontend/pages/projects/merge_requests/creations/new/branch_finder_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..f564116b58c648294bf3fe627ad69bb5af4c5ab6 --- /dev/null +++ b/ee/spec/frontend/pages/projects/merge_requests/creations/new/branch_finder_spec.js @@ -0,0 +1,36 @@ +import MockAdapter from 'axios-mock-adapter'; +import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; +import axios from '~/lib/utils/axios_utils'; +import { findTargetBranch } from 'ee/pages/projects/merge_requests/creations/new/branch_finder'; + +let mock; + +describe('Merge request find target branch', () => { + beforeEach(() => { + mock = new MockAdapter(axios); + mock.onGet('/target_branch').reply(200, { target_branch: 'main' }); + }); + + afterEach(() => { + resetHTMLFixture(); + mock.restore(); + }); + + describe('element does not exist', () => { + it('returns null', async () => { + expect(await findTargetBranch()).toBe(null); + }); + }); + + describe('element exists', () => { + beforeEach(() => { + setHTMLFixture( + '<div class="js-merge-request-new-compare" data-target-branch-finder-path="/target_branch"></div>', + ); + }); + + it('returns target branch', async () => { + expect(await findTargetBranch()).toBe('main'); + }); + }); +}); diff --git a/ee/spec/helpers/merge_requests_helper_spec.rb b/ee/spec/helpers/merge_requests_helper_spec.rb index 195500de9957054d95129793221c885c7c736871..6ff455857d6ee53397dce05e919e6a3abe3c2553 100644 --- a/ee/spec/helpers/merge_requests_helper_spec.rb +++ b/ee/spec/helpers/merge_requests_helper_spec.rb @@ -153,4 +153,42 @@ end end end + + describe '#mr_compare_form_data' do + let_it_be(:project) { build_stubbed(:project) } + let_it_be(:merge_request) { build_stubbed(:merge_request, source_project: project) } + let_it_be(:user) { build_stubbed(:user) } + + subject(:mr_compare_form_data) { helper.mr_compare_form_data(user, merge_request) } + + describe 'when the target_branch_rules_flag flag is disabled' do + before do + stub_feature_flags(target_branch_rules_flag: false) + end + + it 'returns target_branch_finder_path as nil' do + expect(subject[:target_branch_finder_path]).to eq(nil) + end + end + + describe 'when the project does not have the correct license' do + before do + stub_licensed_features(target_branch_rules: false) + end + + it 'returns target_branch_finder_path as nil' do + expect(subject[:target_branch_finder_path]).to eq(nil) + end + end + + describe 'when the project has the correct license' do + before do + stub_licensed_features(target_branch_rules: true) + end + + it 'returns target_branch_finder_path' do + expect(subject[:target_branch_finder_path]).to eq(project_target_branch_rules_path(project)) + end + end + end end diff --git a/ee/spec/requests/projects/target_branch_rules_controller_spec.rb b/ee/spec/requests/projects/target_branch_rules_controller_spec.rb index f7fe42b0685a91c0232371e6e4f88373f3a9f5dd..4a2be8043bb72e029e30fec04024fe2072f27282 100644 --- a/ee/spec/requests/projects/target_branch_rules_controller_spec.rb +++ b/ee/spec/requests/projects/target_branch_rules_controller_spec.rb @@ -10,6 +10,53 @@ login_as(user) end + describe 'GET #index' do + describe 'when the target_branch_rules_flag flag is disabled' do + before do + stub_feature_flags(target_branch_rules_flag: false) + end + + it 'returns 404' do + get project_target_branch_rules_path(project) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'when the project does not have the correct license' do + before do + stub_licensed_features(target_branch_rules: false) + end + + it 'returns 404' do + get project_target_branch_rules_path(project) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'when target_branch_rules_flag is enabled and project has the correct license' do + before do + stub_licensed_features(target_branch_rules: true) + create(:target_branch_rule, project: project, name: 'feature', target_branch: 'other-branch') + end + + it 'calls TargetBranchRules::FindService' do + expect_next_instance_of(TargetBranchRules::FindService) do |service| + expect(service).to receive(:execute).with('feature') + end + + get project_target_branch_rules_path(project), params: { branch_name: 'feature' } + end + + it 'renders JSON with target_branch property' do + get project_target_branch_rules_path(project), params: { branch_name: 'feature' } + + expect(json_response).to include("target_branch" => 'other-branch') + end + end + end + describe 'POST #create' do let(:params) { { name: 'dev/*', target_branch: 'develop' } } diff --git a/ee/spec/routing/project_routing_spec.rb b/ee/spec/routing/project_routing_spec.rb index 96d90db3141d92253ce87bd80b6e6966e9caa9cc..685b980ea11e39488e7899538829da8366153044 100644 --- a/ee/spec/routing/project_routing_spec.rb +++ b/ee/spec/routing/project_routing_spec.rb @@ -86,6 +86,10 @@ end describe Projects::TargetBranchRulesController, 'routing' do + it "to #index" do + expect(get("/gitlab/gitlabhq/-/target_branch_rules")).to route_to('projects/target_branch_rules#index', namespace_id: 'gitlab', project_id: 'gitlabhq') + end + it "to #create" do expect(post("/gitlab/gitlabhq/-/target_branch_rules")).to route_to('projects/target_branch_rules#create', namespace_id: 'gitlab', project_id: 'gitlabhq') end diff --git a/spec/frontend/merge_requests/components/compare_app_spec.js b/spec/frontend/merge_requests/components/compare_app_spec.js index ba129363ffd470a38bb28f885750a83c0dda94c8..887f79f9fadc0cfb21a135fddceb625d4b574763 100644 --- a/spec/frontend/merge_requests/components/compare_app_spec.js +++ b/spec/frontend/merge_requests/components/compare_app_spec.js @@ -1,10 +1,14 @@ -import { shallowMount } from '@vue/test-utils'; +import MockAdapter from 'axios-mock-adapter'; +import waitForPromises from 'helpers/wait_for_promises'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import axios from '~/lib/utils/axios_utils'; import CompareApp from '~/merge_requests/components/compare_app.vue'; let wrapper; +let mock; function factory(provideData = {}) { - wrapper = shallowMount(CompareApp, { + wrapper = shallowMountExtended(CompareApp, { provide: { inputs: { project: { @@ -16,6 +20,7 @@ function factory(provideData = {}) { name: 'branch', }, }, + branchCommitPath: '/commit', toggleClass: { project: 'project', branch: 'branch', @@ -29,7 +34,18 @@ function factory(provideData = {}) { }); } +const findCommitBox = () => wrapper.findByTestId('commit-box'); + describe('Merge requests compare app component', () => { + beforeEach(() => { + mock = new MockAdapter(axios); + mock.onGet('/commit').reply(200, 'commit content'); + }); + + afterEach(() => { + mock.restore(); + }); + it('shows commit box when selected branch is empty', () => { factory({ currentBranch: { @@ -38,9 +54,41 @@ describe('Merge requests compare app component', () => { }, }); - const commitBox = wrapper.find('[data-testid="commit-box"]'); + const commitBox = findCommitBox(); expect(commitBox.exists()).toBe(true); expect(commitBox.text()).toBe('Select a branch to compare'); }); + + it('emits select-branch on selected event', () => { + factory({ + currentBranch: { + text: '', + value: '', + }, + }); + + wrapper.findByTestId('compare-dropdown').vm.$emit('selected', { value: 'main' }); + + expect(wrapper.emitted('select-branch')).toEqual([['main']]); + }); + + describe('currentBranch watcher', () => { + it('changes selected value', async () => { + factory({ + currentBranch: { + text: '', + value: '', + }, + }); + + expect(findCommitBox().text()).toBe('Select a branch to compare'); + + wrapper.setProps({ currentBranch: { text: 'main', value: 'main ' } }); + + await waitForPromises(); + + expect(findCommitBox().text()).toBe('commit content'); + }); + }); });