-
Notifications
You must be signed in to change notification settings - Fork 59
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
Publish container not working for user with write access & getting no success notification when creating a new version/ publishing a container #990
Comments
Thank you for taking the time to create this issue @tsteur . Can you please confirm that the changes were in fact not published. It appears that the changes were actually published, but the Matomo Configuration variables are showing as different (note the date is almost 2 years ago when there have been many releases since). I'm guessing that a migration didn't run correctly and it's showing as different because there's a new field which wasn't added to the previous version. When I tested in my local instance, I didn't see the success notification, but the version was published as expected and the changes list was empty afterward. We definitely need to look into why the success notification doesn't display. We might also want to look into an improvement related to the comparison issue as this keeps popping up when migrations are skipped or don't run correctly. |
@snake14 the publishing worked nicely with super user access. It was publishing with the write user but to no environment. So when I tried to publish to live again a 2nd time, it would show the same changes again. Published, then version without environment was released: Then I click on publish again a 2nd time and same changes show up again It seems the version entry is created, but not the release entry. Looking at the request information there is an error: Stack trace below. Looking at this stack trace and then the data there is Matomo tracking into site 3 configured, but this user does not have access to site 3 while the super user does have access to site 3. So publishing fails for this user. The container is configured in idSite 1. We use 1 container to track the data into 2 sites.
|
Thank you for the additional information @tsteur . It looks like the site permission is why we haven't run into this before. We'll have to look into the best way to fix publishing a container which tracks to a site that the publisher doesn't have access to. |
Just checking if I understand this correctly. The problem is:
If that's accurate and exhaustive, my question is: why should a user be able to publish to a website they don't have access to? I'm not really understanding this UX logic, I would imagine they shouldn't see this container in the first place. |
To clarify, the container is associated with a site the user can access. In Matomo, it's common to track data across multiple sites using a single container rather than setting up and loading separate containers for each site. This configuration means that while the container is set up in a site the user has access to, the user editing the container may not have permissions for all sites referenced by the container. Because the user is only configuring a Matomo variable (specifically, setting an idSite) and cannot gain unauthorized access through this action, a permission check in this context is unnecessary. |
Thanks @tsteur . That makes sense. I believe that the permission error is being thrown when the code is checking if the ID belongs to a valid site, but the Site class also checks permission at the same time. Maybe we can use a try/catch which only cares about whether the site exists and ignores potential permission errors in this specific case. |
This works as a super user. But for a user with write access.
Expected: Get a notification on success, close the modal, assign the correct environment, actually publish the container, (optionally reload the list of published versions to show the newly published version if that page was open previously
The text was updated successfully, but these errors were encountered: