Skip to content

Commit c8aeaf6

Browse files
tzitzi
committed
Tests and feedbacks from Matthieu
1 parent 5ad4de3 commit c8aeaf6

7 files changed

Lines changed: 115 additions & 33 deletions

File tree

‎plugins/CoreHome/vue/dist/CoreHome.umd.js‎

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎plugins/CoreHome/vue/dist/CoreHome.umd.min.js‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎plugins/CoreHome/vue/src/Comparisons/Comparisons.store.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,21 @@ export default class ComparisonsStore {
216216
);
217217
}
218218

219+
removeSegmentComparisonByDefinition(segmentDefinition: string): void {
220+
if (!this.isComparisonEnabled()) {
221+
throw new Error('Comparison disabled.');
222+
}
223+
let segmentIndex = null;
224+
this.getSegmentComparisons().forEach((segment: SegmentComparison, index: number) => {
225+
if (segment && segment.params && segment.params.segment === segmentDefinition) {
226+
segmentIndex = index;
227+
}
228+
});
229+
if (segmentIndex !== null) {
230+
this.removeSegmentComparison(segmentIndex);
231+
}
232+
}
233+
219234
addSegmentComparison(params: { [name: string]: string }): void {
220235
if (!this.isComparisonEnabled()) {
221236
throw new Error('Comparison disabled.');

‎plugins/SegmentEditor/API.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ protected function checkUserCanEditOrDeleteSegment(array $segment): void
192192
throw new Exception($this->getMessageCannotEditSegmentCreatedBySuperUser());
193193
}
194194

195-
if ((int) $segment['enable_only_idsite'] === 0 && !Piwik::hasUserSuperUserAccess()) {
195+
if ((int) $segment['enable_only_idsite'] === 0) {
196196
throw new Exception(Piwik::translate('SegmentEditor_UpdatingAllSitesSegmentPermittedToSuperUser'));
197197
}
198198
}

‎plugins/SegmentEditor/javascripts/Segmentation.js‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ Segmentation = (function($) {
105105
const comparedSegmentsLength = comparisonService.getSegmentComparisons().length;
106106
$('div.segmentList ul li[data-definition] .compareSegment').each(function() {
107107
const $compareButton = $(this);
108-
const $segment = $compareButton.parent();
109108
const currentState = $compareButton.attr('data-state');
110109
if (currentState === 'active') {
111110
return;
@@ -126,8 +125,7 @@ Segmentation = (function($) {
126125

127126
var segmentationTitle = $(this.content).find(".segmentationTitle");
128127
var title;
129-
if( current != "")
130-
{
128+
if (current != "") {
131129
// this code is mad, and may drive you mad.
132130
// the whole segmentation editor needs to be rewritten in Vue with clean code
133131
var selector = 'div.segmentList ul li[data-definition="'+current+'"]';
@@ -368,14 +366,15 @@ Segmentation = (function($) {
368366
}
369367

370368
var filterSegmentList = function (keyword) {
369+
var search = normalizeSearchString(keyword);
371370
var curTitle;
372371
clearFilterSegmentList();
373372
$(self.target).find(".filterNoResults").remove();
374373

375374
$(self.target).find(".segmentList li").each(function () {
376375
curTitle = $(this).find('.segname').prop('title');
377376
$(this).hide();
378-
if (curTitle.toLowerCase().indexOf(keyword.toLowerCase()) !== -1) {
377+
if (normalizeSearchString(curTitle).indexOf(search) !== -1) {
379378
$(this).show();
380379
}
381380
});
@@ -476,9 +475,14 @@ Segmentation = (function($) {
476475
return false;
477476
}
478477
const comparisonService = window.CoreHome.ComparisonsStoreInstance;
479-
comparisonService.addSegmentComparison({
480-
segment: $button.closest('li').data('definition'),
481-
});
478+
const segmentDefinition = $button.closest('li').data('definition');
479+
if ($button.attr('data-state') === 'active') {
480+
comparisonService.removeSegmentComparisonByDefinition(segmentDefinition);
481+
} else {
482+
comparisonService.addSegmentComparison({
483+
segment: segmentDefinition,
484+
});
485+
}
482486
closeAllOpenLists();
483487
});
484488

@@ -622,18 +626,28 @@ Segmentation = (function($) {
622626
if (self.segmentAccess !== 'write') {
623627
return false;
624628
}
625-
return (segment.login === piwik.userLogin || piwik.hasSuperUserAccess);
629+
if (piwik.hasSuperUserAccess) {
630+
return true;
631+
}
632+
633+
return (segment.login === piwik.userLogin);
626634
}
627635

628636
function getStarredByTitlePart(segment) {
629637
const login = segment.starred_by || '';
630638
if (login === piwik.userLogin) {
631-
return ' (' + self.translations['General_StarredByYou'] + ')'
639+
return ' (' + self.translations['General_StarredByYou'] + ')';
632640
}
633-
return ' (' + self.translations['General_StarredBy'] + ' ' + login + ')'
641+
642+
return ' (' + self.translations['General_StarredBy'] + ' ' + login + ')';
634643
}
635644

636645
function getStarSegmentTitle(segment, canEdit) {
646+
// Anonymous users do not have any action
647+
if (piwik.userLogin === 'anonymous') {
648+
return '';
649+
}
650+
637651
// Site-specific segments
638652
if (segment.enable_only_idsite) {
639653
if (canEdit) {

‎plugins/SegmentEditor/stylesheets/segmentation.less‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,10 @@ div.scrollable {
264264
order: 1;
265265
}
266266

267+
.segmentationContainer .submenu ul li.comparedSegment {
268+
font-weight: bold;
269+
}
270+
267271
.segmentationContainer .submenu ul li:hover {
268272
color: #255792;
269273
background: @color-silver-l95;
@@ -344,14 +348,6 @@ div.scrollable {
344348
opacity: 0.2;
345349
cursor: not-allowed;
346350
}
347-
348-
&[data-state=active] {
349-
border-radius: 50%;
350-
background-color: white;
351-
background-size: 70%;
352-
background-position: center;
353-
filter: invert(1);
354-
}
355351
}
356352

357353
.editSegment {
@@ -392,6 +388,11 @@ div.scrollable {
392388
animation-direction: reverse;
393389
}
394390

391+
/* We do not want filled & transparent stars, design wanted by Matthieu */
392+
.segmentStarred .starSegment[data-state=disabled] {
393+
opacity: 0.5;
394+
}
395+
395396
.segmentStarred .starSegment path {
396397
fill: black;
397398
}

‎plugins/SegmentEditor/tests/UI/SegmentSelectorEditor_spec.js‎

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,26 @@ describe("SegmentSelectorEditorTest", function () {
3333
await (await page.jQuery(prefixSelector + ' .metricListBlock .expandableList .secondLevel li:contains(' + name + ')', { waitFor: true })).click();
3434
}
3535

36+
async function switchToAnonymousUser() {
37+
await testEnvironment.callApi('UsersManager.setUserAccess', {
38+
userLogin: 'anonymous',
39+
access: 'view',
40+
idSites: [1],
41+
});
42+
testEnvironment.testUseMockAuth = 0;
43+
testEnvironment.save();
44+
}
45+
46+
async function switchToConnectedUser() {
47+
testEnvironment.testUseMockAuth = 1;
48+
testEnvironment.save();
49+
await testEnvironment.callApi('UsersManager.setUserAccess', {
50+
userLogin: 'anonymous',
51+
access: 'noaccess',
52+
idSites: [1],
53+
});
54+
}
55+
3656
it("should load correctly", async function() {
3757
await page.goto(url);
3858
expect(await page.screenshotSelector(selectorsToCapture)).to.matchImage('0_initial');
@@ -43,21 +63,39 @@ describe("SegmentSelectorEditorTest", function () {
4363
expect(await page.screenshotSelector(selectorsToCapture)).to.matchImage('1_selector_open');
4464
});
4565

46-
it("should unstar all segments", async function() {
47-
await page.click('.segmentList li:nth-child(2) .starSegment');
48-
await page.click('.segmentList li:nth-child(3) .starSegment');
49-
await page.click('.segmentList li:nth-child(4) .starSegment');
50-
expect(await page.screenshotSelector(selectorsToCapture)).to.matchImage('1_selector_unstarred');
66+
it("should star all segments", async function() {
67+
await page.click('.segmentList li:nth-child(2) .starSegment');
68+
await page.click('.segmentList li:nth-child(3) .starSegment');
69+
await page.click('.segmentList li:nth-child(4) .starSegment');
70+
const firstSegment = await page.$('.segmentList li:nth-child(2)');
71+
expect(firstSegment.className).to.contain('segmentStarred');
72+
expect(firstSegment.find('.starSegment').attr('data-state')).to.equal('');
73+
expect(await page.screenshotSelector(selectorsToCapture)).to.matchImage('1_selector_starred');
5174
});
5275

53-
it("should star last segment", async function() {
54-
await page.click('.segmentList li:last-child .starSegment');
55-
expect(await page.screenshotSelector(selectorsToCapture)).to.matchImage('1_selector_starred');
76+
it("should unstar first segment", async function() {
77+
await page.click('.segmentList li:nth-child(2) .starSegment');
78+
const firstSegment = await page.$('.segmentList li:nth-child(2)');
79+
expect(firstSegment.className).to.not.contain('segmentStarred');
80+
expect(firstSegment.find('.starSegment').attr('data-state')).to.equal('');
81+
expect(await page.screenshotSelector(selectorsToCapture)).to.matchImage('1b_selector_unstarred');
82+
});
83+
84+
it("should have disabled star for anonymous users", async function() {
85+
await switchToAnonymousUser();
86+
await page.goto(url);
87+
await page.click('.segmentationContainer .title');
88+
const firstSegment = await page.$('.segmentList li:nth-child(2)');
89+
expect(firstSegment.className).to.contain('segmentStarred');
90+
expect(firstSegment.find('.starSegment').attr('data-state')).to.equal('');
5691
});
5792

5893
it("should open segment editor when edit link clicked for existing segment", async function() {
94+
await switchToConnectedUser();
95+
await page.goto(url);
96+
await page.click('.segmentationContainer .title');
5997
await page.evaluate(function() {
60-
$('.segmentList .editSegment:first').click()
98+
$('.segmentList .editSegment:first').click();
6199
});
62100
await page.waitForNetworkIdle();
63101
expect(await page.screenshotSelector(selectorsToCapture)).to.matchImage('2_segment_editor_update');
@@ -127,10 +165,10 @@ describe("SegmentSelectorEditorTest", function () {
127165

128166
it("should save a new segment and add it to the segment list when the form is filled out and the save button is clicked", async function() {
129167
for (let i = 0; i < 3; i += 1) {
130-
await page.evaluate(function (i) {
131-
$(`.metricValueBlock input:eq(${i})`).val('value ' + i).change();
132-
}, i);
133-
await page.waitForTimeout(250);
168+
await page.evaluate(function (i) {
169+
$(`.metricValueBlock input:eq(${i})`).val('value ' + i).change();
170+
}, i);
171+
await page.waitForTimeout(250);
134172
}
135173

136174
await page.type('input.edit_segment_name', 'new segment');

0 commit comments

Comments
 (0)
⚡