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.

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

Topic landing tod #2137

Merged
merged 28 commits into from
Dec 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
97f3466
test(topics): add test for topic of the day
nsantacruz Nov 28, 2024
c22abe4
feat(topics): add topic of the day api
nsantacruz Nov 28, 2024
f6e6959
chore(topics): remove unnecessary django setup now that we have pytes…
nsantacruz Nov 28, 2024
b7bea0c
chore(topics): add pytest django as a dependency
nsantacruz Nov 28, 2024
2b39b16
chore(topics): add topic of the day function in Sefaria to call corre…
nsantacruz Nov 28, 2024
9219d5c
feat(topics): basic topic of the day rendering
nsantacruz Nov 28, 2024
347167a
feat(topics): add ImageWithAltText
nsantacruz Nov 28, 2024
d7f8846
feat(topics): topic of the day frontend WIP
nsantacruz Nov 30, 2024
633c801
fix(topics): change text transform
nsantacruz Dec 2, 2024
c786ba1
Merge branch 'topic-landing' into topic-landing-tod
nsantacruz Dec 2, 2024
c4474f3
chore(topics): merge
nsantacruz Dec 2, 2024
12f0223
feat(topics): polish topic of the day (to be known henceforth as feat…
nsantacruz Dec 2, 2024
914c9bf
refactor(topics): only fetch topic of the day on first render
nsantacruz Dec 7, 2024
513d286
chore(topics): merge
nsantacruz Dec 14, 2024
e77010b
refactor(topics): rename all mentions of TopicOfTheDay to FeaturedTopic
nsantacruz Dec 15, 2024
611716c
chore(topics): merge master
nsantacruz Dec 15, 2024
dd60de1
fix(topics): fix forked migration
nsantacruz Dec 15, 2024
9478479
chore(topics): leave TopicOfTheDay model name as is to avoid issues w…
nsantacruz Dec 17, 2024
dfc2486
chore(topics): merge
nsantacruz Dec 17, 2024
bf793a7
chore(topics): merge
nsantacruz Dec 17, 2024
f18e75b
chore(topics): upgrade pytest to match pytest-django
nsantacruz Dec 17, 2024
74357c2
fix(topics): use secondary image
nsantacruz Dec 17, 2024
2b574ee
fix(topics): use secondary image
nsantacruz Dec 17, 2024
d2538a4
fix(topics): fix styling
nsantacruz Dec 17, 2024
1904663
fix(topic): make featured topic image correct aspect ratio on chrome
nsantacruz Dec 19, 2024
b49ecc3
fix(topic): handle case where there is no image
nsantacruz Dec 19, 2024
5a04fc8
chore(topic): merge
nsantacruz Dec 19, 2024
ab054c8
chore(topic): remove unused callback parameter
nsantacruz Dec 22, 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
13 changes: 6 additions & 7 deletions django_topics/admin.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from django.contrib import admin, messages
from django.utils.html import format_html
from django_topics.models import Topic, TopicPool, TopicOfTheDayEnglish, TopicOfTheDayHebrew, SeasonalTopicEnglish, SeasonalTopicHebrew
from django_topics.models import Topic, TopicPool, FeaturedTopicEnglish, FeaturedTopicHebrew, SeasonalTopicEnglish, SeasonalTopicHebrew
from django_topics.models.pool import PoolType


Expand Down Expand Up @@ -102,7 +101,7 @@ def sefaria_link(self, obj):
sefaria_link.short_description = "Sefaria Link"


class TopicOfTheDayAdmin(admin.ModelAdmin):
class FeaturedTopicAdmin(admin.ModelAdmin):
exclude = ("lang",) # not for manual editing
list_display = ('start_date', 'topic')
list_filter = ('start_date',)
Expand All @@ -123,16 +122,16 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs):
return super().formfield_for_foreignkey(db_field, request, **kwargs)


@admin.register(TopicOfTheDayEnglish)
class TopicOfTheDayAdminEnglish(TopicOfTheDayAdmin):
@admin.register(FeaturedTopicEnglish)
class FeaturedTopicAdminEnglish(FeaturedTopicAdmin):

