Commit b01fd7ad authored by Filipa Lacerda's avatar Filipa Lacerda

Fixes unresolved discussions rendering the error state instead of the diff

parent 1e54e30f
...@@ -19,7 +19,7 @@ import AjaxCache from '~/lib/utils/ajax_cache'; ...@@ -19,7 +19,7 @@ import AjaxCache from '~/lib/utils/ajax_cache';
import Vue from 'vue'; import Vue from 'vue';
import syntaxHighlight from '~/syntax_highlight'; import syntaxHighlight from '~/syntax_highlight';
import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue';
import { __ } from '~/locale'; import { __, sprintf } from '~/locale';
import axios from './lib/utils/axios_utils'; import axios from './lib/utils/axios_utils';
import { getLocationHash } from './lib/utils/url_utility'; import { getLocationHash } from './lib/utils/url_utility';
import Flash from './flash'; import Flash from './flash';
...@@ -1434,10 +1434,11 @@ export default class Notes { ...@@ -1434,10 +1434,11 @@ export default class Notes {
static renderDiffError($container) { static renderDiffError($container) {
$container.find('.line_content').html( $container.find('.line_content').html(
$(` $(`
<div class="nothing-here-block"> <div class="js-error-load-lazy-diff nothing-here-block">
${__( ${sprintf(__('Unable to load the diff.%{buttonStartTag}Try again%{buttonEndTag}?'), {
'Unable to load the diff.', buttonStartTag: '<button type="button" class="btn-link btn-no-padding js-toggle-lazy-diff">',
)} <a class="js-toggle-lazy-diff" href="javascript:void(0)">Try again</a>? buttonEndTag: '</button>'
}, false)}
</div> </div>
`), `),
); );
...@@ -1455,6 +1456,11 @@ export default class Notes { ...@@ -1455,6 +1456,11 @@ export default class Notes {
const fileHolder = $container.find('.file-holder'); const fileHolder = $container.find('.file-holder');
const url = fileHolder.data('linesPath'); const url = fileHolder.data('linesPath');
/**
* We only fetch resolved discussions.
* Unresolved discussions don't have an endpoint being provided.
*/
if (url) {
axios axios
.get(url) .get(url)
.then(({ data }) => { .then(({ data }) => {
...@@ -1464,6 +1470,7 @@ export default class Notes { ...@@ -1464,6 +1470,7 @@ export default class Notes {
Notes.renderDiffError($container); Notes.renderDiffError($container);
}); });
} }
}
toggleCommitList(e) { toggleCommitList(e) {
const $element = $(e.currentTarget); const $element = $(e.currentTarget);
......
...@@ -485,3 +485,7 @@ fieldset[disabled] .btn, ...@@ -485,3 +485,7 @@ fieldset[disabled] .btn,
@extend %disabled; @extend %disabled;
} }
} }
.btn-no-padding {
padding: 0;
}
\ No newline at end of file
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
- unless expanded - unless expanded
- diff_data = { lines_path: project_merge_request_discussion_path(discussion.project, discussion.noteable, discussion) } - diff_data = { lines_path: project_merge_request_discussion_path(discussion.project, discussion.noteable, discussion) }
.diff-file.file-holder{ class: diff_file_class, data: diff_data } .diff-file.file-holder.js-lazy-load-discussion{ class: diff_file_class, data: diff_data }
.js-file-title.file-title.file-title-flex-parent .js-file-title.file-title.file-title-flex-parent
.file-header-content .file-header-content
= render "projects/diffs/file_header", diff_file: diff_file, url: discussion_path(discussion), show_toggle: false = render "projects/diffs/file_header", diff_file: diff_file, url: discussion_path(discussion), show_toggle: false
......
---
title: Fixes unresolved discussions rendering the error state instead of the diff
merge_request:
author:
type: fixed
...@@ -27,6 +27,24 @@ describe 'Merge request > User scrolls to note on load', :js do ...@@ -27,6 +27,24 @@ describe 'Merge request > User scrolls to note on load', :js do
expect(fragment_position_top).to be < (page_scroll_y + page_height) expect(fragment_position_top).to be < (page_scroll_y + page_height)
end end
it 'renders un-collapsed notes with diff' do
page.current_window.resize_to(1000, 1000)
visit "#{project_merge_request_path(project, merge_request)}#{fragment_id}"
page.execute_script "window.scrollTo(0,0)"
note_element = find(fragment_id)
note_container = note_element.ancestor('.js-toggle-container')
expect(note_element.visible?).to eq true
page.within note_container do
expect(page).not_to have_selector('.js-error-load-lazy-diff')
end
end
it 'expands collapsed notes' do it 'expands collapsed notes' do
visit "#{project_merge_request_path(project, merge_request)}#{collapsed_fragment_id}" visit "#{project_merge_request_path(project, merge_request)}#{collapsed_fragment_id}"
note_element = find(collapsed_fragment_id) note_element = find(collapsed_fragment_id)
......
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