@diosmosis opened this Pull Request on April 14th 2021 Member

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/ no value key. 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

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

@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?

@diosmosis commented on April 14th 2021 Member

@tsteur this is to fix the root cause of L3-65

@diosmosis commented on April 15th 2021 Member

@tsteur updated. Warning looks like:

image

@tsteur commented on April 15th 2021 Member

👍 be great to have an issue to fix the addSite/updateSite API regarding requiring URLs as part of Matomo 5

@diosmosis commented on April 15th 2021 Member
This Pull Request was closed on April 15th 2021
Powered by GitHub Issue Mirror