def get_queryset(self, request):
qs = super().get_queryset(request)
return qs.filter(lang="en")


@admin.register(TopicOfTheDayHebrew)
class TopicOfTheDayAdminHebrew(TopicOfTheDayAdmin):
@admin.register(FeaturedTopicHebrew)
class FeaturedTopicAdminHebrew(FeaturedTopicAdmin):

def get_queryset(self, request):
qs = super().get_queryset(request)
Expand Down
49 changes: 49 additions & 0 deletions django_topics/migrations/0008_auto_20241217_0702.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.29 on 2024-12-17 11:02
from __future__ import unicode_literals

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('django_topics', '0007_auto_20241127_0034'),
]

operations = [
migrations.DeleteModel(
name='TopicOfTheDayEnglish',
),
migrations.DeleteModel(
name='TopicOfTheDayHebrew',
),
migrations.CreateModel(
name='FeaturedTopicEnglish',
fields=[
],
options={
'verbose_name': 'Landing Page - Featured Topic (EN)',
'verbose_name_plural': 'Landing Page - Featured Topic (EN)',
'proxy': True,
'indexes': [],
},
bases=('django_topics.topicoftheday',),
),
migrations.CreateModel(
name='FeaturedTopicHebrew',
fields=[
],
options={
'verbose_name': 'Landing Page - Featured Topic (HE)',
'verbose_name_plural': 'Landing Page - Featured Topic (HE)',
'proxy': True,
'indexes': [],
},
bases=('django_topics.topicoftheday',),
),
migrations.AlterModelOptions(
name='topicoftheday',
options={'verbose_name': 'Landing Page - Featured Topic', 'verbose_name_plural': 'Landing Page - Featured Topic'},
),
]
2 changes: 1 addition & 1 deletion django_topics/models/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .topic import Topic
from .pool import TopicPool, PoolType
from .topic_of_the_day import TopicOfTheDay, TopicOfTheDayEnglish, TopicOfTheDayHebrew
from .featured_topic import TopicOfTheDay, FeaturedTopicEnglish, FeaturedTopicHebrew
from .seasonal_topic import SeasonalTopic, SeasonalTopicEnglish, SeasonalTopicHebrew
62 changes: 62 additions & 0 deletions django_topics/models/featured_topic.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
from django.db import models
from datetime import datetime
from django.utils.timezone import now
from django_topics.models import Topic


class FeaturedTopicManager(models.Manager):

def get_featured_topic(self, lang: str, date: datetime = None) -> 'TopicOfTheDay':
"""
Return featured topic for given date or closest date that is less than or equal to given date
@param lang: language code, "en" or "he"
@param date: datetime object
@return:
"""
date = date or now().date()
return (
self.filter(start_date__lte=date, lang=lang)
.order_by('-start_date')
.first()
)


class TopicOfTheDay(models.Model):
topic = models.ForeignKey(
Topic,
on_delete=models.CASCADE,
related_name='topic_of_the_day'
)
start_date = models.DateField()
lang = models.CharField(max_length=2, choices=[('en', 'English'), ('he', 'Hebrew')])
objects = FeaturedTopicManager()

class Meta:
unique_together = ('topic', 'start_date')
verbose_name = "Landing Page - Featured Topic"
verbose_name_plural = "Landing Page - Featured Topic"

def __str__(self):
return f"{self.topic.slug} ({self.start_date})"


class FeaturedTopicEnglish(TopicOfTheDay):
Copy link
Contributor

@yonadavGit yonadavGit Dec 19, 2024

Choose a reason for hiding this comment

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

Why is child called FeaturedTopicEnglish instead of simply TopicOfTheDayEnglish?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed every variable and class name to be based on the new name of the feature "Featured Topic" EXCEPT for the main django model which I kept as "TopicOfTheDay". This is unfortunate but this is why:

  • TopicOfTheDay has already been merged into master
  • Therefore, all cauldrons (or most) are dependent on this model
  • TopicOfTheDay directly corresponds to a table in Postgres by the same name
  • If we rename the model, it will rename the table which will break all cauldrons
  • I want to wait to do this until my code is in master.

