Commit 03980b40 authored by Bob Van Landuyt's avatar Bob Van Landuyt Committed by Thong Kuah

Return unloaded commit if it couldn't be enriched

When a commit could not be enriched, we want to return the original
commit.

When calling `Commit.lazy` we get a `BatchLoader` instance that
behaves like `nil`. So now we explicitly trigger the loading in batch
and replace the commit with the original commit if it could not be
loaded.

When a commit could not be enriched, we want to return the original
commit.

When calling `Commit.lazy` we get a `BatchLoader` instance that
behaves like `nil`. So now we explicitly trigger the loading in batch
and replace the commit with the original commit if it could not be
loaded.

This makes sure the merge request view can still be loaded for merge
requests for which the source branch became unavailable.
parent 048594ad
......@@ -72,8 +72,15 @@ class CommitCollection
end.compact]
# Replace the commits, keeping the same order
@commits = @commits.map do |c|
replacements.fetch(c.id, c)
@commits = @commits.map do |original_commit|
# Return the original instance: if it didn't need to be batchloaded, it was
# already enriched.
batch_loaded_commit = replacements.fetch(original_commit.id, original_commit)
# If batch loading the commit failed, fall back to the original commit.
# We need to explicitly check `.nil?` since otherwise a `BatchLoader` instance
# that looks like `nil` is returned.
batch_loaded_commit.nil? ? original_commit : batch_loaded_commit
end
self
......
---
title: Fix viewing merge reqeust from a fork that's being deleted
merge_request: 17894
author:
type: fixed
# frozen_string_literal: true
require 'spec_helper'
# This is a feature spec because the problems arrise when rendering the view for
# an actual project for which the repository is removed but the cached not
# updated.
# This can occur when the fork a merge request is created from is in the process
# of being destroyed.
describe 'User views merged merge request from deleted fork' do
include ProjectForksHelper
let(:project) { create(:project, :repository) }
let(:source_project) { fork_project(project, nil, repository: true) }
let(:user) { project.owner }
let!(:merge_request) { create(:merge_request, :merged, source_project: source_project, target_project: project) }
before do
sign_in user
fork_owner = source_project.namespace.owners.first
# Place the source_project in the weird in between state
source_project.update_attribute(:pending_delete, true)
Projects::DestroyService.new(source_project, fork_owner, {}).__send__(:trash_repositories!)
end
it 'correctly shows the merge request' do
visit(merge_request_path(merge_request))
expect(page).to have_content(merge_request.title)
end
end
......@@ -149,6 +149,17 @@ describe CommitCollection do
collection.enrich!
end
it 'returns the original commit if the commit could not be lazy loaded' do
collection = described_class.new(project, [hash_commit])
unexisting_lazy_commit = Commit.lazy(project, Gitlab::Git::BLANK_SHA)
expect(Commit).to receive(:lazy).with(project, hash_commit.id).and_return(unexisting_lazy_commit)
collection.enrich!
expect(collection.commits).to contain_exactly(hash_commit)
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment