Commit bd9ff63e authored by Natalia Tepluhina's avatar Natalia Tepluhina Committed by Nicolò Maria Mezzopera

Fix design overlay test case without adminNote

Here we improve the test case for when we are
trying to ensure that a user without adminNote
permission is unable to move an existing note.

Previously, we were testing arbitrary details of the
component. Here, we simplify to test only
the outcome w.r.t the DOM (in this case,
the element's `style` tag).
parent 6123dff4
...@@ -63,10 +63,12 @@ export default { ...@@ -63,10 +63,12 @@ export default {
query: activeDiscussionQuery, query: activeDiscussionQuery,
result({ data }) { result({ data }) {
const discussionId = data.activeDiscussion.id; const discussionId = data.activeDiscussion.id;
if (this.discussion.resolved && !this.resolvedDiscussionsExpanded) {
return;
}
// We watch any changes to the active discussion from the design pins and scroll to this discussion if it exists // We watch any changes to the active discussion from the design pins and scroll to this discussion if it exists
// We don't want scrollIntoView to be triggered from the discussion click itself // We don't want scrollIntoView to be triggered from the discussion click itself
if ( if (
this.resolvedDiscussionsExpanded &&
discussionId && discussionId &&
data.activeDiscussion.source === ACTIVE_DISCUSSION_SOURCE_TYPES.pin && data.activeDiscussion.source === ACTIVE_DISCUSSION_SOURCE_TYPES.pin &&
discussionId === this.discussion.notes[0].id discussionId === this.discussion.notes[0].id
......
...@@ -144,7 +144,7 @@ export default { ...@@ -144,7 +144,7 @@ export default {
}, },
onExistingNoteMove(e) { onExistingNoteMove(e) {
const note = this.notes.find(({ id }) => id === this.movingNoteStartPosition.noteId); const note = this.notes.find(({ id }) => id === this.movingNoteStartPosition.noteId);
if (!note) return; if (!note || !this.canMoveNote(note)) return;
const { position } = note; const { position } = note;
const { width, height } = position; const { width, height } = position;
...@@ -190,8 +190,6 @@ export default { ...@@ -190,8 +190,6 @@ export default {
}); });
}, },
onNoteMousedown({ clientX, clientY }, note) { onNoteMousedown({ clientX, clientY }, note) {
if (note && !this.canMoveNote(note)) return;
this.movingNoteStartPosition = { this.movingNoteStartPosition = {
noteId: note?.id, noteId: note?.id,
discussionId: note?.discussion.id, discussionId: note?.discussion.id,
......
---
title: Fix design note scrolling
merge_request: 33939
author:
type: fixed
...@@ -10,18 +10,6 @@ describe('Design overlay component', () => { ...@@ -10,18 +10,6 @@ describe('Design overlay component', () => {
let wrapper; let wrapper;
const mockDimensions = { width: 100, height: 100 }; const mockDimensions = { width: 100, height: 100 };
const mockNoteNotAuthorised = {
id: 'note-not-authorised',
index: 1,
discussion: { id: 'discussion-not-authorised' },
position: {
x: 1,
y: 80,
...mockDimensions,
},
userPermissions: {},
resolved: false,
};
const findOverlay = () => wrapper.find('.image-diff-overlay'); const findOverlay = () => wrapper.find('.image-diff-overlay');
const findAllNotes = () => wrapper.findAll('.js-image-badge'); const findAllNotes = () => wrapper.findAll('.js-image-badge');
...@@ -230,20 +218,32 @@ describe('Design overlay component', () => { ...@@ -230,20 +218,32 @@ describe('Design overlay component', () => {
}); });
}); });
it('should do nothing if [adminNote] permission is not present', () => { describe('without [adminNote] permission', () => {
createComponent({ const mockNoteNotAuthorised = {
dimensions: mockDimensions, ...notes[0],
notes: [mockNoteNotAuthorised], userPermissions: {
}); adminNote: false,
},
};
const badge = findAllNotes().at(0); const mockNoteCoordinates = {
return clickAndDragBadge( x: mockNoteNotAuthorised.position.x,
badge, y: mockNoteNotAuthorised.position.y,
{ x: mockNoteNotAuthorised.x, y: mockNoteNotAuthorised.y }, };
{ x: 20, y: 20 },
).then(() => { it('should be unable to move a note', () => {
expect(wrapper.vm.movingNoteStartPosition).toBeNull(); createComponent({
expect(findFirstBadge().attributes().style).toBe('left: 1px; top: 80px;'); dimensions: mockDimensions,
notes: [mockNoteNotAuthorised],
});
const badge = findAllNotes().at(0);
return clickAndDragBadge(badge, { ...mockNoteCoordinates }, { x: 20, y: 20 }).then(() => {
// note position should not change after a click-and-drag attempt
expect(findFirstBadge().attributes().style).toContain(
`left: ${mockNoteCoordinates.x}px; top: ${mockNoteCoordinates.y}px;`,
);
});
}); });
}); });
}); });
......
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