Skip to content

fix: prevent 'null' display when instructor family_name is missing #4711

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

Open
rjv31 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: prevent 'null' display when instructor family_name is missing #4711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion course_discovery/apps/course_metadata/people.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ def _get_api_client(self, partner):
)

def _get_node_data(self, person):
full_name = " ".join(filter(None, [person.given_name, person.family_name]))
Copy link
Copy Markdown

@ankit-sonata ankit-sonata Apr 10, 2026

Choose a reason for hiding this comment

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

@rjv31

This has two issues:

It silently drops the salutation field — if a person has salutation="Dr.", given_name="Jane", family_name="Smith", the model's full_name returns "Dr. Jane Smith", but this code would send "Jane Smith" to the marketing site.

It's redundant — person.full_name already handles None family_name correctly, so the fix doesn't actually change the behavior for the stated bug scenario (null family_name).


return {
'field_person_slug': person.slug,
'title': person.full_name,
'title': full_name,
'type': 'person',
'status': 1 if person.published else 0
}
Expand Down
27 changes: 27 additions & 0 deletions course_discovery/apps/course_metadata/tests/test_people.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ class MarketingSitePublisherTests(MarketingSitePublisherTestMixin):
"""
def setUp(self):
super().setUp()
patcher = mock.patch("django_elasticsearch_dsl.registries.registry.update")
self.mock_es = patcher.start()
self.addCleanup(patcher.stop)
self.partner = PartnerFactory()
self.partner.marketing_site_url_root = self.api_root
self.partner.marketing_site_api_username = self.username
Expand Down Expand Up @@ -148,6 +151,30 @@ def test_person_create_json(self, mock_create_node):
})
self.assertDictEqual(data, expected)

def test_person_without_family_name(self):
person = PersonFactory(
partner=self.partner,
given_name='Test',
family_name=None
)

people = MarketingSitePeople()
data = people._get_node_data(person) # pylint: disable=protected-access

assert data['title'] == 'Test'

def test_person_with_empty_family_name(self):
person = PersonFactory(
partner=self.partner,
given_name='Test',
family_name=''
)

people = MarketingSitePeople()
data = people._get_node_data(person) # pylint: disable=protected-access

assert data['title'] == 'Test'

@responses.activate
def test_person_create_failed(self):
self.mock_api_client(200)
Expand Down
Loading