From d6e776e2df94e84b4efe104fae4d55ce60db8161 Mon Sep 17 00:00:00 2001 From: Joe Woodward Date: Tue, 28 Oct 2025 16:37:08 +0000 Subject: [PATCH 1/2] Fetch correct pipeline when viewing commits in detacted state If you are viewing the commits and have a branch or tag selected in the ref selector we can correctly fetch the pipeline using the ref.name. However, you can also view commits when you've checked out a commit i.e. instead of viewing commits/master you're viewing commits/SHA and viewing all of the commits in the history of SHA. When we are in this detatched state we need to remove the ref and rely on the SHA instead. We don't know which branch or tag we're on so we just find the most recent pipeline within any ref and show that one. Unfortunately we don't have ref_type available in the commits page so here we are checking if the ref looks like a SHA. This does have a caveat that any branches that look like SHAs will show the latest pipelines for the commit, not the commit within the current branch. This trade-off is much better than the current situation. It's highly unlikely for users to be naming branches with SHA values. It is much more likely for users to view the commits page while viewing a commit. --- app/helpers/ci/status_helper.rb | 2 +- app/views/projects/commits/_commit.html.haml | 9 ++- spec/features/commits_spec.rb | 78 ++++++++++++++++++-- 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/app/helpers/ci/status_helper.rb b/app/helpers/ci/status_helper.rb index 965887c1074d4a..eb2e424afb9e78 100644 --- a/app/helpers/ci/status_helper.rb +++ b/app/helpers/ci/status_helper.rb @@ -53,7 +53,7 @@ def ci_icon_for_status(status, size: 24) def render_commit_status(commit, status, ref: nil, tooltip_placement: 'left') project = commit.project - path = pipelines_project_commit_path(project, commit, ref: ref) + path = pipelines_project_commit_path(project, commit.id, ref: ref) render_ci_icon( status, diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 24eb971943dc1e..94bb381075d894 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -13,11 +13,12 @@ - merge_request = local_assigns.fetch(:merge_request, nil) - project = local_assigns.fetch(:project) { merge_request&.project } - ref = local_assigns.fetch(:ref) { merge_request&.source_branch } +- is_commits_page = local_assigns.fetch(:is_commits_page, false) +- pipeline_ref = ref unless is_commits_page && !@ref_type && /^([a-f0-9]{7,40})/.match?(ref) - commit = commit.present(current_user: current_user) -- commit_status = commit.detailed_status_for(ref) +- commit_status = commit.detailed_status_for(pipeline_ref) - show_legacy_ci_icon = local_assigns.fetch(:show_legacy_ci_icon, true) - is_blob_page = local_assigns.fetch(:is_blob_page, false) -- is_commits_page = local_assigns.fetch(:is_commits_page, false) - tags = commit.tags_for_display - collapsible = local_assigns.fetch(:collapsible, true) - link_data_attrs = local_assigns.fetch(:link_data_attrs, {}) @@ -79,8 +80,8 @@ - else = render partial: 'projects/commit/ajax_signature', locals: { commit: commit } - if show_legacy_ci_icon && commit_status - = render_commit_status(commit, commit_status, ref: ref) - .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: ref) } } + = render_commit_status(commit, commit_status, ref: pipeline_ref) + .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: pipeline_ref) } } .btn-group.gl-hidden{ class: '@sm/panel:gl-flex' } = render Pajamas::ButtonComponent.new(label: true, button_text_classes: 'gl-font-monospace', button_options: { class: 'dark:!gl-bg-neutral-800' }) do = commit.short_id diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 6a6c497fa6d604..f3a8906fe7f4c9 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -75,14 +75,80 @@ ) end - before do - visit project_commits_path(project, :master) + context 'while viewing a branch' do + before do + visit project_commits_path(project, :master) + end + + it 'shows correct build status from default branch' do + page.within("//li[@id='commit-#{pipeline.short_sha}']") do + expect(page).to have_css("[data-testid='ci-icon']") + expect(page).to have_css('[data-testid="status_success_borderless-icon"]') + end + end end - it 'shows correct build status from default branch' do - page.within("//li[@id='commit-#{pipeline.short_sha}']") do - expect(page).to have_css("[data-testid='ci-icon']") - expect(page).to have_css('[data-testid="status_success_borderless-icon"]') + context 'while viewing a commit' do + let_it_be(:sha) { project.commit.sha } + let_it_be(:short_sha) { sha[..7] } + + before_all do + project.repository.add_branch(user, sha, 'master', skip_ci: true) + project.repository.add_branch(user, short_sha, 'master', skip_ci: true) + end + + context 'when ref is a commit short sha' do + context 'without ref_type' do + before do + visit project_commits_path(project, short_sha) + end + + it 'shows latest build status for the commit sha' do + page.within("//li[@id='commit-#{short_sha}']") do + expect(page).to have_css("[data-testid='ci-icon']") + expect(page).to have_css('[data-testid="status_failed_borderless-icon"]') + end + end + end + + context 'with ref_type' do + before do + visit project_commits_path(project, short_sha, ref_type: 'heads') + end + + it 'does not show any build status' do + page.within("//li[@id='commit-#{short_sha}']") do + expect(page).not_to have_css("[data-testid='ci-icon']") + end + end + end + end + + context 'when ref is a commit sha' do + context 'without ref_type' do + before do + visit project_commits_path(project, sha) + end + + it 'shows latest build status for the commit sha' do + page.within("//li[@id='commit-#{short_sha}']") do + expect(page).to have_css("[data-testid='ci-icon']") + expect(page).to have_css('[data-testid="status_failed_borderless-icon"]') + end + end + end + + context 'with ref_type' do + before do + visit project_commits_path(project, sha, ref_type: 'heads') + end + + it 'does not show any build status' do + page.within("//li[@id='commit-#{short_sha}']") do + expect(page).not_to have_css("[data-testid='ci-icon']") + end + end + end end end end -- GitLab From ea8c94076441fcbc2c0afec7a8d4d2bc2112c7f4 Mon Sep 17 00:00:00 2001 From: Joe Woodward Date: Thu, 30 Oct 2025 16:12:23 +0000 Subject: [PATCH 2/2] Use Commit.valid_hash? to validate ref --- app/views/projects/commits/_commit.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 94bb381075d894..75b9fbb56fff33 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -14,7 +14,7 @@ - project = local_assigns.fetch(:project) { merge_request&.project } - ref = local_assigns.fetch(:ref) { merge_request&.source_branch } - is_commits_page = local_assigns.fetch(:is_commits_page, false) -- pipeline_ref = ref unless is_commits_page && !@ref_type && /^([a-f0-9]{7,40})/.match?(ref) +- pipeline_ref = ref unless is_commits_page && !@ref_type && Commit.valid_hash?(ref) - commit = commit.present(current_user: current_user) - commit_status = commit.detailed_status_for(pipeline_ref) - show_legacy_ci_icon = local_assigns.fetch(:show_legacy_ci_icon, true) -- GitLab