IBX-6773: Fixed loading Bookmarks for non-accessible content items - #476
IBX-6773: Fixed loading Bookmarks for non-accessible content items#476vidarl wants to merge 15 commits into
Conversation
bdac332 to
9164daa
Compare
|
9164daa to
ec96060
Compare
konradoboza
left a comment
There was a problem hiding this comment.
Did you make sure none of the deprecated classes are still in use?
|
@konradoboza : FYI : Had to add related PR : ibexa/admin-ui#1835 |
alongosz
left a comment
There was a problem hiding this comment.
It looks like it's almost done, great work @vidarl 💪
I have one general remark to test coverage - it's extensive, but I don't see an integration test case that reproduces the scenario which created this bug report - remove permissions for an item user previously has bookmarked.
Other remarks:
fb6c2bd to
4ea28c1
Compare
@alongosz : Added in 4ea28c1: Added regression tests for bookmarks of inaccessible and removed items PR should not be ready again for your sharp eyes |
8f23c67 to
7ce51b5
Compare
Co-authored-by: Paweł Niedzielski <pawel.niedzielski@ibexa.co> Co-authored-by: Andrew Longosz <alongosz@users.noreply.github.com>
…tFilteringTest and LocationFilteringTest
…ck in IsBookmarkedQueryBuilder
…y in IsBookmarked criterion
… in IdSortClauseQueryBuilder
…itoryException, log at error level
7ce51b5 to
daec766
Compare
|



Warning
Related PRs:
loadBookmarkscall with Repository filtering requirements admin-ui#1835Description:
If user bookmark some location which he later looses access too, then the bookmark list in admin-ui fails with an exception.
Simply fixing
BookmarkService::loadBookmarks()would be easy. The problem is to implementcountUserBookmarks()in persistence layer and having it taking into account user permissions so that BC would be kept.Talked with Adam on how to solve this without breaking BC and he suggested implementing it using filtering
The Bookmark filter will only work with location filtering (LocationService::find()), not with content (ContentService::find())Code was initially based on ezsystems/ezplatform-kernel#408 which was not approved and merge in time before 3.3 went EO, but has been heavily modified since.
For QA:
Read ticket for info on how to reproduce
Documentation:
The IsBookmarked criterion existed before this PR, but it now also supports filtering.