Skip to content

Commit

Permalink
🗑️(back) filter search on current lti_context
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
carofun committed Jun 17, 2021
1 parent 1210445 commit 83ccbf6
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
25 changes: 24 additions & 1 deletion src/ashley/machina_extensions/forum_search/forms.py
Expand Up @@ -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):
Expand All @@ -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
Expand Down
25 changes: 25 additions & 0 deletions 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
138 changes: 137 additions & 1 deletion tests/ashley/test_forum_search.py
Expand Up @@ -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")

Expand Down Expand Up @@ -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 <b>2</b> 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 <b>1</b> 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 <b>1</b> result", html=True
)

0 comments on commit 83ccbf6

Please sign in to comment.