class Meta:
proxy = True
verbose_name = "Landing Page - Featured Topic (EN)"
verbose_name_plural = "Landing Page - Featured Topic (EN)"

def save(self, *args, **kwargs):
self.lang = "en"
super().save(*args, **kwargs)


class FeaturedTopicHebrew(TopicOfTheDay):
class Meta:
proxy = True
verbose_name = "Landing Page - Featured Topic (HE)"
verbose_name_plural = "Landing Page - Featured Topic (HE)"

def save(self, *args, **kwargs):
self.lang = "he"
super().save(*args, **kwargs)
47 changes: 47 additions & 0 deletions django_topics/models/tests/featured_topic_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import pytest
from datetime import date
from django_topics.models import TopicOfTheDay, Topic


@pytest.fixture
def topic(db):
"""Fixture to create a Topic instance."""
return Topic.objects.create(slug="test-topic")


@pytest.fixture
def featured_topics(db, topic):
"""Fixture to create TopicOfTheDay instances."""
topics = [
TopicOfTheDay.objects.create(topic=topic, start_date=date(2024, 11, 26), lang="en"),
TopicOfTheDay.objects.create(topic=topic, start_date=date(2024, 11, 25), lang="en"),
TopicOfTheDay.objects.create(topic=topic, start_date=date(2024, 11, 24), lang="en"),
]
return topics


@pytest.mark.django_db
def test_get_featured_topic_with_exact_date_db(featured_topics):
"""Test for exact match."""
result = TopicOfTheDay.objects.get_featured_topic(lang="en", date=date(2024, 11, 26))

assert result.start_date == date(2024, 11, 26)
assert result.lang == "en"


@pytest.mark.django_db
def test_get_featured_topic_with_closest_date_db(featured_topics):
"""Test for the closest date less than or equal to the given date."""
result = TopicOfTheDay.objects.get_featured_topic(lang="en", date=date(2024, 11, 27))

assert result.start_date == date(2024, 11, 26)
assert result.lang == "en"


@pytest.mark.django_db
def test_get_featured_topic_with_no_matching_date_db(db, topic):
"""Test when there is no matching date."""
TopicOfTheDay.objects.create(topic=topic, start_date=date(2024, 11, 20), lang="en")
result = TopicOfTheDay.objects.get_featured_topic(lang="en", date=date(2024, 11, 19))

assert result is None
43 changes: 0 additions & 43 deletions django_topics/models/topic_of_the_day.py

This file was deleted.

13 changes: 13 additions & 0 deletions reader/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3250,6 +3250,19 @@ def topic_pool_api(request, pool_name):
return jsonResponse(response, callback=request.GET.get("callback", None))


@catch_error_as_json
def featured_topic_api(request):
from django_topics.models import TopicOfTheDay

lang = request.GET.get("lang")
featured_topic = TopicOfTheDay.objects.get_featured_topic(lang)
if not featured_topic:
return jsonResponse({'error': f'No featured topic found for lang "{lang}"'}, status=404)
mongo_topic = Topic.init(featured_topic.topic.slug)
response = {'topic': mongo_topic.contents(), 'date': featured_topic.start_date.isoformat()}
return jsonResponse(response)


@staff_member_required
def reorder_topics(request):
topics = json.loads(request.POST["json"]).get("topics", [])
Expand Down
3 changes: 2 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pillow==8.0.1; sys_platform == 'linux'
psycopg2==2.8.6 #for dev: psycopg2-binary==2.8.6
py2-py3-django-email-as-username==1.7.1
pymongo==3.12.*
pytest==7.0.0
pytest~=7.4.4
python-bidi
pytz
pyyaml==6.0.1
Expand All @@ -71,6 +71,7 @@ unidecode==1.1.1
user-agents==2.2.0
pytest-django==4.9.*


