local-it/mitgliederverwaltung
2
3
Fork 0
Code Issues 51 Pull requests 14 Projects 1 Releases Packages 1 Activity Actions

Implement full-text search for members closes #11 #163

Merged
carla merged 6 commits from feature/11-fulltext-search into main 2025-09-29 14:26:38 +02:00
Conversation 6 Commits 6 Files changed 8 +406
carla commented 2025-09-17 14:57:13 +02:00
Owner
Copy link

Description of the implemented changes

The changes were:

  • Bugfixing
  • New Feature
  • Breaking Change
  • Refactoring
    Implement full-text search

What has been changed?

  • added searchbarcomponent
  • added tsvector for search to members ressource

Definition of Done

Code Quality

  • No new technical depths
  • Linting passed
  • Documentation is added were needed

Accessibility

  • New elements are properly defined with html-tags
  • Colour contrast follows WCAG criteria
  • Aria labels are added when needed
  • Everything is accessible by keyboard
  • Tab-Order is comprehensible
  • All interactive elements have a visible focus

Testing

  • Tests for new code are written
  • All tests pass
  • axe-core dev tools show no critical or major issues

Additional Notes

Futty / Substring search not part fo this PR

## Description of the implemented changes The changes were: - [ ] Bugfixing - [X] New Feature - [ ] Breaking Change - [ ] Refactoring Implement full-text search ## What has been changed? - added searchbarcomponent - added tsvector for search to members ressource ## Definition of Done ### Code Quality - [X] No new technical depths - [X] Linting passed - [X] Documentation is added were needed ### Accessibility - [X] New elements are properly defined with html-tags - [X] Colour contrast follows WCAG criteria - [X] Aria labels are added when needed - [X] Everything is accessible by keyboard - [X] Tab-Order is comprehensible - [ ] All interactive elements have a visible focus ### Testing - [X] Tests for new code are written - [X] All tests pass - [X] axe-core dev tools show no critical or major issues ## Additional Notes Futty / Substring search not part fo this PR
carla added 4 commits 2025-09-17 14:57:14 +02:00
carla added this to the Sprint 6 - 11.09 - 02.10. project 2025-09-17 14:57:20 +02:00
requested reviews from rafael, simon, moritz 2025-09-17 14:57:33 +02:00
rafael requested changes 2025-09-25 14:41:05 +02:00
rafael left a comment
Collaborator
Copy link

Great work! I think there are two things we should address before merging, but overall this is really well done.

Great work! I think there are two things we should address before merging, but overall this is really well done.
lib/mv_web/live/member_live/index.ex Outdated
@ -19,0 +28,4 @@
def handle_info({:search_changed, q}, socket) do
members =
Mv.Membership.Member
|> Ash.Query.filter(expr(fragment("search_vector @@ plainto_tsquery('simple', ?)", ^q)))
rafael commented 2025-09-25 14:39:03 +02:00
Collaborator
Copy link

It seems like this returns an empty list when the search string is empty. Maybe we need a special case for that, so an empty search field shows all members instead?

It seems like this returns an empty list when the search string is empty. Maybe we need a special case for that, so an empty search field shows all members instead?
👍 1
carla marked this conversation as resolved
lib/mv_web/live/member_live/index.ex Outdated
@ -19,0 +31,4 @@
|> Ash.Query.filter(expr(fragment("search_vector @@ plainto_tsquery('simple', ?)", ^q)))
|> Ash.read!()
IO.inspect(members)
rafael commented 2025-09-25 14:39:58 +02:00
Collaborator
Copy link

Let's remove this before merging.

Let's remove this before merging.
👍 1
carla marked this conversation as resolved
priv/repo/migrations/20250912085235_AddSearchVectorToMembers.exs
@ -0,0 +44,4 @@
CREATE TRIGGER update_search_vector
BEFORE INSERT OR UPDATE ON members
FOR EACH ROW
EXECUTE FUNCTION members_search_vector_trigger()
rafael commented 2025-09-25 14:36:47 +02:00
Collaborator
Copy link

This constructs the search vector for all new future members, but can we run it in this migration for all existing members as well?

This constructs the search vector for all new future members, but can we run it in this migration for all existing members as well?
carla commented 2025-09-26 09:22:14 +02:00
Author
Owner
Copy link

But is this a use case?
I would rather put that in an own low priority issue due to priorisation if that's fine?

But is this a use case? I would rather put that in an own low priority issue due to priorisation if that's fine?
rafael commented 2025-09-26 14:56:15 +02:00
Collaborator
Copy link

Yeah, I think if we all just reset our database we should be fine as well. No need to create an issue. I was just confused that I could not find all members in my database :D

Yeah, I think if we all just reset our database we should be fine as well. No need to create an issue. I was just confused that I could not find all members in my database :D
👍 1
carla marked this conversation as resolved
carla added 1 commit 2025-09-26 09:23:01 +02:00
simon commented 2025-09-26 14:31:40 +02:00
Owner
Copy link

