Commit 9b9a3814 authored by Thomas Randolph's avatar Thomas Randolph

Use actual file IDs to retrieve reviews

It's not very robust to retrieve a value by its position in an array.

Instead, because the files all have unique IDs we return an
object where each key is a file ID and it's value is whether that
file has been marked as reviewed.

This fixes the issue where the list of files in single-file mode
always has a length of 1, which means the fetched review is
always the first file in the MR.
parent 9222b48d
...@@ -514,7 +514,7 @@ export default { ...@@ -514,7 +514,7 @@ export default {
v-for="(file, index) in diffs" v-for="(file, index) in diffs"
:key="file.newPath" :key="file.newPath"
:file="file" :file="file"
:reviewed="fileReviews[index]" :reviewed="fileReviews[file.id]"
:is-first-file="index === 0" :is-first-file="index === 0"
:is-last-file="index === diffFilesLength - 1" :is-last-file="index === diffFilesLength - 1"
:help-page-path="helpPagePath" :help-page-path="helpPagePath"
......
...@@ -9,7 +9,12 @@ export function isFileReviewed(reviews, file) { ...@@ -9,7 +9,12 @@ export function isFileReviewed(reviews, file) {
} }
export function reviewStatuses(files, reviews) { export function reviewStatuses(files, reviews) {
return files.map((file) => isFileReviewed(reviews, file)); return files.reduce((flat, file) => {
return {
...flat,
[file.id]: isFileReviewed(reviews, file),
};
}, {});
} }
export function getReviewsForMergeRequest(mrPath) { export function getReviewsForMergeRequest(mrPath) {
......
...@@ -49,11 +49,11 @@ describe('File Review(s) utilities', () => { ...@@ -49,11 +49,11 @@ describe('File Review(s) utilities', () => {
it.each` it.each`
mrReviews | files | fileReviews mrReviews | files | fileReviews
${{}} | ${[file1, file2]} | ${[false, false]} ${{}} | ${[file1, file2]} | ${{ 123: false, '098': false }}
${{ abc: ['123'] }} | ${[file1, file2]} | ${[true, false]} ${{ abc: ['123'] }} | ${[file1, file2]} | ${{ 123: true, '098': false }}
${{ abc: ['098'] }} | ${[file1, file2]} | ${[false, true]} ${{ abc: ['098'] }} | ${[file1, file2]} | ${{ 123: false, '098': true }}
${{ def: ['123'] }} | ${[file1, file2]} | ${[false, false]} ${{ def: ['123'] }} | ${[file1, file2]} | ${{ 123: false, '098': false }}
${{ abc: ['123'], def: ['098'] }} | ${[]} | ${[]} ${{ abc: ['123'], def: ['098'] }} | ${[]} | ${{}}
`( `(
'returns $fileReviews based on the diff files in state and the existing reviews $reviews', 'returns $fileReviews based on the diff files in state and the existing reviews $reviews',
({ mrReviews, files, fileReviews }) => { ({ mrReviews, files, fileReviews }) => {
......
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