Commit ce7471bf authored by Phil Hughes's avatar Phil Hughes

Merge branch 'feature/file-reviews/use-blob-id' into 'master'

Use blob ID instead of `content_sha`

See merge request gitlab-org/gitlab!50022
parents 0bfd480e b7fd97d6
...@@ -35,7 +35,7 @@ function collapsed(file) { ...@@ -35,7 +35,7 @@ function collapsed(file) {
function identifier(file) { function identifier(file) {
return uuids({ return uuids({
seeds: [file.file_identifier_hash, file.content_sha], seeds: [file.file_identifier_hash, file.blob?.id],
})[0]; })[0];
} }
...@@ -51,7 +51,7 @@ export function prepareRawDiffFile({ file, allFiles, meta = false }) { ...@@ -51,7 +51,7 @@ export function prepareRawDiffFile({ file, allFiles, meta = false }) {
// It's possible, but not confirmed, that `content_sha` isn't available sometimes // It's possible, but not confirmed, that `content_sha` isn't available sometimes
// See: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49506#note_464692057 // See: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49506#note_464692057
// We don't want duplicate IDs if that's the case, so we just don't assign an ID // We don't want duplicate IDs if that's the case, so we just don't assign an ID
if (!meta && file.content_sha) { if (!meta && file.blob?.id) {
additionalProperties.id = identifier(file); additionalProperties.id = identifier(file);
} }
......
...@@ -3,35 +3,45 @@ import { prepareRawDiffFile } from '~/diffs/utils/diff_file'; ...@@ -3,35 +3,45 @@ import { prepareRawDiffFile } from '~/diffs/utils/diff_file';
function getDiffFiles() { function getDiffFiles() {
return [ return [
{ {
blob: {
id: 'C0473471',
},
file_hash: 'ABC', // This file is just a normal file file_hash: 'ABC', // This file is just a normal file
file_identifier_hash: 'ABC1', file_identifier_hash: 'ABC1',
content_sha: 'C047347',
}, },
{ {
blob: {
id: 'C0473472',
},
file_hash: 'DEF', // This file replaces a symlink file_hash: 'DEF', // This file replaces a symlink
file_identifier_hash: 'DEF1', file_identifier_hash: 'DEF1',
content_sha: 'C047347',
a_mode: '0', a_mode: '0',
b_mode: '0755', b_mode: '0755',
}, },
{ {
blob: {
id: 'C0473473',
},
file_hash: 'DEF', // This symlink is replaced by a file file_hash: 'DEF', // This symlink is replaced by a file
file_identifier_hash: 'DEF2', file_identifier_hash: 'DEF2',
content_sha: 'C047347',
a_mode: '120000', a_mode: '120000',
b_mode: '0', b_mode: '0',
}, },
{ {
blob: {
id: 'C0473474',
},
file_hash: 'GHI', // This symlink replaces a file file_hash: 'GHI', // This symlink replaces a file
file_identifier_hash: 'GHI1', file_identifier_hash: 'GHI1',
content_sha: 'C047347',
a_mode: '0', a_mode: '0',
b_mode: '120000', b_mode: '120000',
}, },
{ {
blob: {
id: 'C0473475',
},
file_hash: 'GHI', // This file is replaced by a symlink file_hash: 'GHI', // This file is replaced by a symlink
file_identifier_hash: 'GHI2', file_identifier_hash: 'GHI2',
content_sha: 'C047347',
a_mode: '0755', a_mode: '0755',
b_mode: '0', b_mode: '0',
}, },
...@@ -76,11 +86,11 @@ describe('diff_file utilities', () => { ...@@ -76,11 +86,11 @@ describe('diff_file utilities', () => {
it.each` it.each`
fileIndex | id fileIndex | id
${0} | ${'e075da30-4ec7-4e1c-a505-fe0fb0efe2d8'} ${0} | ${'8dcd585e-a421-4dab-a04e-6f88c81b7b4c'}
${1} | ${'5ab05419-123e-4d18-8454-0b8c3d9f3f91'} ${1} | ${'3f178b78-392b-44a4-bd7d-5d6192208a97'}
${2} | ${'94eb6bba-575c-4504-bd8e-5d302364d31e'} ${2} | ${'3d9e1354-cddf-4a11-8234-f0413521b2e5'}
${3} | ${'06d669b2-29b7-4f47-9731-33fc38a8db61'} ${3} | ${'460f005b-d29d-43c1-9a08-099a7c7f08de'}
${4} | ${'edd3e8f9-07f9-4647-8171-544c72e5a175'} ${4} | ${'d8c89733-6ce1-4455-ae3d-f8aad6ee99f9'}
`('sets the file id properly { id: $id } on normal diff files', ({ fileIndex, id }) => { `('sets the file id properly { id: $id } on normal diff files', ({ fileIndex, id }) => {
const preppedFile = prepareRawDiffFile({ const preppedFile = prepareRawDiffFile({
file: files[fileIndex], file: files[fileIndex],
...@@ -100,10 +110,10 @@ describe('diff_file utilities', () => { ...@@ -100,10 +110,10 @@ describe('diff_file utilities', () => {
expect(preppedFile).not.toHaveProp('id'); expect(preppedFile).not.toHaveProp('id');
}); });
it('does not set the id property if the file is missing a `content_sha`', () => { it('does not set the id property if the file is missing a `blob.id`', () => {
const fileMissingContentSha = { ...files[0] }; const fileMissingContentSha = { ...files[0] };
delete fileMissingContentSha.content_sha; delete fileMissingContentSha.blob.id;
const preppedFile = prepareRawDiffFile({ const preppedFile = prepareRawDiffFile({
file: fileMissingContentSha, file: fileMissingContentSha,
......
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