I think we also should check for substrings of names.
e.g. I just tried to search for Friedrich, typed "Fried" and wondered, why I got no results

I think we also should check for substrings of names. e.g. I just tried to search for Friedrich, typed "Fried" and wondered, why I got no results
rafael commented 2025-09-26 14:57:44 +02:00
Collaborator
Copy link

@simon wrote in #163 (comment):

I think we also should check for substrings of names. e.g. I just tried to search for Friedrich, typed "Fried" and wondered, why I got no results

This is intentional as the current implementation only uses full-text search which only matches whole words, and is optimized to search large volumes of text. I think the idea was to open an issue and a follow-up MR implementing fuzzy and substring search.

@simon wrote in https://git.local-it.org/local-it/mitgliederverwaltung/pulls/163#issuecomment-13467: > I think we also should check for substrings of names. e.g. I just tried to search for Friedrich, typed "Fried" and wondered, why I got no results This is intentional as the current implementation only uses full-text search which only matches whole words, and is optimized to search large volumes of text. I think the idea was to open an issue and a follow-up MR implementing fuzzy and substring search.
simon commented 2025-09-26 15:08:14 +02:00
Owner
Copy link

ah yes, I see! ty

ah yes, I see! ty
simon requested changes 2025-09-26 15:08:47 +02:00
Dismissed
lib/mv_web/live/components/search_bar_component.ex Outdated
@ -0,0 +8,4 @@
use MvWeb, :live_component
@impl true
def update(assigns, socket) do
simon commented 2025-09-26 14:28:01 +02:00
Owner
Copy link
mix test
Compiling 5 files (.ex)
    warning: variable "assigns" is unused (if the variable is not meant to be used, prefix it with an underscore)
    │
 11 │   def update(assigns, socket) do
    │              ~~~~~~~
    │
    └─ lib/mv_web/live/components/search_bar_component.ex:11:14: MvWeb.Components.SearchBarComponent.update/2
``` mix test Compiling 5 files (.ex) warning: variable "assigns" is unused (if the variable is not meant to be used, prefix it with an underscore) │ 11 │ def update(assigns, socket) do │ ~~~~~~~ │ └─ lib/mv_web/live/components/search_bar_component.ex:11:14: MvWeb.Components.SearchBarComponent.update/2 ```
simon marked this conversation as resolved
test/mv_web/components/search_bar_component_test.exs Outdated
@ -0,0 +20,4 @@
|> element("form[role=search]")
|> render_change(%{"query" => "Friedrich"})
assert html =~ "Friedrich"
simon commented 2025-09-26 14:34:39 +02:00
Owner
Copy link

Doesn't this just assert that "Friedrich" should occur anywhere in the html?
If I'm not wrong, this would also apply to the exact same search string "Friedrich" which is rendered in html, too, and therefore it doesn't matter for the test which result is presented?

Doesn't this just assert that "Friedrich" should occur anywhere in the html? If I'm not wrong, this would also apply to the exact same search string "Friedrich" which is rendered in html, too, and therefore it doesn't matter for the test which result is presented?
carla commented 2025-09-29 09:01:23 +02:00
Author
Owner
Copy link

Yes, you are right :) Changed that to checking if members are not present instead

Yes, you are right :) Changed that to checking if members are not present instead
simon marked this conversation as resolved
test/mv_web/member_live/index_test.exs
@ -76,0 +80,4 @@
send(view.pid, {:search_changed, "Friedrich"})
# State aus dem LiveView-Prozess holen
simon commented 2025-09-26 14:35:51 +02:00
Owner
Copy link

a bit picky, but this comment should be in english :)

a bit picky, but this comment should be in english :)
👍 1
carla marked this conversation as resolved
carla added 1 commit 2025-09-29 09:00:43 +02:00
test: update test for search bar component
Some checks failed
continuous-integration/drone/push Build is failing
Details
2095d9b0da
simon approved these changes 2025-09-29 11:33:42 +02:00
carla merged commit 80b79d80cd into main 2025-09-29 14:26:38 +02:00
carla deleted branch feature/11-fulltext-search 2025-09-29 14:26:39 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
moritz
rafael
simon
Labels
Clear labels   
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
high priority

   
invalid

Something is wrong

  
L

5 h

  
low priority

   
M

3 h

  
medium priority

   
needs refinement

   
question

More information is needed

  
S

1 h

  
UX research

   
wontfix

This won't be fixed

No labels
bug
duplicate
enhancement
help wanted
high priority
invalid
L
low priority
M
medium priority
needs refinement
question
S
UX research
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Sprint 6 - 11.09 - 02.10.
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: local-it/mitgliederverwaltung#163
No description provided.
Delete branch "feature/11-fulltext-search"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?