@diosmosis opened this Pull Request on December 2nd 2021 Member

Description:

This PR is based off of #18430.

Changes:

  • Add an adapter for sitesstore => siteselector model so the angularjs code can be removed.\
  • Fix issue in SiteSelector.vue, the previous angularjs version only set the current site to the first site in the list if there was only one site in the Matomo overall. Changing that breaks the manage rollup field.

image

Review

@diosmosis commented on December 5th 2021 Member

UI tests should be passing for this PR

@github-actions[bot] commented on December 14th 2021 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@peterhashair commented on December 14th 2021 Contributor

@diosmosis working as expected, test VUE parts, interface, and CSS. I did conflicts resolve for this PR. Can you review it, in case I merge the wrong parts?

@diosmosis commented on December 14th 2021 Member

@peterhashair I'll check eventually, but these PRs are all based on each other like a chain. The next one to review is https://github.com/matomo-org/matomo/pull/18430, then when that gets merged I'll merge into the next PR, etc. This PR, eg, contains changes in other PRs so I wouldn't want to get this merged right away, instead would look at the PR it's based on first.

This is the link I've been using to look at the PRs in the order that they were created: https://github.com/matomo-org/matomo/pulls?q=is%3Apr+%5BVue%5D+sort%3Acreated-asc+is%3Aopen

@diosmosis commented on December 15th 2021 Member
@sgiehl commented on December 15th 2021 Member

@diosmosis this seems to work fine. While testing I discovered anther bug in RollUpReporting that seems to be caused by the vue migration (at least it worked with Matomo 4.5).
When the all sites object is added to an roll up, it can't be removed anymore. There is no javascript error, but the dom keeps updating the site elements, without really removing it...
Feel free to merge this one and create another PR for that if you find a fix.

@diosmosis commented on December 15th 2021 Member

@sgiehl :+1: just found the fix, will create a new PR

This Pull Request was closed on December 15th 2021
Powered by GitHub Issue Mirror