Workaround error in Overlay when site has no URLs #17457
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sgiehl
requested changes
Apr 14, 2021
plugins/SitesManager/API.php
Outdated
| @@ -598,7 +598,7 @@ public function addSite($siteName, | |||
| } | |||
|
|
|||
| $coreProperties = array(); | |||
| $coreProperties = $this->setSettingValue('urls', $urls, $coreProperties, $settingValues); | |||
| $coreProperties = $this->setSettingValue('urls', $urls, $coreProperties, $settingValues, $isRequired = true); | |||
There was a problem hiding this comment.
This makes it impossible to create any new roll ups, as for roll ups you can't define any urls.
There was a problem hiding this comment.
Good point. I guess I can make it required if type == 'website'. (cc @tsteur)
|
@diosmosis this would be breaking API (even though in the code it should have already worked but from a user perspective things are potentially still breaking as it probably wasn't working for a while) and not sure if want to require URLs now or maybe wait to Matomo 5. Is there an issue for this PR? |
|
@tsteur this is to fix the root cause of L3-65 |
|
@tsteur updated. Warning looks like:
|
|
👍 be great to have an issue to fix the addSite/updateSite API regarding requiring URLs as part of Matomo 5 |
diosmosis
changed the title
|
Created #17459 |
diosmosis
added a commit
that referenced
this pull request
Apr 18, 2021
* get segment hash * convert tab indentations to spaces * Create 4.3.0-b2.php * update tests and update file * bump version * Update 4.3.0-b3.php * Update ApiTest.php * fixing urlencode bugs * cache segment hashes * update segment caching * update segment caching * add segment cache test * add testdox to phpunit.xml * revert phunit.xml * test investigation * Update phpunit.xml.dist * update blobreportlimitingt test * Revert "update blobreportlimitingt test" This reverts commit 90fe735. * Update phpunit.xml.dist * Update BlobReportLimitingTest.php * Update BlobReportLimitingTest.php * Update SystemTestCase.php * Update BlobReportLimitingTest.php * Update SystemTestCase.php * Update phpunit.xml.dist * modify mem limit for travis * Update .travis.yml * revert travis.yml * Update SystemTestCase.php * try test without cache * test witch cache and gc_disabled * Revert "test witch cache and gc_disabled" This reverts commit 7e1d370. * test witch cache and gc_disabled * use other model method * refactor test * Workaround error in Overlay when site has no URLs (#17457) * Set setting value even if set to NULL so it will still be validated. * Make sure when creating a site that the urls options is set. * workaround in Overlay for instances that have an invalid site URL set for some reason * Add integration tests for changes to SitesManager API. * revert non-overlay changes * Add warning if site has no URLs when viewing Overlay. * add more tests for segment caches * get segment hash * convert tab indentations to spaces * Create 4.3.0-b2.php * update tests and update file * bump version * Update 4.3.0-b3.php * Update ApiTest.php * fixing urlencode bugs * cache segment hashes * update segment caching * update segment caching * add segment cache test * add testdox to phpunit.xml * revert phunit.xml * test investigation * Update phpunit.xml.dist * update blobreportlimitingt test * Revert "update blobreportlimitingt test" This reverts commit 90fe735. * Update phpunit.xml.dist * Update BlobReportLimitingTest.php * Update BlobReportLimitingTest.php * Update SystemTestCase.php * Update BlobReportLimitingTest.php * Update SystemTestCase.php * Update phpunit.xml.dist * modify mem limit for travis * Update .travis.yml * revert travis.yml * Update SystemTestCase.php * try test without cache * test witch cache and gc_disabled * Revert "test witch cache and gc_disabled" This reverts commit 7e1d370. * test witch cache and gc_disabled * use other model method * refactor test * add more tests for segment caches * revert phpunit.xml * revert phpunit.xml * add more tests Co-authored-by: dizzy <diosmosis@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
When creating a site it is possible to not specify URLs. In this case, we send a plugin setting entry in the HTTP request as
['name' => 'urls']w/ novaluekey. Currently this is ignored by Matomo, and the validation that happens does not occur. Which allows sites to have no URLs.If this happens and Overlay is loaded, the JS will error when trying to find the host of the site URLs.
This is fixed in this PR w/ tests. The issue in Overlay is also worked around in case there are users who created sites w/ no URLs.
Review