Search minutes #635
Search minutes #635
Conversation
| <form action="{% url 'minutes:search' group_id %}" method="post" id="text_search"> | ||
| {% csrf_token %} | ||
| {{search_form}} | ||
| </form> |
There was a problem hiding this comment.
please add some additional space after the search field
| @@ -0,0 +1,62 @@ | |||
| {% extends 'minutes_list.html' %} | |||
There was a problem hiding this comment.
this copies quite a lot of code. please think of a way to reuse more of the existing minutes_list.html file.
_1327/minutes/views.py
Outdated
| search_text = form.cleaned_data['search_phrase'] | ||
|
|
||
| # filter for documents that contain the searched for string | ||
| minutes = MinutesDocument.objects.filter(text__icontains=search_text).prefetch_related('labels').order_by('-date') |
There was a problem hiding this comment.
this fails if the page was requested via GET instead of POST (e.g., because the search url was manually requested again), because search_phrase is then unbound. you should handle this case and display an error message or just redirect to the minutes list.
_1327/minutes/views.py
Outdated
| for m in minutes: | ||
| # find line with the searched for string | ||
| document_content = m.text | ||
| lines = document_content.splitlines() |
There was a problem hiding this comment.
can you please combine the two lines above to just lines = m.text.splitlines()?
_1327/minutes/tests.py
Outdated
| text2 = "in both minutes notO \n one notB \n substring notB notO" | ||
| text3 = "this will never show up notB notO" | ||
|
|
||
| cls.minutes_document1 = mommy.make(MinutesDocument, text = text1, title = "MinutesOne") |
There was a problem hiding this comment.
here and at some other places the code doesn't follow our styleguide. please check the linter errors for more information. the linter is part of the travis checks and you can run it locally via python manage.py lint.
_1327/settings.py
Outdated
| @@ -78,6 +78,7 @@ | |||
| 'django.contrib.sessions', | |||
| 'django.contrib.messages', | |||
| 'django.contrib.staticfiles', | |||
| 'django.contrib.postgres', | |||
There was a problem hiding this comment.
this is likely the reason why the tests fail.
it's needed for using the postgres full text search functionality (see here and here, but that's not used right now in this PR as far as i can see.
by using that you would gain stuff like searching for multiple words which would then not need to be adjacent, and weighting the results by some relevancy metric, so it might be worth checking out?
There was a problem hiding this comment.
yes, i would suggest that for #212
| <td style="width: 55%;"> | ||
| <a href="{{ minute.get_view_url }}">{{ minute.title }}</a> | ||
| {% for line in lines %} | ||
| <br>{{line}} |
There was a problem hiding this comment.
Maybe you could display all lines as <li> elements so that it gets clear which text belongs to the same snippet. Also it would be really nice to highlight (e.g. in bold text) where the search term occurs in the respective line.
i think we're done after that - you could then squash all your commits into one and rebase it on the current master branch.
| <em> | ||
| {% blocktrans %} No documents containing "{{ phrase }}" found.{% endblocktrans %} | ||
| </em> | ||
| {% endblock %} |
There was a problem hiding this comment.
please add a newline at the end of the file
| @@ -8,6 +8,10 @@ | |||
| {% block sidebar %} | |||
| <div class="toc hidden-print"> | |||
| <ul> | |||
| <form class="pb-2" action="{% url 'minutes:search' group_id %}" method="post" id="text_search"> | |||
| {% csrf_token %} | |||
| {{search_form}} | |||
| <tr> | ||
| <td style="width: 15%;"> | ||
| <a href="{{ minute.get_view_url }}">{{ minute.date | date:"d.m.Y" }}</a> | ||
| </td> | ||
|
|
There was a problem hiding this comment.
please remove the empty line
_1327/minutes/views.py
Outdated
|
|
||
|
|
||
| def search(request, groupid): | ||
|
|
There was a problem hiding this comment.
please remove the empty line
_1327/minutes/views.py
Outdated
|
|
||
| if(request.method == 'POST'): | ||
| form = SearchForm(request.POST) | ||
|
|
There was a problem hiding this comment.
please remove the empty line
julkw
force-pushed
the
search_minutes
branch
3 times, most recently
from
8808d4c to
3aa4aba
Compare
julkw
force-pushed
the
search_minutes
branch
from
3aa4aba to
6c9c5e4
Compare
add a search bar in minutes lists with which you ca search through all minutes at once