@Starker3 opened this Issue on November 27th 2022 Contributor

This is present in 4.12.3 and the latest 4.13.0-rc2 release.

Steps to reproduce:

  1. Go to Administration > Measurable/Websites > Manage
  2. Click edit on a measurable that has a URL set
  3. Remove all lines from the setting (I.e. the field should be empty)
  4. Click Save
  5. Reload the page

The green prompt at the top of the page says the settings are saved, but when refreshing the page we can see that the URLs that were previously set are still present.

@peterhashair commented on November 28th 2022 Contributor

@Starker3 thanks for reporting this, I can reproduce this locally. Our product team will prioritize this.

@peterhashair commented on November 28th 2022 Contributor

maybe we should add a new test for that one.

    public function testUpdateSiteRemoveAllTheUrls()
    {
        $idSite = API::getInstance()->addSite("site1", ['http://main.url', 'http://main2.url']);

        $settings['WebsiteMeasurable'][0]['name'] = 'urls';
        $settings['WebsiteMeasurable'][0]['value'] = null;
        API::getInstance()->updateSite(
            $idSite,
            'site1',
            $urls = null,
            $ecommerce = null,
            $siteSearch = null,
            $searchKeywordParams = null,
            $searchCategoryParams = null,
            $excludedIps = null,
            $excludedQueryParameters = null,
            $timeZone = null,
            $currency = null,
            $group = null,
            $startDate = null,
            $excludedUserAgents = null,
            $keepUrlFragments = null,
            $type = null,
            $settings,
            $excludeUnknownUrls = true);

        $urls = API::getInstance()->getSiteUrlsFromId($idSite);
        $this->assertEmpty($urls);
    }
@sgiehl commented on November 28th 2022 Member

It should actually be required to set at least one url for a website measurable. So we first need to clarify what the expected behaviour would be. Either it should be possible to remove all urls, or if not, an error message should shown.

@heurteph-ei commented on November 29th 2022

By default, when a new measurable is created, this field is not mandatory.

Powered by GitHub Issue Mirror