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

Fix super-admin website IDs check #14417

Closed
wants to merge 3 commits into from

Conversation

Valair
Copy link

@Valair Valair commented May 6, 2019

Force website IDs existence check even for super-admin.
Resolve #8697

@tsteur
Copy link
Member

tsteur commented May 6, 2019

@Valair thanks for this 👍
As I thought a LOT of tests are now failing see eg https://travis-ci.org/matomo-org/matomo/jobs/528746645
https://travis-ci.org/matomo-org/matomo/jobs/528746646 and https://travis-ci.org/matomo-org/matomo/jobs/528746647

In our test fixtures a lot of tests will need to be corrected.

I see it complains eg that in an AnnotationTest the user doesn't have view access to idSite 2. Looking at https://github.com/matomo-org/matomo/blob/3.10.0-b1/tests/PHPUnit/Fixtures/TwoSitesWithAnnotations.php#L27 the two websites seem actually created. So not sure if the tests needs adjusting or if there are more things to change in the Access class itself.

Fixing all those tests can take quite a long time and possibly also some debugging etc. Please let us know if you're keen looking into it or not.

@Valair
Copy link
Author

Valair commented May 7, 2019

Ok, I will try to setup e dev/test environment to look into those failed tests.

@mattab
Copy link
Member

mattab commented Jun 10, 2019

Hi @Valair Thank you for the PR! Do you think you will be able to setup the test environment to adjust tests? Be awesome if you can. We'd like to merge the PR 👍

@mattab mattab added this to the 3.11.0 milestone Jun 10, 2019
@mattab mattab added the Needs Review PRs that need a code review label Jun 10, 2019
@Valair
Copy link
Author

Valair commented Jun 11, 2019

Hi @mattab, I've setup a test environment and I'm slowly working on fixing tests

@tsteur tsteur modified the milestones: 3.11.0, 3.12.0 Jul 16, 2019
@tsteur
Copy link
Member

tsteur commented Nov 25, 2019

@Valair closing this PR for now. Let me know if you're planning to work on it again and I can reopen. Cheers

@tsteur tsteur closed this Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

do not allow to set access to non existing websites
4 participants