www.fgks.org   »   [go: up one dir, main page]
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

Always show CTA banner if no reading goal exists #8606

Merged
merged 3 commits into from
Dec 20, 2023
Merged

Always show CTA banner if no reading goal exists #8606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
37ca538
Remove `within_date_range()` unit tests
jimchamp Dec 13, 2023
fa0b486
Show CTA banner iff current year's goal not set
jimchamp Dec 13, 2023
7a45bd3
Add clarifying comment
jimchamp Dec 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion openlibrary/templates/account/mybooks.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
<div class="mybooks-details-specific">

$if owners_page:
$ year = get_reading_goals_year()
$ year = current_year()
$ current_goal = get_reading_goals(year=year)
$ hidden = 'hidden' if current_goal else ''
<div class="yearly-goal-section">
$# Hide "Set reading goal" call to action with `hidden` class if a goal has already been set:
<div class="chip-group $hidden">
<span class="chip value-chip category-chip chip--selectable goal-chip">
$ year_markup = year_span(year)
Expand Down
8 changes: 4 additions & 4 deletions openlibrary/templates/account/yrg_banner.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
$def with(component_times)

$ component_times['Yearly Goal Banner'] = time()
$ year = get_reading_goals_year()
$ year = current_year()
$ goal = get_reading_goals(year=year)
$if not goal and not within_date_range(1, 1, 11, 30):
$if not goal:
$ cta_copy = _('Set your %(year)s Yearly Reading Goal:', year=year)
$ btn_copy = _('Set my goal')
$ announcement = '%s <a class="btn primary set-reading-goal-link" data-ol-link-track="MyBooksLandingPage|SetReadingGoal"href="javascript:;">%s</a>' % (cta_copy, btn_copy)
$ cookie_name = 'yrg'
$:render_template('site/banner', announcement, cookie_name, cookie_duration_days=65)
$ cookie_name = 'yrg%d' % (year % 100)
$:render_template('site/banner', announcement, cookie_name, cookie_duration_days=365)
Copy link
Member

Choose a reason for hiding this comment

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

Instead of 365, we should use the number of days until the end of the year.

Note: either way, this shouldn't impact site behavior because the cookie name is pinned to the year and so one year cannot bleed and effect another year.

$ component_times['Yearly Goal Banner'] = time() - component_times['Yearly Goal Banner']
47 changes: 0 additions & 47 deletions openlibrary/utils/dateutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,53 +111,6 @@ def current_year():
return datetime.datetime.now().year


@public
def get_reading_goals_year():
now = datetime.datetime.now()
year = now.year
return year if now.month < 12 else year + 1


@public
def within_date_range(
start_month: int,
start_day: int,
end_month: int,
end_day: int,
current_date: datetime.datetime | None = None,
) -> bool:
"""
Checks if the current date is within the given duration.
If now current_date is given, the actual current date is instead.
Year is not used when determining if current date is within range.
"""
now = current_date or datetime.datetime.now()
current_month = now.month
current_day = now.day

if start_month < end_month: # Duration spans a single calendar year
if (
(current_month < start_month or current_month > end_month)
or (current_month == start_month and current_day < start_day)
or (current_month == end_month and current_day > end_day)
):
return False
elif start_month > end_month: # Duration spans two years
if (
(current_month > end_month and current_month < start_month)
or (current_month == start_month and current_day < start_day)
or (current_month == end_month and current_day > end_day)
):
return False
else: # Duration is within a single month
if (current_month != start_month) or (
current_day < start_day or current_day > end_day
):
return False

return True


@contextmanager
def elapsed_time(name="elapsed_time"):
"""
Expand Down
195 changes: 0 additions & 195 deletions openlibrary/utils/tests/test_dateutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,198 +43,3 @@ def test_parse_daterange():
datetime.date(2010, 2, 3),
datetime.date(2010, 2, 4),
)


def test_within_date_range():
# Test single-year date range:
start_date = datetime.datetime(2030, 1, 2)
end_date = datetime.datetime(2030, 5, 20)

# Positive cases:
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=start_date,
)
is True
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2030, 2, 1),
)
is True
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=end_date,
)
is True
)
# Negative cases:
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2030, 1, 1),
)
is False
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2030, 5, 25),
)
is False
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2030, 10, 13),
)
is False
)

# Test multi-year date range:
start_date = datetime.datetime(2030, 12, 1)
end_date = datetime.datetime(2031, 2, 1)

# Positive cases:
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=start_date,
)
is True
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2031, 1, 11),
)
is True
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=end_date,
)
is True
)

# Negative cases:
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2030, 11, 15),
)
is False
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2031, 2, 2),
)
is False
)

# Test single-month date range:
start_date = datetime.datetime(2030, 6, 3)
end_date = datetime.datetime(2030, 6, 10)

# Positive cases:
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=start_date,
)
is True
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2030, 6, 8),
)
is True
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=end_date,
)
is True
)

# Negative cases:
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2030, 5, 8),
)
is False
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2031, 6, 2),
)
is False
)
assert (
dateutil.within_date_range(
start_date.month,
start_date.day,
end_date.month,
end_date.day,
current_date=datetime.datetime(2031, 6, 20),
)
is False
)