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.

Sign up for GitHub

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

Jump to bottom

fix: add partner field to person documents #4497

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+22 −14
Open

fix: add partner field to person documents #4497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1688fd8
fix: add partner field to person documents
zawan-ila Nov 28, 2024
3d7845d
fix: remove field from serializer
zawan-ila Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions course_discovery/apps/api/v1/tests/test_views/mixins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
from course_discovery.apps.api.tests.mixins import SiteMixin
from course_discovery.apps.core.tests.factories import USER_PASSWORD, UserFactory
from course_discovery.apps.course_metadata.search_indexes.documents import (
CourseDocument, CourseRunDocument, LearnerPathwayDocument, ProgramDocument
CourseDocument, CourseRunDocument, LearnerPathwayDocument, ProgramDocument, PersonDocument
)
from course_discovery.apps.course_metadata.search_indexes.serializers import (
CourseRunSearchDocumentSerializer, CourseSearchDocumentSerializer, LearnerPathwaySearchDocumentSerializer,
ProgramSearchDocumentSerializer
ProgramSearchDocumentSerializer, PersonSearchDocumentSerializer
)
from course_discovery.apps.course_metadata.tests import factories

Expand Down Expand Up @@ -82,6 +82,10 @@ def serialize_course_run_search(self, run, serializer=None):
def serialize_person(self, person, many=False, format=None, extra_context=None):
return self._serialize_object(serializers.PersonSerializer, person, many, format, extra_context)

def serialize_person_search(self, person, serializer=None):
obj = self._get_search_result(PersonDocument, uuid=person.uuid)
return self._serialize_object(serializer or PersonSearchDocumentSerializer, obj)

def serialize_program(self, program, many=False, format=None, extra_context=None):
return self._serialize_object(
serializers.MinimalProgramSerializer if many else serializers.ProgramSerializer,
Expand Down
9 changes: 9 additions & 0 deletions course_discovery/apps/api/v1/tests/test_views/test_search.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,15 @@ def test_results_include_aggregation_key(self):
)
assert expected == actual

def test_results_include_person_objects(self):
""" Verify that a get against search/all includes person objects. """
person = PersonFactory(partner=self.partner)
response = self.client.get(self.list_path)
assert response.status_code == 200
response_data = response.json()
assert response_data["count"] == 1
assert response_data["results"] == [self.serialize_person_search(person)]

@ddt.data((True, 10, 8), (False, 0, 4))
@ddt.unpack
def test_learner_pathway_feature_flag(self, include_learner_pathways, expected_result_count, expected_query_count):
Expand Down
8 changes: 4 additions & 4 deletions course_discovery/apps/course_metadata/search_indexes/documents/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ def __init__(self, *args, **kwargs):
self._object = None

aggregation_key = fields.KeywordField()
partner = fields.TextField(
analyzer=html_strip,
fields={'raw': fields.KeywordField(), 'lower': fields.TextField(analyzer=case_insensitive_keyword)}
)
content_type = fields.KeywordField()
id = fields.KeywordField()
organizations = fields.TextField(analyzer=html_strip, multi=True, fields={'raw': fields.KeywordField(multi=True)})
Expand Down Expand Up @@ -219,10 +223,6 @@ class BaseCourseDocument(OrganizationsMixin, BaseDocument):
level_type = fields.TextField(
fields={'raw': fields.KeywordField(), 'lower': fields.TextField(analyzer=case_insensitive_keyword)}
)
partner = fields.TextField(
analyzer=html_strip,
fields={'raw': fields.KeywordField(), 'lower': fields.TextField(analyzer=case_insensitive_keyword)}
)
outcome = fields.TextField()
org = fields.TextField(
analyzer=html_strip,
Expand Down
4 changes: 0 additions & 4 deletions course_discovery/apps/course_metadata/search_indexes/documents/learner_pathway.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ class LearnerPathwayDocument(BaseDocument, OrganizationsMixin):
'edge_ngram_completion': fields.TextField(analyzer=edge_ngram_completion),
},
)
partner = fields.TextField(
analyzer=html_strip,
fields={'raw': fields.KeywordField(), 'lower': fields.TextField(analyzer=case_insensitive_keyword)}
)
visible_via_association = fields.BooleanField()
status = fields.TextField()
overview = fields.TextField()
Expand Down
3 changes: 3 additions & 0 deletions course_discovery/apps/course_metadata/search_indexes/documents/person.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ class PersonDocument(BaseDocument):
def prepare_aggregation_key(self, obj):
return 'person:{}'.format(obj.uuid)

def prepare_partner(self, obj):
return obj.partner.short_code if obj.partner else ''

Comment on lines +32 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

should this not be added in common too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could but it wouldn't be as neat since some documents have "different" implementations for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

If only course run has a different implementation, it makes sense to override it there. But if all others have the same pattern, it would be good to move this in common as you have done for field itself.

def prepare_bio_language(self, obj):
if obj.bio_language:
return obj.bio_language.name
Expand Down
4 changes: 0 additions & 4 deletions course_discovery/apps/course_metadata/search_indexes/documents/program.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ class ProgramDocument(BaseDocument, OrganizationsMixin):
marketing_url = fields.TextField()
min_hours_effort_per_week = fields.IntegerField()
max_hours_effort_per_week = fields.IntegerField()
partner = fields.TextField(
analyzer=html_strip,
fields={'raw': fields.KeywordField(), 'lower': fields.TextField(analyzer=case_insensitive_keyword)}
)
published = fields.BooleanField()
subtitle = fields.TextField(analyzer=html_strip)
status = fields.KeywordField()
Expand Down
Loading