-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Amend user start node handling #16094
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the integration tests can be improved as there is some multi assert smell present.
IEntitySlim[] userStartEntities = userStartNodeIds.Any() | ||
? _entityService.GetAll(umbracoObjectType, userStartNodeIds).ToArray() | ||
: Array.Empty<IEntitySlim>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥰
tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Update.cs
Show resolved
Hide resolved
Bit of a strange message when trying the recycle bin root endpoint
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor remarks about the code.
Tests out as described.
src/Umbraco.Cms.Api.Management/ViewModels/User/Current/CurrentUserResponseModel.cs
Outdated
Show resolved
Hide resolved
# Conflicts: # src/Umbraco.Cms.Api.Management/Factories/UserPresentationFactory.cs
Prerequisites
🚨 Please coordinate the merge of this PR with the client 🚨
The matching client PR is here: umbraco/Umbraco.CMS.Backoffice#1727
Description
A long time ago we hardcoded tree root access, because we did not have user permissions properly wired up. We do now 😄 so now it's time to enforce it.
Another thing missing is the ability to define root access at user level in V14, just as we can in V13. It is currently possible at group level in V14, but not yet at user level.
The last piece of this particular puzzle is making sure we don't request the recycle bins if the current user does not have root access. This will yield a 401, which will trigger a "please log in again" dialog --- and 'round and 'round we go. In other words, the "current user" model needs to be enriched with "has root access" to document and media, respectively. The client can then use this to hide the recycle bin when applicable.
Testing this PR
It's probably best to test this in Swagger. The initial setup of users and groups can be done in the V14 UI, though.