Commit dc2558cc authored by Thomas Randolph's avatar Thomas Randolph Committed by Phil Hughes

Convert fileReviews getter into a simpler helper utility

Big wins:
 - Not hooked into the Vuex sequence / memory load
 - Results aren't cached!!
parent e716fbf4
...@@ -26,6 +26,7 @@ import CollapsedFilesWarning from './collapsed_files_warning.vue'; ...@@ -26,6 +26,7 @@ import CollapsedFilesWarning from './collapsed_files_warning.vue';
import { diffsApp } from '../utils/performance'; import { diffsApp } from '../utils/performance';
import { fileByFile } from '../utils/preferences'; import { fileByFile } from '../utils/preferences';
import { reviewStatuses } from '../utils/file_reviews';
import { import {
TREE_LIST_WIDTH_STORAGE_KEY, TREE_LIST_WIDTH_STORAGE_KEY,
...@@ -169,12 +170,7 @@ export default { ...@@ -169,12 +170,7 @@ export default {
'hasConflicts', 'hasConflicts',
'viewDiffsFileByFile', 'viewDiffsFileByFile',
]), ]),
...mapGetters('diffs', [ ...mapGetters('diffs', ['whichCollapsedTypes', 'isParallelView', 'currentDiffIndex']),
'whichCollapsedTypes',
'isParallelView',
'currentDiffIndex',
'fileReviews',
]),
...mapGetters(['isNotesFetched', 'getNoteableData']), ...mapGetters(['isNotesFetched', 'getNoteableData']),
diffs() { diffs() {
if (!this.viewDiffsFileByFile) { if (!this.viewDiffsFileByFile) {
...@@ -232,6 +228,9 @@ export default { ...@@ -232,6 +228,9 @@ export default {
return visible; return visible;
}, },
fileReviews() {
return reviewStatuses(this.diffFiles, this.mrReviews);
},
}, },
watch: { watch: {
commit(newCommit, oldCommit) { commit(newCommit, oldCommit) {
......
...@@ -10,7 +10,9 @@ import notesEventHub from '../../notes/event_hub'; ...@@ -10,7 +10,9 @@ import notesEventHub from '../../notes/event_hub';
import DiffFileHeader from './diff_file_header.vue'; import DiffFileHeader from './diff_file_header.vue';
import DiffContent from './diff_content.vue'; import DiffContent from './diff_content.vue';
import { diffViewerErrors } from '~/ide/constants'; import { diffViewerErrors } from '~/ide/constants';
import { collapsedType, isCollapsed } from '../utils/diff_file'; import { collapsedType, isCollapsed } from '../utils/diff_file';
import { import {
DIFF_FILE_AUTOMATIC_COLLAPSE, DIFF_FILE_AUTOMATIC_COLLAPSE,
DIFF_FILE_MANUAL_COLLAPSE, DIFF_FILE_MANUAL_COLLAPSE,
...@@ -144,6 +146,12 @@ export default { ...@@ -144,6 +146,12 @@ export default {
showContent() { showContent() {
return !this.isCollapsed && !this.isFileTooLarge; return !this.isCollapsed && !this.isFileTooLarge;
}, },
showLocalFileReviews() {
const loggedIn = Boolean(gon.current_user_id);
const featureOn = this.glFeatures.localFileReviews;
return loggedIn && featureOn;
},
}, },
watch: { watch: {
'file.file_hash': { 'file.file_hash': {
...@@ -181,6 +189,10 @@ export default { ...@@ -181,6 +189,10 @@ export default {
if (this.hasDiff) { if (this.hasDiff) {
this.postRender(); this.postRender();
} }
if (this.reviewed && !this.isCollapsed && this.showLocalFileReviews) {
this.handleToggle();
}
}, },
beforeDestroy() { beforeDestroy() {
eventHub.$off(EVT_EXPAND_ALL_FILES, this.expandAllListener); eventHub.$off(EVT_EXPAND_ALL_FILES, this.expandAllListener);
...@@ -273,9 +285,11 @@ export default { ...@@ -273,9 +285,11 @@ export default {
:can-current-user-fork="canCurrentUserFork" :can-current-user-fork="canCurrentUserFork"
:diff-file="file" :diff-file="file"
:collapsible="true" :collapsible="true"
:reviewed="reviewed"
:expanded="!isCollapsed" :expanded="!isCollapsed"
:add-merge-request-buttons="true" :add-merge-request-buttons="true"
:view-diffs-file-by-file="viewDiffsFileByFile" :view-diffs-file-by-file="viewDiffsFileByFile"
:show-local-file-reviews="showLocalFileReviews"
class="js-file-title file-title gl-border-1 gl-border-solid gl-border-gray-100" class="js-file-title file-title gl-border-1 gl-border-solid gl-border-gray-100"
:class="hasBodyClasses.header" :class="hasBodyClasses.header"
@toggleFile="handleToggle" @toggleFile="handleToggle"
......
<script> <script>
import { escape } from 'lodash'; import { escape } from 'lodash';
import { mapActions, mapGetters } from 'vuex'; import { mapActions, mapGetters, mapState } from 'vuex';
import { import {
GlTooltipDirective, GlTooltipDirective,
GlSafeHtmlDirective, GlSafeHtmlDirective,
...@@ -10,16 +10,23 @@ import { ...@@ -10,16 +10,23 @@ import {
GlDropdown, GlDropdown,
GlDropdownItem, GlDropdownItem,
GlDropdownDivider, GlDropdownDivider,
GlFormCheckbox,
GlLoadingIcon, GlLoadingIcon,
} from '@gitlab/ui'; } from '@gitlab/ui';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
import FileIcon from '~/vue_shared/components/file_icon.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue';
import { truncateSha } from '~/lib/utils/text_utility'; import { truncateSha } from '~/lib/utils/text_utility';
import { __, s__, sprintf } from '~/locale'; import { __, s__, sprintf } from '~/locale';
import { diffViewerModes } from '~/ide/constants';
import DiffStats from './diff_stats.vue'; import DiffStats from './diff_stats.vue';
import { scrollToElement } from '~/lib/utils/common_utils'; import { scrollToElement } from '~/lib/utils/common_utils';
import { isCollapsed } from '../utils/diff_file';
import { collapsedType, isCollapsed } from '../utils/diff_file';
import { reviewable } from '../utils/file_reviews';
import { diffViewerModes } from '~/ide/constants';
import { DIFF_FILE_AUTOMATIC_COLLAPSE } from '../constants';
import { DIFF_FILE_HEADER } from '../i18n'; import { DIFF_FILE_HEADER } from '../i18n';
export default { export default {
...@@ -33,12 +40,14 @@ export default { ...@@ -33,12 +40,14 @@ export default {
GlDropdown, GlDropdown,
GlDropdownItem, GlDropdownItem,
GlDropdownDivider, GlDropdownDivider,
GlFormCheckbox,
GlLoadingIcon, GlLoadingIcon,
}, },
directives: { directives: {
GlTooltip: GlTooltipDirective, GlTooltip: GlTooltipDirective,
SafeHtml: GlSafeHtmlDirective, SafeHtml: GlSafeHtmlDirective,
}, },
mixins: [glFeatureFlagsMixin()],
i18n: { i18n: {
...DIFF_FILE_HEADER, ...DIFF_FILE_HEADER,
}, },
...@@ -76,6 +85,16 @@ export default { ...@@ -76,6 +85,16 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
showLocalFileReviews: {
type: Boolean,
required: false,
default: false,
},
reviewed: {
type: Boolean,
required: false,
default: false,
},
}, },
data() { data() {
return { return {
...@@ -83,6 +102,7 @@ export default { ...@@ -83,6 +102,7 @@ export default {
}; };
}, },
computed: { computed: {
...mapState('diffs', ['latestDiff']),
...mapGetters('diffs', ['diffHasExpandedDiscussions', 'diffHasDiscussions']), ...mapGetters('diffs', ['diffHasExpandedDiscussions', 'diffHasDiscussions']),
diffContentIDSelector() { diffContentIDSelector() {
return `#diff-content-${this.diffFile.file_hash}`; return `#diff-content-${this.diffFile.file_hash}`;
...@@ -170,6 +190,9 @@ export default { ...@@ -170,6 +190,9 @@ export default {
(this.diffFile.edit_path || this.diffFile.ide_edit_path) (this.diffFile.edit_path || this.diffFile.ide_edit_path)
); );
}, },
isReviewable() {
return reviewable(this.diffFile);
},
}, },
methods: { methods: {
...mapActions('diffs', [ ...mapActions('diffs', [
...@@ -177,6 +200,8 @@ export default { ...@@ -177,6 +200,8 @@ export default {
'toggleFileDiscussionWrappers', 'toggleFileDiscussionWrappers',
'toggleFullDiff', 'toggleFullDiff',
'toggleActiveFileByHash', 'toggleActiveFileByHash',
'reviewFile',
'setFileCollapsedByUser',
]), ]),
handleToggleFile() { handleToggleFile() {
this.$emit('toggleFile'); this.$emit('toggleFile');
...@@ -204,6 +229,26 @@ export default { ...@@ -204,6 +229,26 @@ export default {
setMoreActionsShown(val) { setMoreActionsShown(val) {
this.moreActionsShown = val; this.moreActionsShown = val;
}, },
toggleReview(newReviewedStatus) {
const autoCollapsed =
this.isCollapsed && collapsedType(this.diffFile) === DIFF_FILE_AUTOMATIC_COLLAPSE;
const open = this.expanded;
const closed = !open;
const reviewed = newReviewedStatus;
this.reviewFile({ file: this.diffFile, reviewed });
if (reviewed && autoCollapsed) {
this.setFileCollapsedByUser({
filePath: this.diffFile.file_path,
collapsed: true,
});
}
if ((open && reviewed) || (closed && !reviewed)) {
this.$emit('toggleFile');
}
},
}, },
}; };
</script> </script>
...@@ -291,6 +336,19 @@ export default { ...@@ -291,6 +336,19 @@ export default {
class="file-actions d-flex align-items-center gl-ml-auto gl-align-self-start" class="file-actions d-flex align-items-center gl-ml-auto gl-align-self-start"
> >
<diff-stats :added-lines="diffFile.added_lines" :removed-lines="diffFile.removed_lines" /> <diff-stats :added-lines="diffFile.added_lines" :removed-lines="diffFile.removed_lines" />
<gl-form-checkbox
v-if="isReviewable && showLocalFileReviews"
v-gl-tooltip.hover
data-testid="fileReviewCheckbox"
class="gl-mb-0"
:title="$options.i18n.fileReviewTooltip"
:checked="reviewed"
@change="toggleReview"
>
<span class="gl-line-height-20">
{{ $options.i18n.fileReviewLabel }}
</span>
</gl-form-checkbox>
<gl-button-group class="gl-pt-0!"> <gl-button-group class="gl-pt-0!">
<gl-button <gl-button
v-if="diffFile.external_url" v-if="diffFile.external_url"
......
...@@ -4,6 +4,8 @@ export const GENERIC_ERROR = __('Something went wrong on our end. Please try aga ...@@ -4,6 +4,8 @@ export const GENERIC_ERROR = __('Something went wrong on our end. Please try aga
export const DIFF_FILE_HEADER = { export const DIFF_FILE_HEADER = {
optionsDropdownTitle: __('Options'), optionsDropdownTitle: __('Options'),
fileReviewLabel: __('Viewed'),
fileReviewTooltip: __('Collapses this file (only for you) until it’s changed again.'),
}; };
export const DIFF_FILE = { export const DIFF_FILE = {
......
...@@ -749,12 +749,10 @@ export const setFileByFile = ({ commit }, { fileByFile }) => { ...@@ -749,12 +749,10 @@ export const setFileByFile = ({ commit }, { fileByFile }) => {
); );
}; };
export function reviewFile({ commit, state, getters }, { file, reviewed = true }) { export function reviewFile({ commit, state }, { file, reviewed = true }) {
const { mrPath } = getDerivedMergeRequestInformation({ endpoint: file.load_collapsed_diff_url }); const { mrPath } = getDerivedMergeRequestInformation({ endpoint: file.load_collapsed_diff_url });
const reviews = setReviewsForMergeRequest( const reviews = markFileReview(state.mrReviews, file, reviewed);
mrPath,
markFileReview(getters.fileReviews(state), file, reviewed),
);
setReviewsForMergeRequest(mrPath, reviews);
commit(types.SET_MR_FILE_REVIEWS, reviews); commit(types.SET_MR_FILE_REVIEWS, reviews);
} }
import { __, n__ } from '~/locale'; import { __, n__ } from '~/locale';
import { parallelizeDiffLines } from './utils'; import { parallelizeDiffLines } from './utils';
import { isFileReviewed } from '../utils/file_reviews';
import { import {
PARALLEL_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE,
INLINE_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE,
...@@ -155,7 +154,3 @@ export const diffLines = (state) => (file, unifiedDiffComponents) => { ...@@ -155,7 +154,3 @@ export const diffLines = (state) => (file, unifiedDiffComponents) => {
state.diffViewType === INLINE_DIFF_VIEW_TYPE, state.diffViewType === INLINE_DIFF_VIEW_TYPE,
); );
}; };
export function fileReviews(state) {
return state.diffFiles.map((file) => isFileReviewed(state.mrReviews, file));
}
...@@ -47,4 +47,5 @@ export default () => ({ ...@@ -47,4 +47,5 @@ export default () => ({
showSuggestPopover: true, showSuggestPopover: true,
defaultSuggestionCommitMessage: '', defaultSuggestionCommitMessage: '',
mrReviews: {}, mrReviews: {},
latestDiff: true,
}); });
...@@ -2,6 +2,16 @@ function getFileReviewsKey(mrPath) { ...@@ -2,6 +2,16 @@ function getFileReviewsKey(mrPath) {
return `${mrPath}-file-reviews`; return `${mrPath}-file-reviews`;
} }
export function isFileReviewed(reviews, file) {
const fileReviews = reviews[file.file_identifier_hash];
return file?.id && fileReviews?.length ? new Set(fileReviews).has(file.id) : false;
}
export function reviewStatuses(files, reviews) {
return files.map((file) => isFileReviewed(reviews, file));
}
export function getReviewsForMergeRequest(mrPath) { export function getReviewsForMergeRequest(mrPath) {
const reviewsForMr = localStorage.getItem(getFileReviewsKey(mrPath)); const reviewsForMr = localStorage.getItem(getFileReviewsKey(mrPath));
let reviews = {}; let reviews = {};
...@@ -23,23 +33,17 @@ export function setReviewsForMergeRequest(mrPath, reviews) { ...@@ -23,23 +33,17 @@ export function setReviewsForMergeRequest(mrPath, reviews) {
return reviews; return reviews;
} }
export function isFileReviewed(reviews, file) {
const fileReviews = reviews[file.file_identifier_hash];
return file?.id && fileReviews?.length ? new Set(fileReviews).has(file.id) : false;
}
export function reviewable(file) { export function reviewable(file) {
return Boolean(file.id) && Boolean(file.file_identifier_hash); return Boolean(file.id) && Boolean(file.file_identifier_hash);
} }
export function markFileReview(reviews, file, reviewed = true) { export function markFileReview(reviews, file, reviewed = true) {
const usableReviews = { ...(reviews || {}) }; const usableReviews = { ...(reviews || {}) };
let updatedReviews = usableReviews; const updatedReviews = usableReviews;
let fileReviews; let fileReviews;
if (reviewable(file)) { if (reviewable(file)) {
fileReviews = new Set([...(usableReviews[file.file_identifier_hash] || [])]); fileReviews = new Set(usableReviews[file.file_identifier_hash] || []);
if (reviewed) { if (reviewed) {
fileReviews.add(file.id); fileReviews.add(file.id);
...@@ -47,10 +51,7 @@ export function markFileReview(reviews, file, reviewed = true) { ...@@ -47,10 +51,7 @@ export function markFileReview(reviews, file, reviewed = true) {
fileReviews.delete(file.id); fileReviews.delete(file.id);
} }
updatedReviews = { updatedReviews[file.file_identifier_hash] = Array.from(fileReviews);
...usableReviews,
[file.file_identifier_hash]: Array.from(fileReviews),
};
if (updatedReviews[file.file_identifier_hash].length === 0) { if (updatedReviews[file.file_identifier_hash].length === 0) {
delete updatedReviews[file.file_identifier_hash]; delete updatedReviews[file.file_identifier_hash];
......
...@@ -42,6 +42,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -42,6 +42,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:diffs_gradual_load, @project, default_enabled: true) push_frontend_feature_flag(:diffs_gradual_load, @project, default_enabled: true)
push_frontend_feature_flag(:codequality_mr_diff, @project) push_frontend_feature_flag(:codequality_mr_diff, @project)
push_frontend_feature_flag(:suggestions_custom_commit, @project) push_frontend_feature_flag(:suggestions_custom_commit, @project)
push_frontend_feature_flag(:local_file_reviews, default_enabled: :yaml)
record_experiment_user(:invite_members_version_a) record_experiment_user(:invite_members_version_a)
record_experiment_user(:invite_members_version_b) record_experiment_user(:invite_members_version_b)
......
---
title: Mark files as reviewed locally
merge_request: 51513
author:
type: added
---
name: local_file_reviews
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48976
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/296674
milestone: '13.8'
type: development
group: group::code review
default_enabled: false
...@@ -7081,6 +7081,9 @@ msgstr "" ...@@ -7081,6 +7081,9 @@ msgstr ""
msgid "Collapse sidebar" msgid "Collapse sidebar"
msgstr "" msgstr ""
msgid "Collapses this file (only for you) until it’s changed again."
msgstr ""
msgid "Collector hostname" msgid "Collector hostname"
msgstr "" msgstr ""
...@@ -31401,6 +31404,9 @@ msgstr "" ...@@ -31401,6 +31404,9 @@ msgstr ""
msgid "View users statistics" msgid "View users statistics"
msgstr "" msgstr ""
msgid "Viewed"
msgstr ""
msgid "Viewing commit" msgid "Viewing commit"
msgstr "" msgstr ""
......
...@@ -13,10 +13,18 @@ import { diffViewerModes } from '~/ide/constants'; ...@@ -13,10 +13,18 @@ import { diffViewerModes } from '~/ide/constants';
import { __, sprintf } from '~/locale'; import { __, sprintf } from '~/locale';
import { scrollToElement } from '~/lib/utils/common_utils'; import { scrollToElement } from '~/lib/utils/common_utils';
import testAction from '../../__helpers__/vuex_action_helper';
import { SET_MR_FILE_REVIEWS } from '~/diffs/store/mutation_types';
import { reviewFile } from '~/diffs/store/actions';
import { DIFF_FILE_AUTOMATIC_COLLAPSE, DIFF_FILE_MANUAL_COLLAPSE } from '~/diffs/constants';
jest.mock('~/lib/utils/common_utils'); jest.mock('~/lib/utils/common_utils');
const diffFile = Object.freeze( const diffFile = Object.freeze(
Object.assign(diffDiscussionsMockData.diff_file, { Object.assign(diffDiscussionsMockData.diff_file, {
id: '123',
file_identifier_hash: 'abc',
edit_path: 'link:/to/edit/path', edit_path: 'link:/to/edit/path',
blob: { blob: {
id: '848ed9407c6730ff16edb3dd24485a0eea24292a', id: '848ed9407c6730ff16edb3dd24485a0eea24292a',
...@@ -52,6 +60,8 @@ describe('DiffFileHeader component', () => { ...@@ -52,6 +60,8 @@ describe('DiffFileHeader component', () => {
toggleFileDiscussionWrappers: jest.fn(), toggleFileDiscussionWrappers: jest.fn(),
toggleFullDiff: jest.fn(), toggleFullDiff: jest.fn(),
toggleActiveFileByHash: jest.fn(), toggleActiveFileByHash: jest.fn(),
setFileCollapsedByUser: jest.fn(),
reviewFile: jest.fn(),
}, },
}, },
}, },
...@@ -79,10 +89,11 @@ describe('DiffFileHeader component', () => { ...@@ -79,10 +89,11 @@ describe('DiffFileHeader component', () => {
const findViewFileButton = () => wrapper.find({ ref: 'viewButton' }); const findViewFileButton = () => wrapper.find({ ref: 'viewButton' });
const findCollapseIcon = () => wrapper.find({ ref: 'collapseIcon' }); const findCollapseIcon = () => wrapper.find({ ref: 'collapseIcon' });
const findEditButton = () => wrapper.find({ ref: 'editButton' }); const findEditButton = () => wrapper.find({ ref: 'editButton' });
const findReviewFileCheckbox = () => wrapper.find("[data-testid='fileReviewCheckbox']");
const createComponent = (props) => { const createComponent = ({ props, options = {} } = {}) => {
mockStoreConfig = cloneDeep(defaultMockStoreConfig); mockStoreConfig = cloneDeep(defaultMockStoreConfig);
const store = new Vuex.Store(mockStoreConfig); const store = new Vuex.Store({ ...mockStoreConfig, ...(options.store || {}) });
wrapper = shallowMount(DiffFileHeader, { wrapper = shallowMount(DiffFileHeader, {
propsData: { propsData: {
...@@ -91,6 +102,7 @@ describe('DiffFileHeader component', () => { ...@@ -91,6 +102,7 @@ describe('DiffFileHeader component', () => {
viewDiffsFileByFile: false, viewDiffsFileByFile: false,
...props, ...props,
}, },
...options,
localVue, localVue,
store, store,
}); });
...@@ -101,7 +113,7 @@ describe('DiffFileHeader component', () => { ...@@ -101,7 +113,7 @@ describe('DiffFileHeader component', () => {
${'visible'} | ${true} ${'visible'} | ${true}
${'hidden'} | ${false} ${'hidden'} | ${false}
`('collapse toggle is $visibility if collapsible is $collapsible', ({ collapsible }) => { `('collapse toggle is $visibility if collapsible is $collapsible', ({ collapsible }) => {
createComponent({ collapsible }); createComponent({ props: { collapsible } });
expect(findCollapseIcon().exists()).toBe(collapsible); expect(findCollapseIcon().exists()).toBe(collapsible);
}); });
...@@ -110,7 +122,7 @@ describe('DiffFileHeader component', () => { ...@@ -110,7 +122,7 @@ describe('DiffFileHeader component', () => {
${true} | ${'chevron-down'} ${true} | ${'chevron-down'}
${false} | ${'chevron-right'} ${false} | ${'chevron-right'}
`('collapse icon is $icon if expanded is $expanded', ({ icon, expanded }) => { `('collapse icon is $icon if expanded is $expanded', ({ icon, expanded }) => {
createComponent({ expanded, collapsible: true }); createComponent({ props: { expanded, collapsible: true } });
expect(findCollapseIcon().props('name')).toBe(icon); expect(findCollapseIcon().props('name')).toBe(icon);
}); });
...@@ -124,7 +136,7 @@ describe('DiffFileHeader component', () => { ...@@ -124,7 +136,7 @@ describe('DiffFileHeader component', () => {
}); });
it('when collapseIcon is clicked emits toggleFile', () => { it('when collapseIcon is clicked emits toggleFile', () => {
createComponent({ collapsible: true }); createComponent({ props: { collapsible: true } });
findCollapseIcon().vm.$emit('click', new Event('click')); findCollapseIcon().vm.$emit('click', new Event('click'));
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
expect(wrapper.emitted().toggleFile).toBeDefined(); expect(wrapper.emitted().toggleFile).toBeDefined();
...@@ -132,7 +144,7 @@ describe('DiffFileHeader component', () => { ...@@ -132,7 +144,7 @@ describe('DiffFileHeader component', () => {
}); });
it('when other element in header is clicked does not emits toggleFile', () => { it('when other element in header is clicked does not emits toggleFile', () => {
createComponent({ collapsible: true }); createComponent({ props: { collapsible: true } });
findTitleLink().trigger('click'); findTitleLink().trigger('click');
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
...@@ -171,11 +183,13 @@ describe('DiffFileHeader component', () => { ...@@ -171,11 +183,13 @@ describe('DiffFileHeader component', () => {
it('prefers submodule_tree_url over submodule_link for href', () => { it('prefers submodule_tree_url over submodule_link for href', () => {
const submoduleTreeUrl = 'some://tree/url'; const submoduleTreeUrl = 'some://tree/url';
createComponent({ createComponent({
props: {
discussionLink: 'discussionLink', discussionLink: 'discussionLink',
diffFile: { diffFile: {
...submoduleDiffFile, ...submoduleDiffFile,
submodule_tree_url: 'some://tree/url', submodule_tree_url: 'some://tree/url',
}, },
},
}); });
expect(findTitleLink().attributes('href')).toBe(submoduleTreeUrl); expect(findTitleLink().attributes('href')).toBe(submoduleTreeUrl);
...@@ -184,8 +198,10 @@ describe('DiffFileHeader component', () => { ...@@ -184,8 +198,10 @@ describe('DiffFileHeader component', () => {
it('uses submodule_link for href if submodule_tree_url does not exists', () => { it('uses submodule_link for href if submodule_tree_url does not exists', () => {
const submoduleLink = 'link://to/submodule'; const submoduleLink = 'link://to/submodule';
createComponent({ createComponent({
props: {
discussionLink: 'discussionLink', discussionLink: 'discussionLink',
diffFile: submoduleDiffFile, diffFile: submoduleDiffFile,
},
}); });
expect(findTitleLink().attributes('href')).toBe(submoduleLink); expect(findTitleLink().attributes('href')).toBe(submoduleLink);
...@@ -193,7 +209,9 @@ describe('DiffFileHeader component', () => { ...@@ -193,7 +209,9 @@ describe('DiffFileHeader component', () => {
it('uses file_path + SHA as link text', () => { it('uses file_path + SHA as link text', () => {
createComponent({ createComponent({
props: {
diffFile: submoduleDiffFile, diffFile: submoduleDiffFile,
},
}); });
expect(findTitleLink().text()).toContain( expect(findTitleLink().text()).toContain(
...@@ -203,15 +221,19 @@ describe('DiffFileHeader component', () => { ...@@ -203,15 +221,19 @@ describe('DiffFileHeader component', () => {
it('does not render file actions', () => { it('does not render file actions', () => {
createComponent({ createComponent({
props: {
diffFile: submoduleDiffFile, diffFile: submoduleDiffFile,
addMergeRequestButtons: true, addMergeRequestButtons: true,
},
}); });
expect(findFileActions().exists()).toBe(false); expect(findFileActions().exists()).toBe(false);
}); });
it('renders submodule icon', () => { it('renders submodule icon', () => {
createComponent({ createComponent({
props: {
diffFile: submoduleDiffFile, diffFile: submoduleDiffFile,
},
}); });
expect(wrapper.find(FileIcon).props('submodule')).toBe(true); expect(wrapper.find(FileIcon).props('submodule')).toBe(true);
...@@ -223,6 +245,7 @@ describe('DiffFileHeader component', () => { ...@@ -223,6 +245,7 @@ describe('DiffFileHeader component', () => {
it('for mode_changed file mode displays mode changes', () => { it('for mode_changed file mode displays mode changes', () => {
createComponent({ createComponent({
props: {
diffFile: { diffFile: {
...diffFile, ...diffFile,
a_mode: 'old-mode', a_mode: 'old-mode',
...@@ -232,6 +255,7 @@ describe('DiffFileHeader component', () => { ...@@ -232,6 +255,7 @@ describe('DiffFileHeader component', () => {
name: diffViewerModes.mode_changed, name: diffViewerModes.mode_changed,
}, },
}, },
},
}); });
expect(findModeChangedLine().text()).toMatch(/old-mode.+new-mode/); expect(findModeChangedLine().text()).toMatch(/old-mode.+new-mode/);
}); });
...@@ -240,6 +264,7 @@ describe('DiffFileHeader component', () => { ...@@ -240,6 +264,7 @@ describe('DiffFileHeader component', () => {
'for %s file mode does not display mode changes', 'for %s file mode does not display mode changes',
(mode) => { (mode) => {
createComponent({ createComponent({
props: {
diffFile: { diffFile: {
...diffFile, ...diffFile,
a_mode: 'old-mode', a_mode: 'old-mode',
...@@ -249,6 +274,7 @@ describe('DiffFileHeader component', () => { ...@@ -249,6 +274,7 @@ describe('DiffFileHeader component', () => {
name: diffViewerModes[mode], name: diffViewerModes[mode],
}, },
}, },
},
}); });
expect(findModeChangedLine().exists()).toBeFalsy(); expect(findModeChangedLine().exists()).toBeFalsy();
}, },
...@@ -256,32 +282,38 @@ describe('DiffFileHeader component', () => { ...@@ -256,32 +282,38 @@ describe('DiffFileHeader component', () => {
it('displays the LFS label for files stored in LFS', () => { it('displays the LFS label for files stored in LFS', () => {
createComponent({ createComponent({
props: {
diffFile: { ...diffFile, stored_externally: true, external_storage: 'lfs' }, diffFile: { ...diffFile, stored_externally: true, external_storage: 'lfs' },
},
}); });
expect(findLfsLabel().exists()).toBe(true); expect(findLfsLabel().exists()).toBe(true);
}); });
it('does not display the LFS label for files stored in repository', () => { it('does not display the LFS label for files stored in repository', () => {
createComponent({ createComponent({
props: {
diffFile: { ...diffFile, stored_externally: false }, diffFile: { ...diffFile, stored_externally: false },
},
}); });
expect(findLfsLabel().exists()).toBe(false); expect(findLfsLabel().exists()).toBe(false);
}); });
it('does not render view replaced file button if no replaced view path is present', () => { it('does not render view replaced file button if no replaced view path is present', () => {
createComponent({ createComponent({
props: {
diffFile: { ...diffFile, replaced_view_path: null }, diffFile: { ...diffFile, replaced_view_path: null },
},
}); });
expect(findReplacedFileButton().exists()).toBe(false); expect(findReplacedFileButton().exists()).toBe(false);
}); });
describe('when addMergeRequestButtons is false', () => { describe('when addMergeRequestButtons is false', () => {
it('does not render file actions', () => { it('does not render file actions', () => {
createComponent({ addMergeRequestButtons: false }); createComponent({ props: { addMergeRequestButtons: false } });
expect(findFileActions().exists()).toBe(false); expect(findFileActions().exists()).toBe(false);
}); });
it('should not render edit button', () => { it('should not render edit button', () => {
createComponent({ addMergeRequestButtons: false }); createComponent({ props: { addMergeRequestButtons: false } });
expect(findEditButton().exists()).toBe(false); expect(findEditButton().exists()).toBe(false);
}); });
}); });
...@@ -290,7 +322,7 @@ describe('DiffFileHeader component', () => { ...@@ -290,7 +322,7 @@ describe('DiffFileHeader component', () => {
describe('without discussions', () => { describe('without discussions', () => {
it('does not render a toggle discussions button', () => { it('does not render a toggle discussions button', () => {
diffHasDiscussionsResultMock.mockReturnValue(false); diffHasDiscussionsResultMock.mockReturnValue(false);
createComponent({ addMergeRequestButtons: true }); createComponent({ props: { addMergeRequestButtons: true } });
expect(findToggleDiscussionsButton().exists()).toBe(false); expect(findToggleDiscussionsButton().exists()).toBe(false);
}); });
}); });
...@@ -298,7 +330,7 @@ describe('DiffFileHeader component', () => { ...@@ -298,7 +330,7 @@ describe('DiffFileHeader component', () => {
describe('with discussions', () => { describe('with discussions', () => {
it('dispatches toggleFileDiscussionWrappers when user clicks on toggle discussions button', () => { it('dispatches toggleFileDiscussionWrappers when user clicks on toggle discussions button', () => {
diffHasDiscussionsResultMock.mockReturnValue(true); diffHasDiscussionsResultMock.mockReturnValue(true);
createComponent({ addMergeRequestButtons: true }); createComponent({ props: { addMergeRequestButtons: true } });
expect(findToggleDiscussionsButton().exists()).toBe(true); expect(findToggleDiscussionsButton().exists()).toBe(true);
findToggleDiscussionsButton().vm.$emit('click'); findToggleDiscussionsButton().vm.$emit('click');
expect( expect(
...@@ -309,7 +341,9 @@ describe('DiffFileHeader component', () => { ...@@ -309,7 +341,9 @@ describe('DiffFileHeader component', () => {
it('should show edit button', () => { it('should show edit button', () => {
createComponent({ createComponent({
props: {
addMergeRequestButtons: true, addMergeRequestButtons: true,
},
}); });
expect(findEditButton().exists()).toBe(true); expect(findEditButton().exists()).toBe(true);
}); });
...@@ -319,25 +353,27 @@ describe('DiffFileHeader component', () => { ...@@ -319,25 +353,27 @@ describe('DiffFileHeader component', () => {
const externalUrl = 'link://to/external'; const externalUrl = 'link://to/external';
const formattedExternalUrl = 'link://formatted'; const formattedExternalUrl = 'link://formatted';
createComponent({ createComponent({
props: {
diffFile: { diffFile: {
...diffFile, ...diffFile,
external_url: externalUrl, external_url: externalUrl,
formatted_external_url: formattedExternalUrl, formatted_external_url: formattedExternalUrl,
}, },
addMergeRequestButtons: true, addMergeRequestButtons: true,
},
}); });
expect(findExternalLink().exists()).toBe(true); expect(findExternalLink().exists()).toBe(true);
}); });
it('is hidden by default', () => { it('is hidden by default', () => {
createComponent({ addMergeRequestButtons: true }); createComponent({ props: { addMergeRequestButtons: true } });
expect(findExternalLink().exists()).toBe(false); expect(findExternalLink().exists()).toBe(false);
}); });
}); });
describe('without file blob', () => { describe('without file blob', () => {
beforeEach(() => { beforeEach(() => {
createComponent({ diffFile: { ...diffFile, blob: false } }); createComponent({ props: { diffFile: { ...diffFile, blob: false } } });
}); });
it('should not render toggle discussions button', () => { it('should not render toggle discussions button', () => {
...@@ -352,8 +388,10 @@ describe('DiffFileHeader component', () => { ...@@ -352,8 +388,10 @@ describe('DiffFileHeader component', () => {
it('should render correct file view button', () => { it('should render correct file view button', () => {
const viewPath = 'link://view-path'; const viewPath = 'link://view-path';
createComponent({ createComponent({
props: {
diffFile: { ...diffFile, view_path: viewPath }, diffFile: { ...diffFile, view_path: viewPath },
addMergeRequestButtons: true, addMergeRequestButtons: true,
},
}); });
expect(findViewFileButton().attributes('href')).toBe(viewPath); expect(findViewFileButton().attributes('href')).toBe(viewPath);
expect(findViewFileButton().text()).toEqual( expect(findViewFileButton().text()).toEqual(
...@@ -367,10 +405,12 @@ describe('DiffFileHeader component', () => { ...@@ -367,10 +405,12 @@ describe('DiffFileHeader component', () => {
describe('when diff is fully expanded', () => { describe('when diff is fully expanded', () => {
it('is not rendered', () => { it('is not rendered', () => {
createComponent({ createComponent({
props: {
diffFile: { diffFile: {
...diffFile, ...diffFile,
is_fully_expanded: true, is_fully_expanded: true,
}, },
},
}); });
expect(findExpandButton().exists()).toBe(false); expect(findExpandButton().exists()).toBe(false);
}); });
...@@ -387,17 +427,17 @@ describe('DiffFileHeader component', () => { ...@@ -387,17 +427,17 @@ describe('DiffFileHeader component', () => {
}; };
it('renders expand to full file button if not showing full file already', () => { it('renders expand to full file button if not showing full file already', () => {
createComponent(fullyNotExpandedFileProps); createComponent({ props: fullyNotExpandedFileProps });
expect(findExpandButton().exists()).toBe(true); expect(findExpandButton().exists()).toBe(true);
}); });
it('renders loading icon when loading full file', () => { it('renders loading icon when loading full file', () => {
createComponent(fullyNotExpandedFileProps); createComponent({ props: fullyNotExpandedFileProps });
expect(findExpandButton().exists()).toBe(true); expect(findExpandButton().exists()).toBe(true);
}); });
it('toggles full diff on click', () => { it('toggles full diff on click', () => {
createComponent(fullyNotExpandedFileProps); createComponent({ props: fullyNotExpandedFileProps });
findExpandButton().vm.$emit('click'); findExpandButton().vm.$emit('click');
expect(mockStoreConfig.modules.diffs.actions.toggleFullDiff).toHaveBeenCalled(); expect(mockStoreConfig.modules.diffs.actions.toggleFullDiff).toHaveBeenCalled();
}); });
...@@ -407,7 +447,9 @@ describe('DiffFileHeader component', () => { ...@@ -407,7 +447,9 @@ describe('DiffFileHeader component', () => {
it('uses discussionPath for link if it is defined', () => { it('uses discussionPath for link if it is defined', () => {
const discussionPath = 'link://to/discussion'; const discussionPath = 'link://to/discussion';
createComponent({ createComponent({
props: {
discussionPath, discussionPath,
},
}); });
expect(findTitleLink().attributes('href')).toBe(discussionPath); expect(findTitleLink().attributes('href')).toBe(discussionPath);
}); });
...@@ -436,21 +478,21 @@ describe('DiffFileHeader component', () => { ...@@ -436,21 +478,21 @@ describe('DiffFileHeader component', () => {
describe('for new file', () => { describe('for new file', () => {
it('displays the path', () => { it('displays the path', () => {
createComponent({ diffFile: { ...diffFile, new_file: true } }); createComponent({ props: { diffFile: { ...diffFile, new_file: true } } });
expect(findTitleLink().text()).toBe(diffFile.file_path); expect(findTitleLink().text()).toBe(diffFile.file_path);
}); });
}); });
describe('for deleted file', () => { describe('for deleted file', () => {
it('displays the path', () => { it('displays the path', () => {
createComponent({ diffFile: { ...diffFile, deleted_file: true } }); createComponent({ props: { diffFile: { ...diffFile, deleted_file: true } } });
expect(findTitleLink().text()).toBe( expect(findTitleLink().text()).toBe(
sprintf(__('%{filePath} deleted'), { filePath: diffFile.file_path }, false), sprintf(__('%{filePath} deleted'), { filePath: diffFile.file_path }, false),
); );
}); });
it('does not show edit button', () => { it('does not show edit button', () => {
createComponent({ diffFile: { ...diffFile, deleted_file: true } }); createComponent({ props: { diffFile: { ...diffFile, deleted_file: true } } });
expect(findEditButton().exists()).toBe(false); expect(findEditButton().exists()).toBe(false);
}); });
}); });
...@@ -458,12 +500,14 @@ describe('DiffFileHeader component', () => { ...@@ -458,12 +500,14 @@ describe('DiffFileHeader component', () => {
describe('for renamed file', () => { describe('for renamed file', () => {
it('displays old and new path if the file was renamed', () => { it('displays old and new path if the file was renamed', () => {
createComponent({ createComponent({
props: {
diffFile: { diffFile: {
...diffFile, ...diffFile,
renamed_file: true, renamed_file: true,
old_path_html: 'old', old_path_html: 'old',
new_path_html: 'new', new_path_html: 'new',
}, },
},
}); });
expect(findTitleLink().text()).toMatch(/^old.+new/s); expect(findTitleLink().text()).toMatch(/^old.+new/s);
}); });
...@@ -473,13 +517,132 @@ describe('DiffFileHeader component', () => { ...@@ -473,13 +517,132 @@ describe('DiffFileHeader component', () => {
it('renders view replaced file button', () => { it('renders view replaced file button', () => {
const replacedViewPath = 'some/path'; const replacedViewPath = 'some/path';
createComponent({ createComponent({
props: {
diffFile: { diffFile: {
...diffFile, ...diffFile,
replaced_view_path: replacedViewPath, replaced_view_path: replacedViewPath,
}, },
addMergeRequestButtons: true, addMergeRequestButtons: true,
},
}); });
expect(findReplacedFileButton().exists()).toBe(true); expect(findReplacedFileButton().exists()).toBe(true);
}); });
}); });
describe('file reviews', () => {
it('calls the action to set the new review', () => {
createComponent({
props: {
diffFile: {
...diffFile,
viewer: {
...diffFile.viewer,
automaticallyCollapsed: false,
manuallyCollapsed: null,
},
},
showLocalFileReviews: true,
addMergeRequestButtons: true,
},
});
const file = wrapper.vm.diffFile;
findReviewFileCheckbox().vm.$emit('change', true);
return testAction(
reviewFile,
{ file, reviewed: true },
{},
[{ type: SET_MR_FILE_REVIEWS, payload: { [file.file_identifier_hash]: [file.id] } }],
[],
);
});
it.each`
description | newReviewedStatus | collapseType | aCollapse | mCollapse | callAction
${'does nothing'} | ${true} | ${DIFF_FILE_MANUAL_COLLAPSE} | ${false} | ${true} | ${false}
${'does nothing'} | ${false} | ${DIFF_FILE_AUTOMATIC_COLLAPSE} | ${true} | ${null} | ${false}
${'does nothing'} | ${true} | ${'not collapsed'} | ${false} | ${null} | ${false}
${'does nothing'} | ${false} | ${'not collapsed'} | ${false} | ${null} | ${false}
${'collapses the file'} | ${true} | ${DIFF_FILE_AUTOMATIC_COLLAPSE} | ${true} | ${null} | ${true}
`(
"$description if the new review status is reviewed = $newReviewedStatus and the file's collapse type is collapse = $collapseType",
({ newReviewedStatus, aCollapse, mCollapse, callAction }) => {
createComponent({
props: {
diffFile: {
...diffFile,
viewer: {
...diffFile.viewer,
automaticallyCollapsed: aCollapse,
manuallyCollapsed: mCollapse,
},
},
showLocalFileReviews: true,
addMergeRequestButtons: true,
},
});
findReviewFileCheckbox().vm.$emit('change', newReviewedStatus);
if (callAction) {
expect(mockStoreConfig.modules.diffs.actions.setFileCollapsedByUser).toHaveBeenCalled();
} else {
expect(
mockStoreConfig.modules.diffs.actions.setFileCollapsedByUser,
).not.toHaveBeenCalled();
}
},
);
it.each`
description | show | visible
${'shows'} | ${true} | ${true}
${'hides'} | ${false} | ${false}
`(
'$description the file review feature given { showLocalFileReviewsProp: $show }',
({ show, visible }) => {
createComponent({
props: {
showLocalFileReviews: show,
addMergeRequestButtons: true,
},
});
expect(findReviewFileCheckbox().exists()).toEqual(visible);
},
);
it.each`
open | status | fires
${true} | ${true} | ${true}
${false} | ${false} | ${true}
${true} | ${false} | ${false}
${false} | ${true} | ${false}
`(
'toggles appropriately when { fileExpanded: $open, newReviewStatus: $status }',
({ open, status, fires }) => {
createComponent({
props: {
diffFile: {
...diffFile,
viewer: {
...diffFile.viewer,
automaticallyCollapsed: false,
manuallyCollapsed: null,
},
},
showLocalFileReviews: true,
addMergeRequestButtons: true,
expanded: open,
},
});
findReviewFileCheckbox().vm.$emit('change', status);
expect(Boolean(wrapper.emitted().toggleFile)).toBe(fires);
},
);
});
}); });
...@@ -66,7 +66,7 @@ function markFileToBeRendered(store, index = 0) { ...@@ -66,7 +66,7 @@ function markFileToBeRendered(store, index = 0) {
}); });
} }
function createComponent({ file, first = false, last = false }) { function createComponent({ file, first = false, last = false, options = {}, props = {} }) {
const localVue = createLocalVue(); const localVue = createLocalVue();
localVue.use(Vuex); localVue.use(Vuex);
...@@ -89,7 +89,9 @@ function createComponent({ file, first = false, last = false }) { ...@@ -89,7 +89,9 @@ function createComponent({ file, first = false, last = false }) {
viewDiffsFileByFile: false, viewDiffsFileByFile: false,
isFirstFile: first, isFirstFile: first,
isLastFile: last, isLastFile: last,
...props,
}, },
...options,
}); });
return { return {
...@@ -220,6 +222,53 @@ describe('DiffFile', () => { ...@@ -220,6 +222,53 @@ describe('DiffFile', () => {
}); });
}); });
describe('computed', () => {
describe('showLocalFileReviews', () => {
let gon;
function setLoggedIn(bool) {
window.gon.current_user_id = bool;
}
beforeAll(() => {
gon = window.gon;
window.gon = {};
});
afterEach(() => {
window.gon = gon;
});
it.each`
loggedIn | featureOn | bool
${true} | ${true} | ${true}
${false} | ${true} | ${false}
${true} | ${false} | ${false}
${false} | ${false} | ${false}
`(
'should be $bool when { userIsLoggedIn: $loggedIn, featureEnabled: $featureOn }',
({ loggedIn, featureOn, bool }) => {
setLoggedIn(loggedIn);
({ wrapper } = createComponent({
options: {
provide: {
glFeatures: {
localFileReviews: featureOn,
},
},
},
props: {
file: store.state.diffs.diffFiles[0],
},
}));
expect(wrapper.vm.showLocalFileReviews).toBe(bool);
},
);
});
});
describe('collapsing', () => { describe('collapsing', () => {
describe(`\`${EVT_EXPAND_ALL_FILES}\` event`, () => { describe(`\`${EVT_EXPAND_ALL_FILES}\` event`, () => {
beforeEach(() => { beforeEach(() => {
......
...@@ -375,26 +375,4 @@ describe('Diffs Module Getters', () => { ...@@ -375,26 +375,4 @@ describe('Diffs Module Getters', () => {
}); });
}); });
}); });
describe('fileReviews', () => {
const file1 = { id: '123', file_identifier_hash: 'abc' };
const file2 = { id: '098', file_identifier_hash: 'abc' };
it.each`
reviews | files | fileReviews
${{}} | ${[file1, file2]} | ${[false, false]}
${{ abc: ['123'] }} | ${[file1, file2]} | ${[true, false]}
${{ abc: ['098'] }} | ${[file1, file2]} | ${[false, true]}
${{ def: ['123'] }} | ${[file1, file2]} | ${[false, false]}
${{ abc: ['123'], def: ['098'] }} | ${[]} | ${[]}
`(
'returns $fileReviews based on the diff files in state and the existing reviews $reviews',
({ reviews, files, fileReviews }) => {
localState.diffFiles = files;
localState.mrReviews = reviews;
expect(getters.fileReviews(localState)).toStrictEqual(fileReviews);
},
);
});
}); });
...@@ -5,6 +5,7 @@ import { ...@@ -5,6 +5,7 @@ import {
setReviewsForMergeRequest, setReviewsForMergeRequest,
isFileReviewed, isFileReviewed,
markFileReview, markFileReview,
reviewStatuses,
reviewable, reviewable,
} from '~/diffs/utils/file_reviews'; } from '~/diffs/utils/file_reviews';
...@@ -28,6 +29,39 @@ describe('File Review(s) utilities', () => { ...@@ -28,6 +29,39 @@ describe('File Review(s) utilities', () => {
localStorage.clear(); localStorage.clear();
}); });
describe('isFileReviewed', () => {
it.each`
description | diffFile | fileReviews
${'the file does not have an `id`'} | ${{ ...file, id: undefined }} | ${getDefaultReviews()}
${'there are no reviews for the file'} | ${file} | ${{ ...getDefaultReviews(), abc: undefined }}
`('returns `false` if $description', ({ diffFile, fileReviews }) => {
expect(isFileReviewed(fileReviews, diffFile)).toBe(false);
});
it("returns `true` for a file if it's available in the provided reviews", () => {
expect(isFileReviewed(reviews, file)).toBe(true);
});
});
describe('reviewStatuses', () => {
const file1 = { id: '123', file_identifier_hash: 'abc' };
const file2 = { id: '098', file_identifier_hash: 'abc' };
it.each`
mrReviews | files | fileReviews
${{}} | ${[file1, file2]} | ${[false, false]}
${{ abc: ['123'] }} | ${[file1, file2]} | ${[true, false]}
${{ abc: ['098'] }} | ${[file1, file2]} | ${[false, true]}
${{ def: ['123'] }} | ${[file1, file2]} | ${[false, false]}
${{ abc: ['123'], def: ['098'] }} | ${[]} | ${[]}
`(
'returns $fileReviews based on the diff files in state and the existing reviews $reviews',
({ mrReviews, files, fileReviews }) => {
expect(reviewStatuses(files, mrReviews)).toStrictEqual(fileReviews);
},
);
});
describe('getReviewsForMergeRequest', () => { describe('getReviewsForMergeRequest', () => {
it('fetches the appropriate stored reviews from localStorage', () => { it('fetches the appropriate stored reviews from localStorage', () => {
getReviewsForMergeRequest(mrPath); getReviewsForMergeRequest(mrPath);
...@@ -73,20 +107,6 @@ describe('File Review(s) utilities', () => { ...@@ -73,20 +107,6 @@ describe('File Review(s) utilities', () => {
}); });
}); });
describe('isFileReviewed', () => {
it.each`
description | diffFile | fileReviews
${'the file does not have an `id`'} | ${{ ...file, id: undefined }} | ${getDefaultReviews()}
${'there are no reviews for the file'} | ${file} | ${{ ...getDefaultReviews(), abc: undefined }}
`('returns `false` if $description', ({ diffFile, fileReviews }) => {
expect(isFileReviewed(fileReviews, diffFile)).toBe(false);
});
it("returns `true` for a file if it's available in the provided reviews", () => {
expect(isFileReviewed(reviews, file)).toBe(true);
});
});
describe('reviewable', () => { describe('reviewable', () => {
it.each` it.each`
response | diffFile | description response | diffFile | description
......
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