Commit e1fd7d0d authored by Miguel Rincon's avatar Miguel Rincon

Respond to frontend review

- Receive selector instead of element in load_branches.js
- Use `data-testid` to find mini pipeline instead of class.
parent d96abce6
......@@ -36,7 +36,7 @@ export default {
};
</script>
<template>
<div data-testid="widget-mini-pipeline-graph">
<div data-testid="pipeline-mini-graph">
<div
v-for="stage in stages"
:key="stage.name"
......
......@@ -3,11 +3,9 @@ import { initCommitPipelineMiniGraph } from './init_commit_pipeline_mini_graph';
import { initDetailsButton } from './init_details_button';
import { loadBranches } from './load_branches';
export const initCommitBoxInfo = (containerSelector = '.js-commit-box-info') => {
const containerEl = document.querySelector(containerSelector);
export const initCommitBoxInfo = () => {
// Display commit related branches
loadBranches(containerEl);
loadBranches();
// Related merge requests to this commit
fetchCommitMergeRequests();
......
......@@ -2,7 +2,8 @@ import axios from 'axios';
import { sanitize } from '~/lib/dompurify';
import { __ } from '~/locale';
export const loadBranches = (containerEl) => {
export const loadBranches = (containerSelector = '.js-commit-box-info') => {
const containerEl = document.querySelector(containerSelector);
if (!containerEl) {
return;
}
......
......@@ -23,7 +23,7 @@ RSpec.describe 'Merge request < User sees mini pipeline graph', :js do
end
it 'displays a mini pipeline graph' do
expect(page).to have_selector('.mr-widget-pipeline-graph')
expect(page).to have_selector('[data-testid="pipeline-mini-graph"]')
end
context 'as json' do
......
......@@ -26,7 +26,7 @@ RSpec.describe 'Mini Pipeline Graph in Commit View', :js do
end
it 'displays a mini pipeline graph' do
expect(page).to have_selector('.mr-widget-pipeline-graph')
expect(page).to have_selector('[data-testid="pipeline-mini-graph"]')
first('.mini-pipeline-graph-dropdown-toggle').click
......@@ -47,7 +47,7 @@ RSpec.describe 'Mini Pipeline Graph in Commit View', :js do
end
it 'does not display a mini pipeline graph' do
expect(page).not_to have_selector('.mr-widget-pipeline-graph')
expect(page).not_to have_selector('[data-testid="pipeline-mini-graph"]')
end
end
end
......@@ -534,7 +534,7 @@ RSpec.describe 'Pipelines', :js do
end
it 'renders a mini pipeline graph' do
expect(page).to have_selector('[data-testid="widget-mini-pipeline-graph"]')
expect(page).to have_selector('[data-testid="pipeline-mini-graph"]')
expect(page).to have_selector(dropdown_selector)
end
......
import axios from 'axios';
import MockAdapter from 'axios-mock-adapter';
import { setHTMLFixture } from 'helpers/fixtures';
import waitForPromises from 'helpers/wait_for_promises';
import { loadBranches } from '~/projects/commit_box/info/load_branches';
......@@ -9,32 +10,38 @@ const mockBranchesRes =
describe('~/projects/commit_box/info/load_branches', () => {
let mock;
let el;
const getElInnerHtml = () => document.querySelector('.js-commit-box-info').innerHTML;
beforeEach(() => {
setHTMLFixture(`
<div class="js-commit-box-info" data-commit-path="${mockCommitPath}">
<div class="commit-info branches">
<span class="spinner"/>
</div>
</div>`);
mock = new MockAdapter(axios);
mock.onGet(mockCommitPath).reply(200, mockBranchesRes);
el = document.createElement('div');
el.dataset.commitPath = mockCommitPath;
el.innerHTML = '<div class="commit-info branches"><span class="spinner"/></div>';
});
it('loads and renders branches info', async () => {
loadBranches(el);
loadBranches();
await waitForPromises();
expect(el.innerHTML).toBe(`<div class="commit-info branches">${mockBranchesRes}</div>`);
expect(getElInnerHtml()).toMatchInterpolatedText(
`<div class="commit-info branches">${mockBranchesRes}</div>`,
);
});
it('does not load when no container is provided', async () => {
loadBranches(null);
loadBranches('.js-another-class');
await waitForPromises();
expect(mock.history.get).toHaveLength(0);
});
describe('when braches request returns unsafe content', () => {
describe('when branches request returns unsafe content', () => {
beforeEach(() => {
mock
.onGet(mockCommitPath)
......@@ -42,25 +49,25 @@ describe('~/projects/commit_box/info/load_branches', () => {
});
it('displays sanitized html', async () => {
loadBranches(el);
loadBranches();
await waitForPromises();
expect(el.innerHTML).toBe(
expect(getElInnerHtml()).toMatchInterpolatedText(
'<div class="commit-info branches"><a href="/-/commits/master">master</a></div>',
);
});
});
describe('when braches request fails', () => {
describe('when branches request fails', () => {
beforeEach(() => {
mock.onGet(mockCommitPath).reply(500, 'Error!');
});
it('attempts to load and renders an error', async () => {
loadBranches(el);
loadBranches();
await waitForPromises();
expect(el.innerHTML).toBe(
expect(getElInnerHtml()).toMatchInterpolatedText(
'<div class="commit-info branches">Failed to load branches. Please try again.</div>',
);
});
......
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