Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to remove all URLs for Measurables #20057

Open
Starker3 opened this issue Nov 27, 2022 · 5 comments
Open

Unable to remove all URLs for Measurables #20057

Starker3 opened this issue Nov 27, 2022 · 5 comments
Labels
Bug For errors / faults / flaws / inconsistencies etc.

Comments

@Starker3
Copy link
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.

@Starker3 Starker3 added Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. To Triage An issue awaiting triage by a Matomo core team member labels Nov 27, 2022
@peterhashair
Copy link
Contributor

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

@peterhashair peterhashair added Regression Indicates a feature used to work in a certain way but it no longer does even though it should. Needs priority decision This issue may need to be added to the current milestone by Product Manager and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. To Triage An issue awaiting triage by a Matomo core team member labels Nov 28, 2022
@peterhashair
Copy link
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
Copy link
Member

sgiehl commented Nov 28, 2022

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
Copy link

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

@michalkleiner michalkleiner removed Regression Indicates a feature used to work in a certain way but it no longer does even though it should. Needs priority decision This issue may need to be added to the current milestone by Product Manager labels Aug 28, 2023
@Stan-vw
Copy link
Contributor

Stan-vw commented Aug 28, 2023

I understand that the ability to create a website without a URL was introduced deliberately as a feature, so we're removing the regression label. However, it's not in line with the current design decision where you can't remove a URL after you've saved one before.

Since we allow people to track items that don't have a URL (such as an app), I suggest we adjust/fix the ability to save the deletion of the URL. I.e. if you previously had a URL saved, you can now delete and save it and it will successfully save without a URL.

@michalkleiner michalkleiner added the Bug For errors / faults / flaws / inconsistencies etc. label Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

No branches or pull requests

7 participants