#opentelemetry-distro
#opentelemetry-exporter-otlp
#opentelemetry-propagator-b3
Expand Down
1 change: 1 addition & 0 deletions sefaria/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@
url(r'^api/topics-graph/(?P<topic>.+)$', reader_views.topic_graph_api),
url(r'^_api/topics/seasonal-topic/?$', reader_views.seasonal_topic_api),
url(r'^api/topics/pools/(?P<pool_name>.+)$', reader_views.topic_pool_api),
url(r'^_api/topics/featured-topic/?$', reader_views.featured_topic_api),
url(r'^api/ref-topic-links/bulk$', reader_views.topic_ref_bulk_api),
url(r'^api/ref-topic-links/(?P<tref>.+)$', reader_views.topic_ref_api),
url(r'^api/v2/topics/(?P<topic>.+)$', reader_views.topics_api, {'v2': True}),
Expand Down
49 changes: 48 additions & 1 deletion static/css/s2.css
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ a, a:hover {
--highlight-blue: #DDEEFF;
--highlight-blue-light: #F0F7FF;
--beit-midrash-grey: #333333;
--darkest-grey: #333333;
--dark-grey: #666666;
--medium-grey: #999999;
--light-grey: #CCCCCC;
Expand Down Expand Up @@ -4959,11 +4960,57 @@ body .ui-autocomplete.dictionary-toc-autocomplete .ui-menu-item a.ui-state-focus
padding: 0;
visibility: hidden;
}

.topic-landing-search-container .readerNavMenuSearchButton{
top: 6px;
padding-inline-start: 11px;
}
.featuredTopic {
display: flex;
flex-direction: column;
margin-top: 68px;
}
.featuredTopicContent {
display: flex;
flex-direction: row;
}
.featuredTopicImgWrapper {
width: 239px;
height: 100%;
}
.featuredTopicContent img {
object-fit: contain;
}
.featuredTopicContent h3 {
font-size: var(--serif-h3-font-size);
text-transform: none;
margin: 0 0 15px 0;
}
.featuredTopicText {
flex: 1;
}
.featuredTopicText .int-en,
.featuredTopicText .int-he {
display: flex;
flex-direction: column;
color: var(--dark-grey);
margin-inline-start: 30px;
}
.featuedTopicText h3 {
text-transform: none;
margin: 0 0 15px 0;
}
.featuedTopicGoToLink {
margin-top: 25px;
}
.featuredTopicText .topicDescription {
font-size: var(--sans-serif-small-font-size);
}
.featuredTopic h1 {
font-size: var(--sans-serif-h2-font-size);
border-bottom: 1px solid #ccc;
width: fit-content;
padding-bottom: 10px;
}
.topic-landing-upper-rainbow{
margin-top: 88px;
margin-bottom: 43px;
Expand Down
7 changes: 5 additions & 2 deletions static/js/Misc.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3034,18 +3034,20 @@ const Autocompleter = ({getSuggestions, showSuggestionsOnSelect, inputPlaceholde
}

const getImgAltText = (caption) => {
return Sefaria._v(caption) || Sefaria._('Illustrative image');
return (caption && Sefaria._v(caption)) || Sefaria._('Illustrative image');
}
const ImageWithCaption = ({photoLink, caption }) => {
return (
<div>
<img className="imageWithCaptionPhoto" src={photoLink} alt={getImgAltText(caption)}/>
<ImageWithAltText photoLink={photoLink} altText={caption} />
<div className="imageCaption">
<InterfaceText text={caption} />
</div>
</div>);
}

const ImageWithAltText = ({photoLink, altText}) => (<img className="imageWithCaptionPhoto" src={photoLink} alt={getImgAltText(altText)}/>);

const AppStoreButton = ({ platform, href, altText }) => {
const isIOS = platform === 'ios';
const aClasses = classNames({button: 1, small: 1, white: 1, appButton: 1, ios: isIOS});
Expand Down Expand Up @@ -3241,6 +3243,7 @@ export {
TitleVariants,
OnInView,
ImageWithCaption,
ImageWithAltText,
handleAnalyticsOnMarkdown,
LangSelectInterface,
PencilSourceEditor,
Expand Down
Loading
Loading