@flamisz opened this Pull Request on March 23rd 2021 Contributor

Description:

fixes #16790

Review

  • [x] Functional review done
  • [x] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • [x] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [x] Security review done see checklist
  • [x] Code review done
  • [x] Tests were added if useful/possible
  • [x] Reviewed for breaking changes
  • [x] Developer changelog updated if needed
  • [x] Documentation added if needed
  • [x] Existing documentation updated if needed
@flamisz commented on March 23rd 2021 Contributor

The original issue says:

For now only apply when "no segment" is select (report shows all visits)

This solution works with segments as well but easy to disable if we don't want to support yet. Is it ok if it works with segments @tsteur?

Update:

the Live.getLastVisitsDetails does not work properly with segments, so I disable it for now

@flamisz commented on March 30th 2021 Contributor

I tried to add that test, but couldn't make it work. I tried this inside the _spec.js file: (I tried with form params as well instead of params in the URL)

       const trackerUrl = config.piwikUrl + "matomo.php?action_name=&idsite=1&url=http://piwik.net/test/url";

        await request({
            method: 'POST',
            uri: trackerUrl,
        });

        await page.goto(pageUrl);
        expect(await page.screenshot({ fullPage: true })).to.matchImage('show_notification_when_only_raw_data_exists');

But there is no new log in the db...
Any thoughts on this @sgiehl?
I found one example here:
https://github.com/matomo-org/matomo/blob/3ab0d4ee6af2eccfd3e1e7e0d192a0032f42edcb/tests/UI/specs/OneClickUpdate_spec.js#L85-L97

EDIT: I talked to @tsteur about this, he showed me a couple of things in the tests, so I try to make it work.

@flamisz commented on March 30th 2021 Contributor

@tsteur and @sgiehl I figured what was the problem and what is the solution. Even when I called the tracker it didn't work, because when we load the page that triggers the archiving, so I need to add this to test, and I don't need any extra tracker call this way.:

       testEnvironment.configOverride.General = {
            browser_archiving_disabled_enforce: '1',
            enable_browser_archiving_triggering: '0',
        };
        testEnvironment.optionsOverride = {
            enableBrowserTriggerArchiving: '0',
        };

Only one thing that came up now. We show this notification only if today's date is in the date range. I couldn't find a way to mock the today in the UI tests. Are we happy with it if the test uses today every time? It means we can have some false negatives, when the tests run around midnight.

This Pull Request was closed on April 6th 2021
Powered by GitHub Issue Mirror