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.

Sign up for GitHub

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

Jump to bottom

IBX-8726: Added IsBookmarked criterion #417

Merged
merged 35 commits into from
Sep 4, 2024
Merged

IBX-8726: Added IsBookmarked criterion #417

merged 35 commits into from
Sep 4, 2024

Conversation

ciastektk
Copy link
Contributor

@ciastektk ciastektk commented Aug 16, 2024 •
edited
Loading

🎫 Issue IBX-8726

Description:

This PR adds IsBookmarked criterion which allow to filter by bookmarked locations.
Main use case is DAM, where we need to filter bookmarked locations by content type.

Response will be automatically updated after added new or deleted Bookmark. The BookmarkEventSubscriber is responsible for triggering serach index updates.

Persistence API

New method loadUserIdsByLocation have been added to Bookmark Handler, which allows to load all id of users which already bookmarked given location.

Supported search engines

LSE, Solr, and Elasticsearch supports IsBookmarked criterion.

Examples

Criterion has argument $value type of bool with default value true. Criterion works with current user reference, and id taken from it will be used in query.

new IsBookmarked();
new IsBookmarked(false);

For QA:

Documentation:

@mnocon mnocon added the Doc needed The changes require some documentation label Aug 19, 2024
@ciastektk ciastektk force-pushed the ibx-8726-added-is-bookmarked-criterion branch from 28fd67f to dfe7121 Compare August 19, 2024 06:44
@ciastektk ciastektk mentioned this pull request Aug 20, 2024
Merged
@ciastektk ciastektk force-pushed the ibx-8726-added-is-bookmarked-criterion branch 5 times, most recently from d02f69d to 466737f Compare August 20, 2024 12:02
@ciastektk ciastektk marked this pull request as ready for review August 20, 2024 12:19
@ciastektk ciastektk requested a review from a team August 20, 2024 13:17
konradoboza
konradoboza reviewed Aug 21, 2024
View reviewed changes
src/contracts/Repository/Values/Content/Query/Criterion/Location/IsBookmarked.php Show resolved Hide resolved
src/contracts/Repository/Values/Content/Query/Criterion/Value/IsBookmarkedValue.php Outdated Show resolved Hide resolved
src/lib/Search/Common/EventSubscriber/BookmarkEventSubscriber.php Show resolved Hide resolved
src/lib/Search/Common/EventSubscriber/BookmarkEventSubscriber.php Outdated Show resolved Hide resolved
src/lib/Search/Common/EventSubscriber/BookmarkEventSubscriber.php Outdated Show resolved Hide resolved
src/lib/Search/Legacy/Content/Location/Gateway/CriterionHandler/Location/IsBookmarked.php Show resolved Hide resolved
tests/integration/Core/Persistence/Legacy/BookmarkHandlerTest.php Show resolved Hide resolved
tests/integration/Core/Repository/SearchServiceBookmarkTest.php Show resolved Hide resolved
@konradoboza konradoboza requested a review from a team August 21, 2024 06:15
@ciastektk ciastektk requested a review from konradoboza August 21, 2024 07:39
@ciastektk ciastektk force-pushed the ibx-8726-added-is-bookmarked-criterion branch from 953c58d to 3698368 Compare August 21, 2024 07:47
@ciastektk ciastektk requested review from konradoboza and a team August 21, 2024 07:58
@ciastektk ciastektk requested review from konradoboza and a team August 21, 2024 09:51
@konradoboza konradoboza requested a review from alongosz August 21, 2024 09:55
@konradoboza konradoboza requested a review from a team August 21, 2024 10:51
alongosz
alongosz reviewed Aug 21, 2024
View reviewed changes
src/contracts/Repository/Values/Content/Query/Criterion/Location/IsBookmarked.php Outdated Show resolved Hide resolved
src/lib/Resources/settings/search_engines/common.yml Outdated Show resolved Hide resolved
src/lib/Search/Common/EventSubscriber/BookmarkEventSubscriber.php Outdated Show resolved Hide resolved
src/lib/Search/Common/EventSubscriber/BookmarkEventSubscriber.php Outdated Show resolved Hide resolved
src/lib/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php Outdated Show resolved Hide resolved
src/lib/Search/Legacy/Content/Location/Gateway/CriterionHandler/Location/IsBookmarked.php Outdated Show resolved Hide resolved
src/lib/Search/Legacy/Content/Location/Gateway/CriterionHandler/Location/IsBookmarked.php Outdated Show resolved Hide resolved
ciastektk and others added 22 commits September 4, 2024 14:39
@ciastektk
Added missing BookmarkEventSubscriber service definition in integrati…
c4fff34 
…on test setup
@ciastektk @konradoboza
Update src/contracts/Repository/Values/Content/Query/Criterion/Locati…
0e148d1 
…on/IsBookmarked.php

Co-authored-by: Konrad Oboza <konrad.oboza@ibexa.co>
@ciastektk @konradoboza
Update src/contracts/Repository/Values/Content/Query/Criterion/Value/…
e63b43e 
…IsBookmarkedValue.php

Co-authored-by: Konrad Oboza <konrad.oboza@ibexa.co>
@ciastektk @konradoboza
Update src/lib/Search/Common/EventSubscriber/BookmarkEventSubscriber.php
14df7c1 
Co-authored-by: Konrad Oboza <konrad.oboza@ibexa.co>
@ciastektk @konradoboza
Update src/lib/Search/Legacy/Content/Location/Gateway/CriterionHandle…
bd292f1 
…r/Location/IsBookmarked.php

Co-authored-by: Konrad Oboza <konrad.oboza@ibexa.co>
@ciastektk @konradoboza
Update tests/integration/Core/Persistence/Legacy/BookmarkHandlerTest.php
c93a569 
Co-authored-by: Konrad Oboza <konrad.oboza@ibexa.co>
@ciastektk @konradoboza
Update tests/integration/Core/Repository/SearchServiceBookmarkTest.php
7fe733c 
Co-authored-by: Konrad Oboza <konrad.oboza@ibexa.co>
@ciastektk
Used IncrementalIndexer instead of Indexer in BookmarkEventSubscriber
cd3c9f3
@ciastektk
Improved query in IsBookmarked criterion handler to be more efficient
642318e
@ciastektk
Used ParameterType::INTEGER instead Types::INTEGER in DoctrineDatabas…
e35042e 
…e::loadUserIdsByLocation
@ciastektk
Marked BookmarkEventSubscriber as internal
270701c
@ciastektk
[PHPStan] Regenerated baseline
a018380
@ciastektk
Reworked criterion
008cfe5
@ciastektk
Reworked BookmarkEventSubscriber
72756a7
@ciastektk
Dropped ibexa.spi.search.indexer service defintion from integration t…
d35c9b9 
…ests setup
@ciastektk
Used fetchFirstColumn instead of fetchAllAssociative in loadUserIdsBy…
5998842 
…Location
@ciastektk
Added check if criterion value is type of array
a2759a9
@ciastektk @Steveb-p
Update src/lib/Search/Legacy/Content/Location/Gateway/CriterionHandle…
3662f8c 
…r/Location/IsBookmarked.php

Co-authored-by: Paweł Niedzielski <pawel.niedzielski@ibexa.co>
@ciastektk
Added missing comma in IsBookmarked handler
425eff4
@ciastektk
Added removal test coverage for BookmarkHandlerTest::testLoadUserIdsB…
d7a712d 
…yLocation
@ciastektk
Added additional content remote ids check in SearchServiceBookmarkTest
949ab72
@ciastektk
Added comment about working with current user to IsBookmarked criterion
1efafe0
@ciastektk ciastektk force-pushed the ibx-8726-added-is-bookmarked-criterion branch from fe32ae8 to 1efafe0 Compare September 4, 2024 12:39
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Sep 4, 2024

Quality Gate Passed Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@alongosz alongosz merged commit 0ba8cd1 into 4.6 Sep 4, 2024
19 of 22 checks passed
@alongosz alongosz deleted the ibx-8726-added-is-bookmarked-criterion branch September 4, 2024 13:31
@julitafalcondusza julitafalcondusza mentioned this pull request Nov 18, 2024
Merged
4 tasks
@julitafalcondusza julitafalcondusza removed the Doc needed The changes require some documentation label Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Steveb-p Steveb-p Steveb-p approved these changes

@bogusez bogusez bogusez approved these changes

@konradoboza konradoboza konradoboza approved these changes

@ViniTou ViniTou ViniTou approved these changes

@alongosz alongosz alongosz approved these changes

Assignees

@mikadamczyk mikadamczyk

@alongosz alongosz

@tischsoic tischsoic

@bogusez bogusez

Labels
Improvement QA approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@ciastektk @Steveb-p @alongosz @ViniTou @konradoboza @bogusez @mikadamczyk @tischsoic @mnocon @julitafalcondusza