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

feat: integration of unified search #2479

Merged
merged 2 commits into from
Jan 7, 2025
Merged

Conversation

Chartman123
Copy link
Collaborator

@Chartman123 Chartman123 commented Jan 3, 2025

This closes #2392 by implementing a SearchProvider that let's you search the titles and descriptions of the forms that are available to you.

@Chartman123 Chartman123 self-assigned this Jan 3, 2025
@Chartman123 Chartman123 added enhancement New feature or request php PHP related ticket integration Compatibility with other apps/systems 2. developing Work in progress labels Jan 3, 2025
@Chartman123 Chartman123 added this to the 5.0 milestone Jan 3, 2025
Copy link

codecov bot commented Jan 3, 2025

Codecov Report

Attention: Patch coverage is 13.33333% with 65 lines in your changes missing coverage. Please review.

Project coverage is 43.53%. Comparing base (c2c3c31) to head (07aecc9).
Report is 8 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2479      +/-   ##
============================================
- Coverage     44.27%   43.53%   -0.75%     
- Complexity      859      870      +11     
============================================
  Files            73       75       +2     
  Lines          3268     3331      +63     
============================================
+ Hits           1447     1450       +3     
- Misses         1821     1881      +60     

@Chartman123 Chartman123 force-pushed the enh/search-integration branch 14 times, most recently from cd42a29 to 470810c Compare January 4, 2025 21:53
@Chartman123 Chartman123 requested review from susnux and hamza221 January 4, 2025 22:04
@Chartman123
Copy link
Collaborator Author

@hamza221 It's basically working, but apparently it doesn't return all matching forms. Perhaps you can find some time and have a look

@Chartman123 Chartman123 changed the title WIP: enh: integration of universal search WIP: enh: integration of unified search Jan 4, 2025
@Chartman123 Chartman123 force-pushed the enh/search-integration branch from 470810c to bddcdea Compare January 5, 2025 12:54
@Chartman123 Chartman123 changed the title WIP: enh: integration of unified search enh: integration of unified search Jan 5, 2025
@Chartman123 Chartman123 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 5, 2025
@Chartman123 Chartman123 marked this pull request as ready for review January 5, 2025 12:57
@Chartman123 Chartman123 force-pushed the enh/search-integration branch 2 times, most recently from d935091 to b5ec03d Compare January 5, 2025 12:58
@Chartman123
Copy link
Collaborator Author

@hamza221 I found the culprit: I used a simple like-operation for the DB query instead of the case insensitive iLike :)

@Chartman123 Chartman123 force-pushed the enh/search-integration branch from b5ec03d to 68436b8 Compare January 5, 2025 13:13
Signed-off-by: Christian Hartmann <[email protected]>
@Chartman123 Chartman123 force-pushed the enh/search-integration branch from 68436b8 to f69a116 Compare January 5, 2025 13:36
@hamza221
Copy link
Contributor

hamza221 commented Jan 6, 2025

I spent an hour on this and I'm ,not sure what's happening. I'm sure I'm missing something obvious
The shared search is not working for me (when I remove the lines you added it returns all shared as expected)
your code looks correct but checking the db logs I found this
my search term is %adham%

SELECT * FROM oc_forms_v2_forms WHERE ( id IN ( SELECT DISTINCT forms.id FROM oc_forms_v2_forms forms LEFT JOIN oc_forms_v2_shares shares ON forms.id = shares.form_id WHERE ( ( (shares.share_type = '0' AND shares.share_with = 'admin') OR (shares.share_type = '1' AND shares.share_with IN ('admin')) ) OR (access_enum IN (2)) ) AND forms.owner_id <> 'admin' ) ) AND ( title COLLATE utf8mb4_general_ci LIKE '0' OR description COLLATE utf8mb4_general_ci LIKE 'admin' ) ORDER BY last_updated DESC, created DESC;

See how the like value is wrong
Compared to the owned forms search 👇

SELECT * FROM oc_forms_v2_forms WHERE (owner_id = 'admin') AND (title COLLATE utf8mb4_general_ci LIKE '%adham%' OR description COLLATE utf8mb4_general_ci LIKE '%adham%') ORDER BY last_updated DESC, created DESC;

@Chartman123
Copy link
Collaborator Author

Ok, I found the error:

$qbForms->setParameters($qbShares->getParameters(), $qbShares->getParameterTypes());

We set the parameters from qbShares to be the parameters of qbForms. So they get mixed up when I set the new parameters for the search...

Copy link
Contributor

@hamza221 hamza221 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works ✅

lib/Db/FormMapper.php Outdated Show resolved Hide resolved
lib/Db/FormMapper.php Outdated Show resolved Hide resolved
@Chartman123 Chartman123 force-pushed the enh/search-integration branch 2 times, most recently from 443fd92 to 286cb73 Compare January 7, 2025 10:39
@Chartman123 Chartman123 force-pushed the enh/search-integration branch from 286cb73 to 07aecc9 Compare January 7, 2025 10:40
@Chartman123 Chartman123 changed the title enh: integration of unified search feat: integration of unified search Jan 7, 2025
@Chartman123 Chartman123 merged commit f2e9736 into main Jan 7, 2025
51 of 53 checks passed
@Chartman123 Chartman123 deleted the enh/search-integration branch January 7, 2025 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request integration Compatibility with other apps/systems php PHP related ticket
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connect Forms to Unified Search by Implementing Search Provider
2 participants