From 83ccbf699bead323ebc5b944b7adf78a1a6eab79 Mon Sep 17 00:00:00 2001 From: carofun Date: Mon, 14 Jun 2021 16:53:29 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F(back)=20filter=20search?= =?UTF-8?q?=20on=20current=20lti=5Fcontext?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user can be part of multiple forums attached to different classes. We want the search to keep is current scope and only search in forums that are related to the course user is actually logged in. Currently, a user can search in all the forums he has access to and it can be a bit confusing. Clicking on a result that lead to another course, connects the user in a different forum from is original search. Results might not be even relevant as context is different than expected. To be more relevant and not to loose our users, we filter results to current LTIContext and only show results from forum of the current class. --- CHANGELOG.md | 1 + .../machina_extensions/forum_search/forms.py | 25 +++- .../machina_extensions/forum_search/views.py | 25 ++++ tests/ashley/test_forum_search.py | 138 +++++++++++++++++- 4 files changed, 187 insertions(+), 2 deletions(-) create mode 100644 src/ashley/machina_extensions/forum_search/views.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c5c17ac..cad09364 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Versioning](https://semver.org/spec/v2.0.0.html). - Clean built frontend files before each build - Upgrade node to version 14, the current LTS - Fix import of the user model in the factory + - Filter search results to current LTIContext ### Added diff --git a/src/ashley/machina_extensions/forum_search/forms.py b/src/ashley/machina_extensions/forum_search/forms.py index 2e21ebf9..8e30bcfb 100644 --- a/src/ashley/machina_extensions/forum_search/forms.py +++ b/src/ashley/machina_extensions/forum_search/forms.py @@ -3,8 +3,12 @@ ================== This module defines forms provided by the ``forum_search`` application. """ - from machina.apps.forum_search.forms import SearchForm as MachinaSearchForm +from machina.core.db.models import get_model +from machina.core.loading import get_class + +Forum = get_model("forum", "Forum") +PermissionHandler = get_class("forum_permission.handler", "PermissionHandler") class SearchForm(MachinaSearchForm): @@ -13,6 +17,25 @@ class SearchForm(MachinaSearchForm): even with an empty search in the main field. """ + def __init__(self, *args, **kwargs): + """ + Init form, code based on base class MachinaSearchForm. A filter on lti_context + has been added. + """ + # Loads current lti_context to filter forum search + lti_contexts = kwargs.pop("lti_context", None) + super().__init__(*args, **kwargs) + user = kwargs.pop("user", None) + self.allowed_forums = PermissionHandler().get_readable_forums( + Forum.objects.filter(lti_contexts=lti_contexts), user + ) + # self.allowed_forums is used in search method of MachinaSearchForm + if self.allowed_forums: + self.fields["search_forums"].choices = [ + (f.id, "{} {}".format("-" * f.margin_level, f.name)) + for f in self.allowed_forums + ] + def clean(self): """ Set main query to catch all "*" if it is empty and there is a search term for diff --git a/src/ashley/machina_extensions/forum_search/views.py b/src/ashley/machina_extensions/forum_search/views.py new file mode 100644 index 00000000..ce476284 --- /dev/null +++ b/src/ashley/machina_extensions/forum_search/views.py @@ -0,0 +1,25 @@ +""" + Forum search views + ================== + + This module defines views provided by the ``forum_search`` application. + +""" +from haystack import views + +from ashley.context_mixins import get_current_lti_session + + +class FacetedSearchView(views.FacetedSearchView): + """View to show search results""" + + template = "forum_search/search.html" + + def build_form(self, form_kwargs=None): + form = super().build_form( + form_kwargs={ + "user": self.request.user, + "lti_context": get_current_lti_session(self.request), + } + ) + return form diff --git a/tests/ashley/test_forum_search.py b/tests/ashley/test_forum_search.py index 1191d6d9..13189543 100644 --- a/tests/ashley/test_forum_search.py +++ b/tests/ashley/test_forum_search.py @@ -14,7 +14,15 @@ from machina.apps.forum_permission.shortcuts import assign_perm from machina.core.db.models import get_model -from ashley.factories import ForumFactory, PostFactory, TopicFactory, UserFactory +from ashley import SESSION_LTI_CONTEXT_ID +from ashley.factories import ( + ForumFactory, + LTIContextFactory, + PostFactory, + TopicFactory, + UserFactory, +) +from ashley.machina_extensions.forum_search.forms import SearchForm Forum = get_model("forum", "Forum") @@ -445,3 +453,131 @@ def test_forum_search_empty_public_username(self): def tearDown(self): haystack.connections["default"].get_backend().clear() + + def test_forum_search_several_forums_lti_context_search(self): + """ + Creates forum in different lti_context, make sure user can't + search into it, even if he has access to this forum. + """ + user = UserFactory() + + lti_context = LTIContextFactory(lti_consumer=user.lti_consumer) + lti_context2 = LTIContextFactory(lti_consumer=user.lti_consumer) + forum = ForumFactory() + forum2 = ForumFactory() + forum3 = ForumFactory() + forum.lti_contexts.add(lti_context) + forum2.lti_contexts.add(lti_context) + forum3.lti_contexts.add(lti_context2) + + post1 = PostFactory( + topic=TopicFactory(forum=forum), + subject="Forum same lti_context", + text="Hello world", + ) + + post2 = PostFactory( + topic=TopicFactory(forum=forum2), + subject="Forum2 same lti_context", + text="Good morning world", + ) + post3 = PostFactory( + topic=TopicFactory(forum=forum3), + subject="Forum3 different lti_context", + text="Welcome world", + ) + # Creates the session + self.client.force_login(user, "ashley.auth.backend.LTIBackend") + session = self.client.session + session[SESSION_LTI_CONTEXT_ID] = lti_context.id + session.save() + + assign_perm("can_read_forum", user, forum) + assign_perm("can_read_forum", user, forum2) + assign_perm("can_read_forum", user, forum3) + + # Index the post in Elasticsearch + call_command("rebuild_index", interactive=False) + + response = self.client.get("/forum/search/?q=world") + + self.assertContains( + response, "Your search has returned 2 results", html=True + ) + self.assertContains(response, post1.subject) + self.assertContains(response, post2.subject) + self.assertNotContains(response, post3.subject) + + # Change the session to connect user to lti_context2 + self.client.force_login(user, "ashley.auth.backend.LTIBackend") + session = self.client.session + session[SESSION_LTI_CONTEXT_ID] = lti_context2.id + session.save() + + response = self.client.get("/forum/search/?q=world") + # We should only have one result + self.assertContains( + response, "Your search has returned 1 result", html=True + ) + self.assertNotContains(response, post1.subject) + self.assertNotContains(response, post2.subject) + self.assertContains(response, post3.subject) + + def test_forum_search_with_unautorized_forum_from_other_lti_context(self): + """ + Try to search in a forum that is not part of our LTIContext by submitting + in the search form a forum from another LTIContext. + """ + user = UserFactory() + + lti_context = LTIContextFactory(lti_consumer=user.lti_consumer) + lti_context2 = LTIContextFactory(lti_consumer=user.lti_consumer) + forum = ForumFactory() + forum2 = ForumFactory() + forum.lti_contexts.add(lti_context) + forum2.lti_contexts.add(lti_context2) + + PostFactory( + topic=TopicFactory(forum=forum), + text="Good morning world", + ) + + PostFactory( + topic=TopicFactory(forum=forum2), + text="Hello world", + ) + # Index posts in Elasticsearch + call_command("rebuild_index", interactive=False) + + # Connects and gets acces to the forum + self.client.force_login(user, "ashley.auth.backend.LTIBackend") + assign_perm("can_read_forum", user, forum) + assign_perm("can_read_forum", user, forum2) + session = self.client.session + session[SESSION_LTI_CONTEXT_ID] = lti_context.id + session.save() + + form = SearchForm(user=user, lti_context=lti_context) + + # Checks that only the forum that is allowed is proposed as choice + self.assertEqual( + form.fields["search_forums"].choices, + [(forum.id, "{} {}".format("-" * forum.margin_level, forum.name))], + ) + # Despite that, we force the request on the forum that is not allowed + response = self.client.get(f"/forum/search/?q=world&search_forums={forum2.id}") + self.assertEqual(response.status_code, 200) + # Controls that we get an error and the search is not executed + self.assertContains( + response, + f"Select a valid choice. {forum2.id} is not one of the available choices.", + html=True, + ) + + # Valid request, we search on the forum that is allowed, we get only one result + # as forum2 is ignored + response = self.client.get(f"/forum/search/?q=world&search_forums={forum.id}") + self.assertEqual(response.status_code, 200) + self.assertContains( + response, "Your search has returned 1 result", html